scsi (uas) disks no longer using tagged command queuing

Taking the uas.c file from 3.17, and building it for 3.16 restores the use of tcq (debugged by adding a printk blk_rq_tagged + request->tag).

So either uas is doing something wrong which happened to work in 3.16, or something has broken in 3.17.

I've already added debug printk-s of scsi_device->tagged_supported, queue_depth, ordered_tags and simple_tags and those all look good (1, 29, 1, 1).

I've also tried setting disable_blk_mq and that does not help.

Any hints to help debugging this further (other then a bisect) are appreciated. If no-one has any smart ideas I guess I'll end up doing a full bisect.

Quoted 10 lines · Hans de Goede

Taking the uas.c file from 3.17, and building it for 3.16 restores the use of tcq (debugged by adding a printk blk_rq_tagged + request->tag).

So either uas is doing something wrong which happened to work in 3.16, or something has broken in 3.17.

I've already added debug printk-s of scsi_device->tagged_supported, queue_depth, ordered_tags and simple_tags and those all look good (1, 29, 1, 1).

Quoted 5 lines

I've also tried setting disable_blk_mq and that does not help.

Any hints to help debugging this further (other then a bisect) are appreciated. If no-one has any smart ideas I guess I'll end up doing a full bisect.

scsi-mq isn't enabled by default, so setting disable_blk_mq should indeed not make a difference.

One interesting thing with uas is that it uses scsi_init_shared_tag_map, which only few drivers do.

Can you apply a debug patch like the one below and see if that helps to poinpoint down the issue?

I've applied the patch, this results in the following new dmesg output when using uas:

[  120.602632] initialized host-wide tag map!

Thank you for looking into this.

Quoted 5 lines · Hans de Goede

I've applied the patch, this results in the following new dmesg output when using uas:

[  120.602632] initialized host-wide tag map!

Quoted 1 line

Thank you for looking into this.

So we're initializing the tag map, but scsi_activate_tcq doesn't pick it up. I can't really come up with a good explanation for it, but there even without that there is an elephant in the room: as part of the scsi-mq series I moved the bqt field used for this into a union with the new blk_mq_tag_set. Below is a patch to get rid of that union, can you try if that fixes it?

include/scsi/scsi_host.h +0 -0

index d0f69a3..bcffff2 100644
@@ -584,10 +584,8 @@ struct Scsi_Host {
 	 * Area to keep a shared tag map (if needed, will be
 	 * NULL if not).

Quoted 6 lines

So we're initializing the tag map, but scsi_activate_tcq doesn't pick it up. I can't really come up with a good explanation for it, but there even without that there is an elephant in the room: as part of the scsi-mq series I moved the bqt field used for this into a union with the new blk_mq_tag_set. Below is a patch to get rid of that union, can you try if that fixes it?

Unfortunately that does not fix it :|

Quoted 8 lines · Hans de Goede

Quoted 6 lines

So we're initializing the tag map, but scsi_activate_tcq doesn't pick it up. I can't really come up with a good explanation for it, but there even without that there is an elephant in the room: as part of the scsi-mq series I moved the bqt field used for this into a union with the new blk_mq_tag_set. Below is a patch to get rid of that union, can you try if that fixes it?

Unfortunately that does not fix it :|

Alright, I guess we need the big bisect hammer unfortunately.

We should be able to start with trying the first and last commit of the big SCSI merge:

start: fcc95a763444017288b318d48367098850c23c0d
end: c21a2c1a4973c8dde32557970fdb44eaa9489aeb

From 865a19b760d2786fe37d3b5c151a4ecea4c0e95e Mon Sep 17 00:00:00 2001

Please try the fix below, looks like the commit broke TCQ for all drivers using block-level tagging.

---
From 865a19b760d2786fe37d3b5c151a4ecea4c0e95e Mon Sep 17 00:00:00 2001
From: Christoph Hellwig <[email protected]>
Date: Fri, 12 Sep 2014 16:00:19 -0700
Subject: scsi: fix regression that accidentally disabled block-based tcq

The scsi blk-mq support accidentally flipped a conditional, which lead to never enabling block based tcq when using the legacy request path.

Fixes d285203cf647d7c9 scsi: add support for a blk-mq based I/O path.

Reported-by Hans de Goede <[email protected]>

Signed-off-by Christoph Hellwig <[email protected]>

---
 include/scsi/scsi_tcq.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

include/scsi/scsi_tcq.h +1 -1

index cdcc90b..e645835 100644
@@ -68,7 +68,7 @@ static inline void scsi_activate_tcq(struct scsi_device *sdev, int depth)
 		return;
 
 	if (!shost_use_blk_mq(sdev->host) &&
-	    blk_queue_tagged(sdev->request_queue))
+	    !blk_queue_tagged(sdev->request_queue))
 		blk_queue_init_tags(sdev->request_queue, depth,
 				    sdev->host->bqt);
 

