Skip to contentSkip to conversation

io-wq sharing

PATCH Superseded Pavel Begunkov 2 participants 12 messages Jan 28, 2020

Subsystems (2)
VFS io_uring
lore
Share & tools
lore

Pavel Begunkov

author
#0 lore raw

cover io-wq sharing

rip-off of Jens io-wq sharing patches allowing multiple io_uring instances to be bound to a single io-wq. The differences are:
- io-wq, which we would like to be shared, is passed as io_uring fd
- fail, if can't share. IMHO, it's always better to fail fast and loud

I didn't tested it after rebasing, but hopefully won't be a problem.

p.s. on top of ("io_uring/io-wq: don't use static creds/mm assignments")

Pavel Begunkov (2):
io-wq: allow grabbing existing io-wq
io_uring: add io-wq workqueue sharing

 fs/io-wq.c                    | 10 ++++++
 fs/io-wq.h                    |  1 +
 fs/io_uring.c                 | 67 +++++++++++++++++++++++++++--------
 include/uapi/linux/io_uring.h |  4 ++-
 4 files changed, 67 insertions(+), 15 deletions(-)
Signature · 1 line
2.24.0
author
#2 lore raw

PATCH 2/2 io_uring: add io-wq workqueue sharing

If IORING_SETUP_ATTACH_WQ is set, it expects wq_fd in io_uring_params to be a valid io_uring fd io-wq of which will be shared with the newly created io_uring instance. If the flag is set but it can't share io-wq, it fails.

This allows creation of "sibling" io_urings, where we prefer to keep the SQ/CQ private, but want to share the async backend to minimize the amount of overhead associated with having multiple rings that belong to the same backend.

Reported-by Jens Axboe <[email protected]>
Reported-by Daurnimator <[email protected]>
Signed-off-by Pavel Begunkov <[email protected]>
---
 fs/io_uring.c                 | 67 +++++++++++++++++++++++++++--------
 include/uapi/linux/io_uring.h |  4 ++-
 2 files changed, 56 insertions(+), 15 deletions(-)
fs/io_uring.c +53 -14
index 5ec6428933c3..b9fd6efe9d90 100644
@@ -5692,11 +5692,55 @@ static void io_get_work(struct io_wq_work *work)
 	refcount_inc(&req->refs);
 }
 
+static int io_init_wq_offload(struct io_ring_ctx *ctx,
+			      struct io_uring_params *p)
+{
+	struct io_wq_data data;
+	struct fd f;
+	struct io_ring_ctx *ctx_attach;
+	unsigned int concurrency;
+	int ret = 0;
+
+	data.user = ctx->user;
+	data.get_work = io_get_work;
+	data.put_work = io_put_work;
+
+	if (!(p->flags & IORING_SETUP_ATTACH_WQ)) {
+		/* Do QD, or 4 * CPUS, whatever is smallest */
+		concurrency = min(ctx->sq_entries, 4 * num_online_cpus());
+
+		ctx->io_wq = io_wq_create(concurrency, &data);
+		if (IS_ERR(ctx->io_wq)) {
+			ret = PTR_ERR(ctx->io_wq);
+			ctx->io_wq = NULL;
+		}
+		return ret;
+	}
+
+	f = fdget(p->wq_fd);
+	if (!f.file)
+		return -EBADF;
+
+	if (f.file->f_op != &io_uring_fops) {
+		ret = -EOPNOTSUPP;
+		goto out_fput;
+	}
+
+	ctx_attach = f.file->private_data; /* @io_wq is protected the fd */
+	if (!io_wq_get(ctx_attach->io_wq, &data)) {
+		ret = -EINVAL;
+		goto out_fput;
+	}
+
+	ctx->io_wq = ctx_attach->io_wq;
+out_fput:
+	fdput(f);
+	return ret;
+}
+
 static int io_sq_offload_start(struct io_ring_ctx *ctx,
 			       struct io_uring_params *p)
 {
-	struct io_wq_data data;
-	unsigned concurrency;
 	int ret;
 
 	init_waitqueue_head(&ctx->sqo_wait);

@@ -5740,18 +5784,9 @@ static int io_sq_offload_start(struct io_ring_ctx *ctx,
 		goto err;
 	}
 
-	data.user = ctx->user;
-	data.get_work = io_get_work;
-	data.put_work = io_put_work;
-
-	/* Do QD, or 4 * CPUS, whatever is smallest */
-	concurrency = min(ctx->sq_entries, 4 * num_online_cpus());
-	ctx->io_wq = io_wq_create(concurrency, &data);
-	if (IS_ERR(ctx->io_wq)) {
-		ret = PTR_ERR(ctx->io_wq);
-		ctx->io_wq = NULL;
+	ret = io_init_wq_offload(ctx, p);
+	if (ret)
 		goto err;
-	}
 
 	return 0;
 err:

@@ -6577,7 +6612,11 @@ static long io_uring_setup(u32 entries, struct io_uring_params __user *params)
 
 	if (p.flags & ~(IORING_SETUP_IOPOLL | IORING_SETUP_SQPOLL |
 			IORING_SETUP_SQ_AFF | IORING_SETUP_CQSIZE |
-			IORING_SETUP_CLAMP))
+			IORING_SETUP_CLAMP | IORING_SETUP_ATTACH_WQ))
+		return -EINVAL;
+
+	/* id isn't valid without ATTACH_WQ being set */
+	if (!(p.flags & IORING_SETUP_ATTACH_WQ) && p.wq_fd)
 		return -EINVAL;
 
 	ret = io_uring_create(entries, &p);
include/uapi/linux/io_uring.h +3 -1
index 9988e82f858b..e067b92af5ad 100644
@@ -75,6 +75,7 @@ enum {
 #define IORING_SETUP_SQ_AFF	(1U << 2)	/* sq_thread_cpu is valid */
 #define IORING_SETUP_CQSIZE	(1U << 3)	/* app defines CQ size */
 #define IORING_SETUP_CLAMP	(1U << 4)	/* clamp SQ/CQ ring sizes */
+#define IORING_SETUP_ATTACH_WQ	(1U << 5)	/* attach to existing wq */
 
 enum {
 	IORING_OP_NOP,

@@ -183,7 +184,8 @@ struct io_uring_params {
 	__u32 sq_thread_cpu;
 	__u32 sq_thread_idle;
 	__u32 features;
-	__u32 resv[4];
+	__u32 wq_fd;
+	__u32 resv[3];
 	struct io_sqring_offsets sq_off;
 	struct io_cqring_offsets cq_off;
 };
Signature · 1 line
2.24.0
author
#1 lore raw

PATCH 1/2 io-wq: allow grabbing existing io-wq

In reply to Pavel Begunkov

If the id and user/creds match, return an existing io_wq if we can safely grab a reference to it.

Reported-by Jens Axboe <[email protected]>
Signed-off-by Pavel Begunkov <[email protected]>
---
 fs/io-wq.c | 10 ++++++++++
 fs/io-wq.h |  1 +
 2 files changed, 11 insertions(+)
fs/io-wq.c +10 -0
index b45d585cdcc8..36d05503b982 100644
@@ -1110,6 +1110,16 @@ struct io_wq *io_wq_create(unsigned bounded, struct io_wq_data *data)
 	return ERR_PTR(ret);
 }
 
+bool io_wq_get(struct io_wq *wq, struct io_wq_data *data)
+{
+	if (data->creds != wq->creds || data->user != wq->user)
+		return false;
+	if (data->get_work != wq->get_work || data->put_work != wq->put_work)
+		return false;
+
+	return refcount_inc_not_zero(&wq->use_refs);
+}
+
 static bool io_wq_worker_wake(struct io_worker *worker, void *data)
 {
 	wake_up_process(worker->task);
fs/io-wq.h +1 -0
index 167316ad447e..c42602c58c56 100644
@@ -99,6 +99,7 @@ struct io_wq_data {
 };
 
 struct io_wq *io_wq_create(unsigned bounded, struct io_wq_data *data);
+bool io_wq_get(struct io_wq *wq, struct io_wq_data *data);
 void io_wq_destroy(struct io_wq *wq);
 
 void io_wq_enqueue(struct io_wq *wq, struct io_wq_work *work);
Signature · 1 line
2.24.0
author
#3 lore raw
In reply to Pavel Begunkov
Quoted 7 lines · Pavel Begunkov

rip-off of Jens io-wq sharing patches allowing multiple io_uring instances to be bound to a single io-wq. The differences are:
- io-wq, which we would like to be shared, is passed as io_uring fd
- fail, if can't share. IMHO, it's always better to fail fast and loud

I didn't tested it after rebasing, but hopefully won't be a problem.

Ahh, wrong version. Sorry for that

Quoted 12 lines

p.s. on top of ("io_uring/io-wq: don't use static creds/mm assignments")

Pavel Begunkov (2):
io-wq: allow grabbing existing io-wq
io_uring: add io-wq workqueue sharing

 fs/io-wq.c                    | 10 ++++++
 fs/io-wq.h                    |  1 +
 fs/io_uring.c                 | 67 +++++++++++++++++++++++++++--------
 include/uapi/linux/io_uring.h |  4 ++-
 4 files changed, 67 insertions(+), 15 deletions(-)
Signature · 1 line
Pavel Begunkov
author
#4 lore raw

v2 cover io-wq sharing

In reply to Pavel Begunkov

rip-off of Jens io-wq sharing patches allowing multiple io_uring instances to be bound to a single io-wq. The differences are:
- io-wq, which we would like to be shared, is passed as io_uring fd
- fail, if can't share. IMHO, it's always better to fail fast and loud

I didn't tested it after rebasing, but hopefully won't be a problem.

p.s. on top of ("io_uring/io-wq: don't use static creds/mm assignments")

v2: rebased version

Pavel Begunkov (2):
io-wq: allow grabbing existing io-wq
io_uring: add io-wq workqueue sharing

 fs/io-wq.c                    |  8 +++++
 fs/io-wq.h                    |  1 +
 fs/io_uring.c                 | 68 +++++++++++++++++++++++++++--------
 include/uapi/linux/io_uring.h |  4 ++-
 4 files changed, 66 insertions(+), 15 deletions(-)
Signature · 1 line
2.24.0
author
#5 lore raw

PATCH v2 1/2 io-wq: allow grabbing existing io-wq

If the id and user/creds match, return an existing io_wq if we can safely grab a reference to it.

Reported-by Jens Axboe <[email protected]>
Signed-off-by Pavel Begunkov <[email protected]>
---
 fs/io-wq.c | 8 ++++++++
 fs/io-wq.h | 1 +
 2 files changed, 9 insertions(+)
fs/io-wq.c +8 -0
index b45d585cdcc8..ee49e8852d39 100644
@@ -1110,6 +1110,14 @@ struct io_wq *io_wq_create(unsigned bounded, struct io_wq_data *data)
 	return ERR_PTR(ret);
 }
 
+bool io_wq_get(struct io_wq *wq, struct io_wq_data *data)
+{
+	if (data->get_work != wq->get_work || data->put_work != wq->put_work)
+		return false;
+
+	return refcount_inc_not_zero(&wq->use_refs);
+}
+
 static bool io_wq_worker_wake(struct io_worker *worker, void *data)
 {
 	wake_up_process(worker->task);
fs/io-wq.h +1 -0
index 167316ad447e..c42602c58c56 100644
@@ -99,6 +99,7 @@ struct io_wq_data {
 };
 
 struct io_wq *io_wq_create(unsigned bounded, struct io_wq_data *data);
+bool io_wq_get(struct io_wq *wq, struct io_wq_data *data);
 void io_wq_destroy(struct io_wq *wq);
 
 void io_wq_enqueue(struct io_wq *wq, struct io_wq_work *work);
Signature · 1 line
2.24.0
author
#8 lore raw

PATCH v2 2/2 io_uring: add io-wq workqueue sharing

In reply to Pavel Begunkov

If IORING_SETUP_ATTACH_WQ is set, it expects wq_fd in io_uring_params to be a valid io_uring fd io-wq of which will be shared with the newly created io_uring instance. If the flag is set but it can't share io-wq, it fails.

This allows creation of "sibling" io_urings, where we prefer to keep the SQ/CQ private, but want to share the async backend to minimize the amount of overhead associated with having multiple rings that belong to the same backend.

Reported-by Jens Axboe <[email protected]>
Reported-by Daurnimator <[email protected]>
Signed-off-by Pavel Begunkov <[email protected]>
---
 fs/io_uring.c                 | 68 +++++++++++++++++++++++++++--------
 include/uapi/linux/io_uring.h |  4 ++-
 2 files changed, 57 insertions(+), 15 deletions(-)
fs/io_uring.c +54 -14
index 5ec6428933c3..de1cb3135721 100644
@@ -5692,11 +5692,56 @@ static void io_get_work(struct io_wq_work *work)
 	refcount_inc(&req->refs);
 }
 
+static int io_init_wq_offload(struct io_ring_ctx *ctx,
+			      struct io_uring_params *p)
+{
+	struct io_wq_data data;
+	struct fd f;
+	struct io_ring_ctx *ctx_attach;
+	unsigned int concurrency;
+	int ret = 0;
+
+	data.user = ctx->user;
+	data.get_work = io_get_work;
+	data.put_work = io_put_work;
+
+	if (!(p->flags & IORING_SETUP_ATTACH_WQ)) {
+		/* Do QD, or 4 * CPUS, whatever is smallest */
+		concurrency = min(ctx->sq_entries, 4 * num_online_cpus());
+
+		ctx->io_wq = io_wq_create(concurrency, &data);
+		if (IS_ERR(ctx->io_wq)) {
+			ret = PTR_ERR(ctx->io_wq);
+			ctx->io_wq = NULL;
+		}
+		return ret;
+	}
+
+	f = fdget(p->wq_fd);
+	if (!f.file)
+		return -EBADF;
+
+	if (f.file->f_op != &io_uring_fops) {
+		ret = -EOPNOTSUPP;
+		goto out_fput;
+	}
+
+	ctx_attach = f.file->private_data;
+	/* @io_wq is protected by holding the fd */
+	if (!io_wq_get(ctx_attach->io_wq, &data)) {
+		ret = -EINVAL;
+		goto out_fput;
+	}
+
+	ctx->io_wq = ctx_attach->io_wq;
+out_fput:
+	fdput(f);
+	return ret;
+}
+
 static int io_sq_offload_start(struct io_ring_ctx *ctx,
 			       struct io_uring_params *p)
 {
-	struct io_wq_data data;
-	unsigned concurrency;
 	int ret;
 
 	init_waitqueue_head(&ctx->sqo_wait);

@@ -5740,18 +5785,9 @@ static int io_sq_offload_start(struct io_ring_ctx *ctx,
 		goto err;
 	}
 
-	data.user = ctx->user;
-	data.get_work = io_get_work;
-	data.put_work = io_put_work;
-
-	/* Do QD, or 4 * CPUS, whatever is smallest */
-	concurrency = min(ctx->sq_entries, 4 * num_online_cpus());
-	ctx->io_wq = io_wq_create(concurrency, &data);
-	if (IS_ERR(ctx->io_wq)) {
-		ret = PTR_ERR(ctx->io_wq);
-		ctx->io_wq = NULL;
+	ret = io_init_wq_offload(ctx, p);
+	if (ret)
 		goto err;
-	}
 
 	return 0;
 err:

@@ -6577,7 +6613,11 @@ static long io_uring_setup(u32 entries, struct io_uring_params __user *params)
 
 	if (p.flags & ~(IORING_SETUP_IOPOLL | IORING_SETUP_SQPOLL |
 			IORING_SETUP_SQ_AFF | IORING_SETUP_CQSIZE |
-			IORING_SETUP_CLAMP))
+			IORING_SETUP_CLAMP | IORING_SETUP_ATTACH_WQ))
+		return -EINVAL;
+
+	/* wq_fd isn't valid without ATTACH_WQ being set */
+	if (!(p.flags & IORING_SETUP_ATTACH_WQ) && p.wq_fd)
 		return -EINVAL;
 
 	ret = io_uring_create(entries, &p);
