Lock ordering and ksched callbacks
authorBarret Rhoden <brho@cs.berkeley.edu>
Mon, 14 May 2012 20:43:24 +0000 (13:43 -0700)
committerBarret Rhoden <brho@cs.berkeley.edu>
Wed, 5 Sep 2012 21:43:58 +0000 (14:43 -0700)
For now, there is no ordering between the proc lock and the ksched lock.
This commit revises a bunch of the scheduler callbacks (all clearly
prefixed with __sched).  These CBs can be triggered from lots of places,
specifically any proc code, any event code, and any code that kills a
process (like a PF handler!).

Since these grab locks, we needed to make the ksched let go of its lock
before calling into proc/event code (preempt_core() triggers events,
among other things).

This also moves the proc functions (like destroy) back into proc code,
since we no longer need to lock in the ksched before calling proc code.

We also can get rid of the 'ignore_next_idle'.  For now, we just wait
til the core has been given back in __core_req (no deadlock now, just a
race), instead of knowing it will be idle soon and circumventing the CB.
It's a toss-up between these two solutions.

kern/include/process.h
kern/include/schedule.h
kern/src/monitor.c
kern/src/process.c
kern/src/schedule.c

index 9a4ea82..85d63e6 100644 (file)
@@ -71,12 +71,12 @@ void proc_decref(struct proc *p);
 void proc_run_s(struct proc *p);
 void __proc_run_m(struct proc *p);
 void proc_restartcore(void);
-bool __proc_destroy(struct proc *p, uint32_t *pc_arr, uint32_t *nr_revoked);
-int __proc_change_to_m(struct proc *p);
+void proc_destroy(struct proc *p);
+int proc_change_to_m(struct proc *p);
 void __proc_save_context_s(struct proc *p, struct trapframe *tf);
 void proc_yield(struct proc *SAFE p, bool being_nice);
 void proc_notify(struct proc *p, uint32_t vcoreid);
-void __proc_wakeup(struct proc *p);
+void proc_wakeup(struct proc *p);
 bool __proc_is_mcp(struct proc *p);
 void proc_change_to_vcore(struct proc *p, uint32_t new_vcoreid,
                           bool enable_my_notif);
index a23aaa9..55fbf51 100644 (file)
@@ -23,7 +23,6 @@ struct sched_pcore {
        TAILQ_ENTRY(sched_pcore)        alloc_next;                     /* on an alloc list (idle)*/
        struct proc                                     *prov_proc;                     /* who this is prov to */
        struct proc                                     *alloc_proc;            /* who this is alloc to */
-       unsigned int                            ignore_next_idle;       /* helps with lock order */
 };
 TAILQ_HEAD(sched_pcore_tailq, sched_pcore);
 
