a parallel operation problem about softint(9)

[Kengo NAKAHARA <k-nakahara%iij.ad.jp@localhost>]

Hi,

I found a parallel operation problem between softint_execute()
and softint_disestablish().

The following single CPU operation of softint(9) is ok.
    - softint_establish()
      - set function, argument, and so on to the appropriate softint_t
    - softint_schedule()
      - the softhand_t is set SOFTINT_PENDING flag
      - the softhand_t is inserted to si->si_q queue
    - wait for softint is scheduled
    - softint_execute()
      - the softhand_t is removed form si->si_q queue
      - the softhand_t is unset SOFTINT_PENDING flag
      - the softhand_t is set SOFTINT_ACTIVE flag
      - do softint handler of the softhand_t
      - the softhand_t is unset SOFTINT_ACTIVE flag
    - softint_disestablish()
      - wait until softhand_t's SOFTINT_ACTIVE of all CPUs is clear
      - unset function of the softint_t

In contrast, the following parallel operation causes panic.
    (0) softint handler "handler A" is established
    (1) CPU#X do softint_schedule() for "handler A"
        - the softhand_t is set SOFTINT_PENDING flag
        - the softhand_t is NOT set SOFTINT_ACTIVE flag yet
    (2) CPU#X begin other H/W interrupt processing
    (3) CPU#Y do softint_disestablish() for "handler A"
        - wait until softhand_t's SOFTINT_ACTIVE of all CPUs is clear
        - the softhand_t is set not SOFTINT_ACTIVE but SOFTINT_PENDING,
          so CPU#Y does not wait
        - unset function of "handler A"
    (4) CPU#X do softint_execute()
        - the function of "handler A" is already clear, so panic

I think softint_disestablish should wait not only SOFTINT_ACTIVE
but also SOFTINT_PENDING flag, that is, the following patch is
required
====================
--- a/sys/kern/kern_softint.c
+++ b/sys/kern/kern_softint.c
@@ -443,7 +443,7 @@ softint_disestablish(void *arg)
                        flags |= sh->sh_flags;
                }
                /* Inactive on all CPUs? */
-               if ((flags & SOFTINT_ACTIVE) == 0) {
+               if ((flags & (SOFTINT_PENDING | SOFTINT_ACTIVE)) == 0) {
                        break;
                }
                /* Oops, still active.  Wait for it to clear. */
====================
Under my environment, this patch fixes above panic, and does not cause
new problems.

Could you comment this patch? 


Thanks,

-- 
//////////////////////////////////////////////////////////////////////
Internet Initiative Japan Inc.

Device Engineering Section,
Core Product Development Department,
Product Division,
Technology Unit

Kengo NAKAHARA <k-nakahara%iij.ad.jp@localhost>