mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Moessbauer, Felix" <felix.moessbauer@siemens.com>
To: Mikhail Malyshev <mike.malyshev@gmail.com>,
	Jens Axboe <axboe@kernel.dk>,
	"io-uring@vger.kernel.org" <io-uring@vger.kernel.org>
Cc: Pavel Begunkov <asml.silence@gmail.com>,
	Josef Bacik <josef@toxicpanda.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"stable@vger.kernel.org" <stable@vger.kernel.org>
Subject: Re: [PATCH] io_uring/io-wq: apply the current cpuset to new workers
Date: Thu, 12 Nov 2026 07:52:33 +0000	[thread overview]
Message-ID: <b95051ac2caf63d3266bd02b326d0e0166a138b6.camel@siemens.com> (raw)
In-Reply-To: <20261111162705.1150983-1-mike.malyshev@gmail.com>

On Wed, 2026-11-11 at 16:27 +0000, Mikhail Malyshev wrote:
> Since commit 84eacf177faa ("io_uring/io-wq: inherit cpuset of cgroup in
> io worker"), io_wq_create() copies the cpuset of the creating task into
> wq->cpu_mask, and io_init_new_worker() gives each new worker that copy.
> A cpuset change, or a move of the task to another cgroup, updates the
> workers that exist but not the copy, so workers started later run on
> the CPUs the cgroup had when the io-wq was created, which may since
> have been given to other tasks. Workers come and go for the life of the
> io-wq, and a worker that cpuset moved is also retired at its next idle
> timeout and replaced on the old CPUs.

Hi,

that's a valid finding.

> 
> Keep wq->cpu_mask for the affinity of the io-wq itself, all CPUs or the
> mask set with IORING_REGISTER_IOWQ_AFF, and intersect it with the cpuset
> of the worker when the worker starts, falling back to the cpuset if the
> two do not intersect, as __set_cpus_allowed_ptr() does. 
> 

Why not simply fail the worker creation if the intersection is empty?
Just falling back means that the worker threads run on CPUs where they
- intentionally - should not run.

> Use the same
> mask in the idle exit test, and go back to all CPUs when the registered
> affinity is removed.
> 
> Fixes: 84eacf177faa ("io_uring/io-wq: inherit cpuset of cgroup in io worker")
> Cc: stable@vger.kernel.org
> Signed-off-by: Mikhail Malyshev <mike.malyshev@gmail.com>
> ---
> Tested on x86_64 under QEMU/KVM (8 CPUs, cgroup v2, PROVE_LOCKING,
> DEBUG_ATOMIC_SLEEP, CPUMASK_OFFSTACK) on io_uring-7.4 with and without
> this patch. A test program blocks requests in io-wq workers around
> cpuset changes and checks the affinity of every worker:
> 
>   case                                          without   with
>   cpuset 2-3 -> 4-5, new workers                FAIL      pass
>   idle worker retired after the change          FAIL      pass
>   cpuset grows 2-3 -> 2-5                       FAIL      pass
>   task moved to a cgroup with 4-5               FAIL      pass
>   IORING_REGISTER_IOWQ_AFF 2-3, cpuset 3-6,     FAIL      pass
>     then 4-5, then unregister
>   CPU offline and online                        pass      pass

This test program should be added to the liburing testsuite, shouldn't
it?

> 
> No lockdep or sleeping-in-atomic reports. The liburing test suite gives
> the same results with and without the patch (iowait fails in some runs
> on both kernels: 5 and 7 of 20). A liburing test for this,
> test/iowq-cpuset.c, follows separately; it fails without the patch and
> passes with it.
> 
> The cost is one cpuset_cpus_allowed() per worker start and per idle
> timeout of a worker, not per request: 0.3-0.4 us against about 60 us
> for create_io_worker(), measured with the function graph tracer.
> 
> The patch applies unchanged to for-7.5/io_uring and to 6.18.y, where
> the results are the same. The bug was first seen on real hardware with
> 6.18; the fix was tested only under QEMU. Not tested: architectures
> other than x86_64, and cgroup v1 cpusets.

Shouldn't that also apply to 6.12 (it technically does not apply but
semantically it should)? 

Best regards,
Felix

> 
> This patch was developed with Claude Code (Anthropic, Claude Opus 5.5).
> The problem came up while I worked with it on CPU isolation for VMs in
> EVE (LF Edge): asked whether QEMU's io-wq workers can appear after a VM
> has started, it found in the code that workers started after a cpuset
> change keep the old CPUs, and reproduced that. It then wrote the fix,
> the changelog, the test program and the QEMU setup, and ran the tests
> above. I have reviewed the patch and the test results.
> 
>  io_uring/io-wq.c | 44 +++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 39 insertions(+), 5 deletions(-)
> 
> diff --git a/io_uring/io-wq.c b/io_uring/io-wq.c
> index 92df54c6f50ca..f2f3c3aad19b5 100644
> --- a/io_uring/io-wq.c
> +++ b/io_uring/io-wq.c
> @@ -741,6 +741,34 @@ static bool io_worker_retire(struct io_wq_acct *acct, struct io_worker *worker)
>  	return false;
>  }
>  
> +/*
> + * The CPUs a worker of @wq runs on: those of the io-wq affinity that the
> + * worker's cpuset allows, or the cpuset if they have none in common, as the
> + * scheduler does with a task's requested affinity when its cpuset changes.
> + * Read the cpuset each time: it can change while the io-wq exists.
> + */
> +static void io_wq_worker_cpus(struct io_wq *wq, struct task_struct *tsk,
> +			      struct cpumask *cpus)
> +{
> +	cpuset_cpus_allowed(tsk, cpus);
> +	if (cpumask_intersects(cpus, wq->cpu_mask))
> +		cpumask_and(cpus, cpus, wq->cpu_mask);
> +}
> +
> +/* Is the current worker on a CPU that a new worker would not get? */
> +static bool io_wq_worker_off_cpus(struct io_wq *wq)
> +{
> +	cpumask_var_t cpus;
> +	bool off;
> +
> +	if (!alloc_cpumask_var(&cpus, GFP_KERNEL))
> +		return false;
> +	io_wq_worker_cpus(wq, current, cpus);
> +	off = !cpumask_test_cpu(raw_smp_processor_id(), cpus);
> +	free_cpumask_var(cpus);
> +	return off;
> +}
> +
>  static int io_wq_worker(void *data)
>  {
>  	struct io_worker *worker = data;
> @@ -819,8 +847,7 @@ static int io_wq_worker(void *data)
>  		}
>  		if (!ret) {
>  			last_timeout = true;
> -			exit_mask = !cpumask_test_cpu(raw_smp_processor_id(),
> -							wq->cpu_mask);
> +			exit_mask = io_wq_worker_off_cpus(wq);
>  		}
>  	}
>  
> @@ -870,9 +897,16 @@ void io_wq_worker_sleeping(struct task_struct *tsk)
>  static void io_init_new_worker(struct io_wq *wq, struct io_wq_acct *acct, struct io_worker *worker,
>  			       struct task_struct *tsk)
>  {
> +	cpumask_var_t cpus;
> +
>  	tsk->worker_private = worker;
>  	worker->task = tsk;
> -	set_cpus_allowed_ptr(tsk, wq->cpu_mask);
> +	/* Without a mask, the worker keeps its creator's CPUs. */
> +	if (alloc_cpumask_var(&cpus, GFP_KERNEL)) {
> +		io_wq_worker_cpus(wq, tsk, cpus);
> +		set_cpus_allowed_ptr(tsk, cpus);
> +		free_cpumask_var(cpus);
> +	}
>  
>  	raw_spin_lock(&acct->workers_lock);
>  	hlist_nulls_add_head_rcu(&worker->nulls_node, &acct->free_list);
> @@ -1369,7 +1403,7 @@ struct io_wq *io_wq_create(unsigned bounded, struct io_wq_data *data)
>  
>  	if (!alloc_cpumask_var(&wq->cpu_mask, GFP_KERNEL))
>  		goto err;
> -	cpuset_cpus_allowed(data->task, wq->cpu_mask);
> +	cpumask_copy(wq->cpu_mask, cpu_possible_mask);
>  	wq->acct[IO_WQ_ACCT_BOUND].max_workers = bounded;
>  	wq->acct[IO_WQ_ACCT_UNBOUND].max_workers =
>  				task_rlimit(current, RLIMIT_NPROC);
> @@ -1574,7 +1608,7 @@ int io_wq_cpu_affinity(struct io_uring_task *tctx, cpumask_var_t mask)
>  		else
>  			ret = -EINVAL;
>  	} else {
> -		cpumask_copy(tctx->io_wq->cpu_mask, allowed_mask);
> +		cpumask_copy(tctx->io_wq->cpu_mask, cpu_possible_mask);
>  	}
>  	rcu_read_unlock();
>  
> 
> base-commit: 39c7210e970200f8b17de3767c73efe64173a95f
> -- 
> 2.43.0

      reply	other threads:[~2026-11-12  7:52 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-11 16:27 Mikhail Malyshev
2026-11-12  7:52 ` Moessbauer, Felix [this message]

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=b95051ac2caf63d3266bd02b326d0e0166a138b6.camel@siemens.com \
    --to=felix.moessbauer@siemens.com \
    --cc=asml.silence@gmail.com \
    --cc=axboe@kernel.dk \
    --cc=io-uring@vger.kernel.org \
    --cc=josef@toxicpanda.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mike.malyshev@gmail.com \
    --cc=stable@vger.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

all inboxes | Powered by JetHome®