[PATCHSET v2 0/4] Add support for shared io-wq backends

[PATCHSET v2 0/4] Add support for shared io-wq backends

[Jens Axboe]

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

-- 
Jens Axboe

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

[Jens Axboe]

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

Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 fs/io-wq.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/fs/io-wq.c b/fs/io-wq.c
index 4d902c19ee5f..54e270ae12ab 100644
--- a/fs/io-wq.c
+++ b/fs/io-wq.c
@@ -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);
+}
-- 
2.25.0

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

[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.

Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 fs/io-wq.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/fs/io-wq.c b/fs/io-wq.c
index 54e270ae12ab..51b3677d6fbd 100644
--- a/fs/io-wq.c
+++ b/fs/io-wq.c
@@ -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);
+	}
 }
-- 
2.25.0

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

[Jens Axboe]

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

Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 fs/io-wq.c | 37 ++++++++++++++++++++++++++++++++++++-
 fs/io-wq.h | 11 ++++++++++-
 2 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/fs/io-wq.c b/fs/io-wq.c
index 51b3677d6fbd..08ef69b498f6 100644
--- a/fs/io-wq.c
+++ b/fs/io-wq.c
@@ -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;
+}
diff --git a/fs/io-wq.h b/fs/io-wq.h
index 1cd039af8813..dee7bb3b7cd4 100644
--- a/fs/io-wq.h
+++ b/fs/io-wq.h
@@ -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);
-- 
2.25.0

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

[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.

Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 fs/io_uring.c                 | 20 +++++++++++++++++---
 include/uapi/linux/io_uring.h |  4 +++-
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 9f73586dcfb8..3dad12906db3 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -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);
diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
index 57d05cc5e271..f66e53c74a3d 100644
--- a/include/uapi/linux/io_uring.h
+++ b/include/uapi/linux/io_uring.h
@@ -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;
 };
-- 
2.25.0

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

[Stefan Metzmacher]

[-- Attachment #1.1: Type: text/plain, Size: 1167 bytes --]

Am 24.01.20 um 22:31 schrieb 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.

Use the new name here too.

>  	ret = io_uring_create(entries, &p);
> diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
> index 57d05cc5e271..f66e53c74a3d 100644
> --- a/include/uapi/linux/io_uring.h
> +++ b/include/uapi/linux/io_uring.h
> @@ -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;

I think this should be wq_id;

metze



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Jens Axboe]

On 1/25/20 12:45 AM, Stefan Metzmacher wrote:
> Am 24.01.20 um 22:31 schrieb 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.
> 
> Use the new name here too.

Already fixed that up yesterday.

>>  	ret = io_uring_create(entries, &p);
>> diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
>> index 57d05cc5e271..f66e53c74a3d 100644
>> --- a/include/uapi/linux/io_uring.h
>> +++ b/include/uapi/linux/io_uring.h
>> @@ -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;
> 
> I think this should be wq_id;

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

-- 
Jens Axboe

[PATCHSET 0/4] Add support for shared io-wq backends

[Jens Axboe]

Sometimes an applications wants to use multiple smaller rings, because
it's more efficient than sharing a ring. The downside of that is that
we'll create the io-wq backend separately for all of them, while they
would be perfectly happy just sharing that.

This patchset adds support for that. io_uring_params grows an 'id' field,
which denotes an identifier for the async backend. If an application
wants to utilize sharing, it'll simply grab the id from the first ring
created, and pass it in to the next one and set IORING_SETUP_SHARED. This
allows efficient sharing of backend resources, while allowing multiple
rings in the application or library.

Not a huge fan of the IORING_SETUP_SHARED name, we should probably make
that better (I'm taking suggestions).

-- 
Jens Axboe

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

[Jens Axboe]

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

Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 fs/io-wq.c | 42 +++++++++++++++++++++++++++++++++++++++++-
 fs/io-wq.h |  3 +++
 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/fs/io-wq.c b/fs/io-wq.c
index 8cb0cff5f15c..a9985856033d 100644
--- a/fs/io-wq.c
+++ b/fs/io-wq.c
@@ -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, id;
 	struct io_wq *wq;
@@ -1107,6 +1107,41 @@ struct io_wq *io_wq_create(unsigned bounded, struct io_wq_data *data)
 	return ERR_PTR(ret);
 }
 
+struct io_wq *io_wq_create(unsigned bounded, struct io_wq_data *data)
+{
+	return __io_wq_create(bounded, data);
+}
+
+/*
+ * 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)
+{
+	struct io_wq *wq, *ret = NULL;
+
+	mutex_lock(&wq_lock);
+	list_for_each_entry(wq, &wq_list, wq_list) {
+		if (id != wq->id)
+			continue;
+		if (data->creds != wq->creds || data->user != wq->user)
+			continue;
+		if (data->get_work != wq->get_work ||
+		    data->put_work != wq->put_work)
+			continue;
+		if (!refcount_inc_not_zero(&wq->use_refs))
+			continue;
+		ret = wq;
+		break;
+	}
+	mutex_unlock(&wq_lock);
+
+	if (!ret)
+		ret = io_wq_create(bounded, data);
+
+	return ret;
+}
+
 static bool io_wq_worker_wake(struct io_worker *worker, void *data)
 {
 	wake_up_process(worker->task);
@@ -1145,3 +1180,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;
+}
diff --git a/fs/io-wq.h b/fs/io-wq.h
index 1cd039af8813..7abe2c56b535 100644
--- a/fs/io-wq.h
+++ b/fs/io-wq.h
@@ -98,7 +98,10 @@ struct io_wq_data {
 };
 
 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);
 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);
-- 
2.25.0

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

[Stefan Metzmacher]

[-- Attachment #1.1: Type: text/plain, Size: 803 bytes --]

> +/*
> + * 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)
> +{
> +	struct io_wq *wq, *ret = NULL;
> +
> +	mutex_lock(&wq_lock);
> +	list_for_each_entry(wq, &wq_list, wq_list) {
> +		if (id != wq->id)
> +			continue;
> +		if (data->creds != wq->creds || data->user != wq->user)
> +			continue;
> +		if (data->get_work != wq->get_work ||
> +		    data->put_work != wq->put_work)
> +			continue;
> +		if (!refcount_inc_not_zero(&wq->use_refs))
> +			continue;
> +		ret = wq;
> +		break;
> +	}
> +	mutex_unlock(&wq_lock);

Isn't there a more efficient ida_find function in order to avoid
the loop, which won't really scale in the long run.

metze


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Jens Axboe]

On 1/24/20 2:54 AM, Stefan Metzmacher wrote:
>> +/*
>> + * 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)
>> +{
>> +	struct io_wq *wq, *ret = NULL;
>> +
>> +	mutex_lock(&wq_lock);
>> +	list_for_each_entry(wq, &wq_list, wq_list) {
>> +		if (id != wq->id)
>> +			continue;
>> +		if (data->creds != wq->creds || data->user != wq->user)
>> +			continue;
>> +		if (data->get_work != wq->get_work ||
>> +		    data->put_work != wq->put_work)
>> +			continue;
>> +		if (!refcount_inc_not_zero(&wq->use_refs))
>> +			continue;
>> +		ret = wq;
>> +		break;
>> +	}
>> +	mutex_unlock(&wq_lock);
> 
> Isn't there a more efficient ida_find function in order to avoid
> the loop, which won't really scale in the long run.

Yeah that would be a lot better - I initially just used a sequence
for this, but since I'm now using ida, might as well use it for
lookup as well. I'll make the change.

-- 
Jens Axboe