rust/kernel/workqueue.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
Below is a summary of a discussion about the Workqueue API and cpu isolation considerations. Details and more information are available here: "workqueue: Always use wq_select_unbound_cpu() for WORK_CPU_UNBOUND." https://lore.kernel.org/all/20250221112003.1dSuoGyc@linutronix.de/ === 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. === Plan and future plans === This patchset is the first stone on a refactoring needed in order to address the points aforementioned; it will have a positive impact also on the cpu isolation, in the long term, moving away percpu workqueue in favor to an unbound model. These are the main steps: 1) API refactoring (changes introduced by this series) - Make more clear and uniform the system wq names, both per-cpu and unbound. This to avoid any possible confusion on what should be used. - Introduction of WQ_PERCPU: this flag is the complement of WQ_UNBOUND, introduced in this patchset and used on all the callers that are not currently using WQ_UNBOUND. WQ_UNBOUND will be removed in a future release cycle. Most users don't need to be per-cpu, because they don't have locality requirements, because of that, a next future step will be make "unbound" the default behavior. 2) Check who really needs to be per-cpu - Remove the WQ_PERCPU flag when is not strictly required. 3) Add a new API (prefer local cpu) - There are users that don't require a local execution, like mentioned above; despite that, local execution yeld to performance gain. This new API will prefer the local execution, without requiring it. === Introduced Changes by this series === 1) [P 1-2] Replace use 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 renamed in system_percpu_wq, and system_unbound_wq has been renamed in system_dfl_wq. === For Maintainers === There are prerequisites for this series, already merged in the master branch. The commits are: 128ea9f6ccfb6960293ae4212f4f97165e42222d ("workqueue: Add system_percpu_wq and system_dfl_wq") 930c2ea566aff59e962c50b2421d5fcc3b98b8be ("workqueue: Add new WQ_PERCPU flag") Thanks! --- Changes in v2: - added system_percpu() and system_dfl() in order to use the new wq defined in the C code. - fixed misleading paragraph in the commit log (no warnings are currently present). Marco Crivellari (2): rust: replace use of system_unbound_wq with system_dfl_wq rust: replace use of system_wq with system_percpu_wq rust/kernel/workqueue.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) -- 2.51.0
On Mon, Sep 8, 2025 at 6:02 PM Marco Crivellari <marco.crivellari@suse.com> wrote: > > Below is a summary of a discussion about the Workqueue API and cpu isolation > considerations. Details and more information are available here: If I understand correctly, these two patches are "just" following the C side, right? i.e. the summary below comes from that original patch series, but this one is just adding the `system_{dfl,percpu}_wq`s that were added on the C side. In other words, I would have just said that, plus a link to the patch series or commit that actually added the new wqs. I would do so similarly in the commit messages. > === Introduced Changes by this series === > > 1) [P 1-2] Replace use 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 renamed in system_percpu_wq, and > system_unbound_wq has been renamed in system_dfl_wq. This is not what the patches are doing? There is no replacement nor rename. > === For Maintainers === > > There are prerequisites for this series, already merged in the master branch. > The commits are: > > 128ea9f6ccfb6960293ae4212f4f97165e42222d ("workqueue: Add system_percpu_wq and > system_dfl_wq") > > 930c2ea566aff59e962c50b2421d5fcc3b98b8be ("workqueue: Add new WQ_PERCPU flag") Why are these prerequisites if they are already merged? Thanks! Cheers, Miguel
On Mon, Sep 8, 2025 at 6:32 PM Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote: > If I understand correctly, these two patches are "just" following the > C side, right? > > i.e. the summary below comes from that original patch series, but this > one is just adding the `system_{dfl,percpu}_wq`s that were added on > the C side. > > In other words, I would have just said that, plus a link to the patch > series or commit that actually added the new wqs. > > I would do so similarly in the commit messages. Hello Miguel, Exactly. I kept the same cover letter for every subsystem involved to give all the information. > This is not what the patches are doing? There is no replacement nor rename. Yes you're right. This is done on the C side, the rename of all the uses; but the old wq(s) are not removed. > > === For Maintainers === > > > > There are prerequisites for this series, already merged in the master branch. > > The commits are: > > > > 128ea9f6ccfb6960293ae4212f4f97165e42222d ("workqueue: Add system_percpu_wq and > > system_dfl_wq") > > > > 930c2ea566aff59e962c50b2421d5fcc3b98b8be ("workqueue: Add new WQ_PERCPU flag") > > Why are these prerequisites if they are already merged? I kept the old cover letter paragraph name. Unfortunately I noticed I made some mistakes not updating the cover letter accordingly... sorry! Thanks! -- Marco Crivellari L3 Support Engineer, Technology & Product marco.crivellari@suse.com
On Tue, Sep 9, 2025 at 9:25 AM Marco Crivellari <marco.crivellari@suse.com> wrote: > > Unfortunately I noticed I made some mistakes not updating the cover > letter accordingly... sorry! No worries! It was just a bit confusing :) Thanks! Cheers, Miguel
© 2016 - 2025 Red Hat, Inc.