usb: offload: Decouple interrupter lifecycle and refactor usage tracking

PATCH In review Guan-Yu Lin 3 participants 5 messages Apr 1

Subsystems (4)

Stable branch USB Sound USB xHCI driver

lore

Share & tools

lore

Guan-Yu Lin

author

2026-04-01 12:32 UTC #0 lore raw

v4 cover usb: offload: Decouple interrupter lifecycle and refactor usage tracking

The current USB offload implementation couples the allocation of xHCI sideband interrupters with the device's offload usage counter. This coupling is conceptually incorrect, as hardware resource availability and power management state serve distinct purposes.

Furthermore, the reliance on the coarse USB device lock for offload state updates has led to potential recursive locking issues, especially during device disconnect when the lock is already held by the USB core.

This series refactors the offload synchronization by introducing a dedicated spinlock for offload state, allowing for more granular concurrency control and avoiding deadlocks. It also optimizes power management by ensuring that offload state is only modified when the device is already active, avoiding unnecessary auto-resumes.

Patch 1 introduces the offload_lock spinlock and offload_pm_locked synchronization, replacing the coarse udev->lock and the legacy offload_at_suspend flag. It also updates usb_offload_get/put to use pm_runtime_get_if_active().

Patch 2 removes the implicit usage tracking from the xHCI sideband layer and delegates the responsibility to class drivers, who have the correct context for managing offload data stream activity.

---
Changes in v4:
- remove spinlock in usb_offload_check.
- Collect the <Tested-by> tag from the OPPO team.
- Link to v3: https://lore.kernel.org/all/[email protected]/

Changes in v3:
- Replace the coarse USB device lock with a dedicated offload_lock spinlock to reduce contention and prevent recursive locking.
- Introduce offload_pm_locked to synchronize with PM transitions and replace the legacy offload_at_suspend flag.
- Optimize usb_offload_get/put by switching from auto-resume/suspend to pm_runtime_get_if_active(), avoiding unnecessary power transitions.
- Explicitly delegate offload_usage tracking to USB class drivers (e.g., the Qualcomm USB audio offload driver).
- Link to v2: https://lore.kernel.org/all/[email protected]/

Changes in v2:
- Collect the <Tested-by> tag from the OPPO team.
- Link to v1: https://lore.kernel.org/all/[email protected]/

Changes in v1:
- Fix build error when building sound/usb/qcom/qc_audio_offload.o.
- Link to RFC v2: https://lore.kernel.org/all/[email protected]/

Changes in RFC v2:
- Move device locking to callers.
- Decouple sideband from offload counting.
- Link to RFC v1: https://lore.kernel.org/all/[email protected]/
---
Guan-Yu Lin (2):
usb: core: use dedicated spinlock for offload state
usb: host: xhci-sideband: delegate offload_usage tracking to class
drivers

 drivers/usb/core/driver.c         |  23 ++++---
 drivers/usb/core/offload.c        | 102 +++++++++++++++++-------------
 drivers/usb/core/usb.c            |   1 +
 drivers/usb/host/xhci-sideband.c  |  18 +-----
 include/linux/usb.h               |  10 ++-
 sound/usb/qcom/qc_audio_offload.c |  10 ++-
 6 files changed, 94 insertions(+), 70 deletions(-)

Signature · 1 line

2.53.0.1018.g2bb0e51243-goog

Guan-Yu Lin

author

2026-04-01 12:32 UTC #1 lore raw

PATCH v4 1/2 usb: core: use dedicated spinlock for offload state

Replace the coarse USB device lock with a dedicated offload_lock spinlock to reduce contention during offload operations. Use offload_pm_locked to synchronize with PM transitions and replace the legacy offload_at_suspend flag.

Optimize usb_offload_get/put by switching from auto-resume/suspend to pm_runtime_get_if_active(). This ensures offload state is only modified when the device is already active, avoiding unnecessary power transitions.

Cc [email protected]

Fixes ef82a4803aab ("xhci: sideband: add api to trace sideband usage")

Signed-off-by Guan-Yu Lin <[email protected]>

Tested-by Hailong Liu <[email protected]>

