* Re: [PATCH net-next v4 1/6] ibmvnic: Move long delayed work on system_dfl_long_wq
2026-10-12 9:54 ` [PATCH net-next v4 1/6] ibmvnic: Move long delayed work on system_dfl_long_wq Marco Crivellari
@ 2026-10-16 21:19 ` mingming cao
2026-10-19 10:18 ` Marco Crivellari
0 siblings, 1 reply; 3+ messages in thread
From: mingming cao @ 2026-10-16 21:19 UTC (permalink / raw)
To: Marco Crivellari, linux-kernel, netdev
Cc: Tejun Heo, Lai Jiangshan, Frederic Weisbecker,
Sebastian Andrzej Siewior, Michal Hocko, Andrew Lunn,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Haren Myneni, Rick Lindsley, Nick Child, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy (CS GROUP),
linuxppc-dev
On 10/12/26 2:54 AM, Marco Crivellari wrote:
> Currently the code enqueue work items using {queue|mod}_delayed_work(),
> using system_long_wq. This workqueue should be used when long works are
> expected and it is a per-cpu workqueue.
>
> The function(s) end up calling __queue_delayed_work(), which set a global
> timer that could fire anywhere, enqueuing the work where the timer fired.
>
> Unbound works could benefit from scheduler task placement, to optimize
> performance and power consumption. Long work shouldn't stick to a single
> CPU.
>
> Recently, a new unbound workqueue specific for long running work has
> been added:
>
> c116737e972e ("workqueue: Add system_dfl_long_wq for long unbound works")
>
> 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.
>
> Cc: Haren Myneni <haren@linux.ibm.com>
> Cc: Rick Lindsley <ricklind@linux.ibm.com>
> Cc: Nick Child <nnac123@linux.ibm.com>
> Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Nicholas Piggin <npiggin@gmail.com>
> Cc: Christophe Leroy (CS GROUP) <chleroy@kernel.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Marco Crivellari <marco.crivellari@suse.com>
> ---
> drivers/net/ethernet/ibm/ibmvnic.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
> index 5a510eed335e..d4c284c8ef43 100644
> --- a/drivers/net/ethernet/ibm/ibmvnic.c
> +++ b/drivers/net/ethernet/ibm/ibmvnic.c
> @@ -3229,7 +3229,7 @@ static void __ibmvnic_reset(struct work_struct *work)
> if (adapter->state == VNIC_PROBING &&
> !wait_for_completion_timeout(&adapter->probe_done, timeout)) {
> dev_err(dev, "Reset thread timed out on probe");
> - queue_delayed_work(system_long_wq,
> + queue_delayed_work(system_dfl_long_wq,
> &adapter->ibmvnic_delayed_reset,
> IBMVNIC_RESET_DELAY);
> return;
> @@ -3267,7 +3267,7 @@ static void __ibmvnic_reset(struct work_struct *work)
> spin_lock(&adapter->rwi_lock);
> if (!list_empty(&adapter->rwi_list)) {
> if (test_and_set_bit_lock(0, &adapter->resetting)) {
> - queue_delayed_work(system_long_wq,
> + queue_delayed_work(system_dfl_long_wq,
> &adapter->ibmvnic_delayed_reset,
> IBMVNIC_RESET_DELAY);
> } else {
> @@ -3454,7 +3454,7 @@ static int ibmvnic_reset(struct ibmvnic_adapter *adapter,
> list_add_tail(&rwi->list, &adapter->rwi_list);
> netdev_dbg(adapter->netdev, "Scheduling reset (reason %s)\n",
> reset_reason_to_string(reason));
> - queue_work(system_long_wq, &adapter->ibmvnic_reset);
> + queue_work(system_dfl_long_wq, &adapter->ibmvnic_reset);
>
> ret = 0;
> err:
On 10/12/26 2:54 AM, Marco Crivellari wrote:
> Currently the code enqueue work items using {queue|mod}_delayed_work(),
> using system_long_wq. This workqueue should be used when long works are
> expected and it is a per-cpu workqueue.
>
> The function(s) end up calling __queue_delayed_work(), which set a global
> timer that could fire anywhere, enqueuing the work where the timer fired.
>
> Unbound works could benefit from scheduler task placement, to optimize
> performance and power consumption. Long work shouldn't stick to a single
> CPU.
>
> Recently, a new unbound workqueue specific for long running work has
> been added:
>
> c116737e972e ("workqueue: Add system_dfl_long_wq for long
unbound works")
>
> 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.
>
> Cc: Haren Myneni <haren@linux.ibm.com>
> Cc: Rick Lindsley <ricklind@linux.ibm.com>
> Cc: Nick Child <nnac123@linux.ibm.com>
> Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Nicholas Piggin <npiggin@gmail.com>
> Cc: Christophe Leroy (CS GROUP) <chleroy@kernel.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Marco Crivellari <marco.crivellari@suse.com>
> ---
> drivers/net/ethernet/ibm/ibmvnic.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
> index 5a510eed335e..d4c284c8ef43 100644
> --- a/drivers/net/ethernet/ibm/ibmvnic.c
> +++ b/drivers/net/ethernet/ibm/ibmvnic.c
> @@ -3229,7 +3229,7 @@ static void __ibmvnic_reset(struct work_struct
*work)
> if (adapter->state == VNIC_PROBING &&
> !wait_for_completion_timeout(&adapter->probe_done, timeout)) {
> dev_err(dev, "Reset thread timed out on probe");
> - queue_delayed_work(system_long_wq,
> + queue_delayed_work(system_dfl_long_wq,
> &adapter->ibmvnic_delayed_reset,
> IBMVNIC_RESET_DELAY);
> return;
> @@ -3267,7 +3267,7 @@ static void __ibmvnic_reset(struct work_struct
*work)
> spin_lock(&adapter->rwi_lock);
> if (!list_empty(&adapter->rwi_list)) {
> if (test_and_set_bit_lock(0, &adapter->resetting)) {
> - queue_delayed_work(system_long_wq,
> + queue_delayed_work(system_dfl_long_wq,
> &adapter->ibmvnic_delayed_reset,
> IBMVNIC_RESET_DELAY);
> } else {
> @@ -3454,7 +3454,7 @@ static int ibmvnic_reset(struct ibmvnic_adapter
*adapter,
> list_add_tail(&rwi->list, &adapter->rwi_list);
> netdev_dbg(adapter->netdev, "Scheduling reset (reason %s)\n",
> reset_reason_to_string(reason));
> - queue_work(system_long_wq, &adapter->ibmvnic_reset);
> + queue_work(system_dfl_long_wq, &adapter->ibmvnic_reset);
>
> ret = 0;
> err:
I walked through the reset path and the conversion looks fine.
All three system_long_wq users move together and there are no
remaining users in the driver. The work items don't rely on per-CPU
state, so system_dfl_long_wq seems appropriate here.
Minor commit message nit: ibmvnic doesn't use mod_delayed_work(), and
one of the conversions is queue_work() rather than delayed work.
Acked-by: Mingming Cao <mmc@linux.ibm.com>
^ permalink raw reply [flat|nested] 3+ messages in thread