Skip to contentSkip to conversation

Add support for shared io-wq backends

PATCH In review Jens Axboe 2 participants 7 messages Jan 24, 2020 – Jan 25, 2020

Subsystems (2)
lore
Share & tools
lore

Jens Axboe

author maintainer
#0 lore raw

v2 cover Add support for shared io-wq backends

Here's v2 of the shared wq backend patchset. A few minor changes in terms of id validation, and a switch to using idr and also utilizing it for lookups.

Changes since v1:

- Use idr instead of ida, and use it for lookups too
- Rename IORING_SETUP_SHARED to IORING_SETUP_ATTACH_WQ
- Don't allow id specified without IORING_SETUP_ATTACH_WQ
- Use id=0 if we run out of idr space instead of failing
- id==0 is not allowed for attach

Signature · 1 line
Jens Axboe
author maintainer
#1 lore raw

PATCH v2 1/4 io-wq: make the io_wq ref counted

In preparation for sharing an io-wq across different users, add a reference count that manages destruction of it.

---
 fs/io-wq.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
fs/io-wq.c +10 -1
index 4d902c19ee5f..54e270ae12ab 100644
@@ -113,6 +113,8 @@ struct io_wq {
 	struct mm_struct *mm;
 	refcount_t refs;
 	struct completion done;
+
+	refcount_t use_refs;
 };
 
 static bool io_worker_get(struct io_worker *worker)
@@ -1073,6 +1075,7 @@ struct io_wq *io_wq_create(unsigned bounded, struct io_wq_data *data)
 			ret = -ENOMEM;
 			goto err;
 		}
+		refcount_set(&wq->use_refs, 1);
 		reinit_completion(&wq->done);
 		return wq;
 	}
@@ -1093,7 +1096,7 @@ static bool io_wq_worker_wake(struct io_worker *worker, void *data)
 	return false;
 }
 
-void io_wq_destroy(struct io_wq *wq)
+static void __io_wq_destroy(struct io_wq *wq)
 {
 	int node;
 
@@ -1113,3 +1116,9 @@ void io_wq_destroy(struct io_wq *wq)
 	kfree(wq->wqes);
 	kfree(wq);
 }
+
+void io_wq_destroy(struct io_wq *wq)
+{
+	if (refcount_dec_and_test(&wq->use_refs))
+		__io_wq_destroy(wq);
+}
Signature · 1 line
2.25.0
author maintainer
#2 lore raw

PATCH v2 2/4 io-wq: add 'id' to io_wq

In reply to Jens Axboe

Add each io_wq to a global list, and assign an id for each of them. This is in preparation for attaching to an existing io_wq, rather than creating a new one.

---
 fs/io-wq.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)
fs/io-wq.c +24 -1
index 54e270ae12ab..51b3677d6fbd 100644
@@ -16,9 +16,14 @@
 #include <linux/slab.h>
 #include <linux/kthread.h>
 #include <linux/rculist_nulls.h>
+#include <linux/idr.h>
 
 #include "io-wq.h"
 
+static LIST_HEAD(wq_list);
+static DEFINE_MUTEX(wq_lock);
+static DEFINE_IDR(wq_idr);
+
 #define WORKER_IDLE_TIMEOUT	(5 * HZ)
 
 enum {
@@ -115,6 +120,8 @@ struct io_wq {
 	struct completion done;
 
 	refcount_t use_refs;
+	struct list_head wq_list;
+	unsigned int id;
 };
 
 static bool io_worker_get(struct io_worker *worker)
@@ -1076,6 +1083,15 @@ struct io_wq *io_wq_create(unsigned bounded, struct io_wq_data *data)
 			goto err;
 		}
 		refcount_set(&wq->use_refs, 1);
+
+		/* if we're out of IDs or fail to get one, use 0 */
+		mutex_lock(&wq_lock);
+		wq->id = idr_alloc(&wq_idr, wq, 1, INT_MAX, GFP_KERNEL);
+		if (wq->id < 0)
+			wq->id = 0;
+
+		list_add(&wq->wq_list, &wq_list);
+		mutex_unlock(&wq_lock);
 		reinit_completion(&wq->done);
 		return wq;
 	}