Yes this one does the trick and fixes things. Note the git tree I used for testing also had your previous fix to split up the blk_tcq union in 2 separate struct members. Let me know if you want me to re-test without that fix.

Quoted 4 lines · Hans de Goede

Yes this one does the trick and fixes things. Note the git tree I used for testing also had your previous fix to split up the blk_tcq union in 2 separate struct members. Let me know if you want me to re-test without that fix.

I'm pretty sure that isn't needed, so double testing it indeed doesn't would be helpful.

I'd like to add your Tested-by: tag after that, too.

Please try the fix below, looks like the commit broke TCQ for all drivers using block-level tagging.

---
From 865a19b760d2786fe37d3b5c151a4ecea4c0e95e Mon Sep 17 00:00:00 2001
From: Christoph Hellwig <[email protected]>
Date: Fri, 12 Sep 2014 16:00:19 -0700
Subject: scsi: fix regression that accidentally disabled block-based tcq

The scsi blk-mq support accidentally flipped a conditional, which lead to never enabling block based tcq when using the legacy request path.

Fixes d285203cf647d7c9 scsi: add support for a blk-mq based I/O path.

Reported-by Hans de Goede <[email protected]>

Signed-off-by Christoph Hellwig <[email protected]>

---
  include/scsi/scsi_tcq.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

include/scsi/scsi_tcq.h +0 -0

index cdcc90b..e645835 100644

Please try the fix below, looks like the commit broke TCQ for all drivers using block-level tagging.

---
From 865a19b760d2786fe37d3b5c151a4ecea4c0e95e Mon Sep 17 00:00:00 2001
From: Christoph Hellwig <[email protected]>
Date: Fri, 12 Sep 2014 16:00:19 -0700
Subject: scsi: fix regression that accidentally disabled block-based tcq

The scsi blk-mq support accidentally flipped a conditional, which lead to never enabling block based tcq when using the legacy request path.

Fixes d285203cf647d7c9 scsi: add support for a blk-mq based I/O path.

Reported-by Hans de Goede <[email protected]>

Signed-off-by Christoph Hellwig <[email protected]>

---
  include/scsi/scsi_tcq.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

include/scsi/scsi_tcq.h +0 -0

index cdcc90b..e645835 100644

Please try the fix below, looks like the commit broke TCQ for all drivers using block-level tagging.

---
From 865a19b760d2786fe37d3b5c151a4ecea4c0e95e Mon Sep 17 00:00:00 2001
From: Christoph Hellwig <[email protected]>
Date: Fri, 12 Sep 2014 16:00:19 -0700
Subject: scsi: fix regression that accidentally disabled block-based tcq

The scsi blk-mq support accidentally flipped a conditional, which lead to never enabling block based tcq when using the legacy request path.

Fixes d285203cf647d7c9 scsi: add support for a blk-mq based I/O path.

Reported-by Hans de Goede <[email protected]>

Signed-off-by Christoph Hellwig <[email protected]>

---
 include/scsi/scsi_tcq.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

include/scsi/scsi_tcq.h +0 -0

index cdcc90b..e645835 100644