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

Waiman Long posted 1 patch 3 days, 2 hours ago
drivers/usb/core/hub.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
[PATCH] usb: hub: Make usb_hub_wq type depend on isolcpus/nohz_full setting
Posted by Waiman Long 3 days, 2 hours ago
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