public inbox for [email protected]
 help / color / mirror / Atom feed
* [PATCH 1/1] io_uring/sqpoll: do not allow pinning outside of cpuset
@ 2024-09-09 15:00 Felix Moessbauer
  2024-09-09 15:08 ` Jens Axboe
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Felix Moessbauer @ 2024-09-09 15:00 UTC (permalink / raw)
  To: axboe
  Cc: asml.silence, linux-kernel, io-uring, cgroups, dqminh, longman,
	adriaan.schmidt, florian.bezdeka, stable, Felix Moessbauer

The submit queue polling threads are userland threads that just never
exit to the userland. When creating the thread with IORING_SETUP_SQ_AFF,
the affinity of the poller thread is set to the cpu specified in
sq_thread_cpu. However, this CPU can be outside of the cpuset defined
by the cgroup cpuset controller. This violates the rules defined by the
cpuset controller and is a potential issue for realtime applications.

In b7ed6d8ffd6 we fixed the default affinity of the poller thread, in
case no explicit pinning is required by inheriting the one of the
creating task. In case of explicit pinning, the check is more
complicated, as also a cpu outside of the parent cpumask is allowed.
We implemented this by using cpuset_cpus_allowed (that has support for
cgroup cpusets) and testing if the requested cpu is in the set.

Fixes: 37d1e2e3642e ("io_uring: move SQPOLL thread io-wq forked worker")
Cc: [email protected] # 6.1+
Signed-off-by: Felix Moessbauer <[email protected]>
---
Hi,

that's hopefully the last fix of cpu pinnings of the sq poller threads.
However, there is more to come on the io-wq side. E.g the syscalls for
IORING_REGISTER_IOWQ_AFF that can be used to change the affinites are
not yet protected. I'm currently just lacking good reproducers for that.
I also have to admit that I don't feel too comfortable making changes to
the wq part, given that I don't have good tests.

While fixing this, I'm wondering if it makes sense to add tests for the
combination of pinning and cpuset. If yes, where should these tests be
added?

Best regards,
Felix Moessbauer
Siemens AG

 io_uring/sqpoll.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/io_uring/sqpoll.c b/io_uring/sqpoll.c
index 713be7c29388..b8ec8fec99b8 100644
--- a/io_uring/sqpoll.c
+++ b/io_uring/sqpoll.c
@@ -10,6 +10,7 @@
 #include <linux/slab.h>
 #include <linux/audit.h>
 #include <linux/security.h>
+#include <linux/cpuset.h>
 #include <linux/io_uring.h>
 
 #include <uapi/linux/io_uring.h>
@@ -459,10 +460,12 @@ __cold int io_sq_offload_create(struct io_ring_ctx *ctx,
 			return 0;
 
 		if (p->flags & IORING_SETUP_SQ_AFF) {
+			struct cpumask allowed_mask;
 			int cpu = p->sq_thread_cpu;
 
 			ret = -EINVAL;
-			if (cpu >= nr_cpu_ids || !cpu_online(cpu))
+			cpuset_cpus_allowed(current, &allowed_mask);
+			if (!cpumask_test_cpu(cpu, &allowed_mask))
 				goto err_sqpoll;
 			sqd->sq_cpu = cpu;
 		} else {
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-09-18  6:22 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-09 15:00 [PATCH 1/1] io_uring/sqpoll: do not allow pinning outside of cpuset Felix Moessbauer
2024-09-09 15:08 ` Jens Axboe
2024-09-09 15:09 ` Jens Axboe
2024-09-18  3:56 ` Lai, Yi
2024-09-18  6:16   ` Jens Axboe
2024-09-18  6:21     ` Lai, Yi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox