Linux USB
 help / color / mirror / Atom feed
* [PATCH] usb: hub: Make usb_hub_wq type depend on isolcpus/nohz_full setting
@ 2026-05-21 17:06 Waiman Long
  2026-05-27 14:21 ` Frederic Weisbecker
  0 siblings, 1 reply; 2+ messages in thread
From: Waiman Long @ 2026-05-21 17:06 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Mathias Nyman, Alan Stern, Kuen-Han Tsai
  Cc: linux-usb, linux-kernel, Vratislav Bendel, Waiman Long

A Red Hat customer reports a kernel stability problem where hung tasks
are reported with occasional kernel panics. Analysis of the core dump
indicates that USB work items are running on isolcpus+nohz_full cores
competing with RT-class tasks running on those core while holding
usb_hub device mutex transitively blocking other kworkers waiting for
the same mutex leading to hung_task reports.

As the usb_hub_wq uses the WQ_PERCPU flag, it will run the work items on
the same CPU that queues them. For many use cases, it is a more efficient
setup leading to higher throughput as it reduces cacheline bouncing.

It is a different story if the system needs to run latency sensitive RT
workload on dedicated isolated CPUs. Having the kworkers processing work
items on the same set of isolated CPUs will likely break the low latency
requirements of the RT tasks. As the RT tasks have higher priority,
not much CPU time will be left running the kworkers to process work
items which, in turn, will block other tasks that have dependency on
the completion of those work items. In this case, using a WQ_UNBOUND
workqueue to avoid running on isolated CPUs will be more beneficial.

One solution to get the best of both worlds is to make the workqueue
type depending on whether the "isolcpus" or "nohz_full" boot command
line options have been specified. If at least one of those options are
present, usb_hub_wq will be created as an unbound workqueue. Otherwise,
it will remain as a percpu workqueue.

Signed-off-by: Waiman Long <longman@redhat.com>
---
 drivers/usb/core/hub.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 24960ba9caa9..f79e5edd627a 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -33,6 +33,7 @@
 #include <linux/random.h>
 #include <linux/pm_qos.h>
 #include <linux/kobject.h>
+#include <linux/sched/isolation.h>
 
 #include <linux/bitfield.h>
 #include <linux/uaccess.h>
@@ -6066,6 +6067,8 @@ static struct usb_driver hub_driver = {
 
 int usb_hub_init(void)
 {
+	unsigned int wq_flags;
+
 	if (usb_register(&hub_driver) < 0) {
 		printk(KERN_ERR "%s: can't register hub driver\n",
 			usbcore_name);
@@ -6077,8 +6080,17 @@ int usb_hub_init(void)
 	 * USB-PERSIST port handover. Otherwise it might see that a full-speed
 	 * device was gone before the EHCI controller had handed its port
 	 * over to the companion full-speed controller.
+	 *
+	 * Create WQ_UNBOUND workqueue instead of WQ_PERCPU if either isolcpus
+	 * or nohz_full boot option is specified.
 	 */
-	hub_wq = alloc_workqueue("usb_hub_wq", WQ_FREEZABLE | WQ_PERCPU, 0);
+	if (housekeeping_enabled(HK_TYPE_DOMAIN) ||
+	    housekeeping_enabled(HK_TYPE_KERNEL_NOISE))
+		wq_flags = WQ_UNBOUND;
+	else
+		wq_flags = WQ_PERCPU;
+
+	hub_wq = alloc_workqueue("usb_hub_wq", WQ_FREEZABLE | wq_flags, 0);
 	if (hub_wq)
 		return 0;
 
-- 
2.54.0


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

* Re: [PATCH] usb: hub: Make usb_hub_wq type depend on isolcpus/nohz_full setting
  2026-05-21 17:06 [PATCH] usb: hub: Make usb_hub_wq type depend on isolcpus/nohz_full setting Waiman Long
@ 2026-05-27 14:21 ` Frederic Weisbecker
  0 siblings, 0 replies; 2+ messages in thread
From: Frederic Weisbecker @ 2026-05-27 14:21 UTC (permalink / raw)
  To: Waiman Long
  Cc: Greg Kroah-Hartman, Mathias Nyman, Alan Stern, Kuen-Han Tsai,
	linux-usb, linux-kernel, Vratislav Bendel

