rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alice Ryhl <aliceryhl@google.com>
To: Marco Crivellari <marco.crivellari@suse.com>
Cc: linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org,
	 Tejun Heo <tj@kernel.org>,
	Lai Jiangshan <jiangshanlai@gmail.com>,
	 Frederic Weisbecker <frederic@kernel.org>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	 Michal Hocko <mhocko@suse.com>, Miguel Ojeda <ojeda@kernel.org>,
	Alex Gaynor <alex.gaynor@gmail.com>
Subject: Re: [PATCH 0/2] rust: replace wq users and add WQ_PERCPU to alloc_workqueue() users
Date: Fri, 5 Sep 2025 13:33:56 +0200	[thread overview]
Message-ID: <CAH5fLgiZnCbNLpuphv4Kgsu48kRkhf6wJiSLrrgsqyEDvU3X3Q@mail.gmail.com> (raw)
In-Reply-To: <20250905090819.107694-1-marco.crivellari@suse.com>

On Fri, Sep 5, 2025 at 11:12 AM 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:
>
>         "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 (that this patch is introducing)
>     -   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!
>
> 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 | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)

The functions you are changing are intended to match 1-to-1 with the
wq globals defined by the C side. Changing them so that Rust and C no
longer agrees on what the wqs are called seems wrong. How about adding
new functions instead?

Alice

  parent reply	other threads:[~2025-09-05 11:34 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-05  9:08 [PATCH 0/2] rust: replace wq users and add WQ_PERCPU to alloc_workqueue() users Marco Crivellari
2025-09-05  9:08 ` [PATCH 1/2] rust: replace use of system_unbound_wq with system_dfl_wq Marco Crivellari
2025-09-05  9:08 ` [PATCH 2/2] rust: replace use of system_wq with system_percpu_wq Marco Crivellari
2025-09-05 11:33 ` Alice Ryhl [this message]
2025-09-05 16:05   ` [PATCH 0/2] rust: replace wq users and add WQ_PERCPU to alloc_workqueue() users Marco Crivellari
2025-09-07 10:59     ` Alice Ryhl
2025-09-08 10:24       ` Marco Crivellari
2025-09-08 10:27         ` Alice Ryhl

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAH5fLgiZnCbNLpuphv4Kgsu48kRkhf6wJiSLrrgsqyEDvU3X3Q@mail.gmail.com \
    --to=aliceryhl@google.com \
    --cc=alex.gaynor@gmail.com \
    --cc=bigeasy@linutronix.de \
    --cc=frederic@kernel.org \
    --cc=jiangshanlai@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marco.crivellari@suse.com \
    --cc=mhocko@suse.com \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).