core_request() uses current_tf to return
authorBarret Rhoden <brho@cs.berkeley.edu>
Thu, 29 Sep 2011 22:02:11 +0000 (15:02 -0700)
committerKevin Klues <klueska@cs.berkeley.edu>
Thu, 3 Nov 2011 00:36:08 +0000 (17:36 -0700)
Instead of manually calling smp_idle(), we just zero current_tf to
trigger abandoning the core.

Documentation/process-internals.txt
kern/src/resource.c
kern/src/syscall.c

index 8ace357..1fa8a01 100644 (file)
@@ -190,28 +190,7 @@ cores, after it knows all cores unloaded the cr3.  This would be a good use of
 the checklist (possibly with one cacheline per core).  It would take a large
 amount of memory for better scalability.
 
-1.8 Core Request:
----------------------------
-core_request() is run outside of the process code (for now), though it is fairly
-intricate.  It's another function that might not return.  Historically, there
-were a few reasons for it, but now only one is true (and may not be for long):
-       1: The process is moving from _S to _M so the return path to userspace won't
-       happen (and sort of will on the new core / the other side), but that will
-       happen when popping into userspace.
-
-For these reasons, core_request() needs to have an edible reference.
-
-Also, since core_request calls functions that might not return, there are cases
-where it will not be able to call abandon_core() and leave process context.
-This is an example of why we have the fallback case of leaving process context
-in proc_startcore().  See the section below about process context for more
-information.
-
-Eventually, core_request() will be split better, probably with the brutal logic
-in process.c that would call out some functions in resource.c that actually make
-choices.
-
-1.9 Things I Could Have Done But Didn't And Why:
+1.8 Things I Could Have Done But Didn't And Why:
 ---------------------------
 Q: Could we have the first reference (existence) mean it could be on the runnable
 list or otherwise in the proc system (but not other subsystems)?  In this case,
index e27196c..522dabe 100644 (file)
@@ -124,10 +124,13 @@ ssize_t core_request(struct proc *p)
                                 * state. */
                                struct preempt_data *vcpd = &p->procdata->vcore_preempt_data[0];
                                disable_irqsave(&state);        /* protect cur_tf */
+                               /* Note this won't play well with concurrent proc kmsgs, but
+                                * since we're _S and locked, we shouldn't have any. */
                                assert(current_tf);
                                vcpd->preempt_tf = *current_tf;
-                               enable_irqsave(&state);
+                               current_tf = 0;                         /* so we don't restart */
                                save_fp_state(&vcpd->preempt_anc);
+                               enable_irqsave(&state);
                                __seq_start_write(&vcpd->preempt_tf_valid);
                                /* If we remove this, vcore0 will start where the _S left off */
                                vcpd->notif_pending = TRUE;
@@ -164,18 +167,11 @@ ssize_t core_request(struct proc *p)
                __proc_give_cores(p, corelist, num_granted);
                spin_unlock(&p->proc_lock);
                /* if there's a race on state (like DEATH), it'll get handled by
-                * proc_run or proc_destroy */
+                * proc_run or proc_destroy.  TODO: Theoretical race here, since someone
+                * else could make p an _S (in theory), and then we would be calling
+                * this with an inedible ref (which is currently a concern). */
                if (p->state == PROC_RUNNABLE_M)
                        proc_run(p);    /* I dislike this - caller should run it */
-               /* if we are moving to a partitionable core from a RUNNING_S on a
-                * management core, the kernel needs to do something else on this core
-                * (just like in proc_destroy).  it also needs to decref, to consume the
-                * reference that came into this function (since we don't return).  */
-               if (need_to_idle) {
-                       proc_decref(p);
-                       abandon_core();
-                       smp_idle();
-               }
        } else { // nothing granted, just return
                spin_unlock(&p->proc_lock);
        }
index 303e584..c9c664d 100644 (file)
@@ -657,13 +657,10 @@ static int sys_shared_page_free(env_t* p1, void*DANGEROUS addr, pid_t p2)
 static int sys_resource_req(struct proc *p, int type, unsigned int amt_wanted,
                             unsigned int amt_wanted_min, int flags)
 {
-       int retval;
-       finish_current_sysc(0);
-       /* this might not return (if it's a _S -> _M transition) */
-       proc_incref(p, 1);
-       retval = resource_req(p, type, amt_wanted, amt_wanted_min, flags);
-       proc_decref(p);
-       return retval;
+       /* resource_req returns and we'll eventually finish the sysc later.  The
+        * original context may restart on a remote core before we return and
+        * finish, but that's fine thanks to the async kernel interface. */
+       return resource_req(p, type, amt_wanted, amt_wanted_min, flags);
 }
 
 /* Untested.  Will notify the target on the given vcore, if the caller controls