Re: 6.0-BETA2: taskqueue_drain for if_xl.c:2796

On Tuesday 16 August 2005 01:21 pm, John Baldwin wrote:
> On Monday 15 August 2005 02:22 am, Scott Long wrote:
> > John Baldwin wrote:
> > > On Thursday 11 August 2005 07:07 pm, Scott Long wrote:
> >
> > I still don't see what the point is of having a function that only
> > removes pending tasks from the queue if it doesn't also drain in
> > progress tasks.  Simple example:
> >
> > CPU1 calls xl_stop.  The XL_LOCK gets taken.  At the same time, CPU2
> > calls taskqueue_swi, which pops the queue and calls xl_rxeof_task.  It
> > tries to take the XL_LOCK but instead sleeps (or adaptively spins)
> > because CPU1 already has it.  CPU1 calls taskqueue_stop, which doesn't
> > do anything because the task was alrady popped off.
> >
> > Now, at some point, CPU1 **MUST** drop the XL_LOCK and let CPU2 continue
> > that task to completion.  There simply is no way around that, right?  So
> > why muddle the API with a function that really doesn't solve any
> > problems.  The argument that being able to call taskqueue_stop with
> > locks held is a convenience doesn't hold water.  You'll still need to
> > drop locks in order to be sure that a mess isn't created.
>
> Well, you need to think through this longer.  During a normal stop, we
> don't care if the task runs later if it is just going to stop anyway, which
> it will with the IFF_DRV_RUNNING checks I added (which it needs anyway even
> if you don't add a taskqueue_stop()).  See, when CPU2 eventually runs the
> xl_rxeof_task, it will drop the lock and return instantly because the
> xl_stop() function clears IFF_DRV_RUNNING in addition to calling
> taskqueue_stop().

Actually, I thought through this longer some myself on the way home and 
because of the RUNNING checks, the taskqueue_stop() in xl_stop() isn't 
actually needed for correct operation.  It's merely an optimization for the 
case that the task is sitting in the queue, but the code will work fine 
without it with the taskqueue_drain() moved to xl_detach() and the 
IFF_DRV_RUNNING checks added.  Thus, if you just really hate taskqueue_stop() 
or don't think the optimization is worth it, I can leave it out.  I kind of 
would like to call it in xl_detach() at least so that detaching xl0 won't be 
dependent on another driver's task deciding to yield.  However, if I moved it 
there I could add blocking to it.  I could also just use the existing 
taskqueue_drain() and not worry about having to wait for other tasks to 
finish.  I can't call taskqueue_drain() in xl_stop() though since xl_stop() 
is called indirectly from xl_intr().

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