Fix abandon_core()'s halting-with-KMSG bug
authorBarret Rhoden <brho@cs.berkeley.edu>
Mon, 30 Jul 2018 19:31:25 +0000 (15:31 -0400)
committerBarret Rhoden <brho@cs.berkeley.edu>
Mon, 30 Jul 2018 20:06:25 +0000 (16:06 -0400)
This seemed like an RCU problem, but I think it was just an MCP + strace
bug.  Here's the scenario:

- straced MCP exits while another core executes a syscall that eventually
  calls smp_idle.
- There are a few cases where we can get in smp_idle, PRKM and get a
  __death, and still have current set (a process kref).
- PRKM, then try_run_proc(), then abandon_core().  If we had the last kref
  for the proc, we __proc_free().
- If the process was straced, __proc_free() will hangup the qio, which
  sends a message to the calling core to wake strace.
- When abandon_core() returns, it halts.

The code was built assuming (perhaps implicitly) that abandon_core() won't
trigger a kernel message.  That is no longer true.  strace is just one
example.  We could have others due to the cclose() called on 'dot.'
(perhaps if the process CD'd into a directory, of which there were no other
references, and the devtab.close method did something special).

You could trigger this with a bunch of different syscalls too.  We were
running a Go test, so I saw calls like proc_yield (slight red herring
there), poke_ksched, block, etc.

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

index 74948f7..ec98687 100644 (file)
@@ -159,7 +159,7 @@ void proc_preempt_all(struct proc *p, uint64_t usec);
 /* Current / cr3 / context management */
 uintptr_t switch_to(struct proc *new_p);
 void switch_back(struct proc *new_p, uintptr_t old_ret);
-void abandon_core(void);
+bool abandon_core(void);
 void clear_owning_proc(uint32_t coreid);
 void proc_tlbshootdown(struct proc *p, uintptr_t start, uintptr_t end);
 
index 7c69f77..5fa70ba 100644 (file)
@@ -1810,16 +1810,21 @@ void __unmap_vcore(struct proc *p, uint32_t vcoreid)
  * Note this leaves no trace of what was running. This "leaves the process's
  * context.
  *
- * This does not clear the owning proc.  Use the other helper for that. */
-void abandon_core(void)
+ * This does not clear the owning proc.  Use the other helper for that.
+ *
+ * Returns whether or not there was a process present. */
+bool abandon_core(void)
 {
        struct per_cpu_info *pcpui = &per_cpu_info[core_id()];
        /* Syscalls that don't return will ultimately call abadon_core(), so we need
         * to make sure we don't think we are still working on a syscall. */
        pcpui->cur_kthread->sysc = 0;
        pcpui->cur_kthread->errbuf = 0; /* just in case */
-       if (pcpui->cur_proc)
+       if (pcpui->cur_proc) {
                __abandon_core();
+               return true;
+       }
+       return false;
 }
 
 /* Helper to clear the core's owning processor and manage refcnting.  Pass in
index a070a0c..934c516 100644 (file)
@@ -46,8 +46,13 @@ static void try_run_proc(void)
                assert(0);
        } else {
                /* Make sure we have abandoned core.  It's possible to have an owner
-                * without a current (smp_idle, __startcore, __death). */
-               abandon_core();
+                * without a current (smp_idle, __startcore, __death).
+                *
+                * If we had a current process, we might trigger __proc_free, which
+                * could send us a KMSG.  Since we're called after PRKM, let's just
+                * restart the idle loop. */
+               if (abandon_core())
+                       smp_idle();
        }
 }