cvs commit: src/sys/kern subr_taskqueue.c

John Baldwin jhb at freebsd.org
Wed Jan 11 06:20:36 PST 2006


On Wednesday 11 January 2006 08:56 am, Scott Long wrote:
> John Baldwin wrote:
> > On Tuesday 10 January 2006 07:37 pm, Scott Long wrote:
> >>scottl      2006-01-11 00:37:13 UTC
> >>
> >>  FreeBSD src repository
> >>
> >>  Modified files:
> >>    sys/kern             subr_taskqueue.c
> >>  Log:
> >>  The interlock in taskqueue_terminate() is completely wrong for
> >> taskqueues that use spinlocks.  Remove it for now.
> >
> > Eh?  It's waiting for the wakeup that comes from kthread_exit() after the
> > thread has exited which is locked via the proc lock.  Sleeping on the
> > taskqueue itself doesn't buy you anything.  (In fact, it might sleep
> > forever.)   The simplest solution might be to acquire the proc lock a lot
> > earlier before the taskqueue lock in this function so that you don't have
> > to acquire it while holding the taskqueue lock since that is what gives
> > you problems.
>
> With the code the way it was, kthread_exit() in taskqueue_thread_loop
> can wind up blocking on the proc lock while the lock is still held in
> taskqueue_terminate.  I don't know why this is actually a problem, but
> turning on WITNESS to investigate revealed the immediate problem of
> trying to grab the proc lock with a spinlock already held.  The
> interlock is really just a protection against drivers that don't
> adequately quiesce themselves, so I removed it for now until we can
> figure out something better.

The interlock to make sure the thread has terminated before the function 
returns.  This would be important if the taskqueue routine for this thread 
was in a kernel module that was being unloaded to avoid having a kernel page 
fault.  The solution for the LOR you saw is probably to just lock the proc 
lock earlier.  taskqueue_terminate() is not really a critical path operation 
(if it is that's a bug) so holding the proc lock a bit longer there won't 
kill.  As it is, there isn't a matching wakeup to ever resume the sleeper in 
taskqueue_terminate(), so the calling thread will block indefinitely (unless 
it is using a timeout).

-- 
John Baldwin <jhb at FreeBSD.org>  <><  http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve"  =  http://www.FreeBSD.org


More information about the cvs-src mailing list