__proc_give_cores() can be called multiple times
authorBarret Rhoden <brho@cs.berkeley.edu>
Sat, 25 Feb 2012 01:14:26 +0000 (17:14 -0800)
committerBarret Rhoden <brho@cs.berkeley.edu>
Tue, 28 Feb 2012 01:53:31 +0000 (17:53 -0800)
Once you're done giving cores, call __proc_run_m() to actually start it
up.  This patch moves the sending of the bulk preempt list events to
right before we run, so that you can give out small groups of cores
without having to run after each handout.  Check out core_request() for
an example.

This also moves the running of _Ms out of the ksched, though
core_request() is pretty close to the ksched anyway.  Regardless,
schedulers no longer need to track the difference between a RUNNABLE_M
and a RUNNING_M.

kern/src/process.c
kern/src/resource.c
kern/src/schedule.c

index 88049d7..148fc6c 100644 (file)
@@ -464,6 +464,32 @@ void proc_run_s(struct proc *p)
        }
 }
 
+/* Helper: sends preempt messages to all vcores on the bulk preempt list, and
+ * moves them to the inactive list. */
+static void __send_bulkp_events(struct proc *p)
+{
+       struct vcore *vc_i, *vc_temp;
+       struct event_msg preempt_msg = {0};
+       /* Send preempt messages for any left on the BP list.  No need to set any
+        * flags, it all was done on the real preempt.  Now we're just telling the
+        * process about any that didn't get restarted and are still preempted. */
+       TAILQ_FOREACH_SAFE(vc_i, &p->bulk_preempted_vcs, list, vc_temp) {
+               /* Note that if there are no active vcores, send_k_e will post to our
+                * own vcore, the last of which will be put on the inactive list and be
+                * the first to be started.  We could have issues with deadlocking,
+                * since send_k_e() could grab the proclock (if there are no active
+                * vcores) */
+               preempt_msg.ev_type = EV_VCORE_PREEMPT;
+               preempt_msg.ev_arg2 = vcore2vcoreid(p, vc_i);   /* arg2 is 32 bits */
+               send_kernel_event(p, &preempt_msg, 0);
+               /* TODO: we may want a TAILQ_CONCAT_HEAD, or something that does that.
+                * We need a loop for the messages, but not necessarily for the list
+                * changes.  */
+               TAILQ_REMOVE(&p->bulk_preempted_vcs, vc_i, list);
+               TAILQ_INSERT_HEAD(&p->inactive_vcs, vc_i, list);
+       }
+}
+
 /* Run an _M.  Can be called safely on one that is already running.  Hold the
  * lock before calling.  Other than state checks, this just starts up the _M's
  * vcores, much like the second part of give_cores_running.  More specifically,
@@ -485,6 +511,7 @@ void __proc_run_m(struct proc *p)
                         * this process.  It is set outside proc_run.  For the kernel
                         * message, a0 = struct proc*, a1 = struct trapframe*.   */
                        if (p->procinfo->num_vcores) {
+                               __send_bulkp_events(p);
                                __proc_set_state(p, PROC_RUNNING_M);
                                /* Up the refcnt, to avoid the n refcnt upping on the
                                 * destination cores.  Keep in sync with __startcore */
@@ -1182,15 +1209,10 @@ static bool __proc_give_a_pcore(struct proc *p, uint32_t pcore,
 static void __proc_give_cores_runnable(struct proc *p, uint32_t *pc_arr,
                                        uint32_t num)
 {
-       struct vcore *vc_i, *vc_temp;
-       struct event_msg preempt_msg = {0};
-       /* They shouldn't have any vcores yet.  One issue with allowing multiple
-        * calls to _give_cores_ is that the bulk preempt list needs to be handled
-        * in one shot. */
-       assert(!p->procinfo->num_vcores);
+       assert(p->state == PROC_RUNNABLE_M);
        assert(num);    /* catch bugs */
        /* add new items to the vcoremap */
-       __seq_start_write(&p->procinfo->coremap_seqctr);
+       __seq_start_write(&p->procinfo->coremap_seqctr);/* unncessary if offline */
        p->procinfo->num_vcores += num;
        for (int i = 0; i < num; i++) {
                /* Try from the bulk list first */
@@ -1202,24 +1224,6 @@ static void __proc_give_cores_runnable(struct proc *p, uint32_t *pc_arr,
                assert(__proc_give_a_pcore(p, pc_arr[i], &p->inactive_vcs));
        }
        __seq_end_write(&p->procinfo->coremap_seqctr);
-       /* Send preempt messages for any left on the BP list.  No need to set any
-        * flags, it all was done on the real preempt.  Now we're just telling the
-        * process about any that didn't get restarted and are still preempted. */
-       TAILQ_FOREACH_SAFE(vc_i, &p->bulk_preempted_vcs, list, vc_temp) {
-               /* Note that if there are no active vcores, send_k_e will post to our
-                * own vcore, the last of which will be put on the inactive list and be
-                * the first to be started.  We don't have to worry too much, since
-                * we're holding the proc lock */
-               preempt_msg.ev_type = EV_VCORE_PREEMPT;
-               preempt_msg.ev_arg2 = vcore2vcoreid(p, vc_i);   /* arg2 is 32 bits */
-               send_kernel_event(p, &preempt_msg, 0);
-               /* TODO: we may want a TAILQ_CONCAT_HEAD, or something that does that.
-                * We need a loop for the messages, but not necessarily for the list
-                * changes.  */
-               TAILQ_REMOVE(&p->bulk_preempted_vcs, vc_i, list);
-               /* TODO: put on the bulk preempt list, if applicable */
-               TAILQ_INSERT_HEAD(&p->inactive_vcs, vc_i, list);
-       }
 }
 
 static void __proc_give_cores_running(struct proc *p, uint32_t *pc_arr,
index c6d33c6..d676cba 100644 (file)
@@ -79,6 +79,11 @@ bool core_request(struct proc *p)
        if (num_granted) {
                /* give them the cores.  this will start up the extras if RUNNING_M. */
                __proc_give_cores(p, corelist, num_granted);
+               /* at some point after giving cores, call proc_run_m() (harmless on
+                * RUNNING_Ms).  You can give small groups of cores, then run them
+                * (which is more efficient than interleaving runs with the gives for
+                * bulk preempted processes). */
+               __proc_run_m(p); /* harmless to call this on RUNNING_Ms */
                spin_unlock(&p->proc_lock);
                return TRUE;    /* proc can run (if it isn't already) */
        }
index a64e9ba..a464058 100644 (file)
@@ -137,15 +137,6 @@ void schedule(void)
                        /* TODO: this interface sucks, change it */
                        if (!core_request(p))
                                schedule_proc(p);       /* can't run, put it back on the queue */
-                       else
-                               /* if there's a race on state (like DEATH), it'll get handled by
-                                * 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). */
-                               spin_lock(&p->proc_lock);
-                               __proc_run_m(p); /* trying to run a RUNNABLE_M here */
-                               spin_unlock(&p->proc_lock);
                } else {
                        /* _S proc, just run it */
                        proc_run_s(p);