Le Thu, May 21, 2026 at 01:06:59PM -0400, Waiman Long a écrit :
> A Red Hat customer reports a kernel stability problem where hung tasks
> are reported with occasional kernel panics. Analysis of the core dump
> indicates that USB work items are running on isolcpus+nohz_full cores
> competing with RT-class tasks running on those core while holding
> usb_hub device mutex transitively blocking other kworkers waiting for
> the same mutex leading to hung_task reports.
> 
> As the usb_hub_wq uses the WQ_PERCPU flag, it will run the work items on
> the same CPU that queues them. For many use cases, it is a more efficient
> setup leading to higher throughput as it reduces cacheline bouncing.
> 
> It is a different story if the system needs to run latency sensitive RT
> workload on dedicated isolated CPUs. Having the kworkers processing work
> items on the same set of isolated CPUs will likely break the low latency
> requirements of the RT tasks. As the RT tasks have higher priority,
> not much CPU time will be left running the kworkers to process work
> items which, in turn, will block other tasks that have dependency on
> the completion of those work items. In this case, using a WQ_UNBOUND
> workqueue to avoid running on isolated CPUs will be more beneficial.
> 
> One solution to get the best of both worlds is to make the workqueue
> type depending on whether the "isolcpus" or "nohz_full" boot command
> line options have been specified. If at least one of those options are
> present, usb_hub_wq will be created as an unbound workqueue. Otherwise,
> it will remain as a percpu workqueue.
> 
> Signed-off-by: Waiman Long <longman@redhat.com>
> ---
>  drivers/usb/core/hub.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 24960ba9caa9..f79e5edd627a 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -33,6 +33,7 @@
>  #include <linux/random.h>
>  #include <linux/pm_qos.h>
>  #include <linux/kobject.h>
> +#include <linux/sched/isolation.h>
>  
>  #include <linux/bitfield.h>
>  #include <linux/uaccess.h>
> @@ -6066,6 +6067,8 @@ static struct usb_driver hub_driver = {
>  
>  int usb_hub_init(void)
>  {
> +	unsigned int wq_flags;
> +
>  	if (usb_register(&hub_driver) < 0) {
>  		printk(KERN_ERR "%s: can't register hub driver\n",
>  			usbcore_name);
> @@ -6077,8 +6080,17 @@ int usb_hub_init(void)
>  	 * USB-PERSIST port handover. Otherwise it might see that a full-speed
>  	 * device was gone before the EHCI controller had handed its port
>  	 * over to the companion full-speed controller.
> +	 *
> +	 * Create WQ_UNBOUND workqueue instead of WQ_PERCPU if either isolcpus
> +	 * or nohz_full boot option is specified.
>  	 */
> -	hub_wq = alloc_workqueue("usb_hub_wq", WQ_FREEZABLE | WQ_PERCPU, 0);
> +	if (housekeeping_enabled(HK_TYPE_DOMAIN) ||
> +	    housekeeping_enabled(HK_TYPE_KERNEL_NOISE))

HK_TYPE_DOMAIN is supposed to be a subset of HK_TYPE_KERNEL_NOISE anyway so
the first should be enough.

> +		wq_flags = WQ_UNBOUND;
> +	else
> +		wq_flags = WQ_PERCPU;
> +
> +	hub_wq = alloc_workqueue("usb_hub_wq", WQ_FREEZABLE | wq_flags, 0);

But then what happens if no isolcpus= is passed but later cpuset creates
an isolated partition?

Tejun and Marco thought about introducing a WQ_PREFER_PERCPU flag that would
do what you want above. And the workqueue code should also handle dynamic
isolation, that is switch from per-cpu workqueues to unbound ones or vice-versa
dynamically.

Marco is working on it.

Thanks.

>  	if (hub_wq)
>  		return 0;
>  
> -- 
> 2.54.0
> 
> 

-- 
Frederic Weisbecker
SUSE Labs

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

end of thread, other threads:[~2026-05-27 14:21 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-21 17:06 [PATCH] usb: hub: Make usb_hub_wq type depend on isolcpus/nohz_full setting Waiman Long
2026-05-27 14:21 ` Frederic Weisbecker

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox