Re: Strange block/scsi/workqueue issue

[Tejun Heo <tj@kernel.org>]

Hey, James.

On Tue, Apr 12, 2011 at 10:15:18AM -0500, James Bottomley wrote:
> So your idea is that all final puts should go through a workqueue?  Like
> I said, that would work, but it's not just SCSI ... any call path that
> destroys a queue has to be audited.

Yeap.

> The problem is nothing to do with sleeping context ... it's that any
> work called by the block workqueue can't destroy that queue.  In a
> refcounted model, that's a bit nasty.

I can see your point but please read on.

> > Hmmm... maybe but at least I prefer doing explicit shutdown/draining
> > on destruction even if the base data structure is refcounted.  Things
> > become much more predictable that way.
> 
> It is pretty much instantaneous.  Unless we're executing, we cancel the
> work.  If the work is already running, we just let it complete instead
> of waiting for it.
> 
> Synchronous waits are dangerous because they cause entanglement.

There are two different types of dangers involved.  One is of getting
trapped into deadlock by recursing and ending up waiting for oneself.
The other of continuing operation on objects which could be in dubious
state.  I guess my point is that I prefer the former by a large
margin.

The deadlocks are more reliable in reproducibility.  Lockdep and soft
hang check can detect them easily and a single stack dump will point
us right to where the problem is.  The latter is much trickier.  The
problem is more difficult to trigger and even when it triggers the
effect often wouldn't be obvious.  Auditing for correctness is more
difficult too - which fields are safe to access post-mortem?  Is there
any chance that the ongoing operation might reach out to hardware
which is already gone or claimed by another software entity?

In this particular case, IMHO it's reasonable for block layer to
require that the destruction function not to be called directly from
request queue path although it definitely could have used better
documentation.

Thank you.

-- 
tejun