On Mon, 2014-09-29 at 12:52 -0400, Webb Scales wrote:
> [Hannes, James, Christoph: sorry for the extra copies -- my mail client
> didn't automatically convert to plain-text for the list, so it bounced.]
>
> Could someone enlighten me on when and where a driver might want to or
> be required to set the "tagged_supported" flag?
>
> A quick grep yields a number of places where the flag value is set or
> cleared.
>
> This looks like the mechanism which is supposed to set the flag (e.g.,
> if the device/LUN characteristics suggest that it's appropriate), and so
> I'm not sure when/why the driver would want to do it explicitly:
>
> ./drivers/scsi/scsi_scan.c: sdev->tagged_supported = 1; [in scsi_add_lun()]
That's set based on device capabilities.
> These look like attempts to override the SCSI mid-layer:
>
> ./drivers/scsi/fnic/fnic_main.c: sdev->tagged_supported = 1; [in '_slave_alloc()]
> ./drivers/scsi/stex.c: sdev->tagged_supported = 1; [in '_slave_alloc()]
> ./drivers/scsi/stex.c: sdev->tagged_supported = 1; [in '_slave_configure()]
> ./drivers/scsi/qla4xxx/ql4_os.c: sdev->tagged_supported = 1; [in '_slave_alloc()]
> ./drivers/scsi/qla4xxx/ql4_os.c: sdev->tagged_supported = 1; [in '_slave_configure()]
> ./drivers/scsi/ufs/ufshcd.c: sdev->tagged_supported = 1; [in '_slave_alloc()]
> ./drivers/scsi/scsi_debug.c: sdp->tagged_supported = 1; [in '_slave_configure()]
Slave configure is allowed to override this, so they all have reasons I
would suspect (qla is because tape is untagged, but qla still needs a
tag for the internal queue, for instance).
>
> However, it strikes me that it's unnecessary to set the flag in /both/
> '_slave_alloc() and '_slave_configure() -- I think that either one
> should work, and '_slave_configure() seems like the appropriate choice.
> (But, I'm left wondering why these drivers need to override the SCSI
> ML's choice....)
Stex is the only one that does this; it's probably just a make sure
thing. You might want to set in slave alloc if you need a tagged
Inquiry command.
> These are interesting, but not pertinent to the discussion at hand (they
> seem to be fall-backs triggered during error recovery):
>
> ./drivers/scsi/atari_NCR5380.c: cmd->device->tagged_supported = 0; [error recovery?]
> ./drivers/scsi/53c700.c: SCp->device->tagged_supported = 0; [error recovery?]
> ./drivers/scsi/sun3_NCR5380.c: cmd->device->tagged_supported = 0; [error recovery?]
SPI allows the device to reject the tag enabling messages ... that's
what this lot are all doing.
> And, this one looks spurious (that is, I believe it's clearing the flag
> when it's already clear):
>
> ./drivers/scsi/libsas/sas_scsi_host.c: scsi_dev->tagged_supported = 0; [looks spurious]
Pointless but harmless.
> So, taking the patch below, for example, why is tagged_supported being
> set in /both/ the '_slave_alloc() and the '_slave_configure() routines,
> and, in fact, why is it being set at all -- why doesn't the SCSI
> mid-layer set it correctly in this case?
Because things like the qla have to have tagged inquiry because it's a
shared host tag map used to label the issue queue. Setting it twice is
overkill because we don't actually turn it off, but it could be
construed as prudent in case we did (we could make tagged_supported
exactly reflect the state of the CmdQue bit for instance).
James
��.n��������+%������w��{.n�����{������ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
