uas breakage when using scsi_mod.blk_mq=Y

uas breakage when using scsi_mod.blk_mq=Y

[Hans de Goede]

Hi Christoph,

Douglas Gilbert (in the CC), has been testing uas with
scsi_mod.blk_mq=Y and this fails. When it fails the following
messages appear in dmesg:

kernel: scsi host8: uas
kernel: blk-mq: reduced tag depth to 10240
mtp-probe: checking bus 2, device 3: "/sys/devices/pci0000:00/0000:00:14.0/usb2/2-1"
mtp-probe: bus: 2, device: 3 was not an MTP device
kernel: scsi 8:0:0:0: Direct-Access     INTEL SS DSA2M080G2GC     2CV1 PQ: 0 ANSI: 6
kernel: sd 8:0:0:0: [sdb] 156301484 512-byte logical blocks: (80.0 GB/74.5 GiB)
kernel: sd 8:0:0:0: Attached scsi generic sg1 type 0
kernel: sd 8:0:0:0: [sdb] Write Protect is off
kernel: sd 8:0:0:0: [sdb] Mode Sense: 31 00 00 00
kernel: sd 8:0:0:0: [sdb] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
kernel: sdb: sdb1 sdb2
kernel: sd 8:0:0:0: [sdb] Attached SCSI disk
kernel: xhci_hcd 0000:00:14.0: WARN: Slot ID 8, ep index 14 has stream IDs 1 to 32 allocated, but stream ID 33 is requested.
kernel: sd 8:0:0:0: [sdb] sense submit err -22 tag 33 inflight: s-st a-in s-in a-cmd s-cmd
kernel: sd 8:0:0:0: [sdb] CDB:
kernel: Read(10): 28 00 00 00 01 6c 00 00 04 00

The problematic part here, which I believe is caused by scsi_mod.blk_mq=Y,
is the tag number 33. uas.c does the following in slave_configure:

	scsi_activate_tcq(sdev, devinfo->qdepth - 2);

Where qdepth is 32, so 30 gets passed in. uas.c stranslates scsi tags
to uas stream ids, which means it adds 2 (stream ids start at 1 not 0,
and 1 is reserved for untagged commands).

So the tag 33 above, means that the scsi subsys has called uas.c with
a tagged command with a tag of 31, which should not happen when using
scsi_activate_tcq(sdev, 30).

So should the uas.c code do something different with blk-mq to tell
it to only use tags 0-29, or is this a blk-mq bug ?

Regards,

Hans

Re: uas breakage when using scsi_mod.blk_mq=Y

[Christoph Hellwig]

On Wed, Oct 01, 2014 at 10:17:41AM +0200, Hans de Goede wrote:
> The problematic part here, which I believe is caused by scsi_mod.blk_mq=Y,
> is the tag number 33. uas.c does the following in slave_configure:
> 
> 	scsi_activate_tcq(sdev, devinfo->qdepth - 2);
> 
> Where qdepth is 32, so 30 gets passed in. uas.c stranslates scsi tags
> to uas stream ids, which means it adds 2 (stream ids start at 1 not 0,
> and 1 is reserved for untagged commands).
> 
> So the tag 33 above, means that the scsi subsys has called uas.c with
> a tagged command with a tag of 31, which should not happen when using
> scsi_activate_tcq(sdev, 30).
> 
> So should the uas.c code do something different with blk-mq to tell
> it to only use tags 0-29, or is this a blk-mq bug ?

This is a mismatch between the (undocumented) existing behavior and
what blk-mq implements.  In the old code unless you use host-shared
maps you would never see a tag number greater than the queue depth
when using block layer tags.  With blk-mq we also use host-wide maps,
so you can easily see tag numbers bigger than the queue depth.

So far it seems uas is the only driver with this expectation.
Given how it maps tags to a non-scsi concept it might be better to
just use a separate bitmaps for the streams inside uas than reusing
the tags.  Is this mapping an implementation detail of the Linux uas
driver or part of the spec?

Re: uas breakage when using scsi_mod.blk_mq=Y

[Hans de Goede]

Hi,

On 10/01/2014 02:45 PM, Christoph Hellwig wrote:
> On Wed, Oct 01, 2014 at 10:17:41AM +0200, Hans de Goede wrote:
>> The problematic part here, which I believe is caused by scsi_mod.blk_mq=Y,
>> is the tag number 33. uas.c does the following in slave_configure:
>>
>> 	scsi_activate_tcq(sdev, devinfo->qdepth - 2);
>>
>> Where qdepth is 32, so 30 gets passed in. uas.c stranslates scsi tags
>> to uas stream ids, which means it adds 2 (stream ids start at 1 not 0,
>> and 1 is reserved for untagged commands).
>>
>> So the tag 33 above, means that the scsi subsys has called uas.c with
>> a tagged command with a tag of 31, which should not happen when using
>> scsi_activate_tcq(sdev, 30).
>>
>> So should the uas.c code do something different with blk-mq to tell
>> it to only use tags 0-29, or is this a blk-mq bug ?
> 
> This is a mismatch between the (undocumented) existing behavior and
> what blk-mq implements.  In the old code unless you use host-shared
> maps you would never see a tag number greater than the queue depth
> when using block layer tags.  With blk-mq we also use host-wide maps,
> so you can easily see tag numbers bigger than the queue depth.
> 
> So far it seems uas is the only driver with this expectation.
> Given how it maps tags to a non-scsi concept it might be better to
> just use a separate bitmaps for the streams inside uas than reusing
> the tags.

So let me see if I understand this correctly, blk-mq will never queue
more then qdepth commands, but it will use higher tag numbers ?

If that is the case fixing this should be easy, uas already has
an array to map stream-ids to scsi cmnds so that when it gets a status
urb, it can find the scsi cmnd which belongs to that status urb. We
can just search for a free slot in that array.

> Is this mapping an implementation detail of the Linux uas
> driver or part of the spec?

The mapping is a Linux uas implementation detail, the spec is
silent on how to map things.

Regards,

Hans

Re: uas breakage when using scsi_mod.blk_mq=Y

[Christoph Hellwig]

On Wed, Oct 01, 2014 at 05:53:10PM +0200, Hans de Goede wrote:
> So let me see if I understand this correctly, blk-mq will never queue
> more then qdepth commands, but it will use higher tag numbers ?

Correct.

> If that is the case fixing this should be easy, uas already has
> an array to map stream-ids to scsi cmnds so that when it gets a status
> urb, it can find the scsi cmnd which belongs to that status urb. We
> can just search for a free slot in that array.

That sounds good.