Fix some races with env destruction and dispatch
authorBarret Rhoden <brho@cs.berkeley.edu>
Thu, 4 Jun 2009 23:11:29 +0000 (16:11 -0700)
committerBarret Rhoden <brho@cs.berkeley.edu>
Fri, 5 Jun 2009 23:56:09 +0000 (16:56 -0700)
include/arch/smp.h
include/env.h
kern/src/env.c
kern/src/workqueue.c

index 9ebcd4b..be0f0bb 100644 (file)
@@ -26,6 +26,7 @@ typedef struct HandlerWrapper {
 } handler_wrapper_t;
 
 typedef struct per_cpu_info {
+       uint32_t lock;
        // Once we have a real kmalloc, we can make this dynamic.  Want a queue.
        work_t delayed_work;
        // will want it padded out to an even cacheline
index a082f33..21bad4b 100644 (file)
@@ -8,6 +8,7 @@
 #include <ros/error.h>
 
 extern env_t *envs;            // All environments
+extern uint32_t num_envs;              // Number of envs
 extern env_t* NORACE curenvs[MAX_NUM_CPUS];
 
 LIST_HEAD(env_list_t, env_t);          // Declares 'struct Env_list'
index 0d2730a..9fbe3a2 100644 (file)
@@ -8,7 +8,7 @@
 #include <arch/elf.h>
 #include <arch/apic.h>
 #include <arch/smp.h>
-#include <ros/error.h>
+#include <arch/atomic.h>
 
 #include <string.h>
 #include <assert.h>
 #include <manager.h>
 
 #include <ros/syscall.h>
+#include <ros/error.h>
 
 env_t *envs = NULL;            // All environments
+uint32_t num_envs = 0;         // Number of envs
 // TODO: make this a struct of info including the pointer and cacheline-align it
 // This lets the kernel know what process is running on the core it traps into.
 // A lot of the Env business, including this and its usage, will change when we
@@ -260,6 +262,7 @@ env_alloc(env_t **newenv_store, envid_t parent_id)
        // commit the allocation
        LIST_REMOVE(e, env_link);
        *newenv_store = e;
+       atomic_inc(&num_envs);
 
        e->env_tscfreq = system_timing.tsc_freq;
        // TODO: for now, the only info at procinfo is this env's struct
@@ -452,8 +455,9 @@ env_free(env_t *e)
                page_decref(pa2page(pa));
        }
 
+       // Moved to page_decref
        // need a known good pgdir before releasing the old one
-       lcr3(boot_cr3);
+       //lcr3(boot_cr3);
 
        // free the page directory
        pa = e->env_cr3;
@@ -493,6 +497,10 @@ error_t env_incref(env_t* e)
  */
 void env_decref(env_t* e)
 {
+       // need a known good pgdir before releasing the old one
+       // sometimes env_free is called on a different core than decref
+       lcr3(boot_cr3);
+
        spin_lock(&e->lock);
        e->env_refcnt--;
        spin_unlock(&e->lock);
@@ -512,7 +520,9 @@ env_destroy(env_t *e)
 {
        // TODO: race condition with env statuses, esp when running / destroying
        e->env_status = ENV_DYING;
+
        env_decref(e);
+       atomic_dec(&num_envs);
 
        // for old envs that die on user cores.  since env run never returns, cores
        // never get back to their old hlt/relaxed/spin state, so we need to force
index 8adc62b..edd185c 100644 (file)
@@ -7,6 +7,7 @@
 #include <arch/apic.h>
 #include <arch/smp.h>
 #include <workqueue.h>
+#include <atomic.h>
 
 /*
  * TODO: actually use a queue, which will change some things all over.
 void process_workqueue()
 {
        work_t work;
+       per_cpu_info_t *cpuinfo = &per_cpu_info[lapic_get_id()];
        // copy the work in, since we may never return to this stack frame
-       work = per_cpu_info[lapic_get_id()].delayed_work;
+       spin_lock_irqsave(&cpuinfo->lock);
+       work = cpuinfo->delayed_work;
+       spin_unlock_irqsave(&cpuinfo->lock);
        if (work.func) {
                // TODO: possible race with this.  sort it out when we have a queue.
-               // probably want a spin_lock_irqsave
-               per_cpu_info[lapic_get_id()].delayed_work.func = 0;
+               spin_lock_irqsave(&cpuinfo->lock);
+               cpuinfo->delayed_work.func = 0;
+               spin_unlock_irqsave(&cpuinfo->lock);
                // We may never return from this (if it is env_run)
                work.func(work.data);
        }