On Sun, Sep 03, 2006 at 10:32:54PM +0200, Michael van Elst wrote:
> On Sun, Sep 03, 2006 at 08:00:10PM +0000, Manuel Bouyer wrote:
> 
> >  it's not correct to tsleep() in a strategy routine, which may be called from
> >  interrupt context.
> 
> vndstrategy isn't called from interrupt context.

It is, if called from another driver (e.g. Xen block device).

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

> 
> 
> >  Unfortunably this reopens PR/10731, PR/12189, PR/20296, PR/34293
> >  As for what the correct fix it, this needs to be analysed deeper.
> 
> Then I suggest you do that before you break vnd.

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

> 
> 
> >  I suspect
> >  throttling the caller in vnd only hides the problem;
> 
> No, it prevents the problem. The writer needs to be throttled to
> not overwhelm the consumer because in this case it just eats up
> all memory. That necessary feedback loop is created by the patch.
> 
> 
> >  the same caller writing
> >  to some other device could exaust all buffers as well.
> 
> This is a general problem with every device but so far not seen as
> a problem. If there is memory to buffer the requests, the current
> buffering policy is to allow all buffers to be used. This is more or
> less harmless as normal device drivers will only consume and free
> buffers on the queue. 
> 
> However, vnd is special in that it requires further buffers to be
> allocated to consume and free buffers on the queue. There is also
> no need to double buffer writes to vnd this way, once in the
> device queue and once in the underlying filesystem. That strategy
> is broken and this is also fixed by the patch that limits the
> double-buffer to whatever the device thread is prepared to consume.

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.
> 
> 
> >  If this driver doesn't
> >  need to allocate buffer this won't cause a deadlock, but it's bad for
> >  performances on systems with e.g. multiple drives.
> 
> 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.

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