Re: [patch 2.5] ips queue depths

Doug Ledford
2002-10-16T02:32:31Z

On Tue, Oct 15, 2002 at 04:30:57PM -0700, Patrick Mansfield wrote:
> On Tue, Oct 15, 2002 at 04:52:18PM -0400, Doug Ledford wrote:
> > On Tue, Oct 15, 2002 at 01:04:45PM -0700, Patrick Mansfield wrote:
> > >
> > > I totally agree the queue depth should not be limited based on the number of
> > > devices, but there should be a maximum limit - as Justin noted with the
> > > aic driver, using a large default queue depth (he had 253 or so) is not
> > > good with linux.
> >
> > That's largely due to the scsi_request_fn() in its current form.
>
> I don't understand that.

The current scsi_request_fn() sucks rocks when it comes to any sort of
fair load balancing in a host queue starvation scenario.

> > > The queue depth should be as small as possible without limiting the
> > > performance.
> >
> > Which happens to be a magic number that no one really knows I think ;-)
>
> Yes, but that implies that adapters should not be relied upon to set
> the queue depth.

I draw exactly the opposite conclusion (look at the email from Lubon
Tuikov for several valid reasons why).

> If the adapter is setting queue depth based upon what
> the adapter knows, that is completely wrong

I strongly disagree here. Only the adapter driver knows if the card
itself has a hard limit of X commands at a time (so that deeper queues are
wasted). Only the adapter driver knows if the particular interconnect has
any inherent queue limitations or speed limitations. There are a thousand
things only the adapter driver knows that should be factored into a sane
queue depth.

> - a regular disk and disk
> array can end up with the same queue depth (this is not ture for raid
> cards like ips, where the adapter is the device).

I don't see a problem here. I have yet to meet a reasonable SCSI disk
that doesn't do well with deep queues. Raid devices I'm not so sure of,
but I'll give them the benefit of the doubt and let them have a deep queue
as well. I would say the maximum speed of the interconnect combined with
the maximum size of each command is a better guage upon which to determine
queue depth than whether it's a real disk or logical disk.

> > Current goals of the work I'm doing include making this adjustable so you
> > aren't wasting queue depth on devices that have a hard limit less than
>
> But, this won't lower it to some optimal magic number.

No, the controller driver in question should already have it's "optimum"
number in mind, set the drive to that, then if it's too high it can come
down. There is no magical "Hey, send me more commands" status code, so
you have to start high and go low, not the other way around. That, of
course, is why an adjustable depth is important. The controller should
set this "optimum" number based upon it's own capabilities and assume the
drive is just as capable.

> > your initial queue depth value and changing the scsi_request_fn() to be
> > more fair with command allocation on a controller in the presence of
> > starvation (and of course, starvation is only possible when the queue
> > depth of all devices is greater than the depth of the controller, so
> > obviously the starvation code in scsi_request_fn() is something
> > controllers have been avoiding by limiting queue depths on individual
> > devices).
>
> It would nice to have more device model/kernfs device attributes.
>
> With your new queueing code, if we expose and allow setting new_queue_depth,
> user code can easily modify the queue depth.

No. For the reasons that the driver knows what it's capable of, there
really is *very* little tunability here. But, should you really need it,
the drivers should provide it (both mine and Justin's aic7xxx drivers do
so run time via module options or boot command options). Make the default
a good common default, let users override if necessary. But I wouldn't
make it a fiddle knob that people can tweak without thinking. Regardless
of where you make it adjustable though, it has to be passed through the
low level drivers so that they can adjust their internal data structs as
needed (some won't have to do anything, some will have to do allocations
or frees on every adjustment, so you *have* to pass all adjustments
through them).

> I wanted to write some code in this area but don't have enoough time. It would
> not be hard to have a scsi_sdev_attrs.c file, that using macros could
> automagically create device attr files and functions for any selection of
> Scsi_Device fields. Locking might be an issue - since we don't have a
> Scsi_Device lock for queue depth, just one big lock, this is true for
> some of the other Scsi_Device fields.
>
> A single function could be called in scsi_scan.c to setup all Scsi_Device
> device attribute files (i.e. calls device_create_file for multiple
> xxx_attr_types).
>
> -- Patrick Mansfield
> -
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Doug Ledford <dledford@redhat.com> 919-754-3700 x44233
Red Hat, Inc.
1801 Varsity Dr.
Raleigh, NC 27606