Skip to content

Commit bd3d245

Browse files
Guan-Yu Lingregkh
authored andcommitted
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: stable <stable@kernel.org> Fixes: ef82a48 ("xhci: sideband: add api to trace sideband usage") Signed-off-by: Guan-Yu Lin <guanyulin@google.com> Tested-by: Hailong Liu <hailong.liu@oppo.com> Acked-by: Mathias Nyman <mathias.nyman@linux.intel.com> Link: https://patch.msgid.link/20260401123238.3790062-2-guanyulin@google.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent c32f874 commit bd3d245

5 files changed

Lines changed: 84 additions & 56 deletions

File tree

drivers/usb/core/driver.c

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1415,14 +1415,16 @@ static int usb_suspend_both(struct usb_device *udev, pm_message_t msg)
14151415
int status = 0;
14161416
int i = 0, n = 0;
14171417
struct usb_interface *intf;
1418+
bool offload_active = false;
14181419

14191420
if (udev->state == USB_STATE_NOTATTACHED ||
14201421
udev->state == USB_STATE_SUSPENDED)
14211422
goto done;
14221423

1424+
usb_offload_set_pm_locked(udev, true);
14231425
if (msg.event == PM_EVENT_SUSPEND && usb_offload_check(udev)) {
14241426
dev_dbg(&udev->dev, "device offloaded, skip suspend.\n");
1425-
udev->offload_at_suspend = 1;
1427+
offload_active = true;
14261428
}
14271429