@@ -39,20 +38,24 @@ struct sched_proc_data {
 
 void schedule_init(void);
 
-/************** Process management **************/
+/************** Process Management Callbacks **************/
 /* Tell the ksched about the process, which it will track cradle-to-grave */
-void register_proc(struct proc *p);
+void __sched_proc_register(struct proc *p);
 
-/* Makes sure p is runnable.  Callers include event delivery, SCP yield, and new
- * SCPs.  Will trigger the __sched_.cp_wakeup() callbacks. */
-void proc_wakeup(struct proc *p);
+/* The proc was an SCP and is becoming an MCP */
+void __sched_proc_change_to_m(struct proc *p);
 
-/* The ksched starts the death process (lock ordering issue), which calls back
- * to proc.c's __proc_destroy while holding the locks (or whatever) */
-void proc_destroy(struct proc *p);
+/* The proc is dying */
+void __sched_proc_destroy(struct proc *p, uint32_t *pc_arr, uint32_t nr_cores);
 
-/* Changes the proc from an SCP to an MCP */
-int proc_change_to_m(struct proc *p);
+/* Makes sure p is runnable. */
+void __sched_mcp_wakeup(struct proc *p);
+void __sched_scp_wakeup(struct proc *p);
+
+/* Gets called when a pcore becomes idle (like in proc yield).  These are 'cg'
+ * cores, given to MCPs, that have been async returned to the ksched. */
+void __sched_put_idle_core(struct proc *p, uint32_t coreid);
+void __sched_put_idle_cores(struct proc *p, uint32_t *pc_arr, uint32_t num);
 
 /************** Decision making **************/
 /* Call the main scheduling algorithm.  Not clear yet if the main kernel will
@@ -64,21 +67,11 @@ void schedule(void);
  * be careful of abuse. */
 void poke_ksched(struct proc *p, int res_type);
 
-/* Callbacks triggered from proc_wakeup() */
-void __sched_mcp_wakeup(struct proc *p);
-void __sched_scp_wakeup(struct proc *p);
-
 /* The calling cpu/core has nothing to do and plans to idle/halt.  This is an
  * opportunity to pick the nature of that halting (low power state, etc), or
  * provide some other work (_Ss on LL cores). */
 void cpu_bored(void);
 
-/* Gets called when a pcore becomes idle (like in proc yield).  These are 'cg'
- * cores, given to MCPs, that have been async returned to the ksched.  If the
- * ksched preempts a core, this won't get called (unless it yielded first). */
-void put_idle_core(struct proc *p, uint32_t coreid);
-void put_idle_cores(struct proc *p, uint32_t *pc_arr, uint32_t num);
-
 /* Available resources changed (plus or minus).  Some parts of the kernel may
  * call this if a particular resource that is 'quantity-based' changes.  Things
  * like available RAM to processes, bandwidth, etc.  Cores would probably be
index a50b229..adbfbdf 100644 (file)
@@ -528,7 +528,7 @@ int mon_measure(int argc, char *NTS *NT COUNT(argc) argv, trapframe_t *tf)
                        uint32_t pcoreid = strtol(argv[3], 0, 0);
                        begin = start_timing();
                        if (proc_preempt_core(p, pcoreid, 1000000)) {
-                               put_idle_core(p, pcoreid);
+                               __sched_put_idle_core(p, pcoreid);
                                /* done when unmapped (right before abandoning) */
                                spin_on(p->procinfo->pcoremap[pcoreid].valid);
                        } else {
@@ -606,7 +606,7 @@ int mon_measure(int argc, char *NTS *NT COUNT(argc) argv, trapframe_t *tf)
                                __proc_set_state(p, PROC_RUNNABLE_M);
                        spin_unlock(&p->proc_lock);
                        /* ghetto, since the ksched should be calling all of this */
-                       put_idle_core(p, pcoreid);
+                       __sched_put_idle_core(p, pcoreid);
                        /* done when unmapped (right before abandoning) */
                        spin_on(p->procinfo->pcoremap[pcoreid].valid);
                        diff = stop_timing(begin);
@@ -620,7 +620,7 @@ int mon_measure(int argc, char *NTS *NT COUNT(argc) argv, trapframe_t *tf)
                        __proc_set_state(p, PROC_RUNNABLE_M);
                        spin_unlock(&p->proc_lock);
                        if (num_revoked)
-                               put_idle_cores(p, pc_arr, num_revoked);
+                               __sched_put_idle_cores(p, pc_arr, num_revoked);
                        /* a little ghetto, implies no one else is using p */
                        spin_on(kref_refcnt(&p->p_kref) != end_refcnt);
                        diff = stop_timing(begin);
index f939df8..dc075c2 100644 (file)
@@ -307,8 +307,9 @@ error_t proc_alloc(struct proc **pp, struct proc *parent)
  * push setting the state to CREATED into here. */
 void __proc_ready(struct proc *p)
 {
-       /* Tell the ksched about us */
-       register_proc(p);
+       /* Tell the ksched about us.  TODO: do we need to worry about the ksched
+        * doing stuff to us before we're added to the pid_hash? */
+       __sched_proc_register(p);
        spin_lock(&pid_hash_lock);
        hashtable_insert(pid_hash, (void*)(long)p->pid, p);
        spin_unlock(&pid_hash_lock);
@@ -662,12 +663,8 @@ void proc_restartcore(void)
        __proc_startcore(pcpui->owning_proc, pcpui->cur_tf);
 }
 
-/* Destroys the process.  This should be called by the ksched, which needs to
- * hold the lock.  It will destroy the process and return any cores allocated to
- * the proc via pc_arr and nr_revoked.  It's up to the caller to have enough
- * space for pc_arr.  This will return TRUE if we successfully killed it, FALSE
- * otherwise.  Failure isn't a big deal either - it can happen due to concurrent
- * calls to proc_destroy. 
+/* Destroys the process.  It will destroy the process and return any cores
+ * to the ksched via the __sched_proc_destroy() CB.
  *
  * Here's the way process death works:
  * 0. grab the lock (protects state transition and core map)
@@ -687,23 +684,26 @@ void proc_restartcore(void)
  * come in, making you abandon_core, as if you weren't running.  It may be that
  * the only reference to p is the one you passed in, and when you decref, it'll
  * get __proc_free()d. */
-bool __proc_destroy(struct proc *p, uint32_t *pc_arr, uint32_t *nr_revoked)
+void proc_destroy(struct proc *p)
 {
+       uint32_t nr_cores_revoked = 0;
        struct kthread *sleeper;
+       spin_lock(&p->proc_lock);
+       /* storage for pc_arr is alloced at decl, which is after grabbing the lock*/
+       uint32_t pc_arr[p->procinfo->num_vcores];
        switch (p->state) {
-               case PROC_DYING: // someone else killed this already.
-                       return FALSE;
-               case PROC_RUNNABLE_M:
-                       /* Need to reclaim any cores this proc might have, even though it's
-                        * not running yet. */
-                       *nr_revoked = __proc_take_allcores(p, pc_arr, FALSE);
-                       // fallthrough
+               case PROC_DYING: /* someone else killed this already. */
+                       spin_unlock(&p->proc_lock);
+                       return;
+               case PROC_CREATED:
                case PROC_RUNNABLE_S:
-                       /* might need to pull from lists, though i'm currently a fan of the
-                        * model where external refs notice DYING (if it matters to them)
-                        * and decref when they are done.  the ksched will notice the proc
-                        * is dying and handle it accordingly (which delay the reaping til
-                        * the next call to schedule()) */
+               case PROC_WAITING:
+                       break;
+               case PROC_RUNNABLE_M:
+               case PROC_RUNNING_M:
+                       /* Need to reclaim any cores this proc might have, even if it's not
+                        * running yet.  Those running will receive a __death */
+                       nr_cores_revoked = __proc_take_allcores(p, pc_arr, FALSE);
                        break;
                case PROC_RUNNING_S:
                        #if 0
@@ -723,45 +723,43 @@ bool __proc_destroy(struct proc *p, uint32_t *pc_arr, uint32_t *nr_revoked)
                        /* If we ever have RUNNING_S run on non-mgmt cores, we'll need to
                         * tell the ksched about this now-idle core (after unlocking) */
                        break;
-               case PROC_RUNNING_M:
-                       /* Send the DEATH message to every core running this process, and
-                        * deallocate the cores.
-                        * The rule is that the vcoremap is set before proc_run, and reset
-                        * within proc_destroy */
-                       *nr_revoked = __proc_take_allcores(p, pc_arr, FALSE);
-                       break;
-               case PROC_CREATED:
-                       break;
                default:
                        warn("Weird state(%s) in %s()", procstate2str(p->state),
                             __FUNCTION__);
-                       return FALSE;
+                       spin_unlock(&p->proc_lock);
+                       return;
        }
        /* At this point, a death IPI should be on its way, either from the
         * RUNNING_S one, or from proc_take_cores with a __death.  in general,
         * interrupts should be on when you call proc_destroy locally, but currently
         * aren't for all things (like traphandlers). */
        __proc_set_state(p, PROC_DYING);
+       spin_unlock(&p->proc_lock);
        /* This prevents processes from accessing their old files while dying, and
         * will help if these files (or similar objects in the future) hold
-        * references to p (preventing a __proc_free()). */
+        * references to p (preventing a __proc_free()).  Need to unlock before
+        * doing this - the proclock doesn't protect the files (not proc state), and
+        * closing these might block (can't block while spinning). */
+       /* TODO: might need some sync protection */
        close_all_files(&p->open_files, FALSE);
+       /* Tell the ksched about our death, and which cores we freed up */
+       __sched_proc_destroy(p, pc_arr, nr_cores_revoked);
        /* Signal our state change.  Assuming we only have one waiter right now. */
        sleeper = __up_sem(&p->state_change, TRUE);
        if (sleeper)
                kthread_runnable(sleeper);
-       return TRUE;
 }
 
 /* Turns *p into an MCP.  Needs to be called from a local syscall of a RUNNING_S
- * process.  Returns 0 if it succeeded, an error code otherwise.  You should
- * hold the lock before calling. */
-int __proc_change_to_m(struct proc *p)
+ * process.  Returns 0 if it succeeded, an error code otherwise. */
+int proc_change_to_m(struct proc *p)
 {
+       int retval = 0;
        int8_t state = 0;
+       spin_lock(&p->proc_lock);
        /* in case userspace erroneously tries to change more than once */
        if (__proc_is_mcp(p))
-               return -EINVAL;
+               goto error_out;
        switch (p->state) {
                case (PROC_RUNNING_S):
                        /* issue with if we're async or not (need to preempt it)
@@ -802,21 +800,26 @@ int __proc_change_to_m(struct proc *p)
                        /* change to runnable_m (it's TF is already saved) */
                        __proc_set_state(p, PROC_RUNNABLE_M);
                        p->procinfo->is_mcp = TRUE;
-                       break;
+                       spin_unlock(&p->proc_lock);
+                       /* Tell the ksched that we're a real MCP now! */
+                       __sched_proc_change_to_m(p);
+                       return 0;
                case (PROC_RUNNABLE_S):
                        /* Issues: being on the runnable_list, proc_set_state not liking
                         * it, and not clearly thinking through how this would happen.
                         * Perhaps an async call that gets serviced after you're
                         * descheduled? */
                        warn("Not supporting RUNNABLE_S -> RUNNABLE_M yet.\n");
-                       return -EINVAL;
+                       goto error_out;
                case (PROC_DYING):
                        warn("Dying, core request coming from %d\n", core_id());
-                       return -EINVAL;
+                       goto error_out;
                default:
-                       return -EINVAL;
+                       goto error_out;
        }
-       return 0;
+error_out:
+       spin_unlock(&p->proc_lock);
+       return -EINVAL;
 }
 
 /* Old code to turn a RUNNING_M to a RUNNING_S, with the calling context
@@ -1047,7 +1050,7 @@ void proc_yield(struct proc *SAFE p, bool being_nice)
        }
        spin_unlock(&p->proc_lock);
        /* Hand the now-idle core to the ksched */
-       put_idle_core(p, pcoreid);
+       __sched_put_idle_core(p, pcoreid);
        goto out_yield_core;
 out_failed:
        /* for some reason we just want to return, either to take a KMSG that cleans
@@ -1058,7 +1061,7 @@ out_failed:
 out_yield_core:                                /* successfully yielded the core */
        proc_decref(p);                 /* need to eat the ref passed in */
        /* Clean up the core and idle.  Need to do this before enabling interrupts,
-        * since once we put_idle_core() and unlock, we could get a startcore. */
+        * since once we call __sched_put_idle_core(), we could get a startcore. */
        clear_owning_proc(pcoreid);     /* so we don't restart */
        abandon_core();
        smp_idle();                             /* will reenable interrupts */
@@ -1092,25 +1095,24 @@ void proc_notify(struct proc *p, uint32_t vcoreid)
        }
 }
 
