[PATCH] usb: core: sysfs: show product, manufacturer and serial without the device lock

[PATCH] usb: core: sysfs: show product, manufacturer and serial without the device lock

[Joe Finzel via B4 Relay]

From: Joe Finzel <joe@sudmix.de>

Reading the product, manufacturer or serial attribute of a USB device
takes the device lock. A hub holds its lock in hub_event() while it
enumerates all of its ports, including the debounce and reset delays of
every child, so a reader of the hub's strings waits until the last port
is done -- seconds on a hub with several devices.

udev reads these strings for every USB device (the usb_id builtin, run
from 50-udev-default.rules). Its worker blocks SIGTERM, so when systemd
stops udevd in the initrd before switching root, it waits for that
worker. On a desktop with a few hubs this delays every boot:

  i7-13700K, ASUS ROG STRIX Z790-F, four hubs with HID devices behind
  them: udevd took 1.7 s to stop, while the hubs were still enumerating.

  QEMU, qemu-xhci with 8 usb-hubs of 8 HID devices each, second boot,
  three runs: udevd stops in 2.55-2.58 s (switch-root at 5.56-5.59 s);
  with this patch in 11-12 ms (switch-root at 3.02 s). Without usb_id in
  the udev rules, it stops in 9 ms as well.

  -device qemu-xhci,id=xhci,p2=8,p3=8
  -device usb-hub,bus=xhci.0,port=N       (N = 1..8)
  -device usb-kbd,bus=xhci.0,port=N.M   (M = 1..8; also mouse, tablet)

The lock is not needed: usb_enumerate_device() sets the strings once,
before device_add() makes the attributes visible, and usb_release_dev()
frees them only after the attributes are gone. Nothing changes them in
between: authorizing or deauthorizing a device no longer touches them
since Wireless USB, for which they could appear and disappear, was
removed. dev_string_attrs_are_visible() already reads them unlocked.

