proc_restartcore() only used for current_tf, etc
authorBarret Rhoden <brho@cs.berkeley.edu>
Fri, 19 Nov 2010 15:42:28 +0000 (07:42 -0800)
committerKevin Klues <klueska@cs.berkeley.edu>
Thu, 3 Nov 2011 00:35:56 +0000 (17:35 -0700)
It has been like this for a while, with the exception of proc_run()'s _S
case.  With the usage of cur_tf as a ptr, this works out well.  This is
also the place to intercept the "returning twice" problem.

kern/arch/i686/trap.c
kern/arch/sparc/trap.c
kern/include/process.h
kern/src/process.c
kern/src/smp.c

index 4b03b4b..b294495 100644 (file)
@@ -302,7 +302,7 @@ void trap(struct trapframe *tf)
         * to still be okay (might not be after blocking) */
        if (in_kernel(tf))
                return; /* TODO: think about this, might want a helper instead. */
-       proc_restartcore(current, tf);
+       proc_restartcore();
        assert(0);
 }
 
@@ -348,7 +348,7 @@ void irq_handler(struct trapframe *tf)
         * to still be okay (might not be after blocking) */
        if (in_kernel(tf))
                return; /* TODO: think about this, might want a helper instead. */
-       proc_restartcore(current, tf);
+       proc_restartcore();
        assert(0);
 }
 
@@ -409,7 +409,7 @@ void sysenter_callwrapper(struct trapframe *tf)
         * process, which could be weird if the syscall blocked.  it would need to
         * restore the proper value in current before returning to here.
         * likewise, tf could be pointing to random gibberish. */
-       proc_restartcore(current, tf);
+       proc_restartcore();
 }
 
 struct kmem_cache *kernel_msg_cache;
index 0fe5be3..3c3f1fd 100644 (file)
@@ -444,7 +444,7 @@ handle_pop_tf(trapframe_t* state)
 
        proc_secure_trapframe(&tf);
        set_current_tf(pcpui, &tf_p);
-       proc_restartcore(current,&tf);
+       proc_restartcore();
 }
 
 void
@@ -478,7 +478,7 @@ handle_syscall(trapframe_t* state)
        run_local_syscall();
        warn("No syscalls on a trap!");
 
-       proc_restartcore(current,state);
+       proc_restartcore();
 }
 
 void
index 3f4900f..36042ff 100644 (file)
@@ -77,7 +77,7 @@ int __proc_set_state(struct proc *p, uint32_t state) WRITES(p->state);
 struct proc *pid2proc(pid_t pid);
 bool proc_controls(struct proc *SAFE actor, struct proc *SAFE target);
 void proc_run(struct proc *SAFE p);
-void proc_restartcore(struct proc *SAFE p, trapframe_t *SAFE tf);
+void proc_restartcore(void);
 void proc_destroy(struct proc *SAFE p);
 void __proc_yield_s(struct proc *p, struct trapframe *tf);
 void proc_yield(struct proc *SAFE p, bool being_nice);
index c72a72b..af3c7ba 100644 (file)
@@ -422,6 +422,28 @@ bool proc_controls(struct proc *actor, struct proc *target)
        return ((actor == target) || (target->ppid == actor->pid));
 }
 
+/* Helper, makes p the 'current' process, dropping the old current/cr3.  Don't
+ * incref - this assumes the passed in reference already counted 'current'. */
+static void __set_proc_current(struct proc *p)
+{
+       /* We use the pcpui to access 'current' to cut down on the core_id() calls,
+        * though who know how expensive/painful they are. */
+       struct per_cpu_info *pcpui = &per_cpu_info[core_id()];
+       /* If the process wasn't here, then we need to load its address space. */
+       if (p != pcpui->cur_proc) {
+               /* Do not incref here.  We were given the reference to current,
+                * pre-upped. */
+               lcr3(p->env_cr3);
+               /* This is "leaving the process context" of the previous proc.  The
+                * previous lcr3 unloaded the previous proc's context.  This should
+                * rarely happen, since we usually proactively leave process context,
+                * but this is the fallback. */
+               if (pcpui->cur_proc)
+                       kref_put(&pcpui->cur_proc->kref);
+               pcpui->cur_proc = p;
+       }
+}
+
 /* Dispatches a process to run, either on the current core in the case of a
  * RUNNABLE_S, or on its partition in the case of a RUNNABLE_M.  This should
  * never be called to "restart" a core.  This expects that the "instructions"
@@ -460,16 +482,19 @@ void proc_run(struct proc *p)
                        p->procinfo->num_vcores = 0;
                        __map_vcore(p, 0, core_id()); // sort of.  this needs work.
                        __seq_end_write(&p->procinfo->coremap_seqctr);
-                       /* proc_restartcore assumes the reference we give it is for current.
-                        * Decref if current is already properly set, otherwise ensure
-                        * current is set. */
+                       /* __set_proc_current assumes the reference we give it is for
+                        * current.  Decref if current is already properly set, otherwise
+                        * ensure current is set. */
                        if (p == current)
                                kref_put(&p->kref);