14281430
/* 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)
14361438
* interrupt urbs, allowing interrupt events to be
14371439
* handled during system suspend.
14381440
*/
1439-
if (udev->offload_at_suspend &&
1440-
intf->needs_remote_wakeup) {
1441+
if (offload_active && intf->needs_remote_wakeup) {
14411442
dev_dbg(&intf->dev,
14421443
"device offloaded, skip suspend.\n");
14431444
continue;
@@ -1452,7 +1453,7 @@ static int usb_suspend_both(struct usb_device *udev, pm_message_t msg)
14521453
}
14531454
}
14541455
if (status == 0) {
1455-
if (!udev->offload_at_suspend)
1456+
if (!offload_active)
14561457
status = usb_suspend_device(udev, msg);
14571458

14581459
/*
@@ -1498,7 +1499,7 @@ static int usb_suspend_both(struct usb_device *udev, pm_message_t msg)
14981499
*/
14991500
} else {
15001501
udev->can_submit = 0;
1501-
if (!udev->offload_at_suspend) {
1502+
if (!offload_active) {
15021503
for (i = 0; i < 16; ++i) {
15031504
usb_hcd_flush_endpoint(udev, udev->ep_out[i]);
15041505
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)
15071508
}
15081509

15091510
done:
1511+
if (status != 0)
1512+
usb_offload_set_pm_locked(udev, false);
15101513
dev_vdbg(&udev->dev, "%s: status %d\n", __func__, status);
15111514
return status;
15121515
}
@@ -1536,16 +1539,19 @@ static int usb_resume_both(struct usb_device *udev, pm_message_t msg)
15361539
int status = 0;
15371540
int i;
15381541
struct usb_interface *intf;
1542+
bool offload_active = false;
15391543

15401544
if (udev->state == USB_STATE_NOTATTACHED) {
15411545
status = -ENODEV;
15421546
goto done;
15431547
}
15441548
udev->can_submit = 1;
1549+
if (msg.event == PM_EVENT_RESUME)
1550+
offload_active = usb_offload_check(udev);
15451551

15461552
/* Resume the device */
15471553
if (udev->state == USB_STATE_SUSPENDED || udev->reset_resume) {
1548-
if (!udev->offload_at_suspend)
1554+
if (!offload_active)
15491555
status = usb_resume_device(udev, msg);
15501556
else
15511557
dev_dbg(&udev->dev,
@@ -1562,8 +1568,7 @@ static int usb_resume_both(struct usb_device *udev, pm_message_t msg)
15621568
* pending interrupt urbs, allowing interrupt events
15631569
* to be handled during system suspend.
15641570
*/
1565-
if (udev->offload_at_suspend &&
1566-
intf->needs_remote_wakeup) {
1571+
if (offload_active && intf->needs_remote_wakeup) {
15671572
dev_dbg(&intf->dev,
15681573
"device offloaded, skip resume.\n");
15691574
continue;
@@ -1572,11 +1577,11 @@ static int usb_resume_both(struct usb_device *udev, pm_message_t msg)
15721577
udev->reset_resume);
15731578
}
15741579
}
1575-
udev->offload_at_suspend = 0;
15761580
usb_mark_last_busy(udev);
15771581

15781582
done:
15791583
dev_vdbg(&udev->dev, "%s: status %d\n", __func__, status);
1584+
usb_offload_set_pm_locked(udev, false);
15801585
if (!status)
15811586
udev->reset_resume = 0;
15821587
return status;

drivers/usb/core/offload.c

Lines changed: 59 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -25,33 +25,30 @@
2525
*/
2626
int usb_offload_get(struct usb_device *udev)
2727
{
28-
int ret;
28+
int ret = 0;
2929

30-
usb_lock_device(udev);
31-
if (udev->state == USB_STATE_NOTATTACHED) {
32-
usb_unlock_device(udev);
30+
if (!usb_get_dev(udev))
3331
return -ENODEV;
34-
}
3532

36-
if (udev->state == USB_STATE_SUSPENDED ||
37-
udev->offload_at_suspend) {
38-
usb_unlock_device(udev);
39-
return -EBUSY;
33+
if (pm_runtime_get_if_active(&udev->dev) != 1) {
34+
ret = -EBUSY;
35+
goto err_rpm;
4036
}
4137

42-
/*
43-
* offload_usage could only be modified when the device is active, since
44-
* it will alter the suspend flow of the device.
45-
*/
46-
ret = usb_autoresume_device(udev);
47-
if (ret < 0) {
48-
usb_unlock_device(udev);
49-
return ret;
38+
spin_lock(&udev->offload_lock);
39+
40+
if (udev->offload_pm_locked) {
41+
ret = -EAGAIN;
42+
goto err;
5043
}
5144

5245
udev->offload_usage++;
53-
usb_autosuspend_device(udev);
54-
usb_unlock_device(udev);
46+
47+
err:
48+
spin_unlock(&udev->offload_lock);
49+
pm_runtime_put_autosuspend(&udev->dev);
50+
err_rpm:
51+
usb_put_dev(udev);
5552

5653
return ret;
5754
}
@@ -69,35 +66,32 @@ EXPORT_SYMBOL_GPL(usb_offload_get);
6966
*/
7067
int usb_offload_put(struct usb_device *udev)
7168
{
72-
int ret;
69+
int ret = 0;
7370

74-
usb_lock_device(udev);
75-
if (udev->state == USB_STATE_NOTATTACHED) {
76-
usb_unlock_device(udev);
71+
if (!usb_get_dev(udev))
7772
return -ENODEV;
78-
}
7973

80-
if (udev->state == USB_STATE_SUSPENDED ||
81-
udev->offload_at_suspend) {
82-
usb_unlock_device(udev);
83-
return -EBUSY;
74+
if (pm_runtime_get_if_active(&udev->dev) != 1) {
75+
ret = -EBUSY;
76+
goto err_rpm;
8477
}
8578

86-
/*
87-
* offload_usage could only be modified when the device is active, since
88-
* it will alter the suspend flow of the device.
89-
*/
90-
ret = usb_autoresume_device(udev);
91-
if (ret < 0) {
92-
usb_unlock_device(udev);
93-
return ret;
79+
spin_lock(&udev->offload_lock);
80+
81+
if (udev->offload_pm_locked) {
82+
ret = -EAGAIN;
83+
goto err;
9484
}
9585

9686
/* Drop the count when it wasn't 0, ignore the operation otherwise. */
9787
if (udev->offload_usage)
9888
udev->offload_usage--;
99-
usb_autosuspend_device(udev);
100-
usb_unlock_device(udev);
89+
90+
err:
91+
spin_unlock(&udev->offload_lock);
92+
pm_runtime_put_autosuspend(&udev->dev);
93+
err_rpm:
94+
usb_put_dev(udev);
10195

10296
return ret;
10397
}
@@ -112,25 +106,47 @@ EXPORT_SYMBOL_GPL(usb_offload_put);
112106
* management.
113107
*
114108
* The caller must hold @udev's device lock. In addition, the caller should
115-
* ensure downstream usb devices are all either suspended or marked as
116-
* "offload_at_suspend" to ensure the correctness of the return value.
109+
* ensure the device itself and the downstream usb devices are all marked as
110+
* "offload_pm_locked" to ensure the correctness of the return value.
117111
*
118112
* Returns true on any offload activity, false otherwise.
119113
*/
120114
bool usb_offload_check(struct usb_device *udev) __must_hold(&udev->dev->mutex)
121115
{
122116
struct usb_device *child;
123-
bool active;
117+
bool active = false;
124118
int port1;
125119

120+
if (udev->offload_usage)
121+
return true;
122+
126123
usb_hub_for_each_child(udev, port1, child) {
127124
usb_lock_device(child);
128125
active = usb_offload_check(child);
129126
usb_unlock_device(child);
127+
130128
if (active)
131-
return true;
129+
break;
132130
}
133131

134-
return !!udev->offload_usage;
132+
return active;
135133
}
136134
EXPORT_SYMBOL_GPL(usb_offload_check);
135+
136+
/**
137+
* usb_offload_set_pm_locked - set the PM lock state of a USB device
138+
* @udev: the USB device to modify
139+
* @locked: the new lock state
140+
*
141+
* Setting @locked to true prevents offload_usage from being modified. This
142+
* ensures that offload activities cannot be started or stopped during critical
143+
* power management transitions, maintaining a stable state for the duration
144+
* of the transition.
145+
*/
146+
void usb_offload_set_pm_locked(struct usb_device *udev, bool locked)
147+
{
148+
spin_lock(&udev->offload_lock);
149+
udev->offload_pm_locked = locked;
150+
spin_unlock(&udev->offload_lock);
151+
}
152+
EXPORT_SYMBOL_GPL(usb_offload_set_pm_locked);

drivers/usb/core/usb.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -671,6 +671,7 @@ struct usb_device *usb_alloc_dev(struct usb_device *parent,
671671
set_dev_node(&dev->dev, dev_to_node(bus->sysdev));
672672
dev->state = USB_STATE_ATTACHED;
673673
dev->lpm_disable_count = 1;
674+
spin_lock_init(&dev->offload_lock);
674675
dev->offload_usage = 0;
675676
atomic_set(&dev->urbnum, 0);
676677

drivers/usb/host/xhci-sideband.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -291,8 +291,8 @@ EXPORT_SYMBOL_GPL(xhci_sideband_get_event_buffer);
291291
* Allow other drivers, such as usb controller driver, to check if there are
292292
* any sideband activity on the host controller. This information could be used
293293
* for power management or other forms of resource management. The caller should
294-
* ensure downstream usb devices are all either suspended or marked as
295-
* "offload_at_suspend" to ensure the correctness of the return value.
294+
* ensure downstream usb devices are all marked as "offload_pm_locked" to ensure
295+
* the correctness of the return value.
296296
*
297297
* Returns true on any active sideband existence, false otherwise.
298298
*/

include/linux/usb.h

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include <linux/completion.h> /* for struct completion */
2222
#include <linux/sched.h> /* for current && schedule_timeout */
2323
#include <linux/mutex.h> /* for struct mutex */
24+
#include <linux/spinlock.h> /* for spinlock_t */
2425
#include <linux/pm_runtime.h> /* for runtime PM */
2526

2627
struct usb_device;
@@ -636,8 +637,9 @@ struct usb3_lpm_parameters {
636637
* @do_remote_wakeup: remote wakeup should be enabled
637638
* @reset_resume: needs reset instead of resume
638639
* @port_is_suspended: the upstream port is suspended (L2 or U3)
639-
* @offload_at_suspend: offload activities during suspend is enabled.
640+
* @offload_pm_locked: prevents offload_usage changes during PM transitions.
640641
* @offload_usage: number of offload activities happening on this usb device.
642+
* @offload_lock: protects offload_usage and offload_pm_locked
641643
* @slot_id: Slot ID assigned by xHCI
642644
* @l1_params: best effor service latency for USB2 L1 LPM state, and L1 timeout.
643645
* @u1_params: exit latencies for USB3 U1 LPM state, and hub-initiated timeout.
@@ -726,8 +728,9 @@ struct usb_device {
726728
unsigned do_remote_wakeup:1;
727729
unsigned reset_resume:1;
728730
unsigned port_is_suspended:1;
729-
unsigned offload_at_suspend:1;
731+
unsigned offload_pm_locked:1;
730732
int offload_usage;
733+
spinlock_t offload_lock;
731734
enum usb_link_tunnel_mode tunnel_mode;
732735
struct device_link *usb4_link;
733736

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

854858
static inline int usb_offload_get(struct usb_device *udev)
@@ -857,6 +861,8 @@ static inline int usb_offload_put(struct usb_device *udev)
857861
{ return 0; }
858862
static inline bool usb_offload_check(struct usb_device *udev)
859863
{ return false; }
864+
static inline void usb_offload_set_pm_locked(struct usb_device *udev, bool locked)
865+
{ }
860866
#endif
861867

862868
extern int usb_disable_lpm(struct usb_device *udev);

0 commit comments

Comments
 (0)