Thread (14 messages) flat view 14 messages, 5 authors, 5d ago

Re: [PATCH v3 net-next 5/6] net: usb: pegasus: Move long delayed work on system_dfl_long_wq

From: Alan Stern <stern@rowland.harvard.edu>
Date: 2026-08-28 14:10:09
Also in: linux-usb, lkml

On Fri, Aug 28, 2026 at 11:43:18AM +0200, Sebastian Andrzej Siewior wrote:
On 2026-08-27 12:01:02 [-0400], Alan Stern wrote:
quoted
quoted
quoted
These drivers have their own work queues because they are part of the block layer.
USB devices can share a device with a block device (storage & UAS) and
USB devices have common, per device operations, in particular reset
and runtime power management and disconnect handling. Because these operations
can be necessary to complete block IO neither they nor anything
they depend on can use IO to allocate memory. That is they need to
perform any memory allocation with GFP_NOIO or GFP_ATOMIC.
This is a networking driver. It has nothing to do with storage and UAS.
It uses USB, yes.
Ah, but a composite USB device can have both a networking interface and 
a mass-storage interface.
okay.
quoted
Suppose you have such a device, and suppose the disk attached to its 
mass-storage interface contains a swap partition.  Now suppose the 
device is being reset, and as part of the preparation for that reset the 
networking driver needs to flush its workqueue.  This means waiting 
until the work routines that are already running have completed.
The individual functions are usually independent. But if the USB core
would reset the whole device it would reset each function.
quoted
Since it's a general-purpose workqueue, you don't know what those work 
routines are going to do.  One of them might try to allocate memory 
using GFP_KERNEL.  Suppose that in order to satisfy the memory request, 
the kernel decides it needs to write some pages to the swap partition on 
the USB mass-storage interface.  But the mass-storage driver is stuck; 
it can't do anything until the device reset finishes.  Deadlock.
So you are saying, the USB-storage device is in reset and we wait until
the networking part finishes its workqueue flush. That flush is stuck
behind behind a memory allocation which waits on the storage device. So
any URB passed to usb_submit_urb() just waits for the reset to complete?
quoted
That's why USB drivers have to use their own workqueues.
While this does make sense I don't see how this is related to this
patch. The pegasus driver uses `system_long_wq'. This is a system wide
workqueue_struct and is not limited to USB or this driver.
This workqueue is per-CPU meaning if you enqueue the work item on CPU3
it will be executed on CPU3. However pegasus uses a delayed work item
and the timer can fire on any CPU so even if it is enqueued on CPU3 it
could be executed on CPU1.
It doesn't matter what CPU the work item runs on.  Here's the deadlock 
sequence, in brief:

	USB device reset cannot proceed until network interface's
	->pre_reset() method returns.

	The ->pre_reset() method cannot return until its call to
	flush_workqueue() returns.

	flush_workqueue() cannot return until the already executing
	work item finishes.

	The work item cannot finish until its kmalloc() call returns.

	kmalloc() won't return until the kernel can free up memory by 
	writing some pages to the swap partition.

	The write to the swap partition cannot take place until the
	usb_storage/uas driver carries it out.

	usb_storage/uas cannot do anything until the USB device reset
	is finished.
Therefore the suggested change system_long_wq -> system_dfl_long_wq
should not make a difference here: it is a different workqueue and it is
unbound (instead of per-CPU) but given the usage it is unchanged but
more obvious. Also its usage recommendations (use this for long running
items) is the same.

The plan is remove system_long_wq from the tree.
The point Oliver was making is that the driver shouldn't be using a 
general-purpose workqueue at all.  Switching from one general-purpose 
workqueue to another ignores this point; it's not the right thing to do.

Alan Stern
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help