What's the state of sdev->tagged_queue

What's the state of sdev->tagged_queue

[Christoph Hellwig]

Hi Doug,

scsi_ioctl.c has the following comment from you:

 * The next two ioctls either need to go or need to be changed to
 * pass tagged queueing changes through the low level drivers.
 * Simply enabling or disabling tagged queueing without the knowledge
 * of the low level driver is a *BAD* thing.

and scsi.h has this:

 unsigned tagged_queue:1;/* This is going away!!!!  Look at simple_tags
                            instead!!!  Please fix your driver now!! */

but several drivers still look at it although it's never touched
outside scsi_ioctl.c and the individual drivers.  Most drivers uses also
look pretty bogus.  Can we just the ioctls and the field?


53c700.c:	if(SCp->device->tagged_supported && !SCp->device->tagged_queue
AM53C974.c:				if (!hostdata->connected->device->tagged_queue)
AM53C974.c:			    cmd->device->tagged_queue = 0;
AM53C974.c:	if (cmd->device->tagged_queue && (tag != TAG_NONE)) {
NCR5380.c:						cmd->device->tagged_queue = 0;
arm/acornscsi.c:	if (SCpnt->device->tagged_queue) {
arm/acornscsi.c:	    host->SCpnt->device->tagged_queue = 0;
arm/acornscsi.c:			     scd->tagged_queue ? "en" : "dis",
arm/fas216.c:	if (SCpnt->device->tagged_queue && SCpnt->cmnd[0] != REQUEST_SENSE &&
arm/fas216.c:				     scd->tagged_queue ? "en" : "dis",
ncr53c8xx.c:	if (lp && !lp->numtags && cmd->device && cmd->device->tagged_queue) {
qla1280.c:			if (cmd->device->tagged_queue)
qla1280.c:			if (cmd->device->tagged_queue)
scsi.h:	unsigned tagged_queue:1;/* This is going away!!!!  Look at simple_tags
scsi_ioctl.c:		sdev->tagged_queue = 1;
scsi_ioctl.c:		sdev->tagged_queue = 0;
sym53c8xx.c:	if (lp && !lp->numtags && cmd->device && cmd->device->tagged_queue) {

Re: What's the state of sdev->tagged_queue

[Doug Ledford]

Christoph Hellwig wrote:
> Hi Doug,

Hi Christoph.

> scsi_ioctl.c has the following comment from you:
> 
>  * The next two ioctls either need to go or need to be changed to
>  * pass tagged queueing changes through the low level drivers.
>  * Simply enabling or disabling tagged queueing without the knowledge
>  * of the low level driver is a *BAD* thing.
> 
> and scsi.h has this:
> 
>  unsigned tagged_queue:1;/* This is going away!!!!  Look at simple_tags
>                             instead!!!  Please fix your driver now!! */
> 
> but several drivers still look at it although it's never touched
> outside scsi_ioctl.c and the individual drivers.  Most drivers uses also
> look pretty bogus.  Can we just the ioctls and the field?

I assume that should read "Can we just kill the ioctls and the field?" :-)

The answer to that is yes I think.  The ioctls for certain, the field 
I'm pretty sure of.  Originally, the field was used as a boolean that 
was when we enabled tagged queueing on a device.  Now, a value of 
simple_tags > 1 should do the same thing.  However, due to usage of 
cmd_per_lun on both tagged and untagged devices (I would have to check 
this, I'm a bit out of touch since my accident) may cause confusion.  If 
we set queue depth to cmd_per_lun the device may look tagged when it is 
in fact not tagged.  The replacement test may need to be if simple_tags 
 > cmd_per_lun to indicate true tagged queueing.


> 53c700.c:	if(SCp->device->tagged_supported && !SCp->device->tagged_queue

This is checking that tagged queueing is supported and not already 
enabled (a holdover from the fact that select_queue_depths would get 
called more than once on a single device if we ever had a scsi 
add-single-device command after the initial device scan).

> AM53C974.c:				if (!hostdata->connected->device->tagged_queue)

Tests tagged queueing not active on a device.

> AM53C974.c:			    cmd->device->tagged_queue = 0;

Disabling tagged queueing on a device.

> AM53C974.c:	if (cmd->device->tagged_queue && (tag != TAG_NONE)) {

Checking for a tagged device with an untagged command.  Others are all 
similar.

> NCR5380.c:						cmd->device->tagged_queue = 0;
> arm/acornscsi.c:	if (SCpnt->device->tagged_queue) {
> arm/acornscsi.c:	    host->SCpnt->device->tagged_queue = 0;
> arm/acornscsi.c:			     scd->tagged_queue ? "en" : "dis",
> arm/fas216.c:	if (SCpnt->device->tagged_queue && SCpnt->cmnd[0] != REQUEST_SENSE &&
> arm/fas216.c:				     scd->tagged_queue ? "en" : "dis",
> ncr53c8xx.c:	if (lp && !lp->numtags && cmd->device && cmd->device->tagged_queue) {
> qla1280.c:			if (cmd->device->tagged_queue)
> qla1280.c:			if (cmd->device->tagged_queue)
> scsi.h:	unsigned tagged_queue:1;/* This is going away!!!!  Look at simple_tags
> scsi_ioctl.c:		sdev->tagged_queue = 1;
> scsi_ioctl.c:		sdev->tagged_queue = 0;
> sym53c8xx.c:	if (lp && !lp->numtags && cmd->device && cmd->device->tagged_queue) {

This does bring up another issue I've been thinking about though so I'll 
suggest it now.

Currently, we use the value of host->this_id to skip the scsi device of 
the host controller in our device scan.  What I would like to do is 
create an sg device entry for this_id that is type processor, is a fake 
device, that implements whatever ioctls at the mid layer level that are 
appropriate, but that also allow the low level driver to register a fall 
through host ioctl routine that can be attached to this device.  This 
would give us a uniform method of accessing host controllers via the 
scsi subsystem.  Generic ioctls that can be handled in the mid layer can 
be done so.  Host adapter specific ioctls can then be handled by the 
driver.  At least this way we wouldn't have to have a different /proc 
method of doing these things in every single driver and we would be able 
to use a normal ioctl interface for things like controlling the various 
smart raid controllers, etc.  Thoughts?

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

Re: What's the state of sdev->tagged_queue

[Christoph Hellwig]

On Fri, Jun 06, 2003 at 10:04:50AM -0400, Doug Ledford wrote:
> >but several drivers still look at it although it's never touched
> >outside scsi_ioctl.c and the individual drivers.  Most drivers uses also
> >look pretty bogus.  Can we just the ioctls and the field?
> 
> I assume that should read "Can we just kill the ioctls and the field?" :-)

Umm, yes..

> The answer to that is yes I think.  The ioctls for certain, the field 
> I'm pretty sure of.  Originally, the field was used as a boolean that 
> was when we enabled tagged queueing on a device.  Now, a value of 
> simple_tags > 1 should do the same thing.

simple_tags == 1?  in my source it's a bitfield.

> However, due to usage of 
> cmd_per_lun on both tagged and untagged devices (I would have to check 
> this, I'm a bit out of touch since my accident)

Sorry, didn't know of that.  I hope you're well already and if not
I wish you to get well soon.

> may cause confusion.  If 
> we set queue depth to cmd_per_lun the device may look tagged when it is 
> in fact not tagged.  The replacement test may need to be if simple_tags 

<snip>

> Currently, we use the value of host->this_id to skip the scsi device of 
> the host controller in our device scan.  What I would like to do is 
> create an sg device entry for this_id that is type processor,

We already have scsi_get_host_dev to get such a device if the LLDD
wants it.  Generalizing it sounds like a good idea.  But before
that we need a 64bit dev_t so we can have sg devices for the
ever-growing number of scsi_devices :)

> is a fake 
> device, that implements whatever ioctls at the mid layer level that are 
> appropriate, but that also allow the low level driver to register a fall 
> through host ioctl routine that can be attached to this device.

I don't think we should another ioctl routine.  Just use the normal
ioctl routine in the host template and let the LLDD check for
host->this_id.

Re: What's the state of sdev->tagged_queue

[Doug Ledford]

Christoph Hellwig wrote:

> simple_tags == 1?  in my source it's a bitfield.

Duh, told you I was out of touch.  I wrote that and I forgot what it was :-)


>>Currently, we use the value of host->this_id to skip the scsi device of 
>>the host controller in our device scan.  What I would like to do is 
>>create an sg device entry for this_id that is type processor,
> 
> 
> We already have scsi_get_host_dev to get such a device if the LLDD
> wants it.  Generalizing it sounds like a good idea.  But before
> that we need a 64bit dev_t so we can have sg devices for the
> ever-growing number of scsi_devices :)
> 
> 
>>is a fake 
>>device, that implements whatever ioctls at the mid layer level that are 
>>appropriate, but that also allow the low level driver to register a fall 
>>through host ioctl routine that can be attached to this device.
> 
> 
> I don't think we should another ioctl routine.  Just use the normal
> ioctl routine in the host template and let the LLDD check for
> host->this_id.

More out-of-touchness...

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