include/uapi/linux/io_uring.h +3 -1
index 9988e82f858b..e067b92af5ad 100644
@@ -75,6 +75,7 @@ enum {
 #define IORING_SETUP_SQ_AFF	(1U << 2)	/* sq_thread_cpu is valid */
 #define IORING_SETUP_CQSIZE	(1U << 3)	/* app defines CQ size */
 #define IORING_SETUP_CLAMP	(1U << 4)	/* clamp SQ/CQ ring sizes */
+#define IORING_SETUP_ATTACH_WQ	(1U << 5)	/* attach to existing wq */
 
 enum {
 	IORING_OP_NOP,

@@ -183,7 +184,8 @@ struct io_uring_params {
 	__u32 sq_thread_cpu;
 	__u32 sq_thread_idle;
 	__u32 features;
-	__u32 resv[4];
+	__u32 wq_fd;
+	__u32 resv[3];
 	struct io_sqring_offsets sq_off;
 	struct io_cqring_offsets cq_off;
 };
Signature · 1 line
2.24.0
author
#6 lore raw

Re: [PATCH v2 1/2] io-wq: allow grabbing existing io-wq

In reply to Pavel Begunkov
Quoted 2 lines · Pavel Begunkov

If the id and user/creds match, return an existing io_wq if we can safely grab a reference to it.

Missed the outdated comment. Apparently, it's too late for continue with it today.

Quoted 40 lines
Reported-by Jens Axboe <[email protected]>
Signed-off-by Pavel Begunkov <[email protected]>
---
 fs/io-wq.c | 8 ++++++++
 fs/io-wq.h | 1 +
 2 files changed, 9 insertions(+)
fs/io-wq.c +8 -0
index b45d585cdcc8..ee49e8852d39 100644
@@ -1110,6 +1110,14 @@ struct io_wq *io_wq_create(unsigned bounded, struct io_wq_data *data)
 	return ERR_PTR(ret);
 }
 
+bool io_wq_get(struct io_wq *wq, struct io_wq_data *data)
+{
+	if (data->get_work != wq->get_work || data->put_work != wq->put_work)
+		return false;
+
+	return refcount_inc_not_zero(&wq->use_refs);
+}
+
 static bool io_wq_worker_wake(struct io_worker *worker, void *data)
 {
 	wake_up_process(worker->task);
fs/io-wq.h +1 -0
index 167316ad447e..c42602c58c56 100644
@@ -99,6 +99,7 @@ struct io_wq_data {
 };
 
 struct io_wq *io_wq_create(unsigned bounded, struct io_wq_data *data);
+bool io_wq_get(struct io_wq *wq, struct io_wq_data *data);
 void io_wq_destroy(struct io_wq *wq);
 
 void io_wq_enqueue(struct io_wq *wq, struct io_wq_work *work);
Signature · 1 line
Pavel Begunkov
maintainer
#7 lore raw

Re: [PATCH v2 1/2] io-wq: allow grabbing existing io-wq

Quoted 5 lines · Pavel Begunkov
Quoted 2 lines · Pavel Begunkov

If the id and user/creds match, return an existing io_wq if we can safely grab a reference to it.

Missed the outdated comment. Apparently, it's too late for continue with it today.

I'll fix it up, testing it now.

Signature · 1 line
Jens Axboe
maintainer
#9 lore raw

Re: [PATCH v2 2/2] io_uring: add io-wq workqueue sharing

In reply to Pavel Begunkov

fs/io_uring.c static long io_uring_setup(u32 entries, struct io_uring_params __user *params) · line 6621

Quoted 11 lines · Pavel Begunkov
fs/io_uring.c +5 -1
@@ -6577,7 +6613,11 @@ static long io_uring_setup(u32 entries, struct io_uring_params __user *params)
 
 	if (p.flags & ~(IORING_SETUP_IOPOLL | IORING_SETUP_SQPOLL |
 			IORING_SETUP_SQ_AFF | IORING_SETUP_CQSIZE |
-			IORING_SETUP_CLAMP))
+			IORING_SETUP_CLAMP | IORING_SETUP_ATTACH_WQ))
+		return -EINVAL;
+
+	/* wq_fd isn't valid without ATTACH_WQ being set */
+	if (!(p.flags & IORING_SETUP_ATTACH_WQ) && p.wq_fd)
 		return -EINVAL;

