| Message ID | 20221031134126.82928-7-dylany@meta.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | io_uring: retarget rsrc nodes periodically | expand |
On 10/31/22 7:41 AM, Dylan Yudaken wrote: > @@ -1849,17 +1866,14 @@ inline struct file *io_file_get_fixed(struct io_kiocb *req, int fd, > unsigned long file_ptr; > > io_ring_submit_lock(ctx, issue_flags); > - > - if (unlikely((unsigned int)fd >= ctx->nr_user_files)) > - goto out; > - fd = array_index_nospec(fd, ctx->nr_user_files); > - file_ptr = io_fixed_file_slot(&ctx->file_table, fd)->file_ptr; > + file_ptr = __io_file_peek_fixed(req, fd); > file = (struct file *) (file_ptr & FFS_MASK); > file_ptr &= ~FFS_MASK; > /* mask in overlapping REQ_F and FFS bits */ > req->flags |= (file_ptr << REQ_F_SUPPORT_NOWAIT_BIT); > io_req_set_rsrc_node(req, ctx, 0); > -out: > + WARN_ON_ONCE(file && !test_bit(fd, ctx->file_table.bitmap)); > + > io_ring_submit_unlock(ctx, issue_flags); > return file; > } Is this WARN_ON_ONCE() a leftover from being originally based on a tree before: commit 4d5059512d283dab7372d282c2fbd43c7f5a2456 Author: Pavel Begunkov <asml.silence@gmail.com> Date: Sun Oct 16 21:30:49 2022 +0100 io_uring: kill hot path fixed file bitmap debug checks got added? Seems not related to the changes here otherwise.
On Mon, 2022-10-31 at 10:04 -0600, Jens Axboe wrote: > On 10/31/22 7:41 AM, Dylan Yudaken wrote: > > @@ -1849,17 +1866,14 @@ inline struct file > > *io_file_get_fixed(struct io_kiocb *req, int fd, > > unsigned long file_ptr; > > > > io_ring_submit_lock(ctx, issue_flags); > > - > > - if (unlikely((unsigned int)fd >= ctx->nr_user_files)) > > - goto out; > > - fd = array_index_nospec(fd, ctx->nr_user_files); > > - file_ptr = io_fixed_file_slot(&ctx->file_table, fd)- > > >file_ptr; > > + file_ptr = __io_file_peek_fixed(req, fd); > > file = (struct file *) (file_ptr & FFS_MASK); > > file_ptr &= ~FFS_MASK; > > /* mask in overlapping REQ_F and FFS bits */ > > req->flags |= (file_ptr << REQ_F_SUPPORT_NOWAIT_BIT); > > io_req_set_rsrc_node(req, ctx, 0); > > -out: > > + WARN_ON_ONCE(file && !test_bit(fd, ctx- > > >file_table.bitmap)); > > + > > io_ring_submit_unlock(ctx, issue_flags); > > return file; > > } > > Is this WARN_ON_ONCE() a leftover from being originally based on a > tree > before: > > commit 4d5059512d283dab7372d282c2fbd43c7f5a2456 > Author: Pavel Begunkov <asml.silence@gmail.com> > Date: Sun Oct 16 21:30:49 2022 +0100 > > io_uring: kill hot path fixed file bitmap debug checks > > got added? Seems not related to the changes here otherwise. > Ah yes. That was a bad merge in that case with the "out:" label. I'll fix that in v2. I am assuming there will be some more comments. Thanks for pointing this out. Dylan
On 10/31/22 13:41, Dylan Yudaken wrote: > Add a helper function to grab the fixed file at a given offset. Will be > useful for retarget op handlers. > > Signed-off-by: Dylan Yudaken <dylany@meta.com> > --- > io_uring/io_uring.c | 26 ++++++++++++++++++++------ > io_uring/io_uring.h | 1 + > 2 files changed, 21 insertions(+), 6 deletions(-) > > diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c > index 32eb305c4ce7..a052653fc65e 100644 > --- a/io_uring/io_uring.c > +++ b/io_uring/io_uring.c > @@ -1841,6 +1841,23 @@ void io_wq_submit_work(struct io_wq_work *work) > io_req_task_queue_fail(req, ret); > } > > +static unsigned long __io_file_peek_fixed(struct io_kiocb *req, int fd) > + __must_hold(&req->ctx->uring_lock) Let's mark it inline, it's in the hot path. Yeah, It's small but I battled compilers enough because from time to time they leave it not inlined. > +{ > + struct io_ring_ctx *ctx = req->ctx; > + > + if (unlikely((unsigned int)fd >= ctx->nr_user_files)) > + return 0; > + fd = array_index_nospec(fd, ctx->nr_user_files); > + return io_fixed_file_slot(&ctx->file_table, fd)->file_ptr; > +} > + > +struct file *io_file_peek_fixed(struct io_kiocb *req, int fd) > + __must_hold(&req->ctx->uring_lock) > +{ > + return (struct file *) (__io_file_peek_fixed(req, fd) & FFS_MASK); > +} > + > inline struct file *io_file_get_fixed(struct io_kiocb *req, int fd, > unsigned int issue_flags) > { > @@ -1849,17 +1866,14 @@ inline struct file *io_file_get_fixed(struct io_kiocb *req, int fd, > unsigned long file_ptr; > > io_ring_submit_lock(ctx, issue_flags); > - > - if (unlikely((unsigned int)fd >= ctx->nr_user_files)) > - goto out; > - fd = array_index_nospec(fd, ctx->nr_user_files); > - file_ptr = io_fixed_file_slot(&ctx->file_table, fd)->file_ptr; > + file_ptr = __io_file_peek_fixed(req, fd); > file = (struct file *) (file_ptr & FFS_MASK); > file_ptr &= ~FFS_MASK; > /* mask in overlapping REQ_F and FFS bits */ > req->flags |= (file_ptr << REQ_F_SUPPORT_NOWAIT_BIT); > io_req_set_rsrc_node(req, ctx, 0); > -out: > + WARN_ON_ONCE(file && !test_bit(fd, ctx->file_table.bitmap)); > + > io_ring_submit_unlock(ctx, issue_flags); > return file; > } > diff --git a/io_uring/io_uring.h b/io_uring/io_uring.h > index ef77d2aa3172..781471bfba12 100644 > --- a/io_uring/io_uring.h > +++ b/io_uring/io_uring.h > @@ -44,6 +44,7 @@ struct page **io_pin_pages(unsigned long ubuf, unsigned long len, int *npages); > struct file *io_file_get_normal(struct io_kiocb *req, int fd); > struct file *io_file_get_fixed(struct io_kiocb *req, int fd, > unsigned issue_flags); > +struct file *io_file_peek_fixed(struct io_kiocb *req, int fd); > > static inline bool io_req_ffs_set(struct io_kiocb *req) > {
diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c index 32eb305c4ce7..a052653fc65e 100644 --- a/io_uring/io_uring.c +++ b/io_uring/io_uring.c @@ -1841,6 +1841,23 @@ void io_wq_submit_work(struct io_wq_work *work) io_req_task_queue_fail(req, ret); } +static unsigned long __io_file_peek_fixed(struct io_kiocb *req, int fd) + __must_hold(&req->ctx->uring_lock) +{ + struct io_ring_ctx *ctx = req->ctx; + + if (unlikely((unsigned int)fd >= ctx->nr_user_files)) + return 0; + fd = array_index_nospec(fd, ctx->nr_user_files); + return io_fixed_file_slot(&ctx->file_table, fd)->file_ptr; +} + +struct file *io_file_peek_fixed(struct io_kiocb *req, int fd) + __must_hold(&req->ctx->uring_lock) +{ + return (struct file *) (__io_file_peek_fixed(req, fd) & FFS_MASK); +} + inline struct file *io_file_get_fixed(struct io_kiocb *req, int fd, unsigned int issue_flags) { @@ -1849,17 +1866,14 @@ inline struct file *io_file_get_fixed(struct io_kiocb *req, int fd, unsigned long file_ptr; io_ring_submit_lock(ctx, issue_flags); - - if (unlikely((unsigned int)fd >= ctx->nr_user_files)) - goto out; - fd = array_index_nospec(fd, ctx->nr_user_files); - file_ptr = io_fixed_file_slot(&ctx->file_table, fd)->file_ptr; + file_ptr = __io_file_peek_fixed(req, fd); file = (struct file *) (file_ptr & FFS_MASK); file_ptr &= ~FFS_MASK; /* mask in overlapping REQ_F and FFS bits */ req->flags |= (file_ptr << REQ_F_SUPPORT_NOWAIT_BIT); io_req_set_rsrc_node(req, ctx, 0); -out: + WARN_ON_ONCE(file && !test_bit(fd, ctx->file_table.bitmap)); + io_ring_submit_unlock(ctx, issue_flags); return file; } diff --git a/io_uring/io_uring.h b/io_uring/io_uring.h index ef77d2aa3172..781471bfba12 100644 --- a/io_uring/io_uring.h +++ b/io_uring/io_uring.h @@ -44,6 +44,7 @@ struct page **io_pin_pages(unsigned long ubuf, unsigned long len, int *npages); struct file *io_file_get_normal(struct io_kiocb *req, int fd); struct file *io_file_get_fixed(struct io_kiocb *req, int fd, unsigned issue_flags); +struct file *io_file_peek_fixed(struct io_kiocb *req, int fd); static inline bool io_req_ffs_set(struct io_kiocb *req) {
Add a helper function to grab the fixed file at a given offset. Will be useful for retarget op handlers. Signed-off-by: Dylan Yudaken <dylany@meta.com> --- io_uring/io_uring.c | 26 ++++++++++++++++++++------ io_uring/io_uring.h | 1 + 2 files changed, 21 insertions(+), 6 deletions(-)