Ensures IRQs are enabled when proc_destroy()ing
authorBarret Rhoden <brho@cs.berkeley.edu>
Mon, 14 May 2012 22:00:49 +0000 (15:00 -0700)
committerBarret Rhoden <brho@cs.berkeley.edu>
Wed, 5 Sep 2012 21:43:58 +0000 (14:43 -0700)
The main reason is that irqs need to be on when grabbing the proclock.
There are some deadlock scenarios where a __preempt (or even __death)
kmsg is sent with the lock held to a core, but if that core has irqs
disabled, it won't process the message (circular waiting, etc).

While I could make proc_destroy() enable irqs, its really up to the
calling code to do that.  I put in asserts where they shouldn't be
disabled, and otherwise turned them on manually.  This mostly applies to
trap handling code.  Note we can't just turn on IRQs immediately - each
case needs to do it where it is possible (preferably sooner than later),
such as after reading cr2() in x86's PF handler.

kern/arch/i686/trap.c
kern/arch/riscv/trap.c
kern/arch/sparc/trap.c
kern/src/manager.c
kern/src/monitor.c
kern/src/process.c
kern/src/syscall.c

index 1672534..732d6fc 100644 (file)
@@ -276,6 +276,7 @@ static void trap_dispatch(struct trapframe *tf)
                                panic("Damn Damn!  Unhandled trap in the kernel!");
                        else {
                                warn("Unexpected trap from userspace");
+                               enable_irq();
                                proc_destroy(current);
                        }
        }
@@ -482,6 +483,8 @@ void page_fault_handler(struct trapframe *tf)
                print_trapframe(tf);
                panic("Page Fault in the Kernel at 0x%08x!", fault_va);
        }
+       /* safe to reenable after rcr2 */
+       enable_irq();
        if ((err = handle_page_fault(current, fault_va, prot))) {
                /* Destroy the faulting process */
                printk("[%08x] user %s fault va %08x ip %08x on core %d with err %d\n",
index 364cf19..96be0b3 100644 (file)
@@ -301,6 +301,7 @@ unhandled_trap(trapframe_t* state, const char* name)
                spin_unlock(&screwup_lock);
 
                assert(current);
+               enable_irq();
                proc_destroy(current);
        }
 }
index 9a1f0e8..577db09 100644 (file)
@@ -357,6 +357,7 @@ unhandled_trap(trapframe_t* state)
                spin_unlock(&screwup_lock);
 
                assert(current);
+               enable_irq();
                proc_destroy(current);
                /* Not sure if SPARC has a central point that would run proc_restartcore
                 */
@@ -468,6 +469,7 @@ handle_pop_tf(trapframe_t* state)
 
        trapframe_t tf, *tf_p = &tf;
        if (memcpy_from_user(current,&tf,(void*)state->gpr[8],sizeof(tf))) {
+               enable_irq();
                proc_destroy(current);
                proc_restartcore();
        }
@@ -483,6 +485,7 @@ handle_set_tf(trapframe_t* state)
        advance_pc(state);
        if (memcpy_to_user(current,(void*)state->gpr[8],state,sizeof(*state))) {
                proc_incref(current, 1);
+               enable_irq();
                proc_destroy(current);
                proc_restartcore();
        }
index 229be5d..f5a28e0 100644 (file)
@@ -172,6 +172,7 @@ void manager_brho(void)
                                spin_unlock(&p->proc_lock);
                                udelay(5000000);
                                printk("Killing p\n");
+                               enable_irq();
                                proc_destroy(p);
                                printk("Killed p\n");
                        panic("This is okay");
index adbfbdf..0c047fb 100644 (file)
@@ -312,6 +312,7 @@ int mon_bin_run(int argc, char *NTS *NT COUNT(argc) argv, trapframe_t *tf)
 
 int mon_procinfo(int argc, char *NTS *NT COUNT(argc) argv, trapframe_t *tf)
 {
+       int8_t irq_state = 0;
        if (argc < 2) {
                printk("Usage: procinfo OPTION\n");
                printk("\tidlecores: show idle core map\n");
@@ -358,7 +359,9 @@ int mon_procinfo(int argc, char *NTS *NT COUNT(argc) argv, trapframe_t *tf)
                        printk("No such proc\n");
                        return 1;
                }
+               enable_irqsave(&irq_state);
                proc_destroy(p);
+               disable_irqsave(&irq_state);
                proc_decref(p);
        } else {
                printk("Bad option\n");
@@ -473,6 +476,7 @@ int mon_measure(int argc, char *NTS *NT COUNT(argc) argv, trapframe_t *tf)
 {
        uint64_t begin = 0, diff = 0;
        uint32_t end_refcnt = 0;
+       int8_t irq_state = 0;
 
        if (argc < 2) {
                printk("Usage: measure OPTION\n");
@@ -500,7 +504,9 @@ int mon_measure(int argc, char *NTS *NT COUNT(argc) argv, trapframe_t *tf)
                printk("Warning: this will be inaccurate due to the appserver.\n");
                end_refcnt = kref_refcnt(&p->p_kref) - p->procinfo->num_vcores - 1;
 #endif /* __CONFIG_APPSERVER__ */
+               enable_irqsave(&irq_state);
                proc_destroy(p);
+               disable_irqsave(&irq_state);
                proc_decref(p);
 #ifdef __CONFIG_APPSERVER__
                /* Won't be that accurate, since it's not actually going through the
index dc075c2..a856ea8 100644 (file)
@@ -688,6 +688,10 @@ void proc_destroy(struct proc *p)
 {
        uint32_t nr_cores_revoked = 0;
        struct kthread *sleeper;
+       /* Can't spin on the proc lock with irq disabled.  This is a problem for all
+        * places where we grab the lock, but it is particularly bad for destroy,
+        * since we tend to call this from trap and irq handlers */
+       assert(irq_is_enabled());
        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];
index d1124a3..b7bd828 100644 (file)
@@ -1497,6 +1497,7 @@ void run_local_syscall(struct syscall *sysc)
        struct per_cpu_info *pcpui = &per_cpu_info[core_id()];
 
        /* TODO: (UMEM) assert / pin the memory for the sysc */
+       assert(irq_is_enabled());       /* in case we proc destroy */
        user_mem_assert(pcpui->cur_proc, sysc, sizeof(struct syscall),
                        sizeof(uintptr_t), PTE_USER_RW);
        pcpui->cur_sysc = sysc;                 /* let the core know which sysc it is */