patch for kern/46168

[Manuel Bouyer <bouyer%antioche.eu.org@localhost>]

Hello,
has someone comments about the patch I propose in kern/46168 before I commit
it ? It's also attached below.
Basically, it looks like a LWP may be added to a proc while lwp_wait1()
has released the p_lock, even if lwp_wait1() returns 0. As a result,
we're waiting for a LWP which doesn't have the LW_WEXIT flag.
What the patch does is always reprocess the lwp list if lwp_wait1() returns
and p_nlwps is still > 1.
This doesn't cause new test failures (I did 2 runs) and I coudln't
reproduce the problem in kern/46168 with this patch.

-- 
Manuel Bouyer <bouyer%antioche.eu.org@localhost>
     NetBSD: 26 ans d'experience feront toujours la difference
--

Index: kern/kern_exit.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_exit.c,v
retrieving revision 1.237
diff -u -p -u -r1.237 kern_exit.c
--- kern/kern_exit.c    19 Feb 2012 21:06:50 -0000      1.237
+++ kern/kern_exit.c    14 Mar 2012 16:22:47 -0000
@@ -598,44 +598,42 @@ exit_lwps(struct lwp *l)
 
        p = l->l_proc;
        KASSERT(mutex_owned(p->p_lock));
-retry:
-       /*
-        * Interrupt LWPs in interruptable sleep, unsuspend suspended
-        * LWPs and then wait for everyone else to finish.
-        */
-       LIST_FOREACH(l2, &p->p_lwps, l_sibling) {
-               if (l2 == l)
-                       continue;
-               lwp_lock(l2);
-               l2->l_flag |= LW_WEXIT;
-               if ((l2->l_stat == LSSLEEP && (l2->l_flag & LW_SINTR)) ||
-                   l2->l_stat == LSSUSPENDED || l2->l_stat == LSSTOP) {
-                       /* setrunnable() will release the lock. */
-                       setrunnable(l2);
-                       DPRINTF(("exit_lwps: Made %d.%d runnable\n",
-                           p->p_pid, l2->l_lid));
-                       continue;
-               }
-               lwp_unlock(l2);
-       }
        while (p->p_nlwps > 1) {
+               /*
+                * Interrupt LWPs in interruptable sleep, unsuspend suspended
+                * LWPs and then wait for everyone else to finish.
+                */
+               LIST_FOREACH(l2, &p->p_lwps, l_sibling) {
+                       if (l2 == l)
+                               continue;
+                       lwp_lock(l2);
+                       l2->l_flag |= LW_WEXIT;
+                       if ((l2->l_stat == LSSLEEP && (l2->l_flag & LW_SINTR)) 
||
+                           l2->l_stat == LSSUSPENDED || l2->l_stat == LSSTOP) {
+                               /* setrunnable() will release the lock. */
+                               setrunnable(l2);
+                               DPRINTF(("exit_lwps: Made %d.%d runnable\n",
+                                   p->p_pid, l2->l_lid));
+                               continue;
+                       }
+                       lwp_unlock(l2);
+               }
                DPRINTF(("exit_lwps: waiting for %d LWPs (%d zombies)\n",
                    p->p_nlwps, p->p_nzlwps));
                error = lwp_wait1(l, 0, &waited, LWPWAIT_EXITCONTROL);
                if (p->p_nlwps == 1)
                        break;
-               if (error == EDEADLK) {
-                       /*
-                        * LWPs can get suspended/slept behind us.
-                        * (eg. sa_setwoken)
-                        * kick them again and retry.
-                        */
-                       goto retry;
-               }
-               if (error)
+               if (error == 0)
+                       DPRINTF(("exit_lwps: Got LWP %d from lwp_wait1()\n",
+                           waited));
+               else if (error != EDEADLK)
                        panic("exit_lwps: lwp_wait1 failed with error %d",
                            error);
-               DPRINTF(("exit_lwps: Got LWP %d from lwp_wait1()\n", waited));
+               /*
+                * LWPs can get suspended/slept behind us.
+                * (eg. sa_setwoken)
+                * kick them again and retry.
+                */
        }
 
        KERNEL_LOCK(nlocks, l);