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

[Greg Kroah-Hartman]

On Fri, Jun 05, 2026 at 02:05:19PM -0400, Waiman Long wrote:
> On 5/27/26 10:21 AM, Frederic Weisbecker wrote:
> > 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@xxxxxxxxxx>
> > > ---
> > >   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.
> Yes, that is the ideal case. However, that is not currently what some users
> are doing as they haven't changed their setup yet.

What does this mean?  If this is a subset, just check that one please.

> > > +		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?

You didn't answer this question :(

thanks,

greg k-h