+                       else
+                               __set_proc_current(p);
                        /* We restartcore, instead of startcore, since startcore is a bit
                         * lower level and we want a chance to process kmsgs before starting
                         * the process. */
                        spin_unlock(&p->proc_lock);
-                       proc_restartcore(p, &p->env_tf);
+                       current_tf = &p->env_tf;
+                       proc_restartcore();
                        break;
                case (PROC_RUNNABLE_M):
                        /* vcoremap[i] holds the coreid of the physical core allocated to
@@ -509,28 +534,6 @@ void proc_run(struct proc *p)
        }
 }
 
-/* Helper, makes p the 'current' process, dropping the old current/cr3.  Don't
- * incref - this assumes the passed in reference already counted 'current'. */
-static void __set_proc_current(struct proc *p)
-{
-       /* We use the pcpui to access 'current' to cut down on the core_id() calls,
-        * though who know how expensive/painful they are. */
-       struct per_cpu_info *pcpui = &per_cpu_info[core_id()];
-       /* If the process wasn't here, then we need to load its address space. */
-       if (p != pcpui->cur_proc) {
-               /* Do not incref here.  We were given the reference to current,
-                * pre-upped. */
-               lcr3(p->env_cr3);
-               /* This is "leaving the process context" of the previous proc.  The
-                * previous lcr3 unloaded the previous proc's context.  This should
-                * rarely happen, since we usually proactively leave process context,
-                * but this is the fallback. */
-               if (pcpui->cur_proc)
-                       kref_put(&pcpui->cur_proc->kref);
-               pcpui->cur_proc = p;
-       }
-}
-
 /* 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.
@@ -564,8 +567,8 @@ static void __proc_startcore(struct proc *p, trapframe_t *tf)
        env_pop_tf(tf);
 }
 
-/* Restarts/runs the given context (trapframe) of process p on the core this
- * code executes on.  Calls an internal function to do the work.
+/* Restarts/runs the current_tf, which must be for the current process, on the
+ * core this code executes on.  Calls an internal function to do the work.
  *
  * In case there are pending routine messages, like __death, __preempt, or
  * __notify, we need to run them.  Alternatively, if there are any, we could
@@ -575,20 +578,23 @@ static void __proc_startcore(struct proc *p, trapframe_t *tf)
  * 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. */
-void proc_restartcore(struct proc *p, trapframe_t *tf)
+void proc_restartcore(void)
 {
-       if ((tf != &p->env_tf) && (tf != current_tf)) {
-               printk("tf: %08p, Current_tf: %08p, env_tf: %08p\n", tf, current_tf,
-                      &p->env_tf);
-               panic("Current TF is jacked...");
+       struct per_cpu_info *pcpui = &per_cpu_info[core_id()];
+       /* If there is no cur_tf, it is because the old one was already restarted
+        * (and we weren't interrupting another one to finish).  In which case, we
+        * should just smp_idle() */
+       if (!pcpui->cur_tf) {
+               assert(!current);       /* might be wrong, but i want to know if it is */
+               smp_idle();
        }
+       /* TODO: this is where we can decide to smp_idle() if there is no cur_tf */
        /* Need ints disabled when we return from processing (race) */
        disable_irq();
-       /* Need to be current, in case a kmsg is there that tries to clobber us.
-        * Yes, this gets called again in __proc_startcore(). */
-       __set_proc_current(p);
-       process_routine_kmsg(tf);
-       __proc_startcore(p, tf);
+       /* Need to be current (set by the caller), in case a kmsg is there that
+        * tries to clobber us. */
+       process_routine_kmsg(pcpui->cur_tf);
+       __proc_startcore(pcpui->cur_proc, pcpui->cur_tf);
 }
 
 /*
index 4392997..69efde1 100644 (file)
@@ -44,13 +44,14 @@ void smp_idle(void)
        per_cpu_info_t *pcpui = &per_cpu_info[core_id()];
 
        /* There was a process running here, and we should return to it */
+       /* TODO: gut this */
        if (pcpui->cur_tf) {                    /* aka, current_tf */
                assert(pcpui->cur_proc);        /* aka, current */
                /* We also check in run_local_syscall().  This is for sys_exec() */
                if (pcpui->nr_syscs)
                        run_local_syscall();
                /* Now we're done, so return */
-               proc_restartcore(pcpui->cur_proc, pcpui->cur_tf);
+               proc_restartcore();
                assert(0);
        }
        /* if we made it here, we truly want to idle */