On Mon, Sep 04, 2006 at 08:55:06PM +0000, Manuel Bouyer wrote:

>  > vndstrategy isn't called from interrupt context.
>  
>  It is, if called from another driver (e.g. Xen block device).

I believe that this is not allowed. The strategy routine is
part of the top half of the driver, it operates in process
context.

Saying that, I scanned other drivers and found several that may
sleep in their strategy() routine. Most promiment scsistrategy()
does this (by virtue of calling scsipi_command which waits for
a free scsi transfer context in scsipi_get_xs).


>  > But if that is
>  > a problem, the feedback loop can be put into both vndread()/vndwrite().
>  I'm not sure we're allowed to tsleep() here either.

vndread()/vndwrite() definitely do already tsleep() in physio() exactly
when they are waiting for buffers. However, I was mistaken that these
routines were passed when accessing the block device but they are
only used by the raw device.


>  This patch broke vnd for all Xen users, where it worked fine before.

I agree. The patch probably leaves a lot space for improvement. But
the real bug is calling the strategy() routine from interrupt context.


>  I understand all of this. But 1) the patch is broken (calling tsleep in
>  a strategy routine) 2) whenever it's not a problem for other drivers is
>  a matter of opinion. If has probably performances inpacts.

Calling tsleep() in strategy is allowed. You are right that sleeps
should be avoided when possible for performance reasons.

However, the patch improves performance significantly by avoiding
the buffer drain.


>  > Preventing vnd from consuming all buffers cannot be bad for
>  > performances on systems with multiple drives.
>  
>  No, I meant if all buffers end up in a single device queue.

That's what happening without the patch. The buffers queue up
for vnd and the vndthread waits for free buffers. Deadlock.



Greetings,
-- 
                                Michael van Elst
Internet: mlelstv@serpens.de
                                "A potential Snark may lurk in every tree."