| Message ID | 20250224213116.3509093-3-kbusch@meta.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | ublk zero copy support | expand |
On 2/24/25 2:31 PM, Keith Busch wrote: > From: Keith Busch <kbusch@kernel.org> > > There is already a field in io_kiocb that can store a registered buffer > index, use that instead of stashing the value into struct io_nop. Only reason it was done this way is that ->buf_index is initially the buffer group ID, and then the buffer ID when a buffer is selected. But I _think_ we always restore that and hence we don't need to do this anymore, should be checked. Maybe you already did?
On Mon, Feb 24, 2025 at 04:30:48PM -0700, Jens Axboe wrote: > On 2/24/25 2:31 PM, Keith Busch wrote: > > From: Keith Busch <kbusch@kernel.org> > > > > There is already a field in io_kiocb that can store a registered buffer > > index, use that instead of stashing the value into struct io_nop. > > Only reason it was done this way is that ->buf_index is initially the > buffer group ID, and then the buffer ID when a buffer is selected. But I > _think_ we always restore that and hence we don't need to do this > anymore, should be checked. Maybe you already did? The IORING_OP_NOP opdef doesn't set the buffer_select flag, so the req buf_index couldn't be used for a buffer select id.
On Mon, Feb 24, 2025 at 01:31:07PM -0800, Keith Busch wrote: > From: Keith Busch <kbusch@kernel.org> > > There is already a field in io_kiocb that can store a registered buffer > index, use that instead of stashing the value into struct io_nop. > > Signed-off-by: Keith Busch <kbusch@kernel.org> Reviewed-by: Ming Lei <ming.lei@redhat.com> Thanks, Ming
On 2/24/25 21:31, Keith Busch wrote: > From: Keith Busch <kbusch@kernel.org> > > There is already a field in io_kiocb that can store a registered buffer > index, use that instead of stashing the value into struct io_nop. Reviewed-by: Pavel Begunkov <asml.silence@gmail.com> > > Signed-off-by: Keith Busch <kbusch@kernel.org> > --- > io_uring/nop.c | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) > > diff --git a/io_uring/nop.c b/io_uring/nop.c > index 5e5196df650a1..ea539531cb5f6 100644 > --- a/io_uring/nop.c > +++ b/io_uring/nop.c > @@ -16,7 +16,6 @@ struct io_nop { > struct file *file; > int result; > int fd; > - int buffer; > unsigned int flags; > }; > > @@ -40,9 +39,7 @@ int io_nop_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe) > else > nop->fd = -1; > if (nop->flags & IORING_NOP_FIXED_BUFFER) > - nop->buffer = READ_ONCE(sqe->buf_index); > - else > - nop->buffer = -1; > + req->buf_index = READ_ONCE(sqe->buf_index); > return 0; > } > > @@ -69,7 +66,7 @@ int io_nop(struct io_kiocb *req, unsigned int issue_flags) > > ret = -EFAULT; > io_ring_submit_lock(ctx, issue_flags); > - node = io_rsrc_node_lookup(&ctx->buf_table, nop->buffer); > + node = io_rsrc_node_lookup(&ctx->buf_table, req->buf_index); > if (node) { > io_req_assign_buf_node(req, node); > ret = 0;
diff --git a/io_uring/nop.c b/io_uring/nop.c index 5e5196df650a1..ea539531cb5f6 100644 --- a/io_uring/nop.c +++ b/io_uring/nop.c @@ -16,7 +16,6 @@ struct io_nop { struct file *file; int result; int fd; - int buffer; unsigned int flags; }; @@ -40,9 +39,7 @@ int io_nop_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe) else nop->fd = -1; if (nop->flags & IORING_NOP_FIXED_BUFFER) - nop->buffer = READ_ONCE(sqe->buf_index); - else - nop->buffer = -1; + req->buf_index = READ_ONCE(sqe->buf_index); return 0; } @@ -69,7 +66,7 @@ int io_nop(struct io_kiocb *req, unsigned int issue_flags) ret = -EFAULT; io_ring_submit_lock(ctx, issue_flags); - node = io_rsrc_node_lookup(&ctx->buf_table, nop->buffer); + node = io_rsrc_node_lookup(&ctx->buf_table, req->buf_index); if (node) { io_req_assign_buf_node(req, node); ret = 0;