Since we're now using file descriptors, this no longer works. Any values (outside of -1) is fair game.

Signature · 1 line
Jens Axboe
maintainer
#10 lore raw
In reply to Pavel Begunkov
Quoted 8 lines · Pavel Begunkov

rip-off of Jens io-wq sharing patches allowing multiple io_uring instances to be bound to a single io-wq. The differences are:
- io-wq, which we would like to be shared, is passed as io_uring fd
- fail, if can't share. IMHO, it's always better to fail fast and loud

I didn't tested it after rebasing, but hopefully won't be a problem.

p.s. on top of ("io_uring/io-wq: don't use static creds/mm assignments")

Applied with the following changes:

- Return -EINVAL for invalid ringfd when attach is specified
- Remove the wq_fd check for attach not specified

Tested here, works for me. Pushing out the updated test case.

Signature · 1 line
Jens Axboe
author
#11 lore raw
Quoted 6 lines · Jens Axboe
Quoted 5 lines · Pavel Begunkov

rip-off of Jens io-wq sharing patches allowing multiple io_uring instances to be bound to a single io-wq. The differences are:
- io-wq, which we would like to be shared, is passed as io_uring fd
- fail, if can't share. IMHO, it's always better to fail fast and loud

Quoted 8 lines
Quoted 3 lines

I didn't tested it after rebasing, but hopefully won't be a problem.

p.s. on top of ("io_uring/io-wq: don't use static creds/mm assignments")

Applied with the following changes:

- Return -EINVAL for invalid ringfd when attach is specified
- Remove the wq_fd check for attach not specified

The check was there to be able to reuse the wq_fd field, if the flag isn't specified.

Quoted 3 lines

Tested here, works for me. Pushing out the updated test case.

Thanks!

Signature · 1 line
Pavel Begunkov