Andy Whitcroft reported the same boot stall in 2010 ("khubd -- switch
USB product/manufacturer/serial handling to RCU"); back then the strings
could still change under Wireless USB.

Signed-off-by: Joe Finzel <joe@sudmix.de>
---
Found while looking into a 1.7 s stall at the end of the initrd on a
desktop with several USB hubs; reproducible in QEMU (see the commit
message). Tested on 7.2.9 with the QEMU reproduction; applies to
usb-next.
---
 drivers/usb/core/sysfs.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/core/sysfs.c b/drivers/usb/core/sysfs.c
index d22dc7845..52e0a227c 100644
--- a/drivers/usb/core/sysfs.c
+++ b/drivers/usb/core/sysfs.c
@@ -120,21 +120,20 @@ static ssize_t devspec_show(struct device *dev, struct device_attribute *attr,
 static DEVICE_ATTR_RO(devspec);
 #endif
 
-/* String fields */
+/*
+ * String fields. usb_enumerate_device() reads them once, before the device
+ * is added, and they are freed only when it is released, so showing them
+ * needs no lock -- dev_string_attrs_are_visible() reads them unlocked, too.
+ * The device lock would make readers wait while a hub holds it to enumerate
+ * all of its ports.
+ */
 #define usb_string_attr(name)						\
 static ssize_t  name##_show(struct device *dev,				\
 		struct device_attribute *attr, char *buf)		\
 {									\
-	struct usb_device *udev;					\
-	int retval;							\
+	struct usb_device *udev = to_usb_device(dev);			\
 									\
-	udev = to_usb_device(dev);					\
-	retval = usb_lock_device_interruptible(udev);			\
-	if (retval < 0)							\
-		return -EINTR;						\
-	retval = sysfs_emit(buf, "%s\n", udev->name);			\
-	usb_unlock_device(udev);					\
-	return retval;							\
+	return sysfs_emit(buf, "%s\n", udev->name);			\
 }									\
 static DEVICE_ATTR_RO(name)
 

---
base-commit: 42bcac323cc3b16566fed8c5b250318ad02a0de0
change-id: 20261011-usb-string-attrs-74abcc8b0195

Best regards,
-- 
Joe Finzel <joe@sudmix.de>

[PATCH] usb: core: sysfs: show product, manufacturer and serial without the device lock

[Joe Finzel]

Reading the product, manufacturer or serial attribute of a USB device
takes the device lock. A hub holds its lock in hub_event() while it
enumerates all of its ports, including the debounce and reset delays of
every child, so a reader of the hub's strings waits until the last port
is done -- seconds on a hub with several devices.

udev reads these strings for every USB device (the usb_id builtin, run
from 50-udev-default.rules). Its worker blocks SIGTERM, so when systemd
stops udevd in the initrd before switching root, it waits for that
worker. On a desktop with a few hubs this delays every boot:

  i7-13700K, ASUS ROG STRIX Z790-F, four hubs with HID devices behind
  them: udevd took 1.7 s to stop, while the hubs were still enumerating.

  QEMU, qemu-xhci with 8 usb-hubs of 8 HID devices each, second boot,
  three runs: udevd stops in 2.55-2.58 s (switch-root at 5.56-5.59 s);
  with this patch in 11-12 ms (switch-root at 3.02 s). Without usb_id in
  the udev rules, it stops in 9 ms as well.

  -device qemu-xhci,id=xhci,p2=8,p3=8
  -device usb-hub,bus=xhci.0,port=N       (N = 1..8)
  -device usb-kbd,bus=xhci.0,port=N.M   (M = 1..8; also mouse, tablet)

The lock is not needed: usb_enumerate_device() sets the strings once,
before device_add() makes the attributes visible, and usb_release_dev()
frees them only after the attributes are gone. Nothing changes them in
between: authorizing or deauthorizing a device no longer touches them
since Wireless USB, for which they could appear and disappear, was
removed. dev_string_attrs_are_visible() already reads them unlocked.

Andy Whitcroft reported the same boot stall in 2010 ("khubd -- switch
USB product/manufacturer/serial handling to RCU"); back then the strings
could still change under Wireless USB.

Signed-off-by: Joe Finzel <joe@sudmix.de>
---
Found while looking into a 1.7 s stall at the end of the initrd on a
desktop with several USB hubs; reproducible in QEMU (see the commit
message). Tested on 7.2.9 with the QEMU reproduction; applies to
usb-next.
---
 drivers/usb/core/sysfs.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/core/sysfs.c b/drivers/usb/core/sysfs.c
index d22dc7845..52e0a227c 100644
--- a/drivers/usb/core/sysfs.c
+++ b/drivers/usb/core/sysfs.c
@@ -120,21 +120,20 @@ static ssize_t devspec_show(struct device *dev, struct device_attribute *attr,
 static DEVICE_ATTR_RO(devspec);
 #endif
 
-/* String fields */
+/*
+ * String fields. usb_enumerate_device() reads them once, before the device
+ * is added, and they are freed only when it is released, so showing them
+ * needs no lock -- dev_string_attrs_are_visible() reads them unlocked, too.
+ * The device lock would make readers wait while a hub holds it to enumerate
+ * all of its ports.
+ */
 #define usb_string_attr(name)						\
 static ssize_t  name##_show(struct device *dev,				\
 		struct device_attribute *attr, char *buf)		\
 {									\
-	struct usb_device *udev;					\
-	int retval;							\
+	struct usb_device *udev = to_usb_device(dev);			\
 									\
-	udev = to_usb_device(dev);					\
-	retval = usb_lock_device_interruptible(udev);			\
-	if (retval < 0)							\
-		return -EINTR;						\
-	retval = sysfs_emit(buf, "%s\n", udev->name);			\
-	usb_unlock_device(udev);					\
-	return retval;							\
+	return sysfs_emit(buf, "%s\n", udev->name);			\
 }									\
 static DEVICE_ATTR_RO(name)
 

---
base-commit: 42bcac323cc3b16566fed8c5b250318ad02a0de0
change-id: 20261011-usb-string-attrs-74abcc8b0195

Best regards,
-- 
Joe Finzel <joe@sudmix.de>

Re: [PATCH] usb: core: sysfs: show product, manufacturer and serial without the device lock

[sashiko-bot]

> From: Joe Finzel <joe@sudmix.de>
> 
> Reading the product, manufacturer or serial attribute of a USB device
> takes the device lock. A hub holds its lock in hub_event() while it
> enumerates all of its ports, including the debounce and reset delays of
> every child, so a reader of the hub's strings waits until the last port
> is done -- seconds on a hub with several devices.
> 
> udev reads these strings for every USB device (the usb_id builtin, run
> from 50-udev-default.rules). Its worker blocks SIGTERM, so when systemd
> stops udevd in the initrd before switching root, it waits for that
> worker. On a desktop with a few hubs this delays every boot:
> 
>   i7-13700K, ASUS ROG STRIX Z790-F, four hubs with HID devices behind
>   them: udevd took 1.7 s to stop, while the hubs were still enumerating.
> [ ... ]
> Andy Whitcroft reported the same boot stall in 2010 ("khubd -- switch
> USB product/manufacturer/serial handling to RCU"); back then the strings
> could still change under Wireless USB.
> 
> Signed-off-by: Joe Finzel <joe@sudmix.de>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261011-usb-string-attrs-v1-1-81c01b035237@sudmix.de?part=1

Re: [PATCH] usb: core: sysfs: show product, manufacturer and serial without the device lock

[Alan Stern]

On Sun, Oct 11, 2026 at 01:15:38AM +0200, Joe Finzel via B4 Relay wrote:
> From: Joe Finzel <joe@sudmix.de>
> 
> Reading the product, manufacturer or serial attribute of a USB device
> takes the device lock. A hub holds its lock in hub_event() while it
> enumerates all of its ports, including the debounce and reset delays of
> every child, so a reader of the hub's strings waits until the last port
> is done -- seconds on a hub with several devices.
> 
> udev reads these strings for every USB device (the usb_id builtin, run
> from 50-udev-default.rules). Its worker blocks SIGTERM, so when systemd
> stops udevd in the initrd before switching root, it waits for that
> worker. On a desktop with a few hubs this delays every boot:
> 
>   i7-13700K, ASUS ROG STRIX Z790-F, four hubs with HID devices behind
>   them: udevd took 1.7 s to stop, while the hubs were still enumerating.
> 
>   QEMU, qemu-xhci with 8 usb-hubs of 8 HID devices each, second boot,
>   three runs: udevd stops in 2.55-2.58 s (switch-root at 5.56-5.59 s);
>   with this patch in 11-12 ms (switch-root at 3.02 s). Without usb_id in
>   the udev rules, it stops in 9 ms as well.
> 
>   -device qemu-xhci,id=xhci,p2=8,p3=8
>   -device usb-hub,bus=xhci.0,port=N       (N = 1..8)
>   -device usb-kbd,bus=xhci.0,port=N.M   (M = 1..8; also mouse, tablet)
> 
> The lock is not needed: usb_enumerate_device() sets the strings once,
> before device_add() makes the attributes visible, and usb_release_dev()
> frees them only after the attributes are gone. Nothing changes them in
> between: authorizing or deauthorizing a device no longer touches them
> since Wireless USB, for which they could appear and disappear, was
> removed. dev_string_attrs_are_visible() already reads them unlocked.

No; it reads the pointers to but not the contents of the strings.  
However, this is a minor point.

> Andy Whitcroft reported the same boot stall in 2010 ("khubd -- switch
> USB product/manufacturer/serial handling to RCU"); back then the strings
> could still change under Wireless USB.
> 
> Signed-off-by: Joe Finzel <joe@sudmix.de>
> ---
> Found while looking into a 1.7 s stall at the end of the initrd on a
> desktop with several USB hubs; reproducible in QEMU (see the commit
> message). Tested on 7.2.9 with the QEMU reproduction; applies to
> usb-next.
> ---

You could submit v2 of this patch with the claim above removed (and also 
removed from the comment below).  When you do, you may add:

Reviewed-by: Alan Stern <stern@rowland.harvard.edu>

Alan Stern

>  drivers/usb/core/sysfs.c | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/usb/core/sysfs.c b/drivers/usb/core/sysfs.c
> index d22dc7845..52e0a227c 100644
> --- a/drivers/usb/core/sysfs.c
> +++ b/drivers/usb/core/sysfs.c
> @@ -120,21 +120,20 @@ static ssize_t devspec_show(struct device *dev, struct device_attribute *attr,
>  static DEVICE_ATTR_RO(devspec);
>  #endif
>  
> -/* String fields */
> +/*
> + * String fields. usb_enumerate_device() reads them once, before the device
> + * is added, and they are freed only when it is released, so showing them
> + * needs no lock -- dev_string_attrs_are_visible() reads them unlocked, too.
> + * The device lock would make readers wait while a hub holds it to enumerate
> + * all of its ports.
> + */
>  #define usb_string_attr(name)						\
>  static ssize_t  name##_show(struct device *dev,				\
>  		struct device_attribute *attr, char *buf)		\
>  {									\
> -	struct usb_device *udev;					\
> -	int retval;							\
> +	struct usb_device *udev = to_usb_device(dev);			\
>  									\
> -	udev = to_usb_device(dev);					\
> -	retval = usb_lock_device_interruptible(udev);			\
> -	if (retval < 0)							\
> -		return -EINTR;						\
> -	retval = sysfs_emit(buf, "%s\n", udev->name);			\
> -	usb_unlock_device(udev);					\
> -	return retval;							\
> +	return sysfs_emit(buf, "%s\n", udev->name);			\
>  }									\
>  static DEVICE_ATTR_RO(name)
>  
> 
> ---
> base-commit: 42bcac323cc3b16566fed8c5b250318ad02a0de0
> change-id: 20261011-usb-string-attrs-74abcc8b0195
> 
> Best regards,
> -- 
> Joe Finzel <joe@sudmix.de>