[PATCH 0/5] random io-wq and io_uring bits

[PATCH 0/5] random io-wq and io_uring bits

[Pavel Begunkov]

A bunch of unconnected patches, easy and straightworward.
Probably could even be picked separately.

The only thing could be of concern is [PATCH 4/5]. I assumed that
work setup is short (switch creds, mm, fs, files with task_[un]lock),
and arm a timeout after it's done.

Pavel Begunkov (5):
  io_uring: clean io_poll_complete
  io_uring: extract kmsg copy helper
  io-wq: remove unused IO_WQ_WORK_HAS_MM
  io_uring: remove IO_WQ_WORK_CB
  io-wq: use BIT for ulong hash

 fs/io-wq.c    | 11 +++--------
 fs/io-wq.h    |  2 --
 fs/io_uring.c | 51 +++++++++++++++++++++------------------------------
 3 files changed, 24 insertions(+), 40 deletions(-)

-- 
2.24.0

[PATCH 1/5] io_uring: clean io_poll_complete

[Pavel Begunkov]

Deduplicate call to io_cqring_fill_event(), plain and easy

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 fs/io_uring.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index d88346d4b781..87dfe397de74 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -3636,10 +3636,7 @@ static void io_poll_complete(struct io_kiocb *req, __poll_t mask, int error)
 	struct io_ring_ctx *ctx = req->ctx;
 
 	req->poll.done = true;
-	if (error)
-		io_cqring_fill_event(req, error);
-	else
-		io_cqring_fill_event(req, mangle_poll(mask));
+	io_cqring_fill_event(req, error ? error : mangle_poll(mask));
 	io_commit_cqring(ctx);
 }
 
-- 
2.24.0

[PATCH 2/5] io_uring: extract kmsg copy helper

[Pavel Begunkov]

io_recvmsg() and io_sendmsg() duplicate nonblock -EAGAIN finilising
part, so add helper for that.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 fs/io_uring.c | 43 +++++++++++++++++++------------------------
 1 file changed, 19 insertions(+), 24 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 87dfe397de74..a32a195407ac 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -3099,6 +3099,21 @@ static int io_sync_file_range(struct io_kiocb *req, struct io_kiocb **nxt,
 	return 0;
 }
 
