* [PATCH 1/5] io_uring: use right helpers for file assign locking
2022-04-18 19:51 [PATCH 0/5] random 5.19 patches Pavel Begunkov
@ 2022-04-18 19:51 ` Pavel Begunkov
2022-04-18 19:51 ` [PATCH 2/5] io_uring: refactor io_assign_file error path Pavel Begunkov
` (4 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Pavel Begunkov @ 2022-04-18 19:51 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe, asml.silence
We have io_ring_submit_[un]lock() functions helping us with conditional
->uring_lock locking, use them in io_file_get_fixed() instead of hand
coding.
Signed-off-by: Pavel Begunkov <[email protected]>
---
fs/io_uring.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 6988bdc182e4..423427e2203f 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -7371,8 +7371,7 @@ static inline struct file *io_file_get_fixed(struct io_kiocb *req, int fd,
struct file *file = NULL;
unsigned long file_ptr;
- if (issue_flags & IO_URING_F_UNLOCKED)
- mutex_lock(&ctx->uring_lock);
+ io_ring_submit_lock(ctx, issue_flags);
if (unlikely((unsigned int)fd >= ctx->nr_user_files))
goto out;
@@ -7384,8 +7383,7 @@ static inline struct file *io_file_get_fixed(struct io_kiocb *req, int fd,
req->flags |= (file_ptr << REQ_F_SUPPORT_NOWAIT_BIT);
io_req_set_rsrc_node(req, ctx, 0);
out:
- if (issue_flags & IO_URING_F_UNLOCKED)
- mutex_unlock(&ctx->uring_lock);
+ io_ring_submit_unlock(ctx, issue_flags);
return file;
}
--
2.35.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/5] io_uring: refactor io_assign_file error path
2022-04-18 19:51 [PATCH 0/5] random 5.19 patches Pavel Begunkov
2022-04-18 19:51 ` [PATCH 1/5] io_uring: use right helpers for file assign locking Pavel Begunkov
@ 2022-04-18 19:51 ` Pavel Begunkov
2022-04-18 20:50 ` Jens Axboe
2022-04-18 19:51 ` [PATCH 3/5] io_uring: store rsrc node in req instead of refs Pavel Begunkov
` (3 subsequent siblings)
5 siblings, 1 reply; 9+ messages in thread
From: Pavel Begunkov @ 2022-04-18 19:51 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe, asml.silence
All io_assign_file() callers do error handling themselves,
req_set_fail() in the io_assign_file()'s fail path needlessly bloats the
kernel and is not the best abstraction to have. Simplify the error path.
Signed-off-by: Pavel Begunkov <[email protected]>
---
fs/io_uring.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 423427e2203f..9626bc1cb0a0 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -7117,12 +7117,8 @@ static bool io_assign_file(struct io_kiocb *req, unsigned int issue_flags)
req->file = io_file_get_fixed(req, req->cqe.fd, issue_flags);
else
req->file = io_file_get_normal(req, req->cqe.fd);
- if (req->file)
- return true;
- req_set_fail(req);
- req->cqe.res = -EBADF;
- return false;
+ return !!req->file;
}
static int io_issue_sqe(struct io_kiocb *req, unsigned int issue_flags)
--
2.35.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/5] io_uring: refactor io_assign_file error path
2022-04-18 19:51 ` [PATCH 2/5] io_uring: refactor io_assign_file error path Pavel Begunkov
@ 2022-04-18 20:50 ` Jens Axboe
2022-04-18 20:51 ` Jens Axboe
0 siblings, 1 reply; 9+ messages in thread
From: Jens Axboe @ 2022-04-18 20:50 UTC (permalink / raw)
To: Pavel Begunkov, io-uring
On 4/18/22 1:51 PM, Pavel Begunkov wrote:
> All io_assign_file() callers do error handling themselves,
> req_set_fail() in the io_assign_file()'s fail path needlessly bloats the
> kernel and is not the best abstraction to have. Simplify the error path.
>
> Signed-off-by: Pavel Begunkov <[email protected]>
> ---
> fs/io_uring.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index 423427e2203f..9626bc1cb0a0 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -7117,12 +7117,8 @@ static bool io_assign_file(struct io_kiocb *req, unsigned int issue_flags)
> req->file = io_file_get_fixed(req, req->cqe.fd, issue_flags);
> else
> req->file = io_file_get_normal(req, req->cqe.fd);
> - if (req->file)
> - return true;
>
> - req_set_fail(req);
> - req->cqe.res = -EBADF;
> - return false;
> + return !!req->file;
Wouldn't it be cleaner to just do:
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 7625b29153b9..b91bcd52cc95 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -7098,15 +7098,8 @@ static bool io_assign_file(struct io_kiocb *req, unsigned int issue_flags)
return true;
if (req->flags & REQ_F_FIXED_FILE)
- req->file = io_file_get_fixed(req, req->fd, issue_flags);
- else
- req->file = io_file_get_normal(req, req->fd);
- if (req->file)
- return true;
-
- req_set_fail(req);
- req->result = -EBADF;
- return false;
+ return io_file_get_fixed(req, req->fd, issue_flags);
+ return io_file_get_normal(req, req->fd);
}
static int io_issue_sqe(struct io_kiocb *req, unsigned int issue_flags)
--
Jens Axboe
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/5] io_uring: refactor io_assign_file error path
2022-04-18 20:50 ` Jens Axboe
@ 2022-04-18 20:51 ` Jens Axboe
0 siblings, 0 replies; 9+ messages in thread
From: Jens Axboe @ 2022-04-18 20:51 UTC (permalink / raw)
To: Pavel Begunkov, io-uring
On 4/18/22 2:50 PM, Jens Axboe wrote:
> On 4/18/22 1:51 PM, Pavel Begunkov wrote:
>> All io_assign_file() callers do error handling themselves,
>> req_set_fail() in the io_assign_file()'s fail path needlessly bloats the
>> kernel and is not the best abstraction to have. Simplify the error path.
>>
>> Signed-off-by: Pavel Begunkov <[email protected]>
>> ---
>> fs/io_uring.c | 6 +-----
>> 1 file changed, 1 insertion(+), 5 deletions(-)
>>
>> diff --git a/fs/io_uring.c b/fs/io_uring.c
>> index 423427e2203f..9626bc1cb0a0 100644
>> --- a/fs/io_uring.c
>> +++ b/fs/io_uring.c
>> @@ -7117,12 +7117,8 @@ static bool io_assign_file(struct io_kiocb *req, unsigned int issue_flags)
>> req->file = io_file_get_fixed(req, req->cqe.fd, issue_flags);
>> else
>> req->file = io_file_get_normal(req, req->cqe.fd);
>> - if (req->file)
>> - return true;
>>
>> - req_set_fail(req);
>> - req->cqe.res = -EBADF;
>> - return false;
>> + return !!req->file;
>
> Wouldn't it be cleaner to just do:
As soon as I sent that, realize we're missing the file assignment
in that case. Stupid. Looks fine as-is.
--
Jens Axboe
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/5] io_uring: store rsrc node in req instead of refs
2022-04-18 19:51 [PATCH 0/5] random 5.19 patches Pavel Begunkov
2022-04-18 19:51 ` [PATCH 1/5] io_uring: use right helpers for file assign locking Pavel Begunkov
2022-04-18 19:51 ` [PATCH 2/5] io_uring: refactor io_assign_file error path Pavel Begunkov
@ 2022-04-18 19:51 ` Pavel Begunkov
2022-04-18 19:51 ` [PATCH 4/5] io_uring: add a helper for putting rsrc nodes Pavel Begunkov
` (2 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Pavel Begunkov @ 2022-04-18 19:51 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe, asml.silence
req->fixed_rsrc_refs keeps a pointer to rsrc node pcpu references, but
it's more natural just to store rsrc node directly. There were some
reasons for that in the past but not anymore.
Signed-off-by: Pavel Begunkov <[email protected]>
---
fs/io_uring.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 9626bc1cb0a0..c26de427b05d 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -931,7 +931,7 @@ struct io_kiocb {
struct io_ring_ctx *ctx;
struct task_struct *task;
- struct percpu_ref *fixed_rsrc_refs;
+ struct io_rsrc_node *rsrc_node;
/* store used ubuf, so we can prevent reloading */
struct io_mapped_ubuf *imu;
@@ -1329,20 +1329,20 @@ static inline void io_req_put_rsrc_locked(struct io_kiocb *req,
struct io_ring_ctx *ctx)
__must_hold(&ctx->uring_lock)
{
- struct percpu_ref *ref = req->fixed_rsrc_refs;
+ struct io_rsrc_node *node = req->rsrc_node;
- if (ref) {
- if (ref == &ctx->rsrc_node->refs)
+ if (node) {
+ if (node == ctx->rsrc_node)
ctx->rsrc_cached_refs++;
else
- percpu_ref_put(ref);
+ percpu_ref_put(&node->refs);
}
}
static inline void io_req_put_rsrc(struct io_kiocb *req, struct io_ring_ctx *ctx)
{
- if (req->fixed_rsrc_refs)
- percpu_ref_put(req->fixed_rsrc_refs);
+ if (req->rsrc_node)
+ percpu_ref_put(&req->rsrc_node->refs);
}
static __cold void io_rsrc_refs_drop(struct io_ring_ctx *ctx)
@@ -1365,8 +1365,8 @@ static inline void io_req_set_rsrc_node(struct io_kiocb *req,
struct io_ring_ctx *ctx,
unsigned int issue_flags)
{
- if (!req->fixed_rsrc_refs) {
- req->fixed_rsrc_refs = &ctx->rsrc_node->refs;
+ if (!req->rsrc_node) {
+ req->rsrc_node = ctx->rsrc_node;
if (!(issue_flags & IO_URING_F_UNLOCKED)) {
lockdep_assert_held(&ctx->uring_lock);
@@ -1374,7 +1374,7 @@ static inline void io_req_set_rsrc_node(struct io_kiocb *req,
if (unlikely(ctx->rsrc_cached_refs < 0))
io_rsrc_refs_refill(ctx);
} else {
- percpu_ref_get(req->fixed_rsrc_refs);
+ percpu_ref_get(&req->rsrc_node->refs);
}
}
}
@@ -7606,7 +7606,7 @@ static int io_init_req(struct io_ring_ctx *ctx, struct io_kiocb *req,
req->flags = sqe_flags = READ_ONCE(sqe->flags);
req->cqe.user_data = READ_ONCE(sqe->user_data);
req->file = NULL;
- req->fixed_rsrc_refs = NULL;
+ req->rsrc_node = NULL;
req->task = current;
if (unlikely(opcode >= IORING_OP_LAST)) {
--
2.35.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 4/5] io_uring: add a helper for putting rsrc nodes
2022-04-18 19:51 [PATCH 0/5] random 5.19 patches Pavel Begunkov
` (2 preceding siblings ...)
2022-04-18 19:51 ` [PATCH 3/5] io_uring: store rsrc node in req instead of refs Pavel Begunkov
@ 2022-04-18 19:51 ` Pavel Begunkov
2022-04-18 19:51 ` [PATCH 5/5] io_uring: kill ctx arg from io_req_put_rsrc Pavel Begunkov
2022-04-18 20:52 ` [PATCH 0/5] random 5.19 patches Jens Axboe
5 siblings, 0 replies; 9+ messages in thread
From: Pavel Begunkov @ 2022-04-18 19:51 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe, asml.silence
Add a simple helper to encapsulating dropping rsrc nodes references,
it's cleaner and will help if we'd change rsrc refcounting or play with
percpu_ref_put() [no]inlining.
Signed-off-by: Pavel Begunkov <[email protected]>
---
fs/io_uring.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index c26de427b05d..c67748eabbd5 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -1325,6 +1325,11 @@ static inline void io_req_set_refcount(struct io_kiocb *req)
#define IO_RSRC_REF_BATCH 100
+static void io_rsrc_put_node(struct io_rsrc_node *node, int nr)
+{
+ percpu_ref_put_many(&node->refs, nr);
+}
+
static inline void io_req_put_rsrc_locked(struct io_kiocb *req,
struct io_ring_ctx *ctx)
__must_hold(&ctx->uring_lock)
@@ -1335,21 +1340,21 @@ static inline void io_req_put_rsrc_locked(struct io_kiocb *req,
if (node == ctx->rsrc_node)
ctx->rsrc_cached_refs++;
else
- percpu_ref_put(&node->refs);
+ io_rsrc_put_node(node, 1);
}
}
static inline void io_req_put_rsrc(struct io_kiocb *req, struct io_ring_ctx *ctx)
{
if (req->rsrc_node)
- percpu_ref_put(&req->rsrc_node->refs);
+ io_rsrc_put_node(req->rsrc_node, 1);
}
static __cold void io_rsrc_refs_drop(struct io_ring_ctx *ctx)
__must_hold(&ctx->uring_lock)
{
if (ctx->rsrc_cached_refs) {
- percpu_ref_put_many(&ctx->rsrc_node->refs, ctx->rsrc_cached_refs);
+ io_rsrc_put_node(ctx->rsrc_node, ctx->rsrc_cached_refs);
ctx->rsrc_cached_refs = 0;
}
}
--
2.35.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 5/5] io_uring: kill ctx arg from io_req_put_rsrc
2022-04-18 19:51 [PATCH 0/5] random 5.19 patches Pavel Begunkov
` (3 preceding siblings ...)
2022-04-18 19:51 ` [PATCH 4/5] io_uring: add a helper for putting rsrc nodes Pavel Begunkov
@ 2022-04-18 19:51 ` Pavel Begunkov
2022-04-18 20:52 ` [PATCH 0/5] random 5.19 patches Jens Axboe
5 siblings, 0 replies; 9+ messages in thread
From: Pavel Begunkov @ 2022-04-18 19:51 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe, asml.silence
The ctx argument of io_req_put_rsrc() is not used, kill it.
Signed-off-by: Pavel Begunkov <[email protected]>
---
fs/io_uring.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index c67748eabbd5..3905b3ec87b8 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -1344,7 +1344,7 @@ static inline void io_req_put_rsrc_locked(struct io_kiocb *req,
}
}
-static inline void io_req_put_rsrc(struct io_kiocb *req, struct io_ring_ctx *ctx)
+static inline void io_req_put_rsrc(struct io_kiocb *req)
{
if (req->rsrc_node)
io_rsrc_put_node(req->rsrc_node, 1);
@@ -2173,7 +2173,7 @@ static void __io_req_complete_post(struct io_kiocb *req, s32 res,
req->link = NULL;
}
}
- io_req_put_rsrc(req, ctx);
+ io_req_put_rsrc(req);
/*
* Selected buffer deallocation in io_clean_op() assumes that
* we don't hold ->completion_lock. Clean them here to avoid
@@ -2336,7 +2336,7 @@ static __cold void io_free_req(struct io_kiocb *req)
{
struct io_ring_ctx *ctx = req->ctx;
- io_req_put_rsrc(req, ctx);
+ io_req_put_rsrc(req);
io_dismantle_req(req);
io_put_task(req->task, 1);
--
2.35.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 0/5] random 5.19 patches
2022-04-18 19:51 [PATCH 0/5] random 5.19 patches Pavel Begunkov
` (4 preceding siblings ...)
2022-04-18 19:51 ` [PATCH 5/5] io_uring: kill ctx arg from io_req_put_rsrc Pavel Begunkov
@ 2022-04-18 20:52 ` Jens Axboe
5 siblings, 0 replies; 9+ messages in thread
From: Jens Axboe @ 2022-04-18 20:52 UTC (permalink / raw)
To: asml.silence, io-uring
On Mon, 18 Apr 2022 20:51:10 +0100, Pavel Begunkov wrote:
> Resending some leftovers
>
> Pavel Begunkov (5):
> io_uring: use right helpers for file assign locking
> io_uring: refactor io_assign_file error path
> io_uring: store rsrc node in req instead of refs
> io_uring: add a helper for putting rsrc nodes
> io_uring: kill ctx arg from io_req_put_rsrc
>
> [...]
Applied, thanks!
[1/5] io_uring: use right helpers for file assign locking
commit: 602b87b4a9cc56c6054b4524a40ecb5b42e9c722
[2/5] io_uring: refactor io_assign_file error path
commit: 6d51914bcd061b6c25d761470b4bbf1ea4470be9
[3/5] io_uring: store rsrc node in req instead of refs
commit: bf9bab6e6369c4b3f03a97345bd401925d8b315c
[4/5] io_uring: add a helper for putting rsrc nodes
commit: 86ec2e629c84f6d57e1ccfc638b6b475aca699a6
[5/5] io_uring: kill ctx arg from io_req_put_rsrc
commit: 111141d5824e947a9a129393315d8473a9356af4
Best regards,
--
Jens Axboe
^ permalink raw reply [flat|nested] 9+ messages in thread