-/* Makes sure p is runnable.  May be spammed, via the ksched.  Called only by
- * the ksched when it holds the ksched lock (or whatever).  We need to lock both
- * the ksched and the proc at some point, so we need to start this call in the
- * ksched (lock ordering).
- *
- * Will call back to the ksched via one of the __sched_.cp_wakeup() calls. */
-void __proc_wakeup(struct proc *p)
+/* Makes sure p is runnable.  Callers may spam this, so it needs to handle
+ * repeated calls for the same event.  Callers include event delivery, SCP
+ * yield, and new SCPs.  Will trigger __sched_.cp_wakeup() CBs.  Will only
+ * trigger the CB once, regardless of how many times we are called, *until* the
+ * proc becomes WAITING again, presumably because of something the ksched did.*/
+void proc_wakeup(struct proc *p)
 {
        spin_lock(&p->proc_lock);
        if (__proc_is_mcp(p)) {
                /* we only wake up WAITING mcps */
-               if (p->state != PROC_WAITING)
-                       goto out_unlock;
-               if (!p->procdata->res_req[RES_CORES].amt_wanted)
-                       p->procdata->res_req[RES_CORES].amt_wanted = 1;
+               if (p->state != PROC_WAITING) {
+                       spin_unlock(&p->proc_lock);
+                       return;
+               }
                __proc_set_state(p, PROC_RUNNABLE_M);
                spin_unlock(&p->proc_lock);
                __sched_mcp_wakeup(p);
-               goto out;
+               return;
        } else {
                /* SCPs can wake up for a variety of reasons.  the only times we need
                 * to do something is if it was waiting or just created.  other cases
@@ -1123,22 +1125,19 @@ void __proc_wakeup(struct proc *p)
                        case (PROC_RUNNABLE_S):
                        case (PROC_RUNNING_S):
                        case (PROC_DYING):
-                               goto out_unlock;
+                               spin_unlock(&p->proc_lock);
+                               return;
                        case (PROC_RUNNABLE_M):
                        case (PROC_RUNNING_M):
                                warn("Weird state(%s) in %s()", procstate2str(p->state),
                                     __FUNCTION__);
-                               goto out_unlock;
+                               spin_unlock(&p->proc_lock);
+                               return;
                }
                printd("[kernel] FYI, waking up an _S proc\n"); /* thanks, past brho! */
                spin_unlock(&p->proc_lock);
                __sched_scp_wakeup(p);
-               goto out;
        }
-out_unlock:
-       spin_unlock(&p->proc_lock);
-out:
-       return;
 }
 
 /* Is the process in multi_mode / is an MCP or not?  */
