From: Jens Axboe <[email protected]>
To: Xiaobing Li <[email protected]>
Cc: [email protected], [email protected],
[email protected], [email protected], [email protected],
[email protected], [email protected],
[email protected]
Subject: Re: [PATCH v3] io_uring/fdinfo: remove need for sqpoll lock for thread/pid retrieval
Date: Wed, 15 Nov 2023 06:49:26 -0700 [thread overview]
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
On 11/15/23 6:42 AM, Jens Axboe wrote:
>> 2. Sometimes it can output, sometimes it outputs -1.
>>
>> The test results are as follows:
>> Every 0.5s: cat /proc/9572/fdinfo/6 | grep Sq
>> SqMask: 0x3
>> SqHead: 6765744
>> SqTail: 6765744
>> CachedSqHead: 6765744
>> SqThread: -1
>> SqThreadCpu: -1
>> SqBusy: 0%
>> -------------------------------------------
>> Every 0.5s: cat /proc/9572/fdinfo/6 | grep Sq
>> SqMask: 0x3
>> SqHead: 7348727
>> SqTail: 7348728
>> CachedSqHead: 7348728
>> SqThread: 9571
>> SqThreadCpu: 174
>> SqBusy: 95%
>
> Right, this is due to the uring_lock. We got rid of the main
> regression, which was the new trylock for the sqd->lock, but the old
> one remains. We can fix this as well for sqpoll info, but it's not a
> regression from past releases, it's always been like that.
>
> Pavel and I discussed it yesterday, and the easy solution is to make
> io_sq_data be under RCU protection. But that requires this patch
> first, so we don't have to fiddle with the sqpoll task itself. I can
> try and hack up the patch if you want to test it, it'd be on top of
> this one and for the next kernel release rather than 6.7.
Something like this, totally untested.
diff --git a/io_uring/fdinfo.c b/io_uring/fdinfo.c
index 976e9500f651..434a21a6b653 100644
--- a/io_uring/fdinfo.c
+++ b/io_uring/fdinfo.c
@@ -142,11 +142,16 @@ __cold void io_uring_show_fdinfo(struct seq_file *m, struct file *f)
*/
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);
--
Jens Axboe
next prev parent reply other threads:[~2023-11-15 13:49 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-15 2:36 [PATCH v3] io_uring/fdinfo: remove need for sqpoll lock for thread/pid retrieval Jens Axboe
[not found] ` <CGME20231115061813epcas5p2bb6bebb451c6e2c65a5e9ec9ffac5f46@epcas5p2.samsung.com>
2023-11-15 6:10 ` Xiaobing Li
2023-11-15 13:42 ` Jens Axboe
2023-11-15 13:49 ` Jens Axboe [this message]
[not found] ` <CGME20231118032740epcas5p20b6aad6264323376fa024bc2a56f0990@epcas5p2.samsung.com>
2023-11-18 3:19 ` Xiaobing Li
2023-11-19 21:23 ` Jens Axboe
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 \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
/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