Re: [PATCH] usb: hub: Make usb_hub_wq type depend on isolcpus/nohz_full setting

[Frederic Weisbecker <frederic@kernel.org>]

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