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

[Oliver Neukum]

On 20.07.26 12:08, Marco Crivellari wrote:
Hi,

> Since the workqueue work doesn't rely on per-cpu variables, there is no
> obvious reason that justify the use of a per-cpu workqueue. So change
> system_long_wq with system_dfl_long_wq so that the work may benefit from
> scheduler task placement.

these changes are problematic, although they look like a good cleanup
in first place. But the test you are using to determine whether USB
devices need their own work queue is incomplete because you are not
considering the reason they allocate their own work queues.

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.

That is also true for any operation on a work queue they need to wait
for to make progress. That means you cannot limit your check to per-cpu
variables. You also need to check for such dependencies. In particular
any usage of flush_work() on such queues can deadlock, if you use
common queues.

Please refrain from making such changes unless you have fully analyzed
the dependencies.

	Regards
		Oliver