Allow ktasks to switch_to()
authorBarret Rhoden <brho@cs.berkeley.edu>
Mon, 7 Dec 2015 21:40:00 +0000 (16:40 -0500)
committerBarret Rhoden <brho@cs.berkeley.edu>
Thu, 10 Dec 2015 16:22:24 +0000 (11:22 -0500)
We had been doing this for a while, but it was buggy.

Ktasks that did a switch_to and then blocked could come back and be in a
different address space, since sem_down()/restart_kthread() didn't know
that suddenly a ktask cared about its address space.  One possibility is
that the kernel would wildly write into the wrong process's address space.

This also may have been a source of bugs where processes wouldn't die
(which is something I found while debugging it).  It sounds familiar; SCPs
not dying under heavy network load to MCPs (IIRC).

Here's the scenario as of a few patches ago:
- A ktask does a switch_to.  old_proc = 0, current = new_proc.  There's no
  refcnting involved.
- The ktask blocks.  Since it was a ktask, in sem_down() kth->proc = 0
- Something else runs, say process 6.  That takes an IRQ that restarts the
  ktask.  That process is still current, and restart_kthread is fine with
that (since ktasks usually ignore whatever process they are in).  Still no
changes to refcnts, and PID 6 is still current.
- The ktask calls switch_back(new_proc, old_proc).  This sets current =
  old_proc, clobbering PID 6's current.  That was a ref counted pointer,
which means we now have an extra ref on PID 6 that will never go away.
That means PID 6 will be stuck DYING forever.

switch_back() had an assumption that new == old, which wasn't playing well
with sem_down's decision to save a ref on current, and restart_kthread's
decision about when to replace current.

Signed-off-by: Barret Rhoden <brho@cs.berkeley.edu>
kern/src/process.c

index a1e80b0..17f5180 100644 (file)
@@ -1797,7 +1797,9 @@ void clear_owning_proc(uint32_t coreid)
 uintptr_t switch_to(struct proc *new_p)
 {
        struct per_cpu_info *pcpui = &per_cpu_info[core_id()];
+       struct kthread *kth = pcpui->cur_kthread;
        struct proc *old_proc;
+       uintptr_t ret;
 
        old_proc = pcpui->cur_proc;                                     /* uncounted ref */
        /* If we aren't the proc already, then switch to it */
@@ -1808,7 +1810,16 @@ uintptr_t switch_to(struct proc *new_p)
                else
                        lcr3(boot_cr3);
        }
-       return (uintptr_t)old_proc;
+       ret = (uintptr_t)old_proc;
+       if (is_ktask(kth)) {
+               if (!(kth->flags & KTH_SAVE_ADDR_SPACE)) {
+                       kth->flags |= KTH_SAVE_ADDR_SPACE;
+                       /* proc pointers are aligned; we can use the lower bit as a signal
+                        * to turn off SAVE_ADDR_SPACE. */
+                       ret |= 0x1;
+               }
+       }
+       return ret;
 }
 
 /* This switches back from new_p to the original process.  Pair it with
@@ -1816,8 +1827,16 @@ uintptr_t switch_to(struct proc *new_p)
 void switch_back(struct proc *new_p, uintptr_t old_ret)
 {
        struct per_cpu_info *pcpui = &per_cpu_info[core_id()];
-       struct proc *old_proc = (struct proc*)old_ret;
+       struct kthread *kth = pcpui->cur_kthread;
+       struct proc *old_proc;
 
+       if (is_ktask(kth)) {
+               if (old_ret & 0x1) {
+                       kth->flags &= ~KTH_SAVE_ADDR_SPACE;
+                       old_ret &= ~0x1;
+               }
+       }
+       old_proc = (struct proc*)old_ret;
        if (old_proc != new_p) {
                pcpui->cur_proc = old_proc;
                if (old_proc)