---
 drivers/usb/core/driver.c        |  23 ++++---
 drivers/usb/core/offload.c       | 102 ++++++++++++++++++-------------
 drivers/usb/core/usb.c           |   1 +
 drivers/usb/host/xhci-sideband.c |   4 +-
 include/linux/usb.h              |  10 ++-
 5 files changed, 84 insertions(+), 56 deletions(-)

drivers/usb/core/driver.c +14 -9

index d29edc7c616a..74b8bdc27dbf 100644
@@ -1415,14 +1415,16 @@ static int usb_suspend_both(struct usb_device *udev, pm_message_t msg)
 	int			status = 0;
 	int			i = 0, n = 0;
 	struct usb_interface	*intf;
+	bool			offload_active = false;
 
 	if (udev->state == USB_STATE_NOTATTACHED ||
 			udev->state == USB_STATE_SUSPENDED)
 		goto done;
 
+	usb_offload_set_pm_locked(udev, true);
 	if (msg.event == PM_EVENT_SUSPEND && usb_offload_check(udev)) {
 		dev_dbg(&udev->dev, "device offloaded, skip suspend.\n");
-		udev->offload_at_suspend = 1;
+		offload_active = true;
 	}
 
 	/* Suspend all the interfaces and then udev itself */

@@ -1436,8 +1438,7 @@ static int usb_suspend_both(struct usb_device *udev, pm_message_t msg)
 			 * interrupt urbs, allowing interrupt events to be
 			 * handled during system suspend.
 			 */
