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

[Sebastian Andrzej Siewior]

On 2026-07-22 10:29:50 [+0200], Oliver Neukum wrote:
> On 20.07.26 12:08, Marco Crivellari wrote:
> Hi,
Hi Oliver,

> > 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.

This driver does not using its own workqueue.

> 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.

> 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.

It has nothing to do with per-CPU variables. In fact its
queue_delayed_work(,, CARRIER_CHECK_DELAY) usage already ensures that it
can be executed on a random and not on the submitting CPU.

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

Did you refer to the wrong patch? As far as this patch goes I can not
reason why you assume Marco did not fully analyze the dependencies. I
don't see anything wrong with this patch.

Reviewed-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>

> 	Regards
> 		Oliver
> 

Sebastian