@@ -1279,7 +1278,7 @@ void proc_preempt_all(struct proc *p, uint64_t usec)
        /* TODO: when we revise this func, look at __put_idle */
        /* Return the cores to the ksched */
        if (num_revoked)
-               put_idle_cores(p, pc_arr, num_revoked);
+               __sched_put_idle_cores(p, pc_arr, num_revoked);
 }
 
 /* Give the specific pcore to proc p.  Lots of assumptions, so don't really use
index 63acd9a..eda43f2 100644 (file)
@@ -153,6 +153,16 @@ void schedule_init(void)
        return;
 }
 
+static uint32_t spc2pcoreid(struct sched_pcore *spc)
+{
+       return spc - all_pcores;
+}
+
+static struct sched_pcore *pcoreid2spc(uint32_t pcoreid)
+{
+       return &all_pcores[pcoreid];
+}
+
 /* Round-robins on whatever list it's on */
 static void add_to_list(struct proc *p, struct proc_list *new)
 {
@@ -186,8 +196,19 @@ static void remove_from_any_list(struct proc *p)
        TAILQ_REMOVE(p->ksched_data.cur_list, p, ksched_data.proc_link);
 }
 
-void register_proc(struct proc *p)
+/************** Process Management Callbacks **************/
+/* a couple notes:
+ * - the proc lock is NOT held for any of these calls.  currently, there is no
+ *   lock ordering between the sched lock and the proc lock.  since the proc
+ *   code doesn't know what we do, it doesn't hold its lock when calling our
+ *   CBs.
+ * - since the proc lock isn't held, the proc could be dying, which means we
+ *   will receive a __sched_proc_destroy() either before or after some of these
+ *   other CBs.  the CBs related to list management need to check and abort if
+ *   DYING */
+void __sched_proc_register(struct proc *p)
 {
+       assert(p->state != PROC_DYING); /* shouldn't be abel to happen yet */
        /* one ref for the proc's existence, cradle-to-grave */
        proc_incref(p, 1);      /* need at least this OR the 'one for existing' */
        spin_lock(&sched_lock);
@@ -198,21 +219,15 @@ void register_proc(struct proc *p)
 }
 
 /* Returns 0 if it succeeded, an error code otherwise. */
