* [PATCH 1/1] io_uring: optimise locking for local tw with submit_wait
@ 2022-10-06 20:42 Pavel Begunkov
2022-10-06 20:59 ` Jens Axboe
2022-10-06 21:14 ` Jens Axboe
0 siblings, 2 replies; 5+ messages in thread
From: Pavel Begunkov @ 2022-10-06 20:42 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe, asml.silence, Dylan Yudaken
Running local task_work requires taking uring_lock, for submit + wait we
can try to run them right after submit while we still hold the lock and
save one lock/unlokc pair. The optimisation was implemented in the first
local tw patches but got dropped for simplicity.
Suggested-by: Dylan Yudaken <[email protected]>
Signed-off-by: Pavel Begunkov <[email protected]>
---
io_uring/io_uring.c | 12 ++++++++++--
io_uring/io_uring.h | 7 +++++++
2 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 355fc1f3083d..b092473eca1d 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -3224,8 +3224,16 @@ SYSCALL_DEFINE6(io_uring_enter, unsigned int, fd, u32, to_submit,
mutex_unlock(&ctx->uring_lock);
goto out;
}
- if ((flags & IORING_ENTER_GETEVENTS) && ctx->syscall_iopoll)
- goto iopoll_locked;
+ if (flags & IORING_ENTER_GETEVENTS) {
+ if (ctx->syscall_iopoll)
+ goto iopoll_locked;
+ /*
+ * Ignore errors, we'll soon call io_cqring_wait() and
+ * it should handle ownership problems if any.
+ */
+ if (ctx->flags & IORING_SETUP_DEFER_TASKRUN)
+ (void)io_run_local_work_locked(ctx);
+ }
mutex_unlock(&ctx->uring_lock);
}
diff --git a/io_uring/io_uring.h b/io_uring/io_uring.h
index e733d31f31d2..8504bc1f3839 100644
--- a/io_uring/io_uring.h
+++ b/io_uring/io_uring.h
@@ -275,6 +275,13 @@ static inline int io_run_task_work_ctx(struct io_ring_ctx *ctx)
return ret;
}
+static inline int io_run_local_work_locked(struct io_ring_ctx *ctx)
+{
+ if (llist_empty(&ctx->work_llist))
+ return 0;
+ return __io_run_local_work(ctx, true);
+}
+
static inline void io_tw_lock(struct io_ring_ctx *ctx, bool *locked)
{
if (!*locked) {
--
2.37.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] io_uring: optimise locking for local tw with submit_wait
2022-10-06 20:42 [PATCH 1/1] io_uring: optimise locking for local tw with submit_wait Pavel Begunkov
@ 2022-10-06 20:59 ` Jens Axboe
2022-10-06 21:09 ` Pavel Begunkov
2022-10-06 21:14 ` Jens Axboe
1 sibling, 1 reply; 5+ messages in thread
From: Jens Axboe @ 2022-10-06 20:59 UTC (permalink / raw)
To: Pavel Begunkov, io-uring; +Cc: Dylan Yudaken
On 10/6/22 2:42 PM, Pavel Begunkov wrote:
> Running local task_work requires taking uring_lock, for submit + wait we
> can try to run them right after submit while we still hold the lock and
> save one lock/unlokc pair. The optimisation was implemented in the first
> local tw patches but got dropped for simplicity.
>
> Suggested-by: Dylan Yudaken <[email protected]>
> Signed-off-by: Pavel Begunkov <[email protected]>
> ---
> io_uring/io_uring.c | 12 ++++++++++--
> io_uring/io_uring.h | 7 +++++++
> 2 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
> index 355fc1f3083d..b092473eca1d 100644
> --- a/io_uring/io_uring.c
> +++ b/io_uring/io_uring.c
> @@ -3224,8 +3224,16 @@ SYSCALL_DEFINE6(io_uring_enter, unsigned int, fd, u32, to_submit,
> mutex_unlock(&ctx->uring_lock);
> goto out;
> }
> - if ((flags & IORING_ENTER_GETEVENTS) && ctx->syscall_iopoll)
> - goto iopoll_locked;
> + if (flags & IORING_ENTER_GETEVENTS) {
> + if (ctx->syscall_iopoll)
> + goto iopoll_locked;
> + /*
> + * Ignore errors, we'll soon call io_cqring_wait() and
> + * it should handle ownership problems if any.
> + */
> + if (ctx->flags & IORING_SETUP_DEFER_TASKRUN)
> + (void)io_run_local_work_locked(ctx);
> + }
> mutex_unlock(&ctx->uring_lock);
> }
>
> diff --git a/io_uring/io_uring.h b/io_uring/io_uring.h
> index e733d31f31d2..8504bc1f3839 100644
> --- a/io_uring/io_uring.h
> +++ b/io_uring/io_uring.h
> @@ -275,6 +275,13 @@ static inline int io_run_task_work_ctx(struct io_ring_ctx *ctx)
> return ret;
> }
>
> +static inline int io_run_local_work_locked(struct io_ring_ctx *ctx)
> +{
> + if (llist_empty(&ctx->work_llist))
> + return 0;
> + return __io_run_local_work(ctx, true);
> +}
Do you have pending patches that also use this? If not, maybe we
should just keep it in io_uring.c? If you do, then this looks fine
to me rather than needing to shuffle it later.
--
Jens Axboe
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] io_uring: optimise locking for local tw with submit_wait
2022-10-06 20:59 ` Jens Axboe
@ 2022-10-06 21:09 ` Pavel Begunkov
2022-10-06 21:11 ` Jens Axboe
0 siblings, 1 reply; 5+ messages in thread
From: Pavel Begunkov @ 2022-10-06 21:09 UTC (permalink / raw)
To: Jens Axboe, io-uring; +Cc: Dylan Yudaken
On 10/6/22 21:59, Jens Axboe wrote:
> On 10/6/22 2:42 PM, Pavel Begunkov wrote:
>> Running local task_work requires taking uring_lock, for submit + wait we
>> can try to run them right after submit while we still hold the lock and
>> save one lock/unlokc pair. The optimisation was implemented in the first
>> local tw patches but got dropped for simplicity.
>>
>> Suggested-by: Dylan Yudaken <[email protected]>
>> Signed-off-by: Pavel Begunkov <[email protected]>
>> ---
>> io_uring/io_uring.c | 12 ++++++++++--
>> io_uring/io_uring.h | 7 +++++++
>> 2 files changed, 17 insertions(+), 2 deletions(-)
>>
>> diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
>> index 355fc1f3083d..b092473eca1d 100644
>> --- a/io_uring/io_uring.c
>> +++ b/io_uring/io_uring.c
>> @@ -3224,8 +3224,16 @@ SYSCALL_DEFINE6(io_uring_enter, unsigned int, fd, u32, to_submit,
>> mutex_unlock(&ctx->uring_lock);
>> goto out;
>> }
>> - if ((flags & IORING_ENTER_GETEVENTS) && ctx->syscall_iopoll)
>> - goto iopoll_locked;
>> + if (flags & IORING_ENTER_GETEVENTS) {
>> + if (ctx->syscall_iopoll)
>> + goto iopoll_locked;
>> + /*
>> + * Ignore errors, we'll soon call io_cqring_wait() and
>> + * it should handle ownership problems if any.
>> + */
>> + if (ctx->flags & IORING_SETUP_DEFER_TASKRUN)
>> + (void)io_run_local_work_locked(ctx);
>> + }
>> mutex_unlock(&ctx->uring_lock);
>> }
>>
>> diff --git a/io_uring/io_uring.h b/io_uring/io_uring.h
>> index e733d31f31d2..8504bc1f3839 100644
>> --- a/io_uring/io_uring.h
>> +++ b/io_uring/io_uring.h
>> @@ -275,6 +275,13 @@ static inline int io_run_task_work_ctx(struct io_ring_ctx *ctx)
>> return ret;
>> }
>>
>> +static inline int io_run_local_work_locked(struct io_ring_ctx *ctx)
>> +{
>> + if (llist_empty(&ctx->work_llist))
>> + return 0;
>> + return __io_run_local_work(ctx, true);
>> +}
>
> Do you have pending patches that also use this? If not, maybe we
> should just keep it in io_uring.c? If you do, then this looks fine
> to me rather than needing to shuffle it later.
No, I don't. I'd argue it's better as a helper because at least it
hides always confusing bool argument, and we'd also need to replace
a similar one in io_iopoll_check(). Add we can stick must_hold there
for even more clarity. But ultimately I don't care much.
--
Pavel Begunkov
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] io_uring: optimise locking for local tw with submit_wait
2022-10-06 21:09 ` Pavel Begunkov
@ 2022-10-06 21:11 ` Jens Axboe
0 siblings, 0 replies; 5+ messages in thread
From: Jens Axboe @ 2022-10-06 21:11 UTC (permalink / raw)
To: Pavel Begunkov, io-uring; +Cc: Dylan Yudaken
On 10/6/22 3:09 PM, Pavel Begunkov wrote:
> On 10/6/22 21:59, Jens Axboe wrote:
>> On 10/6/22 2:42 PM, Pavel Begunkov wrote:
>>> Running local task_work requires taking uring_lock, for submit + wait we
>>> can try to run them right after submit while we still hold the lock and
>>> save one lock/unlokc pair. The optimisation was implemented in the first
>>> local tw patches but got dropped for simplicity.
>>>
>>> Suggested-by: Dylan Yudaken <[email protected]>
>>> Signed-off-by: Pavel Begunkov <[email protected]>
>>> ---
>>> io_uring/io_uring.c | 12 ++++++++++--
>>> io_uring/io_uring.h | 7 +++++++
>>> 2 files changed, 17 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
>>> index 355fc1f3083d..b092473eca1d 100644
>>> --- a/io_uring/io_uring.c
>>> +++ b/io_uring/io_uring.c
>>> @@ -3224,8 +3224,16 @@ SYSCALL_DEFINE6(io_uring_enter, unsigned int, fd, u32, to_submit,
>>> mutex_unlock(&ctx->uring_lock);
>>> goto out;
>>> }
>>> - if ((flags & IORING_ENTER_GETEVENTS) && ctx->syscall_iopoll)
>>> - goto iopoll_locked;
>>> + if (flags & IORING_ENTER_GETEVENTS) {
>>> + if (ctx->syscall_iopoll)
>>> + goto iopoll_locked;
>>> + /*
>>> + * Ignore errors, we'll soon call io_cqring_wait() and
>>> + * it should handle ownership problems if any.
>>> + */
>>> + if (ctx->flags & IORING_SETUP_DEFER_TASKRUN)
>>> + (void)io_run_local_work_locked(ctx);
>>> + }
>>> mutex_unlock(&ctx->uring_lock);
>>> }
>>> diff --git a/io_uring/io_uring.h b/io_uring/io_uring.h
>>> index e733d31f31d2..8504bc1f3839 100644
>>> --- a/io_uring/io_uring.h
>>> +++ b/io_uring/io_uring.h
>>> @@ -275,6 +275,13 @@ static inline int io_run_task_work_ctx(struct io_ring_ctx *ctx)
>>> return ret;
>>> }
>>> +static inline int io_run_local_work_locked(struct io_ring_ctx *ctx)
>>> +{
>>> + if (llist_empty(&ctx->work_llist))
>>> + return 0;
>>> + return __io_run_local_work(ctx, true);
>>> +}
>>
>> Do you have pending patches that also use this? If not, maybe we
>> should just keep it in io_uring.c? If you do, then this looks fine
>> to me rather than needing to shuffle it later.
>
> No, I don't. I'd argue it's better as a helper because at least it
> hides always confusing bool argument, and we'd also need to replace
> a similar one in io_iopoll_check(). Add we can stick must_hold there
> for even more clarity. But ultimately I don't care much.
I really don't feel that strongly about it either, let's just keep
it the way it is.
--
Jens Axboe
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] io_uring: optimise locking for local tw with submit_wait
2022-10-06 20:42 [PATCH 1/1] io_uring: optimise locking for local tw with submit_wait Pavel Begunkov
2022-10-06 20:59 ` Jens Axboe
@ 2022-10-06 21:14 ` Jens Axboe
1 sibling, 0 replies; 5+ messages in thread
From: Jens Axboe @ 2022-10-06 21:14 UTC (permalink / raw)
To: io-uring, Pavel Begunkov; +Cc: Dylan Yudaken
On Thu, 6 Oct 2022 21:42:33 +0100, Pavel Begunkov wrote:
> Running local task_work requires taking uring_lock, for submit + wait we
> can try to run them right after submit while we still hold the lock and
> save one lock/unlokc pair. The optimisation was implemented in the first
> local tw patches but got dropped for simplicity.
>
>
Applied, thanks!
[1/1] io_uring: optimise locking for local tw with submit_wait
commit: a2b61c4d8fcb005007bae5b2f007d43cba89baa1
Best regards,
--
Jens Axboe
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-10-06 21:14 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-10-06 20:42 [PATCH 1/1] io_uring: optimise locking for local tw with submit_wait Pavel Begunkov
2022-10-06 20:59 ` Jens Axboe
2022-10-06 21:09 ` Pavel Begunkov
2022-10-06 21:11 ` Jens Axboe
2022-10-06 21:14 ` Jens Axboe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox