kernel - Fix serious issue w/ smp_invltlb(), plus other issues (3)
authorMatthew Dillon <dillon@apollo.backplane.com>
Sun, 31 Oct 2010 15:52:00 +0000 (08:52 -0700)
committerMatthew Dillon <dillon@apollo.backplane.com>
Sun, 31 Oct 2010 16:36:23 +0000 (09:36 -0700)
* splz() and doreti on both i386 and x86_64 were leaving interrupts
  disabled through calls to e.g. sched_ithd() and IPI function callbacks.

  As before this can lead to a deadlock if these functions call
  smp_invltlb() or some other cpu calls smp_invltlb() while these
  functions are spinning on a full IPIQ.

* sti in appropriate places to fix the problem.

sys/platform/pc32/i386/mp_machdep.c
sys/platform/pc32/isa/ipl.s
sys/platform/pc64/x86_64/ipl.s
sys/platform/pc64/x86_64/mp_machdep.c

index eda9722..2a0cc89 100644 (file)
@@ -2604,6 +2604,7 @@ smitest(void)
  */
 
 static cpumask_t smp_invltlb_req;
+#define SMP_INVLTLB_DEBUG
 
 void
 smp_invltlb(void)
index f0c7d6e..e448d6d 100644 (file)
@@ -118,10 +118,9 @@ doreti:
        jne     5f
        incl    TD_CRITCOUNT(%ebx)      /* force all ints to pending */
 doreti_next:
-       sti                             /* allow new interrupts */
+       cli                             /* re-assert cli on loop */
        movl    %eax,%ecx               /* irq mask unavailable due to BGL */
        notl    %ecx
-       cli                             /* disallow YYY remove */
 #ifdef SMP
        testl   $RQF_IPIQ,PCPU(reqflags)
        jnz     doreti_ipiq
@@ -187,6 +186,11 @@ doreti_popl_ds:
 doreti_iret:
        iret
 
+       /*
+        * Interrupts are likely disabled due to the above interlock
+        * between cli/iretq.  We must enable them before calling any
+        * high level function.
+        */
        ALIGN_TEXT
        .globl  doreti_iret_fault
 doreti_iret_fault:
@@ -204,6 +208,7 @@ doreti_popl_fs_fault:
        pushl   %gs
        .globl  doreti_popl_gs_fault
 doreti_popl_gs_fault:
+       sti
        movl    $0,TF_ERR(%esp) /* XXX should be the error code */
        movl    $T_PROTFLT,TF_TRAPNO(%esp)
        jmp     alltraps_with_regs_pushed
@@ -215,6 +220,7 @@ doreti_popl_gs_fault:
        ALIGN_TEXT
 doreti_fast:
        andl    PCPU(fpending),%ecx     /* only check fast ints */
+       sti
        bsfl    %ecx, %ecx              /* locate the next dispatchable int */
        btrl    %ecx, PCPU(fpending)    /* is it really still pending? */
        jnc     doreti_next
@@ -233,6 +239,7 @@ doreti_fast:
         */
        ALIGN_TEXT
 doreti_soft:
+       sti
        bsfl    %ecx,%ecx               /* locate the next pending softint */
        btrl    %ecx,PCPU(spending)     /* make sure its still pending */
        jnc     doreti_next
@@ -279,6 +286,7 @@ doreti_ipiq:
        movl    %eax,%esi               /* save cpl (can't use stack) */
        incl    PCPU(intr_nesting_level)
        andl    $~RQF_IPIQ,PCPU(reqflags)
+       sti
        subl    $8,%esp                 /* add dummy vec and ppl */
        pushl   %esp                    /* pass frame by reference */
        call    lwkt_process_ipiq_frame
@@ -291,6 +299,7 @@ doreti_timer:
        movl    %eax,%esi               /* save cpl (can't use stack) */
        incl    PCPU(intr_nesting_level)
        andl    $~RQF_TIMER,PCPU(reqflags)
+       sti
        subl    $8,%esp                 /* add dummy vec and ppl */
        pushl   %esp                    /* pass frame by reference */
        call    lapic_timer_process_frame
@@ -357,6 +366,7 @@ splz_next:
        ALIGN_TEXT
 splz_fast:
        andl    PCPU(fpending),%ecx     /* only check fast ints */
+       sti
        bsfl    %ecx, %ecx              /* locate the next dispatchable int */
        btrl    %ecx, PCPU(fpending)    /* is it really still pending? */
        jnc     splz_next
@@ -373,11 +383,11 @@ splz_fast:
         */
        ALIGN_TEXT
 splz_soft:
+       sti
        bsfl    %ecx,%ecx               /* locate the next pending softint */
        btrl    %ecx,PCPU(spending)     /* make sure its still pending */
        jnc     splz_next
        addl    $FIRST_SOFTINT,%ecx     /* actual intr number */
-       sti
        pushl   %eax
        pushl   %ecx
        decl    TD_CRITCOUNT(%ebx)
@@ -392,6 +402,7 @@ splz_soft:
 #ifdef SMP
 splz_ipiq:
        andl    $~RQF_IPIQ,PCPU(reqflags)
+       sti
        pushl   %eax
        call    lwkt_process_ipiq
        popl    %eax
@@ -399,6 +410,7 @@ splz_ipiq:
 
 splz_timer:
        andl    $~RQF_TIMER,PCPU(reqflags)
+       sti
        pushl   %eax
        call    lapic_timer_process
        popl    %eax
index 6577de6..1f7ce0e 100644 (file)
@@ -117,6 +117,10 @@ fastunpend_count:  .long   0
         *      - We have to be careful in regards to local interrupts
         *        occuring simultaniously with our doreti and splz 
         *        processing.
+        *
+        *      - Interrupts must be enabled when calling higher level
+        *        functions in order to avoid deadlocking against things
+        *        like smp_invltlb.
         */
 
        /*
@@ -145,10 +149,9 @@ doreti:
        jne     5f
        incl    TD_CRITCOUNT(%rbx)      /* force all ints to pending */
 doreti_next:
-       sti                             /* allow new interrupts */
+       cli                             /* re-assert cli on loop */
        movl    %eax,%ecx               /* irq mask unavailable due to BGL */
        notl    %ecx
-       cli                             /* disallow YYY remove */
 #ifdef SMP
        testl   $RQF_IPIQ,PCPU(reqflags)
        jnz     doreti_ipiq
@@ -183,6 +186,8 @@ doreti_next:
        MEXITCOUNT
 
        /*
+        * (interrupts are disabled here)
+        *
         * Restore register and iret.  iret can fault on %rip (which is
         * really stupid).  If this occurs we re-fault and vector to
         * doreti_iret_fault().
@@ -208,15 +213,16 @@ doreti_iret:
         * (sys/platform/pc64/x86_64/trap.c) catches this specific * case,
         * sends the process a signal and continues in the corresponding
         * place in the code below.
+        *
+        * Interrupts are likely disabled due to the above interlock
+        * between cli/iretq.  We must enable them before calling any
+        * high level function.
         */
        ALIGN_TEXT
        .globl  doreti_iret_fault
 doreti_iret_fault:
        PUSH_FRAME_NOSWAP
-       testq   $PSL_I,TF_RFLAGS(%rsp)
-       jz      2f
        sti
-2:
        movq    $T_PROTFLT,TF_TRAPNO(%rsp)
        movq    $0,TF_ERR(%rsp) /* XXX should be the error code */
        movq    $0,TF_ADDR(%rsp)
@@ -230,34 +236,15 @@ doreti_iret_fault:
        ALIGN_TEXT
 doreti_fast:
        andl    PCPU(fpending),%ecx     /* only check fast ints */
+       sti
        bsfl    %ecx, %ecx              /* locate the next dispatchable int */
        btrl    %ecx, PCPU(fpending)    /* is it really still pending? */
        jnc     doreti_next
        pushq   %rax                    /* save IRQ mask unavailable for BGL */
                                        /* NOTE: is also CPL in frame */
-#if 0
-#ifdef SMP
-       pushq   %rcx                    /* save ecx */
-       call    try_mplock
-       popq    %rcx
-       testl   %eax,%eax
-       jz      1f
-       /* MP lock successful */
-#endif
-#endif
        call    dofastunpend            /* unpend fast intr %ecx */
-#if 0
-#ifdef SMP
-       call    rel_mplock
-#endif
-#endif
        popq    %rax
        jmp     doreti_next
-1:
-       btsl    %ecx, PCPU(fpending)    /* oops, couldn't get the MP lock */
-       popq    %rax                    /* add to temp. cpl mask to ignore */
-       orl     PCPU(fpending),%eax
-       jmp     doreti_next
 
        /*
         *  SOFT interrupt pending
@@ -268,6 +255,7 @@ doreti_fast:
         */
        ALIGN_TEXT
 doreti_soft:
+       sti
        bsfl    %ecx,%ecx               /* locate the next pending softint */
        btrl    %ecx,PCPU(spending)     /* make sure its still pending */
        jnc     doreti_next
@@ -312,6 +300,7 @@ doreti_ipiq:
        movl    %eax,%r12d              /* save cpl (can't use stack) */
        incl    PCPU(intr_nesting_level)
        andl    $~RQF_IPIQ,PCPU(reqflags)
+       sti
        subq    $8,%rsp                 /* trapframe->intrframe */
        movq    %rsp,%rdi               /* pass frame by ref (C arg) */
        call    lwkt_process_ipiq_frame
@@ -324,6 +313,7 @@ doreti_timer:
        movl    %eax,%r12d              /* save cpl (can't use stack) */
        incl    PCPU(intr_nesting_level)
        andl    $~RQF_TIMER,PCPU(reqflags)
+       sti
        subq    $8,%rsp                 /* trapframe->intrframe */
        movq    %rsp,%rdi               /* pass frame by ref (C arg) */
        call    lapic_timer_process_frame
@@ -390,24 +380,12 @@ splz_next:
        ALIGN_TEXT
 splz_fast:
        andl    PCPU(fpending),%ecx     /* only check fast ints */
+       sti
        bsfl    %ecx, %ecx              /* locate the next dispatchable int */
        btrl    %ecx, PCPU(fpending)    /* is it really still pending? */
        jnc     splz_next
        pushq   %rax
-#if 0
-#ifdef SMP
-       movl    %ecx,%edi               /* argument to try_mplock */
-       call    try_mplock
-       testl   %eax,%eax
-       jz      1f
-#endif
-#endif
        call    dofastunpend            /* unpend fast intr %ecx */
-#if 0
-#ifdef SMP
-       call    rel_mplock
-#endif
-#endif
        popq    %rax
        jmp     splz_next
 1:
@@ -424,6 +402,7 @@ splz_fast:
         */
        ALIGN_TEXT
 splz_soft:
+       sti
        bsfl    %ecx,%ecx               /* locate the next pending softint */
        btrl    %ecx,PCPU(spending)     /* make sure its still pending */
        jnc     splz_next
@@ -431,8 +410,8 @@ splz_soft:
        sti
        pushq   %rax
        movl    %ecx,%edi               /* C argument */
-       decl    TD_CRITCOUNT(%rbx)
        incl    TD_NEST_COUNT(%rbx)     /* prevent doreti/splz nesting */
+       decl    TD_CRITCOUNT(%rbx)
        call    sched_ithd              /* YYY must pull in imasks */
        incl    TD_CRITCOUNT(%rbx)
        decl    TD_NEST_COUNT(%rbx)     /* prevent doreti/splz nesting */
@@ -442,6 +421,7 @@ splz_soft:
 #ifdef SMP
 splz_ipiq:
        andl    $~RQF_IPIQ,PCPU(reqflags)
+       sti
        pushq   %rax
        call    lwkt_process_ipiq
        popq    %rax
@@ -449,6 +429,7 @@ splz_ipiq:
 
 splz_timer:
        andl    $~RQF_TIMER,PCPU(reqflags)
+       sti
        pushq   %rax
        call    lapic_timer_process
        popq    %rax
index 5a7dd68..d9259e9 100644 (file)
@@ -2790,6 +2790,8 @@ smitest(void)
 
 static cpumask_t smp_invltlb_req;
 
+#define SMP_INVLTLB_DEBUG
+
 void
 smp_invltlb(void)
 {