linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC] usb: uas: implement .change_queue_depth to allow per-device queue depth override
@ 2026-10-04 11:41 Luca Cecchi
  2026-10-08 11:00 ` Oliver Neukum
  0 siblings, 1 reply; 16+ messages in thread
From: Luca Cecchi @ 2026-10-04 11:41 UTC (permalink / raw)
  To: linux-usb; +Cc: linux-scsi, oneukum

Hi Oliver,

First time writing to this list, so apologies in advance if anything about
the format is off. I ran into a UAS bridge that locks up under deep
concurrent command queueing, worked around it, and ended up with a small
generic patch that I think could be useful beyond my specific device. I'd
like to hear whether this approach makes sense before trying to turn it
into a proper patch submission.

THE GAP

uas_host_template does not implement .change_queue_depth. Because of that,
the standard sysfs queue_depth attribute stays read-only for every UAS
device, not just mine: scsi_sysfs.c:sdev_store_queue_depth() requires
sht->change_queue_depth to be non-NULL before it allows a write.

uas_probe() already defaults can_queue to qdepth - 2, with a comment
acknowledging that "some bridge firmwares" need extra margin:

      /*
       * 1 tag is reserved for untagged commands +
       * 1 tag to avoid off by one errors in some bridge firmwares
       */
      shost->can_queue = devinfo->qdepth - 2;

That margin isn't enough for every bridge. Right now the only way to work
around a bridge that needs more headroom is the IGNORE_UAS quirk, which
disables UAS entirely and falls back to BOT/usb-storage - a large
performance cost for a queue-depth problem.

THE PATCH

--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -910,6 +910,23 @@ static int uas_sdev_configure(struct scsi_device *sdev,
      return 0;
 }

+/*
+ * uas does not implement .change_queue_depth, so the standard sysfs
+ * queue_depth attribute stays read-only for every UAS device (see
+ * scsi_sysfs.c:sdev_store_queue_depth(), which requires
+ * sht->change_queue_depth != NULL). Some bridge chips become
+ * unstable under deep command queueing; wiring this up lets affected
+ * users lower the depth for just their device (e.g. via a udev rule
+ * matching idVendor/idProduct), without disabling UAS altogether.
+ * Default behaviour (qdepth - 2) is unchanged unless userspace asks
+ * for less.
+ */
+static int uas_change_queue_depth(struct scsi_device *sdev, int depth)
+{
+     struct uas_dev_info *devinfo = (struct uas_dev_info
*)sdev->host->hostdata;
+     int max_depth = devinfo->qdepth - 2;
+
+     if (depth > max_depth)
+             depth = max_depth;
+     return scsi_change_queue_depth(sdev, depth);
+}
+
 static const struct scsi_host_template uas_host_template = {
      .module = THIS_MODULE,
      .name = "uas",
@@ -917,6 +934,7 @@ static const struct scsi_host_template uas_host_template = {
      .target_alloc = uas_target_alloc,
      .sdev_init = uas_sdev_init,
      .sdev_configure = uas_sdev_configure,
+     .change_queue_depth = uas_change_queue_depth,
      .eh_abort_handler = uas_eh_abort_handler,
      .eh_host_reset_handler = uas_eh_host_reset_handler,
      .this_id = -1,

Nothing changes by default for any device. It only becomes possible to
ask for a lower depth, per device, through the existing sysfs interface -
e.g. a udev rule matching a specific idVendor/idProduct:

ACTION=="add", SUBSYSTEM=="scsi", ATTR{type}=="0", \
  ATTRS{idVendor}=="21c4", ATTRS{idProduct}=="0003", \
  RUN+="/bin/sh -c 'echo 24 > /sys%p/queue_depth'"

THE CASE THAT LED HERE

Device: Lexar ES3 external SSD enclosure, VID:PID 21c4:0003, USB 3.x
SuperSpeed (qdepth=32, can_queue=30 by default). Host: Lenovo 82KU, AMD
Ryzen 5 5500U, onboard AMD Renoir/Cezanne USB 3.1 controller [1022:1639].
Kernel: 7.2.7 (CachyOS-BORE; BORE only touches the CPU scheduler, verified
it doesn't touch USB/storage).

Under sustained heavy concurrent random writes (32 processes, one
outstanding command each, 64KiB blocks, O_DIRECT), the bridge stops
responding entirely after a period of healthy operation (anywhere from a
few seconds to several minutes). No STALL, NAK, or malformed reply visible
on the bus - the device simply goes silent. After the 30s default SCSI
timeout, the kernel aborts every in-flight command (uas_eh_abort_handler)
and resets the device (uas_eh_host_reset_handler), successfully.

A usbmon + Wireshark capture of one such event shows a clean 3-pipe UAS
cycle (32B command on EP 0x01, 64KiB data on EP 0x04, status on EP 0x82,
~160us per cycle) right up to the last normal packet at 10:27:45.752012,
followed by complete silence - zero packets in either direction - for
exactly 30.610467 seconds, until the first abort-triggered URB
cancellation at 10:28:16.362479.

I compared against the same device on the same host under Windows 11
(UASPStor driver): no lockup, 733MB/s sustained, max latency 194ms. A
USBPcap capture there showed Windows also keeping up to 30 commands truly
in flight (not serialized), using stream IDs 2-31; Linux stock
(can_queue=30) uses 1-30. Same count, one-off range, both inside the valid
1-31 window for MaxPStreams=5 - so "Linux reuses tags out of range" isn't
the explanation, and tag reuse cadence on Windows was comparable or
faster than what I measured on Linux. I could not fully isolate why the
same chip behaves differently between the two stacks (different
filesystem, and USB scheduling under the hood may differ in ways not
visible at this level), but empirically:

- can_queue=30 (the current default -2): locked up in every single test
  run (4/4), including one using parameters matching the Windows
  comparison exactly.
- Depth capped at 24 (via fio numjobs, before this patch existed): zero
  lockups across multiple 360s runs and a continuous 1-hour soak test
  (1.36TB written), at 819-823MB/s - faster than the Windows run.
- After implementing this patch plus the udev rule above: 6 consecutive
  clean runs, including with the application still requesting 32
  concurrent commands, confirming the kernel-enforced cap protects the
  device even when userspace asks for more than it's given.

QUESTION

Does this seem like a reasonable way to expose this, or would you rather
see it handled differently (e.g. through the quirks table with a numeric
value instead of a boolean, if that fits the existing infrastructure
better)? Happy to adjust the implementation, rerun tests, or send more
logs/captures if useful. I can also follow up with a proper patch
(Signed-off-by, etc.) once the approach itself looks right to you.

Disclosure: the investigation (usbmon/Wireshark analysis, the Windows
comparison, the fio testing methodology) and this patch were carried out
with the assistance of an AI coding assistant (Claude, Anthropic), under
my direction and with every result independently verified against kernel
source and live system behaviour before being included here.

Note: my first attempt at this email bounced from both lists for
containing an HTML part (a mail client default I didn't catch in time) -
apologies for the noise if a duplicate reaches you, Oliver.

Thanks for maintaining this driver.

Cecchi Luca
luca.cecchi.info@gmail.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2026-10-09  8:29 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-04 11:41 [RFC] usb: uas: implement .change_queue_depth to allow per-device queue depth override Luca Cecchi
2026-10-08 11:00 ` Oliver Neukum
2026-10-08 11:26   ` [PATCH] " Luca Cecchi
2026-10-08 11:36     ` sashiko-bot
2026-10-08 12:26     ` [PATCH v2 1/2] " Luca Cecchi
2026-10-08 12:26       ` [PATCH v2 2/2] usb: uas: add US_FL_QDEPTH_075 quirk to cap queue depth at probe time Luca Cecchi
2026-10-08 12:33         ` sashiko-bot
2026-10-08 12:32       ` [PATCH v2 1/2] usb: uas: implement .change_queue_depth to allow per-device queue depth override sashiko-bot
2026-10-08 13:32       ` Oliver Neukum
2026-10-08 14:21       ` Alan Stern
2026-10-08 20:19         ` Luca Cecchi
2026-10-09  8:03           ` Luca Cecchi
2026-10-09  8:12             ` [PATCH v3 " Luca Cecchi
2026-10-09  8:12               ` [PATCH v3 2/2] usb: uas: add US_FL_QDEPTH_075 quirk to cap queue depth at probe time Luca Cecchi
2026-10-09  8:20                 ` sashiko-bot
2026-10-09  8:29               ` [PATCH v3 1/2] usb: uas: implement .change_queue_depth to allow per-device queue depth override sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).