-			if (udev->offload_at_suspend &&
-			    intf->needs_remote_wakeup) {
+			if (offload_active && intf->needs_remote_wakeup) {
 				dev_dbg(&intf->dev,
 					"device offloaded, skip suspend.\n");
 				continue;

@@ -1452,7 +1453,7 @@ static int usb_suspend_both(struct usb_device *udev, pm_message_t msg)
 		}
 	}
 	if (status == 0) {
-		if (!udev->offload_at_suspend)
+		if (!offload_active)
 			status = usb_suspend_device(udev, msg);
 
 		/*

@@ -1498,7 +1499,7 @@ static int usb_suspend_both(struct usb_device *udev, pm_message_t msg)
 	 */
 	} else {
 		udev->can_submit = 0;
-		if (!udev->offload_at_suspend) {
+		if (!offload_active) {
 			for (i = 0; i < 16; ++i) {
 				usb_hcd_flush_endpoint(udev, udev->ep_out[i]);
 				usb_hcd_flush_endpoint(udev, udev->ep_in[i]);

@@ -1507,6 +1508,8 @@ static int usb_suspend_both(struct usb_device *udev, pm_message_t msg)
 	}
 
  done:
+	if (status != 0)
+		usb_offload_set_pm_locked(udev, false);
 	dev_vdbg(&udev->dev, "%s: status %d\n", __func__, status);
 	return status;
 }

@@ -1536,16 +1539,19 @@ static int usb_resume_both(struct usb_device *udev, pm_message_t msg)
 	int			status = 0;
 	int			i;
 	struct usb_interface	*intf;
+	bool			offload_active = false;
 
 	if (udev->state == USB_STATE_NOTATTACHED) {
 		status = -ENODEV;
 		goto done;
 	}
 	udev->can_submit = 1;
+	if (msg.event == PM_EVENT_RESUME)
+		offload_active = usb_offload_check(udev);
 
 	/* Resume the device */
 	if (udev->state == USB_STATE_SUSPENDED || udev->reset_resume) {
-		if (!udev->offload_at_suspend)
+		if (!offload_active)
 			status = usb_resume_device(udev, msg);
 		else
 			dev_dbg(&udev->dev,

@@ -1562,8 +1568,7 @@ static int usb_resume_both(struct usb_device *udev, pm_message_t msg)
 			 * pending interrupt urbs, allowing interrupt events
 			 * to be handled during system suspend.
 			 */
-			if (udev->offload_at_suspend &&
-			    intf->needs_remote_wakeup) {
+			if (offload_active && intf->needs_remote_wakeup) {
 				dev_dbg(&intf->dev,
 					"device offloaded, skip resume.\n");
 				continue;

@@ -1572,11 +1577,11 @@ static int usb_resume_both(struct usb_device *udev, pm_message_t msg)
 					udev->reset_resume);
 		}
 	}
-	udev->offload_at_suspend = 0;
 	usb_mark_last_busy(udev);
 
  done:
 	dev_vdbg(&udev->dev, "%s: status %d\n", __func__, status);
+	usb_offload_set_pm_locked(udev, false);
 	if (!status)
 		udev->reset_resume = 0;
 	return status;

drivers/usb/core/offload.c +59 ~~-43~~

152 lines not on this page · raw message

drivers/usb/core/usb.c +1 -0

index e740f7852bcd..8f7ca084010f 100644
@@ -671,6 +671,7 @@ struct usb_device *usb_alloc_dev(struct usb_device *parent,
 	set_dev_node(&dev->dev, dev_to_node(bus->sysdev));
 	dev->state = USB_STATE_ATTACHED;
 	dev->lpm_disable_count = 1;
+	spin_lock_init(&dev->offload_lock);
 	dev->offload_usage = 0;
 	atomic_set(&dev->urbnum, 0);

drivers/usb/host/xhci-sideband.c +2 -2

index 2bd77255032b..1ddb64b0a48e 100644
@@ -291,8 +291,8 @@ EXPORT_SYMBOL_GPL(xhci_sideband_get_event_buffer);
  * Allow other drivers, such as usb controller driver, to check if there are
  * any sideband activity on the host controller. This information could be used
  * for power management or other forms of resource management. The caller should
- * ensure downstream usb devices are all either suspended or marked as
- * "offload_at_suspend" to ensure the correctness of the return value.
+ * ensure downstream usb devices are all marked as "offload_pm_locked" to ensure
+ * the correctness of the return value.
  *
  * Returns true on any active sideband existence, false otherwise.
  */

include/linux/usb.h +8 -2

index fbfcc70b07fb..a4b031196da3 100644
@@ -21,6 +21,7 @@
 #include <linux/completion.h>	/* for struct completion */
 #include <linux/sched.h>	/* for current && schedule_timeout */
 #include <linux/mutex.h>	/* for struct mutex */
+#include <linux/spinlock.h>	/* for spinlock_t */
 #include <linux/pm_runtime.h>	/* for runtime PM */
 
 struct usb_device;

@@ -636,8 +637,9 @@ struct usb3_lpm_parameters {
  * @do_remote_wakeup:  remote wakeup should be enabled
  * @reset_resume: needs reset instead of resume
  * @port_is_suspended: the upstream port is suspended (L2 or U3)
- * @offload_at_suspend: offload activities during suspend is enabled.
+ * @offload_pm_locked: prevents offload_usage changes during PM transitions.
  * @offload_usage: number of offload activities happening on this usb device.
+ * @offload_lock: protects offload_usage and offload_pm_locked
  * @slot_id: Slot ID assigned by xHCI
  * @l1_params: best effor service latency for USB2 L1 LPM state, and L1 timeout.
  * @u1_params: exit latencies for USB3 U1 LPM state, and hub-initiated timeout.

@@ -726,8 +728,9 @@ struct usb_device {
 	unsigned do_remote_wakeup:1;
 	unsigned reset_resume:1;
 	unsigned port_is_suspended:1;
-	unsigned offload_at_suspend:1;
+	unsigned offload_pm_locked:1;
 	int offload_usage;
+	spinlock_t offload_lock;
 	enum usb_link_tunnel_mode tunnel_mode;
 	struct device_link *usb4_link;

@@ -849,6 +852,7 @@ static inline void usb_mark_last_busy(struct usb_device *udev)
 int usb_offload_get(struct usb_device *udev);
 int usb_offload_put(struct usb_device *udev);
 bool usb_offload_check(struct usb_device *udev);
+void usb_offload_set_pm_locked(struct usb_device *udev, bool locked);
 #else
 
 static inline int usb_offload_get(struct usb_device *udev)

@@ -857,6 +861,8 @@ static inline int usb_offload_put(struct usb_device *udev)
 { return 0; }
 static inline bool usb_offload_check(struct usb_device *udev)
 { return false; }
+static inline void usb_offload_set_pm_locked(struct usb_device *udev, bool locked)
+{ }
 #endif
 
 extern int usb_disable_lpm(struct usb_device *udev);

Signature · 1 line

2.53.0.1018.g2bb0e51243-goog

Guan-Yu Lin

author

2026-04-01 12:32 UTC #2 lore raw

PATCH v4 2/2 usb: host: xhci-sideband: delegate offload_usage tracking to class drivers

In reply to Guan-Yu Lin

Remove usb_offload_get() and usb_offload_put() from the xHCI sideband interrupter creation and removal paths.

The responsibility of manipulating offload_usage now lies entirely with the USB class drivers. They have the precise context of when an offload data stream actually starts and stops, ensuring a much more accurate representation of offload activity for power management.

Cc [email protected]

Fixes ef82a4803aab ("xhci: sideband: add api to trace sideband usage")

Signed-off-by Guan-Yu Lin <[email protected]>

Tested-by Hailong Liu <[email protected]>

---
 drivers/usb/host/xhci-sideband.c  | 14 +-------------
 sound/usb/qcom/qc_audio_offload.c | 10 +++++++++-
 2 files changed, 10 insertions(+), 14 deletions(-)

drivers/usb/host/xhci-sideband.c +1 ~~-13~~

index 1ddb64b0a48e..651973606137 100644
@@ -93,8 +93,6 @@ __xhci_sideband_remove_endpoint(struct xhci_sideband *sb, struct xhci_virt_ep *e
 static void
 __xhci_sideband_remove_interrupter(struct xhci_sideband *sb)
 {
-	struct usb_device *udev;
-
 	lockdep_assert_held(&sb->mutex);
 
 	if (!sb->ir)

@@ -102,10 +100,6 @@ __xhci_sideband_remove_interrupter(struct xhci_sideband *sb)
 
 	xhci_remove_secondary_interrupter(xhci_to_hcd(sb->xhci), sb->ir);
 	sb->ir = NULL;
-	udev = sb->vdev->udev;
-
-	if (udev->state != USB_STATE_NOTATTACHED)
-		usb_offload_put(udev);
 }
 
 /* sideband api functions */

@@ -328,9 +322,6 @@ int
 xhci_sideband_create_interrupter(struct xhci_sideband *sb, int num_seg,
 				 bool ip_autoclear, u32 imod_interval, int intr_num)
 {
-	int ret = 0;
-	struct usb_device *udev;
-
 	if (!sb || !sb->xhci)
 		return -ENODEV;

@@ -348,12 +339,9 @@ xhci_sideband_create_interrupter(struct xhci_sideband *sb, int num_seg,
 	if (!sb->ir)
 		return -ENOMEM;
 
-	udev = sb->vdev->udev;
-	ret = usb_offload_get(udev);
-
 	sb->ir->ip_autoclear = ip_autoclear;
 
-	return ret;
+	return 0;
 }
 EXPORT_SYMBOL_GPL(xhci_sideband_create_interrupter);

sound/usb/qcom/qc_audio_offload.c +9 -1

index cfb30a195364..abafbc78dea9 100644
@@ -699,6 +699,7 @@ static void uaudio_event_ring_cleanup_free(struct uaudio_dev *dev)
 		uaudio_iommu_unmap(MEM_EVENT_RING, IOVA_BASE, PAGE_SIZE,
 				   PAGE_SIZE);
 		xhci_sideband_remove_interrupter(uadev[dev->chip->card->number].sb);
+		usb_offload_put(dev->udev);
 	}
 }

@@ -1182,12 +1183,16 @@ static int uaudio_event_ring_setup(struct snd_usb_substream *subs,
 	dma_coherent = dev_is_dma_coherent(subs->dev->bus->sysdev);
 	er_pa = 0;
 
+	ret = usb_offload_get(subs->dev);
+	if (ret < 0)
+		goto exit;
+
 	/* event ring */
 	ret = xhci_sideband_create_interrupter(uadev[card_num].sb, 1, false,
 					       0, uaudio_qdev->data->intr_num);
 	if (ret < 0) {
 		dev_err(&subs->dev->dev, "failed to fetch interrupter\n");
-		goto exit;
+		goto put_offload;
 	}
 
 	sgt = xhci_sideband_get_event_buffer(uadev[card_num].sb);

@@ -1219,6 +1224,8 @@ static int uaudio_event_ring_setup(struct snd_usb_substream *subs,
 	mem_info->dma = 0;
 remove_interrupter:
 	xhci_sideband_remove_interrupter(uadev[card_num].sb);
+put_offload:
+	usb_offload_put(subs->dev);
 exit:
 	return ret;
 }

@@ -1483,6 +1490,7 @@ static int prepare_qmi_response(struct snd_usb_substream *subs,
 	uaudio_iommu_unmap(MEM_EVENT_RING, IOVA_BASE, PAGE_SIZE, PAGE_SIZE);
 free_sec_ring:
 	xhci_sideband_remove_interrupter(uadev[card_num].sb);
+	usb_offload_put(subs->dev);
 drop_sync_ep:
 	if (subs->sync_endpoint) {
 		uaudio_iommu_unmap(MEM_XFER_RING,

Signature · 1 line

2.53.0.1018.g2bb0e51243-goog

Greg KH

maintainer

2026-04-01 13:38 UTC #3 lore raw

In reply to Guan-Yu Lin

Quoted 16 lines · Guan-Yu Lin

The current USB offload implementation couples the allocation of xHCI sideband interrupters with the device's offload usage counter. This coupling is conceptually incorrect, as hardware resource availability and power management state serve distinct purposes.
Furthermore, the reliance on the coarse USB device lock for offload state updates has led to potential recursive locking issues, especially during device disconnect when the lock is already held by the USB core.
This series refactors the offload synchronization by introducing a dedicated spinlock for offload state, allowing for more granular concurrency control and avoiding deadlocks. It also optimizes power management by ensuring that offload state is only modified when the device is already active, avoiding unnecessary auto-resumes.

Quoted 8 lines

Patch 1 introduces the offload_lock spinlock and offload_pm_locked synchronization, replacing the coarse udev->lock and the legacy offload_at_suspend flag. It also updates usb_offload_get/put to use pm_runtime_get_if_active().
Patch 2 removes the implicit usage tracking from the xHCI sideband layer and delegates the responsibility to class drivers, who have the correct context for managing offload data stream activity.

Ok, this looks much better, thanks for the revisions.

If the xhci maintainer acks these, I'll be glad to apply them to my tree.

thanks,

greg k-h

Mathias Nyman

Acked-by

2026-04-01 14:47 UTC #4 lore raw

Quoted 22 lines · Greg KH

Quoted 21 lines · Guan-Yu Lin
The current USB offload implementation couples the allocation of xHCI sideband interrupters with the device's offload usage counter. This coupling is conceptually incorrect, as hardware resource availability and power management state serve distinct purposes.
Furthermore, the reliance on the coarse USB device lock for offload state updates has led to potential recursive locking issues, especially during device disconnect when the lock is already held by the USB core.
This series refactors the offload synchronization by introducing a dedicated spinlock for offload state, allowing for more granular concurrency control and avoiding deadlocks. It also optimizes power management by ensuring that offload state is only modified when the device is already active, avoiding unnecessary auto-resumes.
Patch 1 introduces the offload_lock spinlock and offload_pm_locked synchronization, replacing the coarse udev->lock and the legacy offload_at_suspend flag. It also updates usb_offload_get/put to use pm_runtime_get_if_active().

Quoted 8 lines

Quoted 3 lines
Patch 2 removes the implicit usage tracking from the xHCI sideband layer and delegates the responsibility to class drivers, who have the correct context for managing offload data stream activity.
Ok, this looks much better, thanks for the revisions.
If the xhci maintainer acks these, I'll be glad to apply them to my tree.

For both patches:

Acked-by Mathias Nyman <[email protected]>