Re: [RFC, PATCH] cfq-iosched: remove redundant queuing detection code

[Jens Axboe <jens.axboe@oracle.com>]

On Tue, Nov 10 2009, Jeff Moyer wrote:
> Jens Axboe <jens.axboe@oracle.com> writes:
> 
> > On Tue, Nov 10 2009, Jeff Moyer wrote:
> >> Jens Axboe <jens.axboe@oracle.com> writes:
> >> 
> >> > On Tue, Nov 10 2009, Corrado Zoccolo wrote:
> >> >> The core block layer already has code to detect presence of command
> >> >> queuing devices. We convert cfq to use that instead of re-doing the
> >> >> computation.
> >> >
> >> > There's is the major difference that the CFQ variant is dynamic and the
> >> > block layer one is not. This change came from Aaron some time ago IIRC,
> >> > see commit 45333d5. It's a bit of a chicken and egg problem.
> >> 
> >> Really?  blk_dequeue_request sure looks like it updates things
> >> dynamically, but only one way (not queueing -> queueing).  Would it make
> >
> > Yes of course the block layer one is dynamically on as well. The ideal
> > goal would be to have every driver use the block layer tagging in which
> > case we'd know without checking, but alas it isn't so (yet). My point is
> > that the CFQ variant is dynamically off as well. Corrado presents his
> > patch as a direct functional equivelant, which it definitely isn't.
> 
> OK.  So we really want to keep track of two things:
> 1) What queue depth does the hardware support?
> 2) What is the command queue depth configured to?
> 
> That second thing can be changed by the administrator (down from or up
> to the maximum value allowed by 1).
> 
> >> sense to just put CFQ's logic into the block layer so that everyone uses
> >> the same implementation?  It makes little sense to have two notions of
> >> whether or not queueing is supported for a device.
> >
> > The one use in the block layer cares about the static property of the
> > device, not the current behaviour. So I'm not sure it makes a lot of
> > sense to unify these. It's not really a case of code duplication either,
> > the block layer one is two checks and a bit. The cfq variant is a bit
> > more involved in that it tracks the state continually.
> 
> Why don't we simply use the value configured via the queue_depth sysfs
> file?

First of all, that only covers SCSI. We could do that by having the tag
on/off functions set the same flag. But even for such devices, actual
tag depth is dependent upon what other devices are on the controller
(since it's often a shared map) and may not even be statically
detectable in the sense that actual depth is only really seen when the
device returns busy on a queue attempt.

In most cases it would work fine, but the dynamic detection is more
reliable. The sysfs setting in reality is max setting.

-- 
Jens Axboe