io-uring.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jens Axboe <axboe@kernel.dk>
To: io-uring@vger.kernel.org
Cc: Jens Axboe <axboe@kernel.dk>
Subject: [PATCH 6/7] io_uring: standardize the prep methods
Date: Fri, 20 Dec 2019 10:47:41 -0700	[thread overview]
Message-ID: <20191220174742.7449-7-axboe@kernel.dk> (raw)
In-Reply-To: <20191220174742.7449-1-axboe@kernel.dk>

We currently have a mix of use cases. Most of the newer ones are pretty
uniform, but we have some older ones that use different calling
calling conventions. This is confusing.

For the opcodes that currently rely on the req->io->sqe copy saving
them from reuse, add a request type struct in the io_kiocb command
union to store the data they need.

Prepare for all opcodes having a standard prep method, so we can call
it in a uniform fashion and outside of the opcode handler. This is in
preparation for passing in the 'sqe' pointer, rather than storing it
in the io_kiocb. Once we have uniform prep handlers, we can leave all
the prep work to that part, and not even pass in the sqe to the opcode
handler. This ensures that we don't reuse sqe data inadvertently.

Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 fs/io_uring.c | 128 +++++++++++++++++++++++++-------------------------
 1 file changed, 63 insertions(+), 65 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 7e8d28750053..2cdfbb451fe2 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -371,7 +371,6 @@ struct io_async_rw {
 };
 
 struct io_async_ctx {
-	struct io_uring_sqe		sqe;
 	union {
 		struct io_async_rw	rw;
 		struct io_async_msghdr	msg;
@@ -433,7 +432,6 @@ struct io_kiocb {
 #define REQ_F_INFLIGHT		16384	/* on inflight list */
 #define REQ_F_COMP_LOCKED	32768	/* completion under lock */
 #define REQ_F_HARDLINK		65536	/* doesn't sever on completion < 0 */
-#define REQ_F_PREPPED		131072	/* request already opcode prepared */
 	u64			user_data;
 	u32			result;
 	u32			sequence;
@@ -1501,6 +1499,8 @@ static int io_prep_rw(struct io_kiocb *req, bool force_nonblock)
 	unsigned ioprio;
 	int ret;
 
+	if (!sqe)
+		return 0;
 	if (!req->file)
 		return -EBADF;
 
@@ -1552,6 +1552,7 @@ static int io_prep_rw(struct io_kiocb *req, bool force_nonblock)
 	/* we own ->private, reuse it for the buffer index */
 	req->rw.kiocb.private = (void *) (unsigned long)
 					READ_ONCE(req->sqe->buf_index);
+	req->sqe = NULL;
 	return 0;
 }
 
@@ -1773,13 +1774,7 @@ static void io_req_map_rw(struct io_kiocb *req, ssize_t io_size,
 static int io_alloc_async_ctx(struct io_kiocb *req)
 {
 	req->io = kmalloc(sizeof(*req->io), GFP_KERNEL);
-	if (req->io) {
-		memcpy(&req->io->sqe, req->sqe, sizeof(req->io->sqe));
-		req->sqe = &req->io->sqe;
-		return 0;
-	}
-
-	return 1;
+	return req->io == NULL;
 }
 
 static void io_rw_async(struct io_wq_work **workptr)
@@ -1810,12 +1805,14 @@ static int io_read_prep(struct io_kiocb *req, struct iovec **iovec,
 {
 	ssize_t ret;
 
-	ret = io_prep_rw(req, force_nonblock);
-	if (ret)
-		return ret;
+	if (req->sqe) {
+		ret = io_prep_rw(req, force_nonblock);
+		if (ret)
+			return ret;
 
-	if (unlikely(!(req->file->f_mode & FMODE_READ)))
-		return -EBADF;
+		if (unlikely(!(req->file->f_mode & FMODE_READ)))
+			return -EBADF;
+	}
 
 	return io_import_iovec(READ, req, iovec, iter);
 }
@@ -1829,15 +1826,9 @@ static int io_read(struct io_kiocb *req, struct io_kiocb **nxt,
 	size_t iov_count;
 	ssize_t io_size, ret;
 
-	if (!req->io) {
-		ret = io_read_prep(req, &iovec, &iter, force_nonblock);
-		if (ret < 0)
-			return ret;
-	} else {
-		ret = io_import_iovec(READ, req, &iovec, &iter);
-		if (ret < 0)
-			return ret;
-	}
+	ret = io_read_prep(req, &iovec, &iter, force_nonblock);
+	if (ret < 0)
+		return ret;
 
 	/* Ensure we clear previously set non-block flag */
 	if (!force_nonblock)
@@ -1901,12 +1892,14 @@ static int io_write_prep(struct io_kiocb *req, struct iovec **iovec,
 {
 	ssize_t ret;
 
-	ret = io_prep_rw(req, force_nonblock);
-	if (ret)
-		return ret;
+	if (req->sqe) {
+		ret = io_prep_rw(req, force_nonblock);
+		if (ret)
+			return ret;
 
-	if (unlikely(!(req->file->f_mode & FMODE_WRITE)))
-		return -EBADF;
+		if (unlikely(!(req->file->f_mode & FMODE_WRITE)))
+			return -EBADF;
+	}
 
 	return io_import_iovec(WRITE, req, iovec, iter);
 }
@@ -1920,15 +1913,9 @@ static int io_write(struct io_kiocb *req, struct io_kiocb **nxt,
 	size_t iov_count;
 	ssize_t ret, io_size;
 
-	if (!req->io) {
-		ret = io_write_prep(req, &iovec, &iter, force_nonblock);
-		if (ret < 0)
-			return ret;
-	} else {
-		ret = io_import_iovec(WRITE, req, &iovec, &iter);
-		if (ret < 0)
-			return ret;
-	}
+	ret = io_write_prep(req, &iovec, &iter, force_nonblock);
+	if (ret < 0)
+		return ret;
 
 	/* Ensure we clear previously set non-block flag */
 	if (!force_nonblock)
@@ -2013,7 +2000,7 @@ static int io_prep_fsync(struct io_kiocb *req)
 	const struct io_uring_sqe *sqe = req->sqe;
 	struct io_ring_ctx *ctx = req->ctx;
 
-	if (req->flags & REQ_F_PREPPED)
+	if (!req->sqe)
 		return 0;
 	if (!req->file)
 		return -EBADF;
@@ -2029,7 +2016,7 @@ static int io_prep_fsync(struct io_kiocb *req)
 
 	req->sync.off = READ_ONCE(sqe->off);
 	req->sync.len = READ_ONCE(sqe->len);
-	req->flags |= REQ_F_PREPPED;
+	req->sqe = NULL;
 	return 0;
 }
 
@@ -2095,7 +2082,7 @@ static int io_prep_sfr(struct io_kiocb *req)
 	const struct io_uring_sqe *sqe = req->sqe;
 	struct io_ring_ctx *ctx = req->ctx;
 
-	if (req->flags & REQ_F_PREPPED)
+	if (!sqe)
 		return 0;
 	if (!req->file)
 		return -EBADF;
@@ -2108,7 +2095,7 @@ static int io_prep_sfr(struct io_kiocb *req)
 	req->sync.off = READ_ONCE(sqe->off);
 	req->sync.len = READ_ONCE(sqe->len);
 	req->sync.flags = READ_ONCE(sqe->sync_range_flags);
-	req->flags |= REQ_F_PREPPED;
+	req->sqe = NULL;
 	return 0;
 }
 
@@ -2173,12 +2160,17 @@ static int io_sendmsg_prep(struct io_kiocb *req, struct io_async_ctx *io)
 #if defined(CONFIG_NET)
 	const struct io_uring_sqe *sqe = req->sqe;
 	struct io_sr_msg *sr = &req->sr_msg;
+	int ret;
 
+	if (!sqe)
+		return 0;
 	sr->msg_flags = READ_ONCE(sqe->msg_flags);
 	sr->msg = u64_to_user_ptr(READ_ONCE(sqe->addr));
 	io->msg.iov = io->msg.fast_iov;
-	return sendmsg_copy_msghdr(&io->msg.msg, sr->msg, sr->msg_flags,
+	ret = sendmsg_copy_msghdr(&io->msg.msg, sr->msg, sr->msg_flags,
 					&io->msg.iov);
+	req->sqe = NULL;
+	return ret;
 #else
 	return -EOPNOTSUPP;
 #endif
@@ -2253,12 +2245,18 @@ static int io_recvmsg_prep(struct io_kiocb *req, struct io_async_ctx *io)
 {
 #if defined(CONFIG_NET)
 	struct io_sr_msg *sr = &req->sr_msg;
+	int ret;
+
+	if (!req->sqe)
+		return 0;
 
 	sr->msg_flags = READ_ONCE(req->sqe->msg_flags);
 	sr->msg = u64_to_user_ptr(READ_ONCE(req->sqe->addr));
 	io->msg.iov = io->msg.fast_iov;
-	return recvmsg_copy_msghdr(&io->msg.msg, sr->msg, sr->msg_flags,
+	ret = recvmsg_copy_msghdr(&io->msg.msg, sr->msg, sr->msg_flags,
 					&io->msg.uaddr, &io->msg.iov);
+	req->sqe = NULL;
+	return ret;
 #else
 	return -EOPNOTSUPP;
 #endif
@@ -2336,7 +2334,7 @@ static int io_accept_prep(struct io_kiocb *req)
 	const struct io_uring_sqe *sqe = req->sqe;
 	struct io_accept *accept = &req->accept;
 
-	if (req->flags & REQ_F_PREPPED)
+	if (!req->sqe)
 		return 0;
 
 	if (unlikely(req->ctx->flags & (IORING_SETUP_IOPOLL|IORING_SETUP_SQPOLL)))
@@ -2347,7 +2345,7 @@ static int io_accept_prep(struct io_kiocb *req)
 	accept->addr = u64_to_user_ptr(READ_ONCE(sqe->addr));
 	accept->addr_len = u64_to_user_ptr(READ_ONCE(sqe->addr2));
 	accept->flags = READ_ONCE(sqe->accept_flags);
-	req->flags |= REQ_F_PREPPED;
+	req->sqe = NULL;
 	return 0;
 #else
 	return -EOPNOTSUPP;
@@ -2416,7 +2414,10 @@ static int io_connect_prep(struct io_kiocb *req, struct io_async_ctx *io)
 {
 #if defined(CONFIG_NET)
 	const struct io_uring_sqe *sqe = req->sqe;
+	int ret;
 
+	if (!sqe)
+		return 0;
 	if (unlikely(req->ctx->flags & (IORING_SETUP_IOPOLL|IORING_SETUP_SQPOLL)))
 		return -EINVAL;
 	if (sqe->ioprio || sqe->len || sqe->buf_index || sqe->rw_flags)
@@ -2424,8 +2425,10 @@ static int io_connect_prep(struct io_kiocb *req, struct io_async_ctx *io)
 
 	req->connect.addr = u64_to_user_ptr(READ_ONCE(sqe->addr));
 	req->connect.addr_len =  READ_ONCE(sqe->addr2);
-	return move_addr_to_kernel(req->connect.addr, req->connect.addr_len,
+	ret = move_addr_to_kernel(req->connect.addr, req->connect.addr_len,
 					&io->connect.address);
+	req->sqe = NULL;
+	return ret;
 #else
 	return -EOPNOTSUPP;
 #endif
@@ -2526,7 +2529,7 @@ static int io_poll_remove_prep(struct io_kiocb *req)
 {
 	const struct io_uring_sqe *sqe = req->sqe;
 
-	if (req->flags & REQ_F_PREPPED)
+	if (!sqe)
 		return 0;
 	if (unlikely(req->ctx->flags & IORING_SETUP_IOPOLL))
 		return -EINVAL;
@@ -2535,7 +2538,7 @@ static int io_poll_remove_prep(struct io_kiocb *req)
 		return -EINVAL;
 
 	req->poll.addr = READ_ONCE(sqe->addr);
-	req->flags |= REQ_F_PREPPED;
+	req->sqe = NULL;
 	return 0;
 }
 
@@ -2696,7 +2699,7 @@ static int io_poll_add_prep(struct io_kiocb *req)
 	struct io_poll_iocb *poll = &req->poll;
 	u16 events;
 
-	if (req->flags & REQ_F_PREPPED)
+	if (!sqe)
 		return 0;
 	if (unlikely(req->ctx->flags & IORING_SETUP_IOPOLL))
 		return -EINVAL;
@@ -2705,9 +2708,9 @@ static int io_poll_add_prep(struct io_kiocb *req)
 	if (!poll->file)
 		return -EBADF;
 
-	req->flags |= REQ_F_PREPPED;
 	events = READ_ONCE(sqe->poll_events);
 	poll->events = demangle_poll(events) | EPOLLERR | EPOLLHUP;
+	req->sqe = NULL;
 	return 0;
 }
 
@@ -2845,7 +2848,7 @@ static int io_timeout_remove_prep(struct io_kiocb *req)
 {
 	const struct io_uring_sqe *sqe = req->sqe;
 
-	if (req->flags & REQ_F_PREPPED)
+	if (!sqe)
 		return 0;
 	if (unlikely(req->ctx->flags & IORING_SETUP_IOPOLL))
 		return -EINVAL;
@@ -2857,7 +2860,7 @@ static int io_timeout_remove_prep(struct io_kiocb *req)
 	if (req->timeout.flags)
 		return -EINVAL;
 
-	req->flags |= REQ_F_PREPPED;
+	req->sqe = NULL;
 	return 0;
 }
 
@@ -2893,6 +2896,8 @@ static int io_timeout_prep(struct io_kiocb *req, struct io_async_ctx *io,
 	struct io_timeout_data *data;
 	unsigned flags;
 
+	if (!sqe)
+		return 0;
 	if (unlikely(req->ctx->flags & IORING_SETUP_IOPOLL))
 		return -EINVAL;
 	if (sqe->ioprio || sqe->buf_index || sqe->len != 1)
@@ -2921,6 +2926,7 @@ static int io_timeout_prep(struct io_kiocb *req, struct io_async_ctx *io,
 		data->mode = HRTIMER_MODE_REL;
 
 	hrtimer_init(&data->timer, CLOCK_MONOTONIC, data->mode);
+	req->sqe = NULL;
 	return 0;
 }
 
@@ -2933,13 +2939,9 @@ static int io_timeout(struct io_kiocb *req)
 	unsigned span = 0;
 	int ret;
 
-	if (!req->io) {
-		if (io_alloc_async_ctx(req))
-			return -ENOMEM;
-		ret = io_timeout_prep(req, req->io, false);
-		if (ret)
-			return ret;
-	}
+	ret = io_timeout_prep(req, req->io, false);
+	if (ret)
+		return ret;
 	data = &req->io->timeout;
 
 	/*
@@ -3069,7 +3071,7 @@ static int io_async_cancel_prep(struct io_kiocb *req)
 {
 	const struct io_uring_sqe *sqe = req->sqe;
 
-	if (req->flags & REQ_F_PREPPED)
+	if (!sqe)
 		return 0;
 	if (unlikely(req->ctx->flags & IORING_SETUP_IOPOLL))
 		return -EINVAL;
@@ -3077,8 +3079,8 @@ static int io_async_cancel_prep(struct io_kiocb *req)
 	    sqe->cancel_flags)
 		return -EINVAL;
 
-	req->flags |= REQ_F_PREPPED;
 	req->cancel.addr = READ_ONCE(sqe->addr);
+	req->sqe = NULL;
 	return 0;
 }
 
@@ -3213,13 +3215,9 @@ static int io_issue_sqe(struct io_kiocb *req, struct io_kiocb **nxt,
 		ret = io_nop(req);
 		break;
 	case IORING_OP_READV:
-		if (unlikely(req->sqe->buf_index))
-			return -EINVAL;
 		ret = io_read(req, nxt, force_nonblock);
 		break;
 	case IORING_OP_WRITEV:
-		if (unlikely(req->sqe->buf_index))
-			return -EINVAL;
 		ret = io_write(req, nxt, force_nonblock);
 		break;
 	case IORING_OP_READ_FIXED:
-- 
2.24.1


  parent reply	other threads:[~2019-12-20 17:47 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-20 17:47 [PATCHSET] Cleanup io_uring sqe handling Jens Axboe
2019-12-20 17:47 ` [PATCH 1/7] io_uring: use u64_to_user_ptr() consistently Jens Axboe
2019-12-20 17:47 ` [PATCH 2/7] io_uring: add and use struct io_rw for read/writes Jens Axboe
2019-12-20 17:47 ` [PATCH 3/7] io_uring: move all prep state for IORING_OP_CONNECT to prep handler Jens Axboe
2019-12-20 17:47 ` [PATCH 4/7] io_uring: move all prep state for IORING_OP_{SEND,RECV}_MGS " Jens Axboe
2019-12-20 17:47 ` [PATCH 5/7] io_uring: read 'count' for IORING_OP_TIMEOUT in " Jens Axboe
2019-12-20 17:47 ` Jens Axboe [this message]
2019-12-20 17:47 ` [PATCH 7/7] io_uring: pass in 'sqe' to the prep handlers Jens Axboe
2019-12-24  4:04 ` [PATCHSET] Cleanup io_uring sqe handling Jens Axboe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191220174742.7449-7-axboe@kernel.dk \
    --to=axboe@kernel.dk \
    --cc=io-uring@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).