kthread_exit(9) unexpectedness

[Lawrence Stewart]

Hi all,

I tracked down a deadlock in some of my code today to some weird 
behaviour in the kthread(9) KPI. The executive summary is that 
kthread_exit() thread termination notification using wakeup() behaves as 
expected intuitively in 8.x, but not in 7.x.

 From sys/kern/kern_kthread.c

----------------------begin 8.x kthread_exit()--------------------------
void
kthread_exit(void)
{
         struct proc *p;

         /* A module may be waiting for us to exit. */
         wakeup(curthread);

         /*
          * We could rely on thread_exit to call exit1() but
          * there is extra work that needs to be done
          */
         if (curthread->td_proc->p_numthreads == 1)
                 kproc_exit(0);  /* never returns */

         p = curthread->td_proc;
         PROC_LOCK(p);
         PROC_SLOCK(p);
         thread_exit();
}
----------------------end 8.x kthread_exit()----------------------------

---------------------begin 7.x kthread_exit()---------------------------
void
kthread_exit(int ecode)
{
         struct thread *td;
         struct proc *p;

         td = curthread;
         p = td->td_proc;

         /*
          * Reparent curthread from proc0 to init so that the zombie
          * is harvested.
          */
         sx_xlock(&proctree_lock);
         PROC_LOCK(p);
         proc_reparent(p, initproc);
         PROC_UNLOCK(p);
         sx_xunlock(&proctree_lock);

         /*
          * Wakeup anyone waiting for us to exit.
          */
         wakeup(p);

         /* Buh-bye! */
         exit1(td, W_EXITCODE(ecode, 0));
}
----------------------end 7.x kthread_exit()----------------------------

 From the 7.x kthread(9) manpage:

"While exiting, the function exit1(9) will initiate a call to wakeup(9) 
on the thread handle."

The 8.x kthread manpage has no mention of the wakeup behaviour whatsoever.

So from the code above, we can see that the 7.x kthread_exit() calls 
wakeup() on the *proc instead of the *thread. In 8.x, kthread_exit() 
calls wakeup() on the *thread and the newly added kproc_exit() function 
will wakeup() anyone waiting on the *proc.

Looking at: 
http://svn.freebsd.org/viewvc/base/head/sys/kern/kern_kthread.c?view=log
the confusion seems to have crept in around r173004 during the KPI 
refactoring to support true kernel threads.

Historically it seems that kthread_exit() called wakeup on the *proc 
(which to my mind seems counter intuitive, but whatever). Then in 
r173052 we switch to the 8.x style of calling wakeup on the *thread, 
which matches the function naming convention and 7.x man page comment.

At a minimum we need a better discussion of the differences in the man 
page, but the behaviour change seems unnecessarily intrusive to me and 
has nasty side effects i.e. deadlock. Keeping consistent wakeup 
behaviour between 7.x and 8.x would I suspect be desirable and avoid 
this issue biting others.

Thoughts?

Cheers,
Lawrence