linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* more I/O context cleanup v2
@ 2021-12-09  6:31 Christoph Hellwig
  2021-12-09  6:31 ` [PATCH 01/11] block: remove the nr_task field from struct io_context Christoph Hellwig
                   ` (11 more replies)
  0 siblings, 12 replies; 18+ messages in thread
From: Christoph Hellwig @ 2021-12-09  6:31 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Paolo Valente, Jan Kara, linux-block

Hi Jens,

this series dusts off a few more bits in the I/O context handling, and
makes a chunk of code only needed by the BFQ I/O scheduler optional.

Changes since v1:
 - fix a comment typo
 - keep the RCU critical section in ioc_clear_queue
 - add a few more cleanups

Diffstat:
 block/Kconfig             |    3 
 block/Kconfig.iosched     |    1 
 block/blk-ioc.c           |  247 ++++++++++++++++++++--------------------------
 block/blk.h               |    6 +
 block/ioprio.c            |   32 -----
 include/linux/iocontext.h |    9 -
 6 files changed, 125 insertions(+), 173 deletions(-)

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 01/11] block: remove the nr_task field from struct io_context
  2021-12-09  6:31 more I/O context cleanup v2 Christoph Hellwig
@ 2021-12-09  6:31 ` Christoph Hellwig
  2021-12-09  6:31 ` [PATCH 02/11] block: simplify struct io_context refcounting Christoph Hellwig
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Christoph Hellwig @ 2021-12-09  6:31 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Paolo Valente, Jan Kara, linux-block

Nothing ever looks at ->nr_tasks, so remove it.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Jan Kara <jack@suse.cz>
---
 block/blk-ioc.c           | 3 ---
 include/linux/iocontext.h | 1 -
 2 files changed, 4 deletions(-)

diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index 536fb496ad763..96336c2134efa 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -207,7 +207,6 @@ void exit_io_context(struct task_struct *task)
 	task->io_context = NULL;
 	task_unlock(task);
 
-	atomic_dec(&ioc->nr_tasks);
 	put_io_context_active(ioc);
 }
 
@@ -259,7 +258,6 @@ static struct io_context *alloc_io_context(gfp_t gfp_flags, int node)
 		return NULL;
 
 	atomic_long_set(&ioc->refcount, 1);
-	atomic_set(&ioc->nr_tasks, 1);
 	atomic_set(&ioc->active_ref, 1);
 	spin_lock_init(&ioc->lock);
 	INIT_RADIX_TREE(&ioc->icq_tree, GFP_ATOMIC);
@@ -339,7 +337,6 @@ int __copy_io(unsigned long clone_flags, struct task_struct *tsk)
 	if (clone_flags & CLONE_IO) {
 		atomic_long_inc(&ioc->refcount);
 		atomic_inc(&ioc->active_ref);
-		atomic_inc(&ioc->nr_tasks);
 		tsk->io_context = ioc;
 	} else if (ioprio_valid(ioc->ioprio)) {
 		tsk->io_context = alloc_io_context(GFP_KERNEL, NUMA_NO_NODE);
diff --git a/include/linux/iocontext.h b/include/linux/iocontext.h
index c1229fbd6691c..82c7f4f5f4f59 100644
--- a/include/linux/iocontext.h
+++ b/include/linux/iocontext.h
@@ -99,7 +99,6 @@ struct io_cq {
 struct io_context {
 	atomic_long_t refcount;
 	atomic_t active_ref;
-	atomic_t nr_tasks;
 
 	/* all the fields below are protected by this lock */
 	spinlock_t lock;
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 02/11] block: simplify struct io_context refcounting
  2021-12-09  6:31 more I/O context cleanup v2 Christoph Hellwig
  2021-12-09  6:31 ` [PATCH 01/11] block: remove the nr_task field from struct io_context Christoph Hellwig
@ 2021-12-09  6:31 ` Christoph Hellwig
  2021-12-09  6:31 ` [PATCH 03/11] block: refactor put_iocontext_active Christoph Hellwig
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Christoph Hellwig @ 2021-12-09  6:31 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Paolo Valente, Jan Kara, linux-block

Don't hold a reference to ->refcount for each active reference, but
just one for all active references.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Jan Kara <jack@suse.cz>
---
 block/blk-ioc.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index 96336c2134efa..9cde3906be3c6 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -180,10 +180,8 @@ static void put_io_context_active(struct io_context *ioc)
 {
 	struct io_cq *icq;
 
-	if (!atomic_dec_and_test(&ioc->active_ref)) {
-		put_io_context(ioc);
+	if (!atomic_dec_and_test(&ioc->active_ref))
 		return;
-	}
 
 	spin_lock_irq(&ioc->lock);
 	hlist_for_each_entry(icq, &ioc->icq_list, ioc_node) {
@@ -335,7 +333,6 @@ int __copy_io(unsigned long clone_flags, struct task_struct *tsk)
 	 * Share io context with parent, if CLONE_IO is set
 	 */
 	if (clone_flags & CLONE_IO) {
-		atomic_long_inc(&ioc->refcount);
 		atomic_inc(&ioc->active_ref);
 		tsk->io_context = ioc;
 	} else if (ioprio_valid(ioc->ioprio)) {
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 03/11] block: refactor put_iocontext_active
  2021-12-09  6:31 more I/O context cleanup v2 Christoph Hellwig
  2021-12-09  6:31 ` [PATCH 01/11] block: remove the nr_task field from struct io_context Christoph Hellwig
  2021-12-09  6:31 ` [PATCH 02/11] block: simplify struct io_context refcounting Christoph Hellwig
@ 2021-12-09  6:31 ` Christoph Hellwig
  2021-12-09  6:31 ` [PATCH 04/11] block: remove the NULL ioc check in put_io_context Christoph Hellwig
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Christoph Hellwig @ 2021-12-09  6:31 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Paolo Valente, Jan Kara, linux-block

Factor out a ioc_exit_icqs helper to tear down the icqs and the fold
the rest of put_iocontext_active into exit_io_context.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Jan Kara <jack@suse.cz>
---
 block/blk-ioc.c | 41 ++++++++++++++---------------------------
 1 file changed, 14 insertions(+), 27 deletions(-)

diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index 9cde3906be3c6..0380e33930e31 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -54,6 +54,16 @@ static void ioc_exit_icq(struct io_cq *icq)
 	icq->flags |= ICQ_EXITED;
 }
 
+static void ioc_exit_icqs(struct io_context *ioc)
+{
+	struct io_cq *icq;
+
+	spin_lock_irq(&ioc->lock);
+	hlist_for_each_entry(icq, &ioc->icq_list, ioc_node)
+		ioc_exit_icq(icq);
+	spin_unlock_irq(&ioc->lock);
+}
+
 /*
  * Release an icq. Called with ioc locked for blk-mq, and with both ioc
  * and queue locked for legacy.
@@ -169,32 +179,6 @@ void put_io_context(struct io_context *ioc)
 }
 EXPORT_SYMBOL_GPL(put_io_context);
 
-/**
- * put_io_context_active - put active reference on ioc
- * @ioc: ioc of interest
- *
- * Put an active reference to an ioc.  If active reference reaches zero after
- * put, @ioc can never issue further IOs and ioscheds are notified.
- */
-static void put_io_context_active(struct io_context *ioc)
-{
-	struct io_cq *icq;
-
-	if (!atomic_dec_and_test(&ioc->active_ref))
-		return;
-
-	spin_lock_irq(&ioc->lock);
-	hlist_for_each_entry(icq, &ioc->icq_list, ioc_node) {
-		if (icq->flags & ICQ_EXITED)
-			continue;
-
-		ioc_exit_icq(icq);
-	}
-	spin_unlock_irq(&ioc->lock);
-
-	put_io_context(ioc);
-}
-
 /* Called by the exiting task */
 void exit_io_context(struct task_struct *task)
 {
@@ -205,7 +189,10 @@ void exit_io_context(struct task_struct *task)
 	task->io_context = NULL;
 	task_unlock(task);
 
-	put_io_context_active(ioc);
+	if (atomic_dec_and_test(&ioc->active_ref)) {
+		ioc_exit_icqs(ioc);
+		put_io_context(ioc);
+	}
 }
 
 static void __ioc_clear_queue(struct list_head *icq_list)
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 04/11] block: remove the NULL ioc check in put_io_context
  2021-12-09  6:31 more I/O context cleanup v2 Christoph Hellwig
                   ` (2 preceding siblings ...)
  2021-12-09  6:31 ` [PATCH 03/11] block: refactor put_iocontext_active Christoph Hellwig
@ 2021-12-09  6:31 ` Christoph Hellwig
  2021-12-09  6:31 ` [PATCH 05/11] block: refactor put_io_context Christoph Hellwig
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Christoph Hellwig @ 2021-12-09  6:31 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Paolo Valente, Jan Kara, linux-block

No caller passes in a NULL pointer, so remove the check.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Jan Kara <jack@suse.cz>
---
 block/blk-ioc.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index 0380e33930e31..04f3d2b0ca7db 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -155,9 +155,6 @@ void put_io_context(struct io_context *ioc)
 	unsigned long flags;
 	bool free_ioc = false;
 
-	if (ioc == NULL)
-		return;
-
 	BUG_ON(atomic_long_read(&ioc->refcount) <= 0);
 
 	/*
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 05/11] block: refactor put_io_context
  2021-12-09  6:31 more I/O context cleanup v2 Christoph Hellwig
                   ` (3 preceding siblings ...)
  2021-12-09  6:31 ` [PATCH 04/11] block: remove the NULL ioc check in put_io_context Christoph Hellwig
@ 2021-12-09  6:31 ` Christoph Hellwig
  2021-12-09  6:31 ` [PATCH 06/11] block: cleanup ioc_clear_queue Christoph Hellwig
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Christoph Hellwig @ 2021-12-09  6:31 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Paolo Valente, Jan Kara, linux-block

Move the code to delay freeing the icqs into a separate helper.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Jan Kara <jack@suse.cz>
---
 block/blk-ioc.c | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index 04f3d2b0ca7db..ca996214c10a6 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -143,6 +143,24 @@ static void ioc_release_fn(struct work_struct *work)
 	kmem_cache_free(iocontext_cachep, ioc);
 }
 
+/*
+ * Releasing icqs requires reverse order double locking and we may already be
+ * holding a queue_lock.  Do it asynchronously from a workqueue.
+ */
+static bool ioc_delay_free(struct io_context *ioc)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&ioc->lock, flags);
+	if (!hlist_empty(&ioc->icq_list)) {
+		queue_work(system_power_efficient_wq, &ioc->release_work);
+		spin_unlock_irqrestore(&ioc->lock, flags);
+		return true;
+	}
+	spin_unlock_irqrestore(&ioc->lock, flags);
+	return false;
+}
+
 /**
  * put_io_context - put a reference of io_context
  * @ioc: io_context to put
@@ -152,26 +170,8 @@ static void ioc_release_fn(struct work_struct *work)
  */
 void put_io_context(struct io_context *ioc)
 {
-	unsigned long flags;
-	bool free_ioc = false;
-
 	BUG_ON(atomic_long_read(&ioc->refcount) <= 0);
-
-	/*
-	 * Releasing ioc requires reverse order double locking and we may
-	 * already be holding a queue_lock.  Do it asynchronously from wq.
-	 */
-	if (atomic_long_dec_and_test(&ioc->refcount)) {
-		spin_lock_irqsave(&ioc->lock, flags);
-		if (!hlist_empty(&ioc->icq_list))
-			queue_work(system_power_efficient_wq,
-					&ioc->release_work);
-		else
-			free_ioc = true;
-		spin_unlock_irqrestore(&ioc->lock, flags);
-	}
-
-	if (free_ioc)
+	if (atomic_long_dec_and_test(&ioc->refcount) && !ioc_delay_free(ioc))
 		kmem_cache_free(iocontext_cachep, ioc);
 }
 EXPORT_SYMBOL_GPL(put_io_context);
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 06/11] block: cleanup ioc_clear_queue
  2021-12-09  6:31 more I/O context cleanup v2 Christoph Hellwig
                   ` (4 preceding siblings ...)
  2021-12-09  6:31 ` [PATCH 05/11] block: refactor put_io_context Christoph Hellwig
@ 2021-12-09  6:31 ` Christoph Hellwig
  2021-12-14 15:50   ` Jan Kara
  2021-12-09  6:31 ` [PATCH 07/11] block: move set_task_ioprio to blk-ioc.c Christoph Hellwig
                   ` (5 subsequent siblings)
  11 siblings, 1 reply; 18+ messages in thread
From: Christoph Hellwig @ 2021-12-09  6:31 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Paolo Valente, Jan Kara, linux-block

Fold __ioc_clear_queue into ioc_clear_queue and switch to always
use plain _irq locking instead of the more expensive _irqsave that
is not needed here.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-ioc.c | 33 +++++++++++----------------------
 1 file changed, 11 insertions(+), 22 deletions(-)

diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index ca996214c10a6..f98a29ee8f362 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -192,27 +192,6 @@ void exit_io_context(struct task_struct *task)
 	}
 }
 
-static void __ioc_clear_queue(struct list_head *icq_list)
-{
-	unsigned long flags;
-
-	rcu_read_lock();
-	while (!list_empty(icq_list)) {
-		struct io_cq *icq = list_entry(icq_list->next,
-						struct io_cq, q_node);
-		struct io_context *ioc = icq->ioc;
-
-		spin_lock_irqsave(&ioc->lock, flags);
-		if (icq->flags & ICQ_DESTROYED) {
-			spin_unlock_irqrestore(&ioc->lock, flags);
-			continue;
-		}
-		ioc_destroy_icq(icq);
-		spin_unlock_irqrestore(&ioc->lock, flags);
-	}
-	rcu_read_unlock();
-}
-
 /**
  * ioc_clear_queue - break any ioc association with the specified queue
  * @q: request_queue being cleared
@@ -227,7 +206,17 @@ void ioc_clear_queue(struct request_queue *q)
 	list_splice_init(&q->icq_list, &icq_list);
 	spin_unlock_irq(&q->queue_lock);
 
-	__ioc_clear_queue(&icq_list);
+	rcu_read_lock();
+	while (!list_empty(&icq_list)) {
+		struct io_cq *icq =
+			list_entry(icq_list.next, struct io_cq, q_node);
+
+		spin_lock_irq(&icq->ioc->lock);
+		if (!(icq->flags & ICQ_DESTROYED))
+			ioc_destroy_icq(icq);
+		spin_unlock_irq(&icq->ioc->lock);
+	}
+	rcu_read_unlock();
 }
 
 static struct io_context *alloc_io_context(gfp_t gfp_flags, int node)
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 07/11] block: move set_task_ioprio to blk-ioc.c
  2021-12-09  6:31 more I/O context cleanup v2 Christoph Hellwig
                   ` (5 preceding siblings ...)
  2021-12-09  6:31 ` [PATCH 06/11] block: cleanup ioc_clear_queue Christoph Hellwig
@ 2021-12-09  6:31 ` Christoph Hellwig
  2021-12-14 15:52   ` Jan Kara
  2021-12-09  6:31 ` [PATCH 08/11] block: fold get_task_io_context into set_task_ioprio Christoph Hellwig
                   ` (4 subsequent siblings)
  11 siblings, 1 reply; 18+ messages in thread
From: Christoph Hellwig @ 2021-12-09  6:31 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Paolo Valente, Jan Kara, linux-block

Keep set_task_ioprio with the other low-level code that accesses the
io_context structure.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-ioc.c           | 34 ++++++++++++++++++++++++++++++++--
 block/ioprio.c            | 32 --------------------------------
 include/linux/iocontext.h |  2 --
 3 files changed, 32 insertions(+), 36 deletions(-)

diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index f98a29ee8f362..c25ce2f3eb191 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -8,6 +8,7 @@
 #include <linux/bio.h>
 #include <linux/blkdev.h>
 #include <linux/slab.h>
+#include <linux/security.h>
 #include <linux/sched/task.h>
 
 #include "blk.h"
@@ -280,8 +281,8 @@ static struct io_context *create_task_io_context(struct task_struct *task,
  * This function always goes through task_lock() and it's better to use
  * %current->io_context + get_io_context() for %current.
  */
-struct io_context *get_task_io_context(struct task_struct *task,
-				       gfp_t gfp_flags, int node)
+static struct io_context *get_task_io_context(struct task_struct *task,
+		gfp_t gfp_flags, int node)
 {
 	struct io_context *ioc;
 
@@ -298,6 +299,35 @@ struct io_context *get_task_io_context(struct task_struct *task,
 	return ioc;
 }
 
+int set_task_ioprio(struct task_struct *task, int ioprio)
+{
+	int err;
+	struct io_context *ioc;
+	const struct cred *cred = current_cred(), *tcred;
+
+	rcu_read_lock();
+	tcred = __task_cred(task);
+	if (!uid_eq(tcred->uid, cred->euid) &&
+	    !uid_eq(tcred->uid, cred->uid) && !capable(CAP_SYS_NICE)) {
+		rcu_read_unlock();
+		return -EPERM;
+	}
+	rcu_read_unlock();
+
+	err = security_task_setioprio(task, ioprio);
+	if (err)
+		return err;
+
+	ioc = get_task_io_context(task, GFP_ATOMIC, NUMA_NO_NODE);
+	if (ioc) {
+		ioc->ioprio = ioprio;
+		put_io_context(ioc);
+	}
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(set_task_ioprio);
+
 int __copy_io(unsigned long clone_flags, struct task_struct *tsk)
 {
 	struct io_context *ioc = current->io_context;
diff --git a/block/ioprio.c b/block/ioprio.c
index 313c14a70bbd3..e118f4bf2dc65 100644
--- a/block/ioprio.c
+++ b/block/ioprio.c
@@ -22,46 +22,14 @@
  */
 #include <linux/gfp.h>
 #include <linux/kernel.h>
-#include <linux/export.h>
 #include <linux/ioprio.h>
 #include <linux/cred.h>
 #include <linux/blkdev.h>
 #include <linux/capability.h>
-#include <linux/sched/user.h>
-#include <linux/sched/task.h>
 #include <linux/syscalls.h>
 #include <linux/security.h>
 #include <linux/pid_namespace.h>
 
-int set_task_ioprio(struct task_struct *task, int ioprio)
-{
-	int err;
-	struct io_context *ioc;
-	const struct cred *cred = current_cred(), *tcred;
-
-	rcu_read_lock();
-	tcred = __task_cred(task);
-	if (!uid_eq(tcred->uid, cred->euid) &&
-	    !uid_eq(tcred->uid, cred->uid) && !capable(CAP_SYS_NICE)) {
-		rcu_read_unlock();
-		return -EPERM;
-	}
-	rcu_read_unlock();
-
-	err = security_task_setioprio(task, ioprio);
-	if (err)
-		return err;
-
-	ioc = get_task_io_context(task, GFP_ATOMIC, NUMA_NO_NODE);
-	if (ioc) {
-		ioc->ioprio = ioprio;
-		put_io_context(ioc);
-	}
-
-	return err;
-}
-EXPORT_SYMBOL_GPL(set_task_ioprio);
-
 int ioprio_check_cap(int ioprio)
 {
 	int class = IOPRIO_PRIO_CLASS(ioprio);
diff --git a/include/linux/iocontext.h b/include/linux/iocontext.h
index 82c7f4f5f4f59..648331f35fc66 100644
--- a/include/linux/iocontext.h
+++ b/include/linux/iocontext.h
@@ -116,8 +116,6 @@ struct task_struct;
 #ifdef CONFIG_BLOCK
 void put_io_context(struct io_context *ioc);
 void exit_io_context(struct task_struct *task);
-struct io_context *get_task_io_context(struct task_struct *task,
-				       gfp_t gfp_flags, int node);
 int __copy_io(unsigned long clone_flags, struct task_struct *tsk);
 static inline int copy_io(unsigned long clone_flags, struct task_struct *tsk)
 {
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 08/11] block: fold get_task_io_context into set_task_ioprio
  2021-12-09  6:31 more I/O context cleanup v2 Christoph Hellwig
                   ` (6 preceding siblings ...)
  2021-12-09  6:31 ` [PATCH 07/11] block: move set_task_ioprio to blk-ioc.c Christoph Hellwig
@ 2021-12-09  6:31 ` Christoph Hellwig
  2021-12-14 15:52   ` Jan Kara
  2021-12-09  6:31 ` [PATCH 09/11] block: open code create_task_io_context in set_task_ioprio Christoph Hellwig
                   ` (3 subsequent siblings)
  11 siblings, 1 reply; 18+ messages in thread
From: Christoph Hellwig @ 2021-12-09  6:31 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Paolo Valente, Jan Kara, linux-block

Fold get_task_io_context into its only caller, and simplify the code
as no reference to the I/O context is required to just set the ioprio
field.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-ioc.c | 52 +++++++++++++------------------------------------
 1 file changed, 14 insertions(+), 38 deletions(-)

diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index c25ce2f3eb191..1ba7cfedca2d9 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -268,41 +268,9 @@ static struct io_context *create_task_io_context(struct task_struct *task,
 	return ioc;
 }
 
-/**
- * get_task_io_context - get io_context of a task
- * @task: task of interest
- * @gfp_flags: allocation flags, used if allocation is necessary
- * @node: allocation node, used if allocation is necessary
- *
- * Return io_context of @task.  If it doesn't exist, it is created with
- * @gfp_flags and @node.  The returned io_context has its reference count
- * incremented.
- *
- * This function always goes through task_lock() and it's better to use
- * %current->io_context + get_io_context() for %current.
- */
-static struct io_context *get_task_io_context(struct task_struct *task,
-		gfp_t gfp_flags, int node)
-{
-	struct io_context *ioc;
-
-	might_sleep_if(gfpflags_allow_blocking(gfp_flags));
-
-	task_lock(task);
-	ioc = task->io_context;
-	if (unlikely(!ioc)) {
-		task_unlock(task);
-		return create_task_io_context(task, gfp_flags, node);
-	}
-	get_io_context(ioc);
-	task_unlock(task);
-	return ioc;
-}
-
 int set_task_ioprio(struct task_struct *task, int ioprio)
 {
 	int err;
-	struct io_context *ioc;
 	const struct cred *cred = current_cred(), *tcred;
 
 	rcu_read_lock();
@@ -318,13 +286,21 @@ int set_task_ioprio(struct task_struct *task, int ioprio)
 	if (err)
 		return err;
 
-	ioc = get_task_io_context(task, GFP_ATOMIC, NUMA_NO_NODE);
-	if (ioc) {
-		ioc->ioprio = ioprio;
-		put_io_context(ioc);
-	}
+	task_lock(task);
+	if (unlikely(!task->io_context)) {
+		struct io_context *ioc;
 
-	return err;
+		task_unlock(task);
+		ioc = create_task_io_context(task, GFP_ATOMIC, NUMA_NO_NODE);
+		if (ioc) {
+			ioc->ioprio = ioprio;
+			put_io_context(ioc);
+		}
+		return 0;
+	}
+	task->io_context->ioprio = ioprio;
+	task_unlock(task);
+	return 0;
 }
 EXPORT_SYMBOL_GPL(set_task_ioprio);
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 09/11] block: open code create_task_io_context in set_task_ioprio
  2021-12-09  6:31 more I/O context cleanup v2 Christoph Hellwig
                   ` (7 preceding siblings ...)
  2021-12-09  6:31 ` [PATCH 08/11] block: fold get_task_io_context into set_task_ioprio Christoph Hellwig
@ 2021-12-09  6:31 ` Christoph Hellwig
  2021-12-14 15:56   ` Jan Kara
  2021-12-09  6:31 ` [PATCH 10/11] block: fold create_task_io_context into ioc_find_get_icq Christoph Hellwig
                   ` (2 subsequent siblings)
  11 siblings, 1 reply; 18+ messages in thread
From: Christoph Hellwig @ 2021-12-09  6:31 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Paolo Valente, Jan Kara, linux-block

The flow in set_task_ioprio can be simplified by simply open coding
create_task_io_context, which removes a refcount roundtrip on the I/O
context.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-ioc.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index 1ba7cfedca2d9..cff0e3bdae53c 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -291,12 +291,18 @@ int set_task_ioprio(struct task_struct *task, int ioprio)
 		struct io_context *ioc;
 
 		task_unlock(task);
-		ioc = create_task_io_context(task, GFP_ATOMIC, NUMA_NO_NODE);
-		if (ioc) {
-			ioc->ioprio = ioprio;
-			put_io_context(ioc);
+
+		ioc = alloc_io_context(GFP_ATOMIC, NUMA_NO_NODE);
+		if (!ioc)
+			return -ENOMEM;
+
+		task_lock(task);
+		if (task->io_context || (task->flags & PF_EXITING)) {
+			kmem_cache_free(iocontext_cachep, ioc);
+			ioc = task->io_context;
+		} else {
+			task->io_context = ioc;
 		}
-		return 0;
 	}
 	task->io_context->ioprio = ioprio;
 	task_unlock(task);
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 10/11] block: fold create_task_io_context into ioc_find_get_icq
  2021-12-09  6:31 more I/O context cleanup v2 Christoph Hellwig
                   ` (8 preceding siblings ...)
  2021-12-09  6:31 ` [PATCH 09/11] block: open code create_task_io_context in set_task_ioprio Christoph Hellwig
@ 2021-12-09  6:31 ` Christoph Hellwig
  2021-12-14 16:01   ` Jan Kara
  2021-12-09  6:31 ` [PATCH 11/11] block: only build the icq tracking code when needed Christoph Hellwig
  2021-12-16 18:02 ` more I/O context cleanup v2 Jens Axboe
  11 siblings, 1 reply; 18+ messages in thread
From: Christoph Hellwig @ 2021-12-09  6:31 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Paolo Valente, Jan Kara, linux-block

Fold create_task_io_context into the only remaining caller.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-ioc.c | 43 ++++++++++++-------------------------------
 1 file changed, 12 insertions(+), 31 deletions(-)

diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index cff0e3bdae53c..dc7fb064fd5f7 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -238,36 +238,6 @@ static struct io_context *alloc_io_context(gfp_t gfp_flags, int node)
 	return ioc;
 }
 
-static struct io_context *create_task_io_context(struct task_struct *task,
-		gfp_t gfp_flags, int node)
-{
-	struct io_context *ioc;
-
-	ioc = alloc_io_context(gfp_flags, node);
-	if (!ioc)
-		return NULL;
-
-	/*
-	 * Try to install.  ioc shouldn't be installed if someone else
-	 * already did or @task, which isn't %current, is exiting.  Note
-	 * that we need to allow ioc creation on exiting %current as exit
-	 * path may issue IOs from e.g. exit_files().  The exit path is
-	 * responsible for not issuing IO after exit_io_context().
-	 */
-	task_lock(task);
-	if (!task->io_context &&
-	    (task == current || !(task->flags & PF_EXITING)))
-		task->io_context = ioc;
-	else
-		kmem_cache_free(iocontext_cachep, ioc);
-
-	ioc = task->io_context;
-	if (ioc)
-		get_io_context(ioc);
-	task_unlock(task);
-	return ioc;
-}
-
 int set_task_ioprio(struct task_struct *task, int ioprio)
 {
 	int err;
@@ -426,9 +396,20 @@ struct io_cq *ioc_find_get_icq(struct request_queue *q)
 	struct io_cq *icq = NULL;
 
 	if (unlikely(!ioc)) {
-		ioc = create_task_io_context(current, GFP_ATOMIC, q->node);
+		ioc = alloc_io_context(GFP_ATOMIC, q->node);
 		if (!ioc)
 			return NULL;
+
+		task_lock(current);
+		if (current->io_context) {
+			kmem_cache_free(iocontext_cachep, ioc);
+			ioc = current->io_context;
+		} else {
+			current->io_context = ioc;
+		}
+
+		get_io_context(ioc);
+		task_unlock(current);
 	} else {
 		get_io_context(ioc);
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 11/11] block: only build the icq tracking code when needed
  2021-12-09  6:31 more I/O context cleanup v2 Christoph Hellwig
                   ` (9 preceding siblings ...)
  2021-12-09  6:31 ` [PATCH 10/11] block: fold create_task_io_context into ioc_find_get_icq Christoph Hellwig
@ 2021-12-09  6:31 ` Christoph Hellwig
  2021-12-16 18:02 ` more I/O context cleanup v2 Jens Axboe
  11 siblings, 0 replies; 18+ messages in thread
From: Christoph Hellwig @ 2021-12-09  6:31 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Paolo Valente, Jan Kara, linux-block

Only bfq needs to code to track icq, so make it conditional.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Jan Kara <jack@suse.cz>
---
 block/Kconfig             |  3 ++
 block/Kconfig.iosched     |  1 +
 block/blk-ioc.c           | 68 +++++++++++++++++++++++----------------
 block/blk.h               |  6 ++++
 include/linux/iocontext.h |  6 ++--
 5 files changed, 55 insertions(+), 29 deletions(-)

diff --git a/block/Kconfig b/block/Kconfig
index c6ce41a5e5b27..d5d4197b7ed2d 100644
--- a/block/Kconfig
+++ b/block/Kconfig
@@ -35,6 +35,9 @@ config BLK_CGROUP_RWSTAT
 config BLK_DEV_BSG_COMMON
 	tristate
 
+config BLK_ICQ
+	bool
+
 config BLK_DEV_BSGLIB
 	bool "Block layer SG support v4 helper lib"
 	select BLK_DEV_BSG_COMMON
diff --git a/block/Kconfig.iosched b/block/Kconfig.iosched
index 885fee86dfcae..6155161460862 100644
--- a/block/Kconfig.iosched
+++ b/block/Kconfig.iosched
@@ -18,6 +18,7 @@ config MQ_IOSCHED_KYBER
 
 config IOSCHED_BFQ
 	tristate "BFQ I/O scheduler"
+	select BLK_ICQ
 	help
 	BFQ I/O scheduler for BLK-MQ. BFQ distributes the bandwidth of
 	of the device among all processes according to their weights,
diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index dc7fb064fd5f7..87bdc9ca82959 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -19,6 +19,7 @@
  */
 static struct kmem_cache *iocontext_cachep;
 
+#ifdef CONFIG_BLK_ICQ
 /**
  * get_io_context - increment reference count to io_context
  * @ioc: io_context to get
@@ -162,6 +163,42 @@ static bool ioc_delay_free(struct io_context *ioc)
 	return false;
 }
 
+/**
+ * ioc_clear_queue - break any ioc association with the specified queue
+ * @q: request_queue being cleared
+ *
+ * Walk @q->icq_list and exit all io_cq's.
+ */
+void ioc_clear_queue(struct request_queue *q)
+{
+	LIST_HEAD(icq_list);
+
+	spin_lock_irq(&q->queue_lock);
+	list_splice_init(&q->icq_list, &icq_list);
+	spin_unlock_irq(&q->queue_lock);
+
+	rcu_read_lock();
+	while (!list_empty(&icq_list)) {
+		struct io_cq *icq =
+			list_entry(icq_list.next, struct io_cq, q_node);
+
+		spin_lock_irq(&icq->ioc->lock);
+		if (!(icq->flags & ICQ_DESTROYED))
+			ioc_destroy_icq(icq);
+		spin_unlock_irq(&icq->ioc->lock);
+	}
+	rcu_read_unlock();
+}
+#else /* CONFIG_BLK_ICQ */
+static inline void ioc_exit_icqs(struct io_context *ioc)
+{
+}
+static inline bool ioc_delay_free(struct io_context *ioc)
+{
+	return false;
+}
+#endif /* CONFIG_BLK_ICQ */
+
 /**
  * put_io_context - put a reference of io_context
  * @ioc: io_context to put
@@ -193,33 +230,6 @@ void exit_io_context(struct task_struct *task)
 	}
 }
 
-/**
- * ioc_clear_queue - break any ioc association with the specified queue
- * @q: request_queue being cleared
- *
- * Walk @q->icq_list and exit all io_cq's.
- */
-void ioc_clear_queue(struct request_queue *q)
-{
-	LIST_HEAD(icq_list);
-
-	spin_lock_irq(&q->queue_lock);
-	list_splice_init(&q->icq_list, &icq_list);
-	spin_unlock_irq(&q->queue_lock);
-
-	rcu_read_lock();
-	while (!list_empty(&icq_list)) {
-		struct io_cq *icq =
-			list_entry(icq_list.next, struct io_cq, q_node);
-
-		spin_lock_irq(&icq->ioc->lock);
-		if (!(icq->flags & ICQ_DESTROYED))
-			ioc_destroy_icq(icq);
-		spin_unlock_irq(&icq->ioc->lock);
-	}
-	rcu_read_unlock();
-}
-
 static struct io_context *alloc_io_context(gfp_t gfp_flags, int node)
 {
 	struct io_context *ioc;
@@ -231,10 +241,12 @@ static struct io_context *alloc_io_context(gfp_t gfp_flags, int node)
 
 	atomic_long_set(&ioc->refcount, 1);
 	atomic_set(&ioc->active_ref, 1);
+#ifdef CONFIG_BLK_ICQ
 	spin_lock_init(&ioc->lock);
 	INIT_RADIX_TREE(&ioc->icq_tree, GFP_ATOMIC);
 	INIT_HLIST_HEAD(&ioc->icq_list);
 	INIT_WORK(&ioc->release_work, ioc_release_fn);
+#endif
 	return ioc;
 }
 
@@ -300,6 +312,7 @@ int __copy_io(unsigned long clone_flags, struct task_struct *tsk)
 	return 0;
 }
 
+#ifdef CONFIG_BLK_ICQ
 /**
  * ioc_lookup_icq - lookup io_cq from ioc
  * @q: the associated request_queue
@@ -428,6 +441,7 @@ struct io_cq *ioc_find_get_icq(struct request_queue *q)
 	return icq;
 }
 EXPORT_SYMBOL_GPL(ioc_find_get_icq);
+#endif /* CONFIG_BLK_ICQ */
 
 static int __init blk_ioc_init(void)
 {
diff --git a/block/blk.h b/block/blk.h
index 7ccb7c7d86b38..8bd43b3ad33d5 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -366,7 +366,13 @@ static inline unsigned int bio_aligned_discard_max_sectors(
  */
 struct io_cq *ioc_find_get_icq(struct request_queue *q);
 struct io_cq *ioc_lookup_icq(struct request_queue *q);
+#ifdef CONFIG_BLK_ICQ
 void ioc_clear_queue(struct request_queue *q);
+#else
+static inline void ioc_clear_queue(struct request_queue *q)
+{
+}
+#endif /* CONFIG_BLK_ICQ */
 
 #ifdef CONFIG_BLK_DEV_THROTTLING_LOW
 extern ssize_t blk_throtl_sample_time_show(struct request_queue *q, char *page);
diff --git a/include/linux/iocontext.h b/include/linux/iocontext.h
index 648331f35fc66..14f7eaf1b4437 100644
--- a/include/linux/iocontext.h
+++ b/include/linux/iocontext.h
@@ -100,16 +100,18 @@ struct io_context {
 	atomic_long_t refcount;
 	atomic_t active_ref;
 
+	unsigned short ioprio;
+
+#ifdef CONFIG_BLK_ICQ
 	/* all the fields below are protected by this lock */
 	spinlock_t lock;
 
-	unsigned short ioprio;
-
 	struct radix_tree_root	icq_tree;
 	struct io_cq __rcu	*icq_hint;
 	struct hlist_head	icq_list;
 
 	struct work_struct release_work;
+#endif /* CONFIG_BLK_ICQ */
 };
 
 struct task_struct;
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 06/11] block: cleanup ioc_clear_queue
  2021-12-09  6:31 ` [PATCH 06/11] block: cleanup ioc_clear_queue Christoph Hellwig
@ 2021-12-14 15:50   ` Jan Kara
  0 siblings, 0 replies; 18+ messages in thread
From: Jan Kara @ 2021-12-14 15:50 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Jens Axboe, Paolo Valente, Jan Kara, linux-block

On Thu 09-12-21 07:31:26, Christoph Hellwig wrote:
> Fold __ioc_clear_queue into ioc_clear_queue and switch to always
> use plain _irq locking instead of the more expensive _irqsave that
> is not needed here.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  block/blk-ioc.c | 33 +++++++++++----------------------
>  1 file changed, 11 insertions(+), 22 deletions(-)
> 
> diff --git a/block/blk-ioc.c b/block/blk-ioc.c
> index ca996214c10a6..f98a29ee8f362 100644
> --- a/block/blk-ioc.c
> +++ b/block/blk-ioc.c
> @@ -192,27 +192,6 @@ void exit_io_context(struct task_struct *task)
>  	}
>  }
>  
> -static void __ioc_clear_queue(struct list_head *icq_list)
> -{
> -	unsigned long flags;
> -
> -	rcu_read_lock();
> -	while (!list_empty(icq_list)) {
> -		struct io_cq *icq = list_entry(icq_list->next,
> -						struct io_cq, q_node);
> -		struct io_context *ioc = icq->ioc;
> -
> -		spin_lock_irqsave(&ioc->lock, flags);
> -		if (icq->flags & ICQ_DESTROYED) {
> -			spin_unlock_irqrestore(&ioc->lock, flags);
> -			continue;
> -		}
> -		ioc_destroy_icq(icq);
> -		spin_unlock_irqrestore(&ioc->lock, flags);
> -	}
> -	rcu_read_unlock();
> -}
> -
>  /**
>   * ioc_clear_queue - break any ioc association with the specified queue
>   * @q: request_queue being cleared
> @@ -227,7 +206,17 @@ void ioc_clear_queue(struct request_queue *q)
>  	list_splice_init(&q->icq_list, &icq_list);
>  	spin_unlock_irq(&q->queue_lock);
>  
> -	__ioc_clear_queue(&icq_list);
> +	rcu_read_lock();
> +	while (!list_empty(&icq_list)) {
> +		struct io_cq *icq =
> +			list_entry(icq_list.next, struct io_cq, q_node);
> +
> +		spin_lock_irq(&icq->ioc->lock);
> +		if (!(icq->flags & ICQ_DESTROYED))
> +			ioc_destroy_icq(icq);
> +		spin_unlock_irq(&icq->ioc->lock);
> +	}
> +	rcu_read_unlock();
>  }
>  
>  static struct io_context *alloc_io_context(gfp_t gfp_flags, int node)
> -- 
> 2.30.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 07/11] block: move set_task_ioprio to blk-ioc.c
  2021-12-09  6:31 ` [PATCH 07/11] block: move set_task_ioprio to blk-ioc.c Christoph Hellwig
@ 2021-12-14 15:52   ` Jan Kara
  0 siblings, 0 replies; 18+ messages in thread
From: Jan Kara @ 2021-12-14 15:52 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Jens Axboe, Paolo Valente, Jan Kara, linux-block

On Thu 09-12-21 07:31:27, Christoph Hellwig wrote:
> Keep set_task_ioprio with the other low-level code that accesses the
> io_context structure.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>


								Honza

> ---
>  block/blk-ioc.c           | 34 ++++++++++++++++++++++++++++++++--
>  block/ioprio.c            | 32 --------------------------------
>  include/linux/iocontext.h |  2 --
>  3 files changed, 32 insertions(+), 36 deletions(-)
> 
> diff --git a/block/blk-ioc.c b/block/blk-ioc.c
> index f98a29ee8f362..c25ce2f3eb191 100644
> --- a/block/blk-ioc.c
> +++ b/block/blk-ioc.c
> @@ -8,6 +8,7 @@
>  #include <linux/bio.h>
>  #include <linux/blkdev.h>
>  #include <linux/slab.h>
> +#include <linux/security.h>
>  #include <linux/sched/task.h>
>  
>  #include "blk.h"
> @@ -280,8 +281,8 @@ static struct io_context *create_task_io_context(struct task_struct *task,
>   * This function always goes through task_lock() and it's better to use
>   * %current->io_context + get_io_context() for %current.
>   */
> -struct io_context *get_task_io_context(struct task_struct *task,
> -				       gfp_t gfp_flags, int node)
> +static struct io_context *get_task_io_context(struct task_struct *task,
> +		gfp_t gfp_flags, int node)
>  {
>  	struct io_context *ioc;
>  
> @@ -298,6 +299,35 @@ struct io_context *get_task_io_context(struct task_struct *task,
>  	return ioc;
>  }
>  
> +int set_task_ioprio(struct task_struct *task, int ioprio)
> +{
> +	int err;
> +	struct io_context *ioc;
> +	const struct cred *cred = current_cred(), *tcred;
> +
> +	rcu_read_lock();
> +	tcred = __task_cred(task);
> +	if (!uid_eq(tcred->uid, cred->euid) &&
> +	    !uid_eq(tcred->uid, cred->uid) && !capable(CAP_SYS_NICE)) {
> +		rcu_read_unlock();
> +		return -EPERM;
> +	}
> +	rcu_read_unlock();
> +
> +	err = security_task_setioprio(task, ioprio);
> +	if (err)
> +		return err;
> +
> +	ioc = get_task_io_context(task, GFP_ATOMIC, NUMA_NO_NODE);
> +	if (ioc) {
> +		ioc->ioprio = ioprio;
> +		put_io_context(ioc);
> +	}
> +
> +	return err;
> +}
> +EXPORT_SYMBOL_GPL(set_task_ioprio);
> +
>  int __copy_io(unsigned long clone_flags, struct task_struct *tsk)
>  {
>  	struct io_context *ioc = current->io_context;
> diff --git a/block/ioprio.c b/block/ioprio.c
> index 313c14a70bbd3..e118f4bf2dc65 100644
> --- a/block/ioprio.c
> +++ b/block/ioprio.c
> @@ -22,46 +22,14 @@
>   */
>  #include <linux/gfp.h>
>  #include <linux/kernel.h>
> -#include <linux/export.h>
>  #include <linux/ioprio.h>
>  #include <linux/cred.h>
>  #include <linux/blkdev.h>
>  #include <linux/capability.h>
> -#include <linux/sched/user.h>
> -#include <linux/sched/task.h>
>  #include <linux/syscalls.h>
>  #include <linux/security.h>
>  #include <linux/pid_namespace.h>
>  
> -int set_task_ioprio(struct task_struct *task, int ioprio)
> -{
> -	int err;
> -	struct io_context *ioc;
> -	const struct cred *cred = current_cred(), *tcred;
> -
> -	rcu_read_lock();
> -	tcred = __task_cred(task);
> -	if (!uid_eq(tcred->uid, cred->euid) &&
> -	    !uid_eq(tcred->uid, cred->uid) && !capable(CAP_SYS_NICE)) {
> -		rcu_read_unlock();
> -		return -EPERM;
> -	}
> -	rcu_read_unlock();
> -
> -	err = security_task_setioprio(task, ioprio);
> -	if (err)
> -		return err;
> -
> -	ioc = get_task_io_context(task, GFP_ATOMIC, NUMA_NO_NODE);
> -	if (ioc) {
> -		ioc->ioprio = ioprio;
> -		put_io_context(ioc);
> -	}
> -
> -	return err;
> -}
> -EXPORT_SYMBOL_GPL(set_task_ioprio);
> -
>  int ioprio_check_cap(int ioprio)
>  {
>  	int class = IOPRIO_PRIO_CLASS(ioprio);
> diff --git a/include/linux/iocontext.h b/include/linux/iocontext.h
> index 82c7f4f5f4f59..648331f35fc66 100644
> --- a/include/linux/iocontext.h
> +++ b/include/linux/iocontext.h
> @@ -116,8 +116,6 @@ struct task_struct;
>  #ifdef CONFIG_BLOCK
>  void put_io_context(struct io_context *ioc);
>  void exit_io_context(struct task_struct *task);
> -struct io_context *get_task_io_context(struct task_struct *task,
> -				       gfp_t gfp_flags, int node);
>  int __copy_io(unsigned long clone_flags, struct task_struct *tsk);
>  static inline int copy_io(unsigned long clone_flags, struct task_struct *tsk)
>  {
> -- 
> 2.30.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 08/11] block: fold get_task_io_context into set_task_ioprio
  2021-12-09  6:31 ` [PATCH 08/11] block: fold get_task_io_context into set_task_ioprio Christoph Hellwig
@ 2021-12-14 15:52   ` Jan Kara
  0 siblings, 0 replies; 18+ messages in thread
From: Jan Kara @ 2021-12-14 15:52 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Jens Axboe, Paolo Valente, Jan Kara, linux-block

On Thu 09-12-21 07:31:28, Christoph Hellwig wrote:
> Fold get_task_io_context into its only caller, and simplify the code
> as no reference to the I/O context is required to just set the ioprio
> field.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  block/blk-ioc.c | 52 +++++++++++++------------------------------------
>  1 file changed, 14 insertions(+), 38 deletions(-)
> 
> diff --git a/block/blk-ioc.c b/block/blk-ioc.c
> index c25ce2f3eb191..1ba7cfedca2d9 100644
> --- a/block/blk-ioc.c
> +++ b/block/blk-ioc.c
> @@ -268,41 +268,9 @@ static struct io_context *create_task_io_context(struct task_struct *task,
>  	return ioc;
>  }
>  
> -/**
> - * get_task_io_context - get io_context of a task
> - * @task: task of interest
> - * @gfp_flags: allocation flags, used if allocation is necessary
> - * @node: allocation node, used if allocation is necessary
> - *
> - * Return io_context of @task.  If it doesn't exist, it is created with
> - * @gfp_flags and @node.  The returned io_context has its reference count
> - * incremented.
> - *
> - * This function always goes through task_lock() and it's better to use
> - * %current->io_context + get_io_context() for %current.
> - */
> -static struct io_context *get_task_io_context(struct task_struct *task,
> -		gfp_t gfp_flags, int node)
> -{
> -	struct io_context *ioc;
> -
> -	might_sleep_if(gfpflags_allow_blocking(gfp_flags));
> -
> -	task_lock(task);
> -	ioc = task->io_context;
> -	if (unlikely(!ioc)) {
> -		task_unlock(task);
> -		return create_task_io_context(task, gfp_flags, node);
> -	}
> -	get_io_context(ioc);
> -	task_unlock(task);
> -	return ioc;
> -}
> -
>  int set_task_ioprio(struct task_struct *task, int ioprio)
>  {
>  	int err;
> -	struct io_context *ioc;
>  	const struct cred *cred = current_cred(), *tcred;
>  
>  	rcu_read_lock();
> @@ -318,13 +286,21 @@ int set_task_ioprio(struct task_struct *task, int ioprio)
>  	if (err)
>  		return err;
>  
> -	ioc = get_task_io_context(task, GFP_ATOMIC, NUMA_NO_NODE);
> -	if (ioc) {
> -		ioc->ioprio = ioprio;
> -		put_io_context(ioc);
> -	}
> +	task_lock(task);
> +	if (unlikely(!task->io_context)) {
> +		struct io_context *ioc;
>  
> -	return err;
> +		task_unlock(task);
> +		ioc = create_task_io_context(task, GFP_ATOMIC, NUMA_NO_NODE);
> +		if (ioc) {
> +			ioc->ioprio = ioprio;
> +			put_io_context(ioc);
> +		}
> +		return 0;
> +	}
> +	task->io_context->ioprio = ioprio;
> +	task_unlock(task);
> +	return 0;
>  }
>  EXPORT_SYMBOL_GPL(set_task_ioprio);
>  
> -- 
> 2.30.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 09/11] block: open code create_task_io_context in set_task_ioprio
  2021-12-09  6:31 ` [PATCH 09/11] block: open code create_task_io_context in set_task_ioprio Christoph Hellwig
@ 2021-12-14 15:56   ` Jan Kara
  0 siblings, 0 replies; 18+ messages in thread
From: Jan Kara @ 2021-12-14 15:56 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Jens Axboe, Paolo Valente, Jan Kara, linux-block

On Thu 09-12-21 07:31:29, Christoph Hellwig wrote:
> The flow in set_task_ioprio can be simplified by simply open coding
> create_task_io_context, which removes a refcount roundtrip on the I/O
> context.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

OK, why not :). Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  block/blk-ioc.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/block/blk-ioc.c b/block/blk-ioc.c
> index 1ba7cfedca2d9..cff0e3bdae53c 100644
> --- a/block/blk-ioc.c
> +++ b/block/blk-ioc.c
> @@ -291,12 +291,18 @@ int set_task_ioprio(struct task_struct *task, int ioprio)
>  		struct io_context *ioc;
>  
>  		task_unlock(task);
> -		ioc = create_task_io_context(task, GFP_ATOMIC, NUMA_NO_NODE);
> -		if (ioc) {
> -			ioc->ioprio = ioprio;
> -			put_io_context(ioc);
> +
> +		ioc = alloc_io_context(GFP_ATOMIC, NUMA_NO_NODE);
> +		if (!ioc)
> +			return -ENOMEM;
> +
> +		task_lock(task);
> +		if (task->io_context || (task->flags & PF_EXITING)) {
> +			kmem_cache_free(iocontext_cachep, ioc);
> +			ioc = task->io_context;
> +		} else {
> +			task->io_context = ioc;
>  		}
> -		return 0;
>  	}
>  	task->io_context->ioprio = ioprio;
>  	task_unlock(task);
> -- 
> 2.30.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 10/11] block: fold create_task_io_context into ioc_find_get_icq
  2021-12-09  6:31 ` [PATCH 10/11] block: fold create_task_io_context into ioc_find_get_icq Christoph Hellwig
@ 2021-12-14 16:01   ` Jan Kara
  0 siblings, 0 replies; 18+ messages in thread
From: Jan Kara @ 2021-12-14 16:01 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Jens Axboe, Paolo Valente, Jan Kara, linux-block

On Thu 09-12-21 07:31:30, Christoph Hellwig wrote:
> Fold create_task_io_context into the only remaining caller.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  block/blk-ioc.c | 43 ++++++++++++-------------------------------
>  1 file changed, 12 insertions(+), 31 deletions(-)
> 
> diff --git a/block/blk-ioc.c b/block/blk-ioc.c
> index cff0e3bdae53c..dc7fb064fd5f7 100644
> --- a/block/blk-ioc.c
> +++ b/block/blk-ioc.c
> @@ -238,36 +238,6 @@ static struct io_context *alloc_io_context(gfp_t gfp_flags, int node)
>  	return ioc;
>  }
>  
> -static struct io_context *create_task_io_context(struct task_struct *task,
> -		gfp_t gfp_flags, int node)
> -{
> -	struct io_context *ioc;
> -
> -	ioc = alloc_io_context(gfp_flags, node);
> -	if (!ioc)
> -		return NULL;
> -
> -	/*
> -	 * Try to install.  ioc shouldn't be installed if someone else
> -	 * already did or @task, which isn't %current, is exiting.  Note
> -	 * that we need to allow ioc creation on exiting %current as exit
> -	 * path may issue IOs from e.g. exit_files().  The exit path is
> -	 * responsible for not issuing IO after exit_io_context().
> -	 */
> -	task_lock(task);
> -	if (!task->io_context &&
> -	    (task == current || !(task->flags & PF_EXITING)))
> -		task->io_context = ioc;
> -	else
> -		kmem_cache_free(iocontext_cachep, ioc);
> -
> -	ioc = task->io_context;
> -	if (ioc)
> -		get_io_context(ioc);
> -	task_unlock(task);
> -	return ioc;
> -}
> -
>  int set_task_ioprio(struct task_struct *task, int ioprio)
>  {
>  	int err;
> @@ -426,9 +396,20 @@ struct io_cq *ioc_find_get_icq(struct request_queue *q)
>  	struct io_cq *icq = NULL;
>  
>  	if (unlikely(!ioc)) {
> -		ioc = create_task_io_context(current, GFP_ATOMIC, q->node);
> +		ioc = alloc_io_context(GFP_ATOMIC, q->node);
>  		if (!ioc)
>  			return NULL;
> +
> +		task_lock(current);
> +		if (current->io_context) {
> +			kmem_cache_free(iocontext_cachep, ioc);
> +			ioc = current->io_context;
> +		} else {
> +			current->io_context = ioc;
> +		}
> +
> +		get_io_context(ioc);
> +		task_unlock(current);
>  	} else {
>  		get_io_context(ioc);
>  
> -- 
> 2.30.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: more I/O context cleanup v2
  2021-12-09  6:31 more I/O context cleanup v2 Christoph Hellwig
                   ` (10 preceding siblings ...)
  2021-12-09  6:31 ` [PATCH 11/11] block: only build the icq tracking code when needed Christoph Hellwig
@ 2021-12-16 18:02 ` Jens Axboe
  11 siblings, 0 replies; 18+ messages in thread
From: Jens Axboe @ 2021-12-16 18:02 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-block, Jan Kara, Paolo Valente

On Thu, 9 Dec 2021 07:31:20 +0100, Christoph Hellwig wrote:
> this series dusts off a few more bits in the I/O context handling, and
> makes a chunk of code only needed by the BFQ I/O scheduler optional.
> 
> Changes since v1:
>  - fix a comment typo
>  - keep the RCU critical section in ioc_clear_queue
>  - add a few more cleanups
> 
> [...]

Applied, thanks!

[01/11] block: remove the nr_task field from struct io_context
        commit: 8a2ba1785c5803d59a63b6320ff54fd4a37a41ce
[02/11] block: simplify struct io_context refcounting
        commit: 0aed2f162bbc7853fe91c0d70492ea73c4e9cb07
[03/11] block: refactor put_iocontext_active
        commit: 4be8a2eaff2e4473b6e8ad9a3857bc9b1e79c8ba
[04/11] block: remove the NULL ioc check in put_io_context
        commit: 8a20c0c7e0cea7eb0c32fd6b63ff514c9ac32b8f
[05/11] block: refactor put_io_context
        commit: edf70ff5a1ed9769da35178454d743828061a6a3
[06/11] block: cleanup ioc_clear_queue
        commit: 091abcb3efd71cb18e80c8f040d9e4a634d8906d
[07/11] block: move set_task_ioprio to blk-ioc.c
        commit: a411cd3cfdc5bbd1329d5b33dbf39e2b5213969d
[08/11] block: fold get_task_io_context into set_task_ioprio
        commit: 8472161b77c41d260c5ba0af6bf940269b297bb6
[09/11] block: open code create_task_io_context in set_task_ioprio
        commit: 5fc11eebb4a98df5324a4de369bb5ab7f0007ff7
[10/11] block: fold create_task_io_context into ioc_find_get_icq
        commit: 90b627f5426ce144cdd4ea585d1f7812359a1a6a
[11/11] block: only build the icq tracking code when needed
        commit: 5ef1630586317e92c9ebd7b4ce48f393b7ff790f

Best regards,
-- 
Jens Axboe



^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2021-12-16 18:02 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-09  6:31 more I/O context cleanup v2 Christoph Hellwig
2021-12-09  6:31 ` [PATCH 01/11] block: remove the nr_task field from struct io_context Christoph Hellwig
2021-12-09  6:31 ` [PATCH 02/11] block: simplify struct io_context refcounting Christoph Hellwig
2021-12-09  6:31 ` [PATCH 03/11] block: refactor put_iocontext_active Christoph Hellwig
2021-12-09  6:31 ` [PATCH 04/11] block: remove the NULL ioc check in put_io_context Christoph Hellwig
2021-12-09  6:31 ` [PATCH 05/11] block: refactor put_io_context Christoph Hellwig
2021-12-09  6:31 ` [PATCH 06/11] block: cleanup ioc_clear_queue Christoph Hellwig
2021-12-14 15:50   ` Jan Kara
2021-12-09  6:31 ` [PATCH 07/11] block: move set_task_ioprio to blk-ioc.c Christoph Hellwig
2021-12-14 15:52   ` Jan Kara
2021-12-09  6:31 ` [PATCH 08/11] block: fold get_task_io_context into set_task_ioprio Christoph Hellwig
2021-12-14 15:52   ` Jan Kara
2021-12-09  6:31 ` [PATCH 09/11] block: open code create_task_io_context in set_task_ioprio Christoph Hellwig
2021-12-14 15:56   ` Jan Kara
2021-12-09  6:31 ` [PATCH 10/11] block: fold create_task_io_context into ioc_find_get_icq Christoph Hellwig
2021-12-14 16:01   ` Jan Kara
2021-12-09  6:31 ` [PATCH 11/11] block: only build the icq tracking code when needed Christoph Hellwig
2021-12-16 18:02 ` more I/O context cleanup v2 Jens Axboe

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