* Re: [PATCH v3] io_uring/fdinfo: remove need for sqpoll lock for thread/pid retrieval [not found] ` <CGME20231118032740epcas5p20b6aad6264323376fa024bc2a56f0990@epcas5p2.samsung.com> @ 2023-11-18 3:19 ` Xiaobing Li 2023-11-19 21:23 ` Jens Axboe 0 siblings, 1 reply; 2+ messages in thread From: Xiaobing Li @ 2023-11-18 3:19 UTC (permalink / raw) To: axboe Cc: asml.silence, linux-kernel, io-uring, kun.dou, peiwei.li, joshi.k, kundan.kumar, wenwen.chen, ruyi.zhang, xiaobing.li On 11/15/23 6:42 AM, Jens Axboe wrote: > */ > has_lock = mutex_trylock(&ctx->uring_lock); > >- if (has_lock && (ctx->flags & IORING_SETUP_SQPOLL)) { >- struct io_sq_data *sq = ctx->sq_data; >- >- sq_pid = sq->task_pid; >- sq_cpu = sq->sq_cpu; >+ if (ctx->flags & IORING_SETUP_SQPOLL) { >+ struct io_sq_data *sq; >+ >+ rcu_read_lock(); >+ sq = READ_ONCE(ctx->sq_data); >+ if (sq) { >+ sq_pid = sq->task_pid; >+ sq_cpu = sq->sq_cpu; >+ } >+ rcu_read_unlock(); > } > > seq_printf(m, "SqThread:\t%d\n", sq_pid); >diff --git a/io_uring/sqpoll.c b/io_uring/sqpoll.c >index 65b5dbe3c850..583c76945cdf 100644 >--- a/io_uring/sqpoll.c >+++ b/io_uring/sqpoll.c >@@ -70,7 +70,7 @@ void io_put_sq_data(struct io_sq_data *sqd) > WARN_ON_ONCE(atomic_read(&sqd->park_pending)); > > io_sq_thread_stop(sqd); >- kfree(sqd); >+ kfree_rcu(sqd, rcu); > } > } > >@@ -313,7 +313,7 @@ static int io_sq_thread(void *data) > } > > io_uring_cancel_generic(true, sqd); >- sqd->thread = NULL; >+ WRITE_ONCE(sqd->thread, NULL); > list_for_each_entry(ctx, &sqd->ctx_list, sqd_list) > atomic_or(IORING_SQ_NEED_WAKEUP, &ctx->rings->sq_flags); > io_run_task_work(); >@@ -411,7 +411,7 @@ __cold int io_sq_offload_create(struct io_ring_ctx *ctx, > goto err_sqpoll; > } > >- sqd->thread = tsk; >+ WRITE_ONCE(sqd->thread, tsk); > ret = io_uring_alloc_task_context(tsk, ctx); > wake_up_new_task(tsk); > if (ret) >diff --git a/io_uring/sqpoll.h b/io_uring/sqpoll.h >index 8df37e8c9149..0cf0c5833a27 100644 >--- a/io_uring/sqpoll.h >+++ b/io_uring/sqpoll.h >@@ -18,6 +18,8 @@ struct io_sq_data { > > unsigned long state; > struct completion exited; >+ >+ struct rcu_head rcu; > }; > > int io_sq_offload_create(struct io_ring_ctx *ctx, struct io_uring_params *p); I tested this and it worked after adding RCU lock. It consistently outputs correct results. The results of a simple test are as follows: Every 0.5s: cat /proc/10212/fdinfo/6 | grep Sq SqMask: 0x3 SqHead: 17422716 SqTail: 17422716 CachedSqHead: 17422716 SqThread: 10212 SqThreadCpu: 73 SqBusy: 97% ------------------------------------------------------------- But the name of the sq thread is "iou-sqp-" + "the PID of its parent process": PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND 10211 root 20 0 184408 8192 0 R 99.9 0.0 4:01.42 fio 10212 root 20 0 184408 8192 0 R 99.9 0.0 4:01.48 iou-sqp-10211 Is this the originally desired effect? ^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH v3] io_uring/fdinfo: remove need for sqpoll lock for thread/pid retrieval 2023-11-18 3:19 ` [PATCH v3] io_uring/fdinfo: remove need for sqpoll lock for thread/pid retrieval Xiaobing Li @ 2023-11-19 21:23 ` Jens Axboe 0 siblings, 0 replies; 2+ messages in thread From: Jens Axboe @ 2023-11-19 21:23 UTC (permalink / raw) To: Xiaobing Li Cc: asml.silence, linux-kernel, io-uring, kun.dou, peiwei.li, joshi.k, kundan.kumar, wenwen.chen, ruyi.zhang On 11/17/23 8:19 PM, Xiaobing Li wrote: > On 11/15/23 6:42 AM, Jens Axboe wrote: >> */ >> has_lock = mutex_trylock(&ctx->uring_lock); >> >> - if (has_lock && (ctx->flags & IORING_SETUP_SQPOLL)) { >> - struct io_sq_data *sq = ctx->sq_data; >> - >> - sq_pid = sq->task_pid; >> - sq_cpu = sq->sq_cpu; >> + if (ctx->flags & IORING_SETUP_SQPOLL) { >> + struct io_sq_data *sq; >> + >> + rcu_read_lock(); >> + sq = READ_ONCE(ctx->sq_data); >> + if (sq) { >> + sq_pid = sq->task_pid; >> + sq_cpu = sq->sq_cpu; >> + } >> + rcu_read_unlock(); >> } >> >> seq_printf(m, "SqThread:\t%d\n", sq_pid); >> diff --git a/io_uring/sqpoll.c b/io_uring/sqpoll.c >> index 65b5dbe3c850..583c76945cdf 100644 >> --- a/io_uring/sqpoll.c >> +++ b/io_uring/sqpoll.c >> @@ -70,7 +70,7 @@ void io_put_sq_data(struct io_sq_data *sqd) >> WARN_ON_ONCE(atomic_read(&sqd->park_pending)); >> >> io_sq_thread_stop(sqd); >> - kfree(sqd); >> + kfree_rcu(sqd, rcu); >> } >> } >> >> @@ -313,7 +313,7 @@ static int io_sq_thread(void *data) >> } >> >> io_uring_cancel_generic(true, sqd); >> - sqd->thread = NULL; >> + WRITE_ONCE(sqd->thread, NULL); >> list_for_each_entry(ctx, &sqd->ctx_list, sqd_list) >> atomic_or(IORING_SQ_NEED_WAKEUP, &ctx->rings->sq_flags); >> io_run_task_work(); >> @@ -411,7 +411,7 @@ __cold int io_sq_offload_create(struct io_ring_ctx *ctx, >> goto err_sqpoll; >> } >> >> - sqd->thread = tsk; >> + WRITE_ONCE(sqd->thread, tsk); >> ret = io_uring_alloc_task_context(tsk, ctx); >> wake_up_new_task(tsk); >> if (ret) >> diff --git a/io_uring/sqpoll.h b/io_uring/sqpoll.h >> index 8df37e8c9149..0cf0c5833a27 100644 >> --- a/io_uring/sqpoll.h >> +++ b/io_uring/sqpoll.h >> @@ -18,6 +18,8 @@ struct io_sq_data { >> >> unsigned long state; >> struct completion exited; >> + >> + struct rcu_head rcu; >> }; >> >> int io_sq_offload_create(struct io_ring_ctx *ctx, struct io_uring_params *p); > > I tested this and it worked after adding RCU lock. > It consistently outputs correct results. > > The results of a simple test are as follows: > Every 0.5s: cat /proc/10212/fdinfo/6 | grep Sq > SqMask: 0x3 > SqHead: 17422716 > SqTail: 17422716 > CachedSqHead: 17422716 > SqThread: 10212 > SqThreadCpu: 73 > SqBusy: 97% > ------------------------------------------------------------- > But the name of the sq thread is "iou-sqp-" + "the PID of its parent process": > PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND > 10211 root 20 0 184408 8192 0 R 99.9 0.0 4:01.42 fio > 10212 root 20 0 184408 8192 0 R 99.9 0.0 4:01.48 iou-sqp-10211 > Is this the originally desired effect? That's how it has always been, it's the sqpoll thread belonging to that parent. -- Jens Axboe ^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-11-19 21:37 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <605eac76-ec47-436b-872a-f6e8b4094293@kernel.dk>
[not found] ` <CGME20231118032740epcas5p20b6aad6264323376fa024bc2a56f0990@epcas5p2.samsung.com>
2023-11-18 3:19 ` [PATCH v3] io_uring/fdinfo: remove need for sqpoll lock for thread/pid retrieval Xiaobing Li
2023-11-19 21:23 ` Jens Axboe
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®