@@ -1119,6 +1135,13 @@ static void __io_wq_destroy(struct io_wq *wq)
 
 void io_wq_destroy(struct io_wq *wq)
 {
-	if (refcount_dec_and_test(&wq->use_refs))
+	if (refcount_dec_and_test(&wq->use_refs)) {
+		mutex_lock(&wq_lock);
+		if (wq->id)
+			idr_remove(&wq_idr, wq->id);
+		list_del(&wq->wq_list);
+		mutex_unlock(&wq_lock);
+
 		__io_wq_destroy(wq);
+	}
 }
Signature · 1 line
2.25.0
author maintainer
#3 lore raw

PATCH v2 3/4 io-wq: allow lookup of existing io_wq with given id

In reply to Jens Axboe

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

---
 fs/io-wq.c | 37 ++++++++++++++++++++++++++++++++++++-
 fs/io-wq.h | 11 ++++++++++-
 2 files changed, 46 insertions(+), 2 deletions(-)
fs/io-wq.c +36 -1
index 51b3677d6fbd..08ef69b498f6 100644
@@ -1024,7 +1024,7 @@ void io_wq_flush(struct io_wq *wq)
 	}
 }
 
-struct io_wq *io_wq_create(unsigned bounded, struct io_wq_data *data)
+static struct io_wq *__io_wq_create(unsigned bounded, struct io_wq_data *data)
 {
 	int ret = -ENOMEM, node;
 	struct io_wq *wq;
@@ -1106,6 +1106,36 @@ struct io_wq *io_wq_create(unsigned bounded, struct io_wq_data *data)
 	return ERR_PTR(ret);
 }
 
+static bool io_wq_match(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);
+}
+
+/*
+ * Find and return io_wq with given id and grab a reference to it.
+ */
+struct io_wq *io_wq_create_id(unsigned bounded, struct io_wq_data *data,
+			      unsigned int id)
+{
+	if (id) {
+		struct io_wq *wq;
+
+		mutex_lock(&wq_lock);
+		wq = idr_find(&wq_idr, id);
+		if (wq && io_wq_match(wq, data)) {
+			mutex_unlock(&wq_lock);
+			return wq;
+		}
+		mutex_unlock(&wq_lock);
+	}
+
+	return __io_wq_create(bounded, data);
+}
+
 static bool io_wq_worker_wake(struct io_worker *worker, void *data)
 {
 	wake_up_process(worker->task);
@@ -1145,3 +1175,8 @@ void io_wq_destroy(struct io_wq *wq)
 		__io_wq_destroy(wq);
 	}
 }
+
+unsigned int io_wq_id(struct io_wq *wq)
+{
+	return wq->id;
+}
fs/io-wq.h +10 -1
index 1cd039af8813..dee7bb3b7cd4 100644
@@ -97,8 +97,17 @@ struct io_wq_data {
 	put_work_fn *put_work;
 };
 
-struct io_wq *io_wq_create(unsigned bounded, struct io_wq_data *data);
+struct io_wq *io_wq_create_id(unsigned bounded, struct io_wq_data *data,
+				unsigned int id);
+
+static inline struct io_wq *io_wq_create(unsigned bounded,
+					 struct io_wq_data *data)
+{
+	return io_wq_create_id(bounded, data, 0);
+}
+
 void io_wq_destroy(struct io_wq *wq);
+unsigned int io_wq_id(struct io_wq *wq);
 
 void io_wq_enqueue(struct io_wq *wq, struct io_wq_work *work);
 void io_wq_enqueue_hashed(struct io_wq *wq, struct io_wq_work *work, void *val);
Signature · 1 line
2.25.0
author maintainer
#4 lore raw

PATCH v2 4/4 io_uring: add support for sharing kernel io-wq workqueue

In reply to Jens Axboe

An id field is added to io_uring_params, which always returns the ID of the io-wq backend that is associated with an io_uring context. If an 'id' is provided and IORING_SETUP_SHARED is set in the creation flags, then we attempt to attach to an existing io-wq instead of setting up a new one.

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.

---
 fs/io_uring.c                 | 20 +++++++++++++++++---
 include/uapi/linux/io_uring.h |  4 +++-
 2 files changed, 20 insertions(+), 4 deletions(-)
fs/io_uring.c +17 -3
index 9f73586dcfb8..3dad12906db3 100644
@@ -5673,7 +5673,7 @@ static int io_sq_offload_start(struct io_ring_ctx *ctx,
 {
 	struct io_wq_data data;
 	unsigned concurrency;
-	int ret;
+	int ret, id;
 
 	init_waitqueue_head(&ctx->sqo_wait);
 	mmgrab(current->mm);
@@ -5724,13 +5724,23 @@ static int io_sq_offload_start(struct io_ring_ctx *ctx,
 
 	/* 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);
+
+	id = 0;
+	if (ctx->flags & IORING_SETUP_ATTACH_WQ) {
+		id = p->id;
+		if (!id) {
+			ret = -EINVAL;
+			goto err;
+		}
+	}
+	ctx->io_wq = io_wq_create_id(concurrency, &data, id);
 	if (IS_ERR(ctx->io_wq)) {
 		ret = PTR_ERR(ctx->io_wq);
 		ctx->io_wq = NULL;
 		goto err;
 	}
 
+	p->id = io_wq_id(ctx->io_wq);
 	return 0;
 err:
 	io_finish_async(ctx);
@@ -6554,7 +6564,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.id)
 		return -EINVAL;
 
 	ret = io_uring_create(entries, &p);
include/uapi/linux/io_uring.h +3 -1
index 57d05cc5e271..f66e53c74a3d 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 id;
+	__u32 resv[3];
 	struct io_sqring_offsets sq_off;
 	struct io_cqring_offsets cq_off;
 };
Signature · 1 line
2.25.0
#5 lore raw

Re: [PATCH 4/4] io_uring: add support for sharing kernel io-wq workqueue

Am 24.01.20 um 22:31 schrieb Jens Axboe:

Quoted 4 lines

An id field is added to io_uring_params, which always returns the ID of the io-wq backend that is associated with an io_uring context. If an 'id' is provided and IORING_SETUP_SHARED is set in the creation flags, then we attempt to attach to an existing io-wq instead of setting up a new one.

Use the new name here too.

Quoted 14 lines
fs/io_uring.c +0 -0
 	ret = io_uring_create(entries, &p);
include/uapi/linux/io_uring.h +1 -0
index 57d05cc5e271..f66e53c74a3d 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,

include/uapi/linux/io_uring.h struct io_uring_params { · line 187

Quoted 5 lines
include/uapi/linux/io_uring.h +1 -1
@@ -183,7 +184,8 @@ struct io_uring_params {
 	__u32 sq_thread_cpu;
 	__u32 sq_thread_idle;
 	__u32 features;
-	__u32 resv[4];
+	__u32 id;

I think this should be wq_id;

metze

author maintainer
#6 lore raw

Re: [PATCH 4/4] io_uring: add support for sharing kernel io-wq workqueue

Quoted 7 lines · Stefan Metzmacher

Am 24.01.20 um 22:31 schrieb Jens Axboe:

Quoted 4 lines

An id field is added to io_uring_params, which always returns the ID of the io-wq backend that is associated with an io_uring context. If an 'id' is provided and IORING_SETUP_SHARED is set in the creation flags, then we attempt to attach to an existing io-wq instead of setting up a new one.

Use the new name here too.

Already fixed that up yesterday.

Quoted 11 lines
Quoted 11 lines
fs/io_uring.c +0 -0
 	ret = io_uring_create(entries, &p);
include/uapi/linux/io_uring.h +1 -0
index 57d05cc5e271..f66e53c74a3d 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 */
 
Quoted 10 lines

include/uapi/linux/io_uring.h struct io_uring_params { · line 187

Quoted 8 lines
include/uapi/linux/io_uring.h +1 -1
 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 id;

I think this should be wq_id;

Yeah, probably clearer, I'll make that change, thanks.

Signature · 1 line
Jens Axboe