Cleans up restartcore/smp_idle, fixes corner case
authorBarret Rhoden <brho@cs.berkeley.edu>
Thu, 25 Jul 2013 18:37:02 +0000 (11:37 -0700)
committerBarret Rhoden <brho@cs.berkeley.edu>
Thu, 25 Jul 2013 18:37:02 +0000 (11:37 -0700)
It was possible to have a proc's address space hang around on a core, if that
core was idle, then received a __startcore, followed quickly by __death, and
then was never used again.

This cleans up this situation (abandon_core() aggressively if there is no
owner), and cleans up proc_restartcore() and smp_idle() a little.

kern/include/process.h
kern/src/process.c
kern/src/smp.c

index 545970b..b8da41e 100644 (file)
@@ -70,6 +70,7 @@ void proc_incref(struct proc *p, unsigned int val);
 void proc_decref(struct proc *p);
 void proc_run_s(struct proc *p);
 void __proc_run_m(struct proc *p);
+void __proc_startcore(struct proc *p, struct user_context *ctx);
 void proc_restartcore(void);
 void proc_destroy(struct proc *p);
 void proc_signal_parent(struct proc *child);
index 753540b..9985e44 100644 (file)
@@ -32,7 +32,6 @@
 struct kmem_cache *proc_cache;
 
 /* Other helpers, implemented later. */
-static void __proc_startcore(struct proc *p, struct user_context *ctx);
 static bool is_mapped_vcore(struct proc *p, uint32_t pcoreid);
 static uint32_t get_vcoreid(struct proc *p, uint32_t pcoreid);
 static uint32_t try_get_pcoreid(struct proc *p, uint32_t vcoreid);
@@ -610,7 +609,9 @@ void __proc_run_m(struct proc *p)
        }
 }
 
-/* Actually runs the given context (trapframe) of process p on the core this
+/* You must disable IRQs and PRKM before calling this.
+ *
+ * Actually runs the given context (trapframe) of process p on the core this
  * code executes on.  This is called directly by __startcore, which needs to
  * bypass the routine_kmsg check.  Interrupts should be off when you call this.
  *
@@ -623,7 +624,7 @@ void __proc_run_m(struct proc *p)
  * in current and you have one reference, like proc_run(non_current_p), then
  * also do nothing.  The refcnt for your *p will count for the reference stored
  * in current. */
-static void __proc_startcore(struct proc *p, struct user_context *ctx)
+void __proc_startcore(struct proc *p, struct user_context *ctx)
 {
        assert(!irq_is_enabled());
        __set_proc_current(p);
@@ -638,11 +639,7 @@ static void __proc_startcore(struct proc *p, struct user_context *ctx)
  * In case there are pending routine messages, like __death, __preempt, or
  * __notify, we need to run them.  Alternatively, if there are any, we could
  * self_ipi, and run the messages immediately after popping back to userspace,
- * but that would have crappy overhead.
- *
- * Refcnting: this will not return, and it assumes that you've accounted for
- * your reference as if it was the ref for "current" (which is what happens when
- * returning from local traps and such. */
+ * but that would have crappy overhead. */
 void proc_restartcore(void)
 {
        struct per_cpu_info *pcpui = &per_cpu_info[core_id()];
@@ -653,7 +650,7 @@ void proc_restartcore(void)
         * do this, we'd just get them in userspace, but this might save us some
         * effort/overhead. */
        enable_irq();
-       /* Need ints disabled when we return from processing (race on missing
+       /* Need ints disabled when we return from PRKM (race on missing
         * messages/IPIs) */
        disable_irq();
        process_routine_kmsg();
index fd29ae2..3586289 100644 (file)
@@ -30,12 +30,16 @@ atomic_t outstanding_calls = 0;
 static void try_run_proc(void)
 {
        struct per_cpu_info *pcpui = &per_cpu_info[core_id()];
-
-       disable_irq();
        /* There was a process running here, and we should return to it. */
        if (pcpui->owning_proc) {
-               proc_restartcore();
+               assert(!pcpui->cur_sysc);
+               assert(pcpui->cur_ctx);
+               __proc_startcore(pcpui->owning_proc, pcpui->cur_ctx);
                assert(0);
+       } else {
+               /* Make sure we have abandoned core.  It's possible to have an owner
+                * without a current (smp_idle, __startcore, __death). */
+               abandon_core();
        }
 }
 
@@ -43,21 +47,15 @@ static void try_run_proc(void)
  * don't know explicitly what to do.  Non-zero cores call it when they are done
  * booting.  Other cases include after getting a DEATH IPI.
  *
- * All cores attempt to run the context of any owning proc.  Barring that, the
- * cores enter a loop.  They halt and wake up when interrupted, do any work on
- * their work queue, then halt again.  In between, the ksched gets a chance to
- * tell it to do something else, or perhaps to halt in another manner. */
+ * All cores attempt to run the context of any owning proc.  Barring that, they
+ * halt and wake up when interrupted, do any work on their work queue, then halt
+ * again.  In between, the ksched gets a chance to tell it to do something else,
+ * or perhaps to halt in another manner. */
 static void __attribute__((noinline, noreturn)) __smp_idle(void)
 {
        struct per_cpu_info *pcpui = &per_cpu_info[core_id()];
        clear_rkmsg(pcpui);
-       /* TODO: idle, abandon_core(), and proc_restartcore() need cleaned up */
-       enable_irq();   /* get any IRQs before we halt later */
-       try_run_proc();
-       /* if we made it here, we truly want to idle */
-       /* in the future, we may need to proactively leave process context here.
-        * for now, it is possible to have a current loaded, even if we are idle
-        * (and presumably about to execute a kmsg or fire up a vcore). */
+       enable_irq();   /* one-shot change to get any IRQs before we halt later */
        while (1) {
                disable_irq();
                process_routine_kmsg();