+static int io_setup_async_msg(struct io_kiocb *req,
+			      struct io_async_msghdr *kmsg)
+{
+	if (req->io)
+		return -EAGAIN;
+	if (io_alloc_async_ctx(req)) {
+		if (kmsg->iov != kmsg->fast_iov)
+			kfree(kmsg->iov);
+		return -ENOMEM;
+	}
+	req->flags |= REQ_F_NEED_CLEANUP;
+	memcpy(&req->io->msg, kmsg, sizeof(*kmsg));
+	return -EAGAIN;
+}
+
 static int io_sendmsg_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 {
 #if defined(CONFIG_NET)
@@ -3170,18 +3185,8 @@ static int io_sendmsg(struct io_kiocb *req, struct io_kiocb **nxt,
 			flags |= MSG_DONTWAIT;
 
 		ret = __sys_sendmsg_sock(sock, &kmsg->msg, flags);
-		if (force_nonblock && ret == -EAGAIN) {
-			if (req->io)
-				return -EAGAIN;
-			if (io_alloc_async_ctx(req)) {
-				if (kmsg->iov != kmsg->fast_iov)
-					kfree(kmsg->iov);
-				return -ENOMEM;
-			}
-			req->flags |= REQ_F_NEED_CLEANUP;
-			memcpy(&req->io->msg, &io.msg, sizeof(io.msg));
-			return -EAGAIN;
-		}
+		if (force_nonblock && ret == -EAGAIN)
+			return io_setup_async_msg(req, kmsg);
 		if (ret == -ERESTARTSYS)
 			ret = -EINTR;
 	}
@@ -3324,18 +3329,8 @@ static int io_recvmsg(struct io_kiocb *req, struct io_kiocb **nxt,
 
 		ret = __sys_recvmsg_sock(sock, &kmsg->msg, req->sr_msg.msg,
 						kmsg->uaddr, flags);
-		if (force_nonblock && ret == -EAGAIN) {
-			if (req->io)
-				return -EAGAIN;
-			if (io_alloc_async_ctx(req)) {
-				if (kmsg->iov != kmsg->fast_iov)
-					kfree(kmsg->iov);
-				return -ENOMEM;
-			}
-			memcpy(&req->io->msg, &io.msg, sizeof(io.msg));
-			req->flags |= REQ_F_NEED_CLEANUP;
-			return -EAGAIN;
-		}
+		if (force_nonblock && ret == -EAGAIN)
+			return io_setup_async_msg(req, kmsg);
 		if (ret == -ERESTARTSYS)
 			ret = -EINTR;
 	}
-- 
2.24.0

[PATCH 3/5] io-wq: remove unused IO_WQ_WORK_HAS_MM

[Pavel Begunkov]

IO_WQ_WORK_HAS_MM is set but never used, remove it.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 fs/io-wq.c | 2 --
 fs/io-wq.h | 1 -
 2 files changed, 3 deletions(-)

diff --git a/fs/io-wq.c b/fs/io-wq.c
index bf8ed1b0b90a..72c73c7b7f28 100644
--- a/fs/io-wq.c
+++ b/fs/io-wq.c
@@ -499,8 +499,6 @@ static void io_worker_handle_work(struct io_worker *worker)
 		 */
 		if (test_bit(IO_WQ_BIT_CANCEL, &wq->state))
 			work->flags |= IO_WQ_WORK_CANCEL;
-		if (worker->mm)
-			work->flags |= IO_WQ_WORK_HAS_MM;
 
 		if (wq->get_work && !(work->flags & IO_WQ_WORK_INTERNAL)) {
 			put_work = work;
diff --git a/fs/io-wq.h b/fs/io-wq.h
index 33baba4370c5..72c860f477d2 100644
--- a/fs/io-wq.h
+++ b/fs/io-wq.h
@@ -5,7 +5,6 @@ struct io_wq;
 
 enum {
 	IO_WQ_WORK_CANCEL	= 1,
-	IO_WQ_WORK_HAS_MM	= 2,
 	IO_WQ_WORK_HASHED	= 4,
 	IO_WQ_WORK_UNBOUND	= 32,
 	IO_WQ_WORK_INTERNAL	= 64,
-- 
2.24.0

[PATCH 4/5] io_uring: remove IO_WQ_WORK_CB

[Pavel Begunkov]

IO_WQ_WORK_CB is used only for linked timeouts, which will be armed
before the work setup (i.e. mm, override creds, etc). The setup
shouldn't take long, so it's ok to arm it a bit later and get rid
of IO_WQ_WORK_CB.

Make io-wq call work->func() only once, callbacks will handle the rest.
i.e. the linked timeout handler will do the actual issue. And as a
bonus, it removes an extra indirect call.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 fs/io-wq.c    | 3 ---
 fs/io-wq.h    | 1 -
 fs/io_uring.c | 3 +--
 3 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/fs/io-wq.c b/fs/io-wq.c
index 72c73c7b7f28..1ceb12c58ae6 100644
--- a/fs/io-wq.c
+++ b/fs/io-wq.c
@@ -479,9 +479,6 @@ static void io_worker_handle_work(struct io_worker *worker)
 		worker->cur_work = work;
 		spin_unlock_irq(&worker->lock);
 
-		if (work->flags & IO_WQ_WORK_CB)
-			work->func(&work);
-
 		if (work->files && current->files != work->files) {
 			task_lock(current);
 			current->files = work->files;
diff --git a/fs/io-wq.h b/fs/io-wq.h
index 72c860f477d2..001194aef6ae 100644
--- a/fs/io-wq.h
+++ b/fs/io-wq.h
@@ -8,7 +8,6 @@ enum {
 	IO_WQ_WORK_HASHED	= 4,
 	IO_WQ_WORK_UNBOUND	= 32,
 	IO_WQ_WORK_INTERNAL	= 64,
-	IO_WQ_WORK_CB		= 128,
 	IO_WQ_WORK_NO_CANCEL	= 256,
 	IO_WQ_WORK_CONCURRENT	= 512,
 
diff --git a/fs/io_uring.c b/fs/io_uring.c
index a32a195407ac..f5fbde552be7 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -2554,7 +2554,7 @@ static void io_link_work_cb(struct io_wq_work **workptr)
 	struct io_kiocb *link = work->data;
 
 	io_queue_linked_timeout(link);
-	work->func = io_wq_submit_work;
+	io_wq_submit_work(workptr);
 }
 
 static void io_wq_assign_next(struct io_wq_work **workptr, struct io_kiocb *nxt)
@@ -2564,7 +2564,6 @@ static void io_wq_assign_next(struct io_wq_work **workptr, struct io_kiocb *nxt)
 	io_prep_next_work(nxt, &link);
 	*workptr = &nxt->work;
 	if (link) {
-		nxt->work.flags |= IO_WQ_WORK_CB;
 		nxt->work.func = io_link_work_cb;
 		nxt->work.data = link;
 	}
-- 
2.24.0

[PATCH 5/5] io-wq: use BIT for ulong hash

[Pavel Begunkov]

@hash_map is unsigned long, but BIT_ULL() is used for manipulations.
BIT() is a better match as it returns exactly unsigned long value.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 fs/io-wq.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/io-wq.c b/fs/io-wq.c
index 1ceb12c58ae6..a05c32df2046 100644
--- a/fs/io-wq.c
+++ b/fs/io-wq.c
@@ -393,8 +393,8 @@ static struct io_wq_work *io_get_next_work(struct io_wqe *wqe, unsigned *hash)
 
 		/* hashed, can run if not already running */
 		*hash = work->flags >> IO_WQ_HASH_SHIFT;
-		if (!(wqe->hash_map & BIT_ULL(*hash))) {
-			wqe->hash_map |= BIT_ULL(*hash);
+		if (!(wqe->hash_map & BIT(*hash))) {
+			wqe->hash_map |= BIT(*hash);
 			wq_node_del(&wqe->work_list, node, prev);
 			return work;
 		}
@@ -512,7 +512,7 @@ static void io_worker_handle_work(struct io_worker *worker)
 		spin_lock_irq(&wqe->lock);
 
 		if (hash != -1U) {
-			wqe->hash_map &= ~BIT_ULL(hash);
+			wqe->hash_map &= ~BIT(hash);
 			wqe->flags &= ~IO_WQE_FLAG_STALLED;
 		}
 		if (work && work != old_work) {
-- 
2.24.0

Re: [PATCH 0/5] random io-wq and io_uring bits

[Jens Axboe]

On 2/28/20 12:36 AM, Pavel Begunkov wrote:
> A bunch of unconnected patches, easy and straightworward.
> Probably could even be picked separately.
> 
> The only thing could be of concern is [PATCH 4/5]. I assumed that
> work setup is short (switch creds, mm, fs, files with task_[un]lock),
> and arm a timeout after it's done.

That's totally fine in terms of timing, the reason it was done in a
callback was so we didn't have a small gap where a cancellation
would trigger via the timeout, but the request wasn't locatable yet.

But you retain that, so I think it should be fine.

-- 
Jens Axboe

Re: [PATCH 0/5] random io-wq and io_uring bits

[Jens Axboe]

On 2/28/20 12:36 AM, Pavel Begunkov wrote:
> A bunch of unconnected patches, easy and straightworward.
> Probably could even be picked separately.
> 
> The only thing could be of concern is [PATCH 4/5]. I assumed that
> work setup is short (switch creds, mm, fs, files with task_[un]lock),
> and arm a timeout after it's done.
> 
> Pavel Begunkov (5):
>   io_uring: clean io_poll_complete
>   io_uring: extract kmsg copy helper
>   io-wq: remove unused IO_WQ_WORK_HAS_MM
>   io_uring: remove IO_WQ_WORK_CB
>   io-wq: use BIT for ulong hash
> 
>  fs/io-wq.c    | 11 +++--------
>  fs/io-wq.h    |  2 --
>  fs/io_uring.c | 51 +++++++++++++++++++++------------------------------
>  3 files changed, 24 insertions(+), 40 deletions(-)

LGTM, and always love a negative diffstat. Applied for 5.7.

-- 
Jens Axboe