diff mbox series

[RFC,1/4] io_uring/rw: Get rid of flags field in struct io_rw

Message ID 20240322185023.131697-2-joshi.k@samsung.com (mailing list archive)
State New
Headers show
Series Read/Write with meta buffer | expand

Commit Message

Kanchan Joshi March 22, 2024, 6:50 p.m. UTC
From: Anuj Gupta <anuj20.g@samsung.com>

Get rid of the flags field in io_rw. Flags can be set in kiocb->flags
during prep rather than doing it while issuing the I/O in io_read/io_write.

Signed-off-by: Anuj Gupta <anuj20.g@samsung.com>
---
 io_uring/rw.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

Comments

David Wei March 27, 2024, 11:30 p.m. UTC | #1
On 2024-03-22 11:50, Kanchan Joshi wrote:
> From: Anuj Gupta <anuj20.g@samsung.com>
> 
> Get rid of the flags field in io_rw. Flags can be set in kiocb->flags
> during prep rather than doing it while issuing the I/O in io_read/io_write.
> 
> Signed-off-by: Anuj Gupta <anuj20.g@samsung.com>
> ---
>  io_uring/rw.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)

This patch looks fine and is a no-op on its own, but I think there is a
subtle semantic change. If the rw_flags is invalid (i.e.
kiocb_set_rw_flags() returns an err) and prep() fails, then the
remaining submissions won't be submitted unless IORING_SETUP_SUBMIT_ALL
is set.

Currently if kiocb_set_rw_flags() fails in prep(), only the request will
fail.
David Wei March 27, 2024, 11:32 p.m. UTC | #2
On 2024-03-27 16:30, David Wei wrote:
> On 2024-03-22 11:50, Kanchan Joshi wrote:
>> From: Anuj Gupta <anuj20.g@samsung.com>
>>
>> Get rid of the flags field in io_rw. Flags can be set in kiocb->flags
>> during prep rather than doing it while issuing the I/O in io_read/io_write.
>>
>> Signed-off-by: Anuj Gupta <anuj20.g@samsung.com>
>> ---
>>  io_uring/rw.c | 22 +++++++++++-----------
>>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> This patch looks fine and is a no-op on its own, but I think there is a
> subtle semantic change. If the rw_flags is invalid (i.e.
> kiocb_set_rw_flags() returns an err) and prep() fails, then the
> remaining submissions won't be submitted unless IORING_SETUP_SUBMIT_ALL
> is set.
> 
> Currently if kiocb_set_rw_flags() fails in prep(), only the request will
> fail.

Sorry, that should say fails in _issue()_.
diff mbox series

Patch

diff --git a/io_uring/rw.c b/io_uring/rw.c
index 47e097ab5d7e..40f6c2a59928 100644
--- a/io_uring/rw.c
+++ b/io_uring/rw.c
@@ -27,7 +27,6 @@  struct io_rw {
 	struct kiocb			kiocb;
 	u64				addr;
 	u32				len;
-	rwf_t				flags;
 };
 
 static inline bool io_file_supports_nowait(struct io_kiocb *req)
@@ -78,10 +77,16 @@  static int io_iov_buffer_select_prep(struct io_kiocb *req)
 int io_prep_rw(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 {
 	struct io_rw *rw = io_kiocb_to_cmd(req, struct io_rw);
+	struct kiocb *kiocb = &rw->kiocb;
 	unsigned ioprio;
 	int ret;
 
-	rw->kiocb.ki_pos = READ_ONCE(sqe->off);
+	kiocb->ki_flags = 0;
+	ret = kiocb_set_rw_flags(kiocb, READ_ONCE(sqe->rw_flags));
+	if (unlikely(ret))
+		return ret;
+
+	kiocb->ki_pos = READ_ONCE(sqe->off);
 	/* used for fixed read/write too - just read unconditionally */
 	req->buf_index = READ_ONCE(sqe->buf_index);
 
@@ -91,15 +96,14 @@  int io_prep_rw(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 		if (ret)
 			return ret;
 
-		rw->kiocb.ki_ioprio = ioprio;
+		kiocb->ki_ioprio = ioprio;
 	} else {
-		rw->kiocb.ki_ioprio = get_current_ioprio();
+		kiocb->ki_ioprio = get_current_ioprio();
 	}
-	rw->kiocb.dio_complete = NULL;
+	kiocb->dio_complete = NULL;
 
 	rw->addr = READ_ONCE(sqe->addr);
 	rw->len = READ_ONCE(sqe->len);
-	rw->flags = READ_ONCE(sqe->rw_flags);
 	return 0;
 }
 
@@ -720,7 +724,6 @@  static int io_rw_init_file(struct io_kiocb *req, fmode_t mode)
 	struct kiocb *kiocb = &rw->kiocb;
 	struct io_ring_ctx *ctx = req->ctx;
 	struct file *file = req->file;
-	int ret;
 
 	if (unlikely(!(file->f_mode & mode)))
 		return -EBADF;
@@ -728,10 +731,7 @@  static int io_rw_init_file(struct io_kiocb *req, fmode_t mode)
 	if (!(req->flags & REQ_F_FIXED_FILE))
 		req->flags |= io_file_get_flags(file);
 
-	kiocb->ki_flags = file->f_iocb_flags;
-	ret = kiocb_set_rw_flags(kiocb, rw->flags);
-	if (unlikely(ret))
-		return ret;
+	kiocb->ki_flags |= file->f_iocb_flags;
 	kiocb->ki_flags |= IOCB_ALLOC_CACHE;
 
 	/*