-int proc_change_to_m(struct proc *p)
+void __sched_proc_change_to_m(struct proc *p)
 {
-       int retval;
        spin_lock(&sched_lock);
-       /* Should only be necessary to lock around the change_to_m call.  It's
-        * definitely necessary to hold the sched lock the whole time - need to
-        * atomically change the proc's state and have the ksched take action (and
-        * not squeeze a proc_destroy in there or something). */
-       spin_lock(&p->proc_lock);
-       retval = __proc_change_to_m(p);
-       spin_unlock(&p->proc_lock);
-       if (retval) {
-               /* Failed for some reason. */
+       /* Need to make sure they aren't dying.  if so, we already dealt with their
+        * list membership, etc (or soon will).  taking advantage of the 'immutable
+        * state' of dying (so long as refs are held). */
+       if (p->state == PROC_DYING) {
                spin_unlock(&sched_lock);
-               return retval;
+               return;
        }
        /* Catch user bugs */
        if (!p->procdata->res_req[RES_CORES].amt_wanted) {
@@ -226,34 +241,9 @@ int proc_change_to_m(struct proc *p)
        add_to_list(p, primary_mcps);
        spin_unlock(&sched_lock);
        //poke_ksched(p, RES_CORES);
-       return retval;
 }
 
-/* Makes sure p is runnable.  Callers may spam this, so it needs to handle
- * repeated calls for the same event.  Callers include event delivery, SCP
- * yield, and new SCPs.  Most every scheduler should do something like this -
- * grab whatever lock you have, then call the proc helper. */
-void proc_wakeup(struct proc *p)
-{
-       /* catch current shitty deadlock... */
-       assert(!per_cpu_info[core_id()].lock_depth);
-       spin_lock(&sched_lock);
-       /* will trigger one of the __sched_.cp_wakeup()s */
-       __proc_wakeup(p);
-       spin_unlock(&sched_lock);
-}
-
-static uint32_t spc2pcoreid(struct sched_pcore *spc)
-{
-       return spc - all_pcores;
-}
-
-static struct sched_pcore *pcoreid2spc(uint32_t pcoreid)
-{
-       return &all_pcores[pcoreid];
-}
-
-/* Helper for proc destroy: unprovisions any pcores for the given list */
+/* Helper for the destroy CB : unprovisions any pcores for the given list */
 static void unprov_pcore_list(struct sched_pcore_tailq *list_head)
 {
        struct sched_pcore *spc_i;
@@ -267,39 +257,97 @@ static void unprov_pcore_list(struct sched_pcore_tailq *list_head)
        TAILQ_INIT(list_head);
 }
 
-/* Destroys the given process.  This may be called from another process, a light
- * kernel thread (no real process context), asynchronously/cross-core, or from
- * the process on its own core.
+/* Sched callback called when the proc dies.  pc_arr holds the cores the proc
+ * had, if any, and nr_cores tells us how many are in the array.
  *
  * An external, edible ref is passed in.  when we return and they decref,
- * __proc_free will be called */
-void proc_destroy(struct proc *p)
+ * __proc_free will be called (when the last one is done). */
+void __sched_proc_destroy(struct proc *p, uint32_t *pc_arr, uint32_t nr_cores)
 {
-       uint32_t nr_cores_revoked = 0;
        spin_lock(&sched_lock);
-       spin_lock(&p->proc_lock);
-       /* storage for pc_arr is alloced at decl, which is after grabbing the lock*/
-       uint32_t pc_arr[p->procinfo->num_vcores];
-       /* If this returns true, it means we successfully destroyed the proc */
-       if (__proc_destroy(p, pc_arr, &nr_cores_revoked)) {
-               /* Do our cleanup.  note that proc_free won't run since we have an
-                * external reference, passed in */
-               /* Unprovision any cores.  Note this is different than track_dealloc.
-                * The latter does bookkeeping when an allocation changes.  This is a
-                * bulk *provisioning* change. */
-               unprov_pcore_list(&p->ksched_data.prov_alloc_me);
-               unprov_pcore_list(&p->ksched_data.prov_not_alloc_me);
-               /* Remove from whatever list we are on */
-               remove_from_any_list(p);
-               /* Drop the cradle-to-the-grave reference, jet-li */
-               proc_decref(p);
-               if (nr_cores_revoked) {
-                       __put_idle_cores(p, pc_arr, nr_cores_revoked);
-                       __prov_track_dealloc_bulk(p, pc_arr, nr_cores_revoked);
-               }
+       /* Unprovision any cores.  Note this is different than track_dealloc.
+        * The latter does bookkeeping when an allocation changes.  This is a
+        * bulk *provisioning* change. */
+       unprov_pcore_list(&p->ksched_data.prov_alloc_me);
+       unprov_pcore_list(&p->ksched_data.prov_not_alloc_me);
+       /* Remove from whatever list we are on */
+       remove_from_any_list(p);
+       if (nr_cores) {
+               __put_idle_cores(p, pc_arr, nr_cores);
+               __prov_track_dealloc_bulk(p, pc_arr, nr_cores);
+       }
+       spin_unlock(&sched_lock);
+       /* Drop the cradle-to-the-grave reference, jet-li */
+       proc_decref(p);
+}
+
+/* ksched callbacks.  p just woke up and is UNLOCKED. */
+void __sched_mcp_wakeup(struct proc *p)
+{
+       spin_lock(&sched_lock);
+       if (p->state == PROC_DYING) {
+               spin_unlock(&sched_lock);
+               return;
        }
-       spin_unlock(&p->proc_lock);
+       /* could try and prioritize p somehow (move it to the front of the list).
+        * for now, just help them out a bit (mild help here, can remove this) */
+       if (!p->procdata->res_req[RES_CORES].amt_wanted)
+               p->procdata->res_req[RES_CORES].amt_wanted = 1;
        spin_unlock(&sched_lock);
+       /* note they could be dying at this point too. */
+       poke(&ksched_poker, p);
+}
+
+/* ksched callbacks.  p just woke up and is UNLOCKED. */
+void __sched_scp_wakeup(struct proc *p)
+{
+       spin_lock(&sched_lock);
+       if (p->state == PROC_DYING) {
+               spin_unlock(&sched_lock);
+               return;
+       }
+       /* might not be on a list if it is new.  o/w, it should be unrunnable */
+       __remove_from_any_list(p);
+       add_to_list(p, &runnable_scps);
+       spin_unlock(&sched_lock);
+}
+
+/* Callback to return a core to the ksched, which tracks it as idle and
+ * deallocated from p.  The proclock is held (__core_req depends on that).
+ *
+ * This also is a trigger, telling us we have more cores.  We could/should make
+ * a scheduling decision (or at least plan to). */
+void __sched_put_idle_core(struct proc *p, uint32_t coreid)
+{
+       struct sched_pcore *spc = pcoreid2spc(coreid);
+       spin_lock(&sched_lock);
+       TAILQ_INSERT_TAIL(&idlecores, spc, alloc_next);
+       __prov_track_dealloc(p, coreid);
+       spin_unlock(&sched_lock);
+}
+
+/* Helper for put_idle and core_req.  Note this does not track_dealloc.  When we
+ * get rid of / revise proc_preempt_all and put_idle_cores, we can get rid of
+ * this.  (the ksched will never need it - only external callers). */
+static void __put_idle_cores(struct proc *p, uint32_t *pc_arr, uint32_t num)
+{
+       struct sched_pcore *spc_i;
+       for (int i = 0; i < num; i++) {
+               spc_i = pcoreid2spc(pc_arr[i]);
+               TAILQ_INSERT_TAIL(&idlecores, spc_i, alloc_next);
+       }
+}
+
+/* Callback, bulk interface for put_idle.  Note this one also calls track_dealloc,
+ * which the internal version does not.  The proclock is held for this. */
+void __sched_put_idle_cores(struct proc *p, uint32_t *pc_arr, uint32_t num)
+{
+       spin_lock(&sched_lock);
+       /* TODO: when we revise this func, look at __put_idle */
+       __put_idle_cores(p, pc_arr, num);
+       __prov_track_dealloc_bulk(p, pc_arr, num);
+       spin_unlock(&sched_lock);
+       /* could trigger a sched decision here */
 }
 
 /* mgmt/LL cores should call this to schedule the calling core and give it to an
@@ -307,6 +355,7 @@ void proc_destroy(struct proc *p)
  * calling.  returns TRUE if it scheduled a proc. */
 static bool __schedule_scp(void)
 {
+       // TODO: sort out lock ordering (proc_run_s also locks)
        struct proc *p;
        uint32_t pcoreid = core_id();
        struct per_cpu_info *pcpui = &per_cpu_info[pcoreid];
@@ -462,21 +511,6 @@ void poke_ksched(struct proc *p, int res_type)
        poke(&ksched_poker, p);
 }
 
-/* ksched callbacks.  p just woke up, is unlocked, and the ksched lock is held */
-void __sched_mcp_wakeup(struct proc *p)
-{
-       /* could try and prioritize p somehow (move it to the front of the list) */
-       poke(&ksched_poker, p);
-}
-
-/* ksched callbacks.  p just woke up, is unlocked, and the ksched lock is held */
-void __sched_scp_wakeup(struct proc *p)
-{
-       /* might not be on a list if it is new.  o/w, it should be unrunnable */
-       __remove_from_any_list(p);
-       add_to_list(p, &runnable_scps);
-}
-
 /* The calling cpu/core has nothing to do and plans to idle/halt.  This is an
  * opportunity to pick the nature of that halting (low power state, etc), or
  * provide some other work (_Ss on LL cores).  Note that interrupts are
@@ -499,60 +533,6 @@ void cpu_bored(void)
         * the 'call of the giraffe' suffices. */
 }
 
-/* Externally called function to return a core to the ksched, which tracks it as
- * idle and deallocated from p.
- *
- * This also is a trigger, telling us we have more cores.  We could/should make
- * a scheduling decision (or at least plan to). */
-void put_idle_core(struct proc *p, uint32_t coreid)
-{
-       struct sched_pcore *spc = pcoreid2spc(coreid);
-       spin_lock(&sched_lock);
-       /* ignore_next_idle gets set if the ksched notices a core is not allocated
-        * before put_idle gets called.  This can happen if the proc yielded the
-        * core while the ksched is holding its lock (protecting lists), and the
-        * proc is spinning on the lock in this function, trying to give it back.
-        * When this happens, the core has already been 'given back', so we ignore
-        * the signal.  We're using a count instead of a bool for cases where this
-        * stacks (would require a change in provisioning, so it shouldn't happen
-        * for now). */
-       if (spc->ignore_next_idle) {
-               spc->ignore_next_idle--;
-       } else {
-               TAILQ_INSERT_TAIL(&idlecores, spc, alloc_next);
-               __prov_track_dealloc(p, coreid);
-       }
-       spin_unlock(&sched_lock);
-}
-
-/* Helper for put_idle and core_req.  Note this does not track_dealloc, but it
- * does handle ignore_next.  When we get rid of / revise proc_preempt_all and
- * put_idle_cores, we can get rid of this.  (the ksched will never need it -
- * only external callers). */
-static void __put_idle_cores(struct proc *p, uint32_t *pc_arr, uint32_t num)
-{
-       struct sched_pcore *spc_i;
-       for (int i = 0; i < num; i++) {
-               spc_i = pcoreid2spc(pc_arr[i]);
-               if (spc_i->ignore_next_idle)
-                       spc_i->ignore_next_idle--;
-               else
-                       TAILQ_INSERT_TAIL(&idlecores, spc_i, alloc_next);
-       }
-}
-
-/* External interface for put_idle.  Note this one also calls track_dealloc,
- * which the internal version does not. */
-void put_idle_cores(struct proc *p, uint32_t *pc_arr, uint32_t num)
-{
-       spin_lock(&sched_lock);
-       /* TODO: when we revise this func, look at __put_idle */
-       __put_idle_cores(p, pc_arr, num);
-       __prov_track_dealloc_bulk(p, pc_arr, num);
-       spin_unlock(&sched_lock);
-       /* could trigger a sched decision here */
-}
-
 /* Available resources changed (plus or minus).  Some parts of the kernel may
  * call this if a particular resource that is 'quantity-based' changes.  Things
  * like available RAM to processes, bandwidth, etc.  Cores would probably be
@@ -634,7 +614,8 @@ static void __core_request(struct proc *p, uint32_t amt_needed)
                                 * trigger) a track_dealloc and put it on the idle list.  our
                                 * signal for this is spc_i->alloc_proc being 0.  We need to
                                 * spin and let whoever is trying to free the core grab the
-                                * ksched lock.
+                                * ksched lock.  We could use an 'ignore_next_idle' flag per
+                                * sched_pcore, but it's not critical anymore.
                                 *
                                 * Note, we're relying on us being the only preemptor - if the
                                 * core was unmapped by *another* preemptor, there would be no