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
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Christoph Hellwig]

Post by Hans de Goede
The problematic part here, which I believe is caused by scsi_mod.blk_mq=Y,
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?
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Hans de Goede]

Hi,

Post by Christoph Hellwig

Post by Hans de Goede
The problematic part here, which I believe is caused by scsi_mod.blk_mq=Y,
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.

Post by Christoph Hellwig
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
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Christoph Hellwig]

Post by Hans de Goede
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.

Post by Hans de Goede
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.

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html