Subject: Re: PR/34293 CVS commit: src/sys/dev
To: Michael van Elst <mlelstv@serpens.de>
From: Manuel Bouyer <bouyer@antioche.eu.org>
List: netbsd-bugs
Date: 09/06/2006 23:23:02
On Mon, Sep 04, 2006 at 11:23:37PM +0200, Michael van Elst wrote:
> 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.

There was a discussion on tech-kern when I introduced the Xen
backend driver, and the conclusion was that it's valid to call
a strategy routine from interrupt context, and drivers should deal
with that.

> 
> 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).

This is a pseudo-strategy, part of the SCSI-specific ioctls implementation.
It's internal details, it's not exported to other drivers.

> >  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.

You're redefining the strategy() interface here.

> >  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.

Where did you get this ?

> >  > 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.

And that's what happens in other driver's queue too.

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 26 ans d'experience feront toujours la difference
--