* [PATCH 0/2] replace system_unbound_wq and system_wq with the new wqs
@ 2025-10-24 9:52 Marco Crivellari
2025-10-24 9:52 ` [PATCH 1/2] accel/ivpu: replace use of system_unbound_wq with system_dfl_wq Marco Crivellari
2025-10-24 9:52 ` [PATCH 2/2] accel/ivpu: replace use of system_wq with system_percpu_wq Marco Crivellari
0 siblings, 2 replies; 5+ messages in thread
From: Marco Crivellari @ 2025-10-24 9:52 UTC (permalink / raw)
To: linux-kernel, dri-devel
Cc: Tejun Heo, Lai Jiangshan, Frederic Weisbecker,
Sebastian Andrzej Siewior, Marco Crivellari, Michal Hocko,
Maciej Falkowski, Karol Wachowski, Oded Gabbay
Hi,
=== Current situation: problems ===
Let's consider a nohz_full system with isolated CPUs: wq_unbound_cpumask is
set to the housekeeping CPUs, for !WQ_UNBOUND the local CPU is selected.
This leads to different scenarios if a work item is scheduled on an isolated
CPU where "delay" value is 0 or greater then 0:
schedule_delayed_work(, 0);
This will be handled by __queue_work() that will queue the work item on the
current local (isolated) CPU, while:
schedule_delayed_work(, 1);
Will move the timer on an housekeeping CPU, and schedule the work there.
Currently if a user enqueue a work item using schedule_delayed_work() the
used wq is "system_wq" (per-cpu wq) while queue_delayed_work() use
WORK_CPU_UNBOUND (used when a cpu is not specified). The same applies to
schedule_work() that is using system_wq and queue_work(), that makes use
again of WORK_CPU_UNBOUND.
This lack of consistentcy cannot be addressed without refactoring the API.
=== Recent changes to the WQ API ===
The following, address the recent changes in the Workqueue API:
- commit 128ea9f6ccfb ("workqueue: Add system_percpu_wq and system_dfl_wq")
- commit 930c2ea566af ("workqueue: Add new WQ_PERCPU flag")
The old workqueues will be removed in a future release cycle.
=== Introduced Changes by this series ===
1) [P 1-2] Replace uses of system_wq and system_unbound_wq
system_wq is a per-CPU workqueue, but his name is not clear.
system_unbound_wq is to be used when locality is not required.
Because of that, system_wq has been replaced with system_percpu_wq, and
system_unbound_wq has been replaced with system_dfl_wq.
Thanks!
Marco Crivellari (2):
accel/ivpu: replace use of system_unbound_wq with system_dfl_wq
accel/ivpu: replace use of system_wq with system_percpu_wq
drivers/accel/ivpu/ivpu_hw_btrs.c | 2 +-
drivers/accel/ivpu/ivpu_ipc.c | 2 +-
drivers/accel/ivpu/ivpu_job.c | 2 +-
drivers/accel/ivpu/ivpu_mmu.c | 2 +-
drivers/accel/ivpu/ivpu_pm.c | 4 ++--
5 files changed, 6 insertions(+), 6 deletions(-)
--
2.51.0
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH 1/2] accel/ivpu: replace use of system_unbound_wq with system_dfl_wq 2025-10-24 9:52 [PATCH 0/2] replace system_unbound_wq and system_wq with the new wqs Marco Crivellari @ 2025-10-24 9:52 ` Marco Crivellari 2025-10-24 9:52 ` [PATCH 2/2] accel/ivpu: replace use of system_wq with system_percpu_wq Marco Crivellari 1 sibling, 0 replies; 5+ messages in thread From: Marco Crivellari @ 2025-10-24 9:52 UTC (permalink / raw) To: linux-kernel, dri-devel Cc: Tejun Heo, Lai Jiangshan, Frederic Weisbecker, Sebastian Andrzej Siewior, Marco Crivellari, Michal Hocko, Maciej Falkowski, Karol Wachowski, Oded Gabbay Currently if a user enqueue a work item using schedule_delayed_work() the used wq is "system_wq" (per-cpu wq) while queue_delayed_work() use WORK_CPU_UNBOUND (used when a cpu is not specified). The same applies to schedule_work() that is using system_wq and queue_work(), that makes use again of WORK_CPU_UNBOUND. This lack of consistentcy cannot be addressed without refactoring the API. system_unbound_wq should be the default workqueue so as not to enforce locality constraints for random work whenever it's not required. Adding system_dfl_wq to encourage its use when unbound work should be used. The old system_unbound_wq will be kept for a few release cycles. Suggested-by: Tejun Heo <tj@kernel.org> Signed-off-by: Marco Crivellari <marco.crivellari@suse.com> --- drivers/accel/ivpu/ivpu_pm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/accel/ivpu/ivpu_pm.c b/drivers/accel/ivpu/ivpu_pm.c index 475ddc94f1cf..ffa2ba7cafe2 100644 --- a/drivers/accel/ivpu/ivpu_pm.c +++ b/drivers/accel/ivpu/ivpu_pm.c @@ -186,7 +186,7 @@ void ivpu_pm_trigger_recovery(struct ivpu_device *vdev, const char *reason) if (atomic_cmpxchg(&vdev->pm->reset_pending, 0, 1) == 0) { ivpu_hw_diagnose_failure(vdev); ivpu_hw_irq_disable(vdev); /* Disable IRQ early to protect from IRQ storm */ - queue_work(system_unbound_wq, &vdev->pm->recovery_work); + queue_work(system_dfl_wq, &vdev->pm->recovery_work); } } -- 2.51.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] accel/ivpu: replace use of system_wq with system_percpu_wq 2025-10-24 9:52 [PATCH 0/2] replace system_unbound_wq and system_wq with the new wqs Marco Crivellari 2025-10-24 9:52 ` [PATCH 1/2] accel/ivpu: replace use of system_unbound_wq with system_dfl_wq Marco Crivellari @ 2025-10-24 9:52 ` Marco Crivellari 2025-10-24 10:48 ` Karol Wachowski 1 sibling, 1 reply; 5+ messages in thread From: Marco Crivellari @ 2025-10-24 9:52 UTC (permalink / raw) To: linux-kernel, dri-devel Cc: Tejun Heo, Lai Jiangshan, Frederic Weisbecker, Sebastian Andrzej Siewior, Marco Crivellari, Michal Hocko, Maciej Falkowski, Karol Wachowski, Oded Gabbay Currently if a user enqueue a work item using schedule_delayed_work() the used wq is "system_wq" (per-cpu wq) while queue_delayed_work() use WORK_CPU_UNBOUND (used when a cpu is not specified). The same applies to schedule_work() that is using system_wq and queue_work(), that makes use again of WORK_CPU_UNBOUND. This lack of consistentcy cannot be addressed without refactoring the API. system_wq should be the per-cpu workqueue, yet in this name nothing makes that clear, so replace system_wq with system_percpu_wq. The old wq (system_wq) will be kept for a few release cycles. Suggested-by: Tejun Heo <tj@kernel.org> Signed-off-by: Marco Crivellari <marco.crivellari@suse.com> --- drivers/accel/ivpu/ivpu_hw_btrs.c | 2 +- drivers/accel/ivpu/ivpu_ipc.c | 2 +- drivers/accel/ivpu/ivpu_job.c | 2 +- drivers/accel/ivpu/ivpu_mmu.c | 2 +- drivers/accel/ivpu/ivpu_pm.c | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/accel/ivpu/ivpu_hw_btrs.c b/drivers/accel/ivpu/ivpu_hw_btrs.c index afdb3b2aa72a..27a345f3befe 100644 --- a/drivers/accel/ivpu/ivpu_hw_btrs.c +++ b/drivers/accel/ivpu/ivpu_hw_btrs.c @@ -673,7 +673,7 @@ bool ivpu_hw_btrs_irq_handler_lnl(struct ivpu_device *vdev, int irq) if (REG_TEST_FLD(VPU_HW_BTRS_LNL_INTERRUPT_STAT, SURV_ERR, status)) { ivpu_dbg(vdev, IRQ, "Survivability IRQ\n"); - queue_work(system_wq, &vdev->irq_dct_work); + queue_work(system_percpu_wq, &vdev->irq_dct_work); } if (REG_TEST_FLD(VPU_HW_BTRS_LNL_INTERRUPT_STAT, FREQ_CHANGE, status)) { diff --git a/drivers/accel/ivpu/ivpu_ipc.c b/drivers/accel/ivpu/ivpu_ipc.c index 5f00809d448a..1f13bf95b2b3 100644 --- a/drivers/accel/ivpu/ivpu_ipc.c +++ b/drivers/accel/ivpu/ivpu_ipc.c @@ -459,7 +459,7 @@ void ivpu_ipc_irq_handler(struct ivpu_device *vdev) } } - queue_work(system_wq, &vdev->irq_ipc_work); + queue_work(system_percpu_wq, &vdev->irq_ipc_work); } void ivpu_ipc_irq_work_fn(struct work_struct *work) diff --git a/drivers/accel/ivpu/ivpu_job.c b/drivers/accel/ivpu/ivpu_job.c index 060f1fc031d3..7a1f78b84b09 100644 --- a/drivers/accel/ivpu/ivpu_job.c +++ b/drivers/accel/ivpu/ivpu_job.c @@ -574,7 +574,7 @@ static int ivpu_job_signal_and_destroy(struct ivpu_device *vdev, u32 job_id, u32 * status and ensure both are handled in the same way */ job->file_priv->has_mmu_faults = true; - queue_work(system_wq, &vdev->context_abort_work); + queue_work(system_percpu_wq, &vdev->context_abort_work); return 0; } diff --git a/drivers/accel/ivpu/ivpu_mmu.c b/drivers/accel/ivpu/ivpu_mmu.c index 5ea010568faa..e1baf6b64935 100644 --- a/drivers/accel/ivpu/ivpu_mmu.c +++ b/drivers/accel/ivpu/ivpu_mmu.c @@ -970,7 +970,7 @@ void ivpu_mmu_irq_evtq_handler(struct ivpu_device *vdev) } } - queue_work(system_wq, &vdev->context_abort_work); + queue_work(system_percpu_wq, &vdev->context_abort_work); } void ivpu_mmu_evtq_dump(struct ivpu_device *vdev) diff --git a/drivers/accel/ivpu/ivpu_pm.c b/drivers/accel/ivpu/ivpu_pm.c index ffa2ba7cafe2..0cff8f808429 100644 --- a/drivers/accel/ivpu/ivpu_pm.c +++ b/drivers/accel/ivpu/ivpu_pm.c @@ -226,7 +226,7 @@ void ivpu_start_job_timeout_detection(struct ivpu_device *vdev) unsigned long timeout_ms = ivpu_tdr_timeout_ms ? ivpu_tdr_timeout_ms : vdev->timeout.tdr; /* No-op if already queued */ - queue_delayed_work(system_wq, &vdev->pm->job_timeout_work, msecs_to_jiffies(timeout_ms)); + queue_delayed_work(system_percpu_wq, &vdev->pm->job_timeout_work, msecs_to_jiffies(timeout_ms)); } void ivpu_stop_job_timeout_detection(struct ivpu_device *vdev) -- 2.51.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] accel/ivpu: replace use of system_wq with system_percpu_wq 2025-10-24 9:52 ` [PATCH 2/2] accel/ivpu: replace use of system_wq with system_percpu_wq Marco Crivellari @ 2025-10-24 10:48 ` Karol Wachowski 2025-10-24 12:58 ` Marco Crivellari 0 siblings, 1 reply; 5+ messages in thread From: Karol Wachowski @ 2025-10-24 10:48 UTC (permalink / raw) To: Marco Crivellari, linux-kernel, dri-devel Cc: Tejun Heo, Lai Jiangshan, Frederic Weisbecker, Sebastian Andrzej Siewior, Michal Hocko, Maciej Falkowski, Oded Gabbay On 10/24/2025 11:52 AM, Marco Crivellari wrote: > Currently if a user enqueue a work item using schedule_delayed_work() the > used wq is "system_wq" (per-cpu wq) while queue_delayed_work() use > WORK_CPU_UNBOUND (used when a cpu is not specified). The same applies to > schedule_work() that is using system_wq and queue_work(), that makes use > again of WORK_CPU_UNBOUND. > > This lack of consistentcy cannot be addressed without refactoring the API. > > system_wq should be the per-cpu workqueue, yet in this name nothing makes > that clear, so replace system_wq with system_percpu_wq. > > The old wq (system_wq) will be kept for a few release cycles. > > Suggested-by: Tejun Heo <tj@kernel.org> > Signed-off-by: Marco Crivellari <marco.crivellari@suse.com> > --- > drivers/accel/ivpu/ivpu_hw_btrs.c | 2 +- > drivers/accel/ivpu/ivpu_ipc.c | 2 +- > drivers/accel/ivpu/ivpu_job.c | 2 +- > drivers/accel/ivpu/ivpu_mmu.c | 2 +- > drivers/accel/ivpu/ivpu_pm.c | 2 +- > 5 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/accel/ivpu/ivpu_hw_btrs.c b/drivers/accel/ivpu/ivpu_hw_btrs.c > index afdb3b2aa72a..27a345f3befe 100644 > --- a/drivers/accel/ivpu/ivpu_hw_btrs.c > +++ b/drivers/accel/ivpu/ivpu_hw_btrs.c > @@ -673,7 +673,7 @@ bool ivpu_hw_btrs_irq_handler_lnl(struct ivpu_device *vdev, int irq) > > if (REG_TEST_FLD(VPU_HW_BTRS_LNL_INTERRUPT_STAT, SURV_ERR, status)) { > ivpu_dbg(vdev, IRQ, "Survivability IRQ\n"); > - queue_work(system_wq, &vdev->irq_dct_work); > + queue_work(system_percpu_wq, &vdev->irq_dct_work); > } > > if (REG_TEST_FLD(VPU_HW_BTRS_LNL_INTERRUPT_STAT, FREQ_CHANGE, status)) { > diff --git a/drivers/accel/ivpu/ivpu_ipc.c b/drivers/accel/ivpu/ivpu_ipc.c > index 5f00809d448a..1f13bf95b2b3 100644 > --- a/drivers/accel/ivpu/ivpu_ipc.c > +++ b/drivers/accel/ivpu/ivpu_ipc.c > @@ -459,7 +459,7 @@ void ivpu_ipc_irq_handler(struct ivpu_device *vdev) > } > } > > - queue_work(system_wq, &vdev->irq_ipc_work); > + queue_work(system_percpu_wq, &vdev->irq_ipc_work); > } > > void ivpu_ipc_irq_work_fn(struct work_struct *work) > diff --git a/drivers/accel/ivpu/ivpu_job.c b/drivers/accel/ivpu/ivpu_job.c > index 060f1fc031d3..7a1f78b84b09 100644 > --- a/drivers/accel/ivpu/ivpu_job.c > +++ b/drivers/accel/ivpu/ivpu_job.c > @@ -574,7 +574,7 @@ static int ivpu_job_signal_and_destroy(struct ivpu_device *vdev, u32 job_id, u32 > * status and ensure both are handled in the same way > */ > job->file_priv->has_mmu_faults = true; > - queue_work(system_wq, &vdev->context_abort_work); > + queue_work(system_percpu_wq, &vdev->context_abort_work); > return 0; > } > > diff --git a/drivers/accel/ivpu/ivpu_mmu.c b/drivers/accel/ivpu/ivpu_mmu.c > index 5ea010568faa..e1baf6b64935 100644 > --- a/drivers/accel/ivpu/ivpu_mmu.c > +++ b/drivers/accel/ivpu/ivpu_mmu.c > @@ -970,7 +970,7 @@ void ivpu_mmu_irq_evtq_handler(struct ivpu_device *vdev) > } > } > > - queue_work(system_wq, &vdev->context_abort_work); > + queue_work(system_percpu_wq, &vdev->context_abort_work); > } > > void ivpu_mmu_evtq_dump(struct ivpu_device *vdev) > diff --git a/drivers/accel/ivpu/ivpu_pm.c b/drivers/accel/ivpu/ivpu_pm.c > index ffa2ba7cafe2..0cff8f808429 100644 > --- a/drivers/accel/ivpu/ivpu_pm.c > +++ b/drivers/accel/ivpu/ivpu_pm.c > @@ -226,7 +226,7 @@ void ivpu_start_job_timeout_detection(struct ivpu_device *vdev) > unsigned long timeout_ms = ivpu_tdr_timeout_ms ? ivpu_tdr_timeout_ms : vdev->timeout.tdr; > > /* No-op if already queued */ > - queue_delayed_work(system_wq, &vdev->pm->job_timeout_work, msecs_to_jiffies(timeout_ms)); > + queue_delayed_work(system_percpu_wq, &vdev->pm->job_timeout_work, msecs_to_jiffies(timeout_ms)); Thanks for the patch. Please fix the checkpatch warning: WARNING: line length of 104 exceeds 100 columns #90: FILE: drivers/accel/ivpu/ivpu_pm.c:229: + queue_delayed_work(system_percpu_wq, &vdev->pm->job_timeout_work, msecs_to_jiffies(timeout_ms)); > } > > void ivpu_stop_job_timeout_detection(struct ivpu_device *vdev) Also there's a typo "consistentcy" -> "consistency" that can get fixed with together with that warning. Tested-by: Karol Wachowski <karol.wachowski@linux.intel.com> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] accel/ivpu: replace use of system_wq with system_percpu_wq 2025-10-24 10:48 ` Karol Wachowski @ 2025-10-24 12:58 ` Marco Crivellari 0 siblings, 0 replies; 5+ messages in thread From: Marco Crivellari @ 2025-10-24 12:58 UTC (permalink / raw) To: Karol Wachowski Cc: linux-kernel, dri-devel, Tejun Heo, Lai Jiangshan, Frederic Weisbecker, Sebastian Andrzej Siewior, Michal Hocko, Maciej Falkowski, Oded Gabbay On Fri, Oct 24, 2025 at 12:49 PM Karol Wachowski <karol.wachowski@linux.intel.com> wrote: > [...] > Thanks for the patch. Please fix the checkpatch warning: > > WARNING: line length of 104 exceeds 100 > columns > Also there's a typo "consistentcy" -> "consistency" that can get fixed > with together with that warning. > > [...] > Tested-by: Karol Wachowski <karol.wachowski@linux.intel.com> > > Hello Karol, thank you, I will fix both. -- Marco Crivellari L3 Support Engineer, Technology & Product ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-10-25 22:07 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-10-24 9:52 [PATCH 0/2] replace system_unbound_wq and system_wq with the new wqs Marco Crivellari 2025-10-24 9:52 ` [PATCH 1/2] accel/ivpu: replace use of system_unbound_wq with system_dfl_wq Marco Crivellari 2025-10-24 9:52 ` [PATCH 2/2] accel/ivpu: replace use of system_wq with system_percpu_wq Marco Crivellari 2025-10-24 10:48 ` Karol Wachowski 2025-10-24 12:58 ` Marco Crivellari
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).