Re: [PATCH] add unschedule_delayed_work to the workqueue API

[Andrew Morton <akpm@osdl.org>]

James Bottomley <James.Bottomley@SteelEye.com> wrote:
>
> > 
> > The probability that the handler is running when you call
> > cancel_delayed_work() is surely very low.  And the probability that there
> > is more than one thing pending in the queue at that time is also low. 
> > Multiplying them both together, then multiplying that by the relative
> > expense of the handler makes me say "show me" ;)
> 
> OK.  In the current code, domain validation is done from the workqueue
> interface.  This can take several seconds per target to complete.  Why
> should I have to wait this extra time.  As I move other SCSI daemon
> threads to being work queue items, these times rise.

You mean the scsi code wants to schedule a work some time in the future and
that when it executes, the handler will then run for several seconds?

If so, it sounds like this is the wrong mechanism to use.  We certainly
don't want keventd gone to lunch for that long, and even if the kernel
threads were privately created by the scsi code, you'll have to create many
such threads to get any sort of parallelism across many disks.  I suspect
I'm missing something.

> However, now there's a worse problem.  If I want to cancel a piece of
> work synchronously, flush_scheduled_work() makes me dependent on the
> execution of all the prior pieces of work.  If some of them are related,
> like Domain Validation and device unlocking say, I have to now be
> extremely careful that the place I cancel and flush from isn't likely to
> deadlock with any other work scheduled on the device.  This makes it a
> hard to use interface.

Well yes, the caller wouldn't want to be holding any locks which cause
currently-queued works to block.

>  By contrast, the proposed patch will *only* wait
> if the item of work is currently executing.  This is a (OK reasonably
> given the aic del_timer_sync() issues) well understood deadlock
> problem---the main point being I now don't have to consider any of the
> other work that might be queued.

OK.