cleanup I/O context handling

cleanup I/O context handling

[Christoph Hellwig]

Hi Jens,

this series does a little spring cleaning of the I/O context handling/

Subject:
 block/bfq-iosched.c                   |   41 ++++++------
 block/blk-ioc.c                       |  115 +++++++++++++++++++++++++---------
 block/blk-mq-sched.c                  |   35 ----------
 block/blk-mq-sched.h                  |    3 
 block/blk-mq.c                        |   14 ----
 block/blk.h                           |    8 --
 drivers/infiniband/hw/qib/qib_verbs.c |    4 -
 include/linux/iocontext.h             |   40 +++--------
 kernel/fork.c                         |   26 -------
 9 files changed, 128 insertions(+), 158 deletions(-)

[PATCH 01/14] RDMA/qib: rename copy_io to qib_copy_io

[Christoph Hellwig]

Add the proper module prefix to avoid conflicts with a function
in the scheduler.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/infiniband/hw/qib/qib_verbs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/qib/qib_verbs.c b/drivers/infiniband/hw/qib/qib_verbs.c
index ef91bff5c23ca..0080f0be72fef 100644
--- a/drivers/infiniband/hw/qib/qib_verbs.c
+++ b/drivers/infiniband/hw/qib/qib_verbs.c
@@ -425,7 +425,7 @@ static inline u32 clear_upper_bytes(u32 data, u32 n, u32 off)
 }
 #endif
 
-static void copy_io(u32 __iomem *piobuf, struct rvt_sge_state *ss,
+static void qib_copy_io(u32 __iomem *piobuf, struct rvt_sge_state *ss,
 		    u32 length, unsigned flush_wc)
 {
 	u32 extra = 0;
@@ -975,7 +975,7 @@ static int qib_verbs_send_pio(struct rvt_qp *qp, struct ib_header *ibhdr,
 			qib_pio_copy(piobuf, addr, dwords);
 		goto done;
 	}
-	copy_io(piobuf, ss, len, flush_wc);
+	qib_copy_io(piobuf, ss, len, flush_wc);
 done:
 	if (dd->flags & QIB_USE_SPCL_TRIG) {
 		u32 spcl_off = (pbufn >= dd->piobcnt2k) ? 2047 : 1023;
-- 
2.30.2

[PATCH 02/14] fork: move copy_io to block/blk-ioc.c

[Christoph Hellwig]

Move the copying of the I/O context to the block layer as that is where
we can use the proper low-level interfaces.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-ioc.c           | 27 +++++++++++++++++++++++++++
 include/linux/iocontext.h | 23 +++++++++++++----------
 kernel/fork.c             | 26 --------------------------
 3 files changed, 40 insertions(+), 36 deletions(-)

diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index 70c99e85aee50..3b31cfad4b75b 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -322,6 +322,33 @@ struct io_context *get_task_io_context(struct task_struct *task,
 	return NULL;
 }
 
+int __copy_io(unsigned long clone_flags, struct task_struct *tsk)
+{
+	struct io_context *ioc = current->io_context;
+	struct io_context *new_ioc;
+
+	/*
+	 * Share io context with parent, if CLONE_IO is set
+	 */
+	if (clone_flags & CLONE_IO) {
+		get_io_context_active(ioc);
+
+		WARN_ON_ONCE(atomic_read(&ioc->nr_tasks) <= 0);
+		atomic_inc(&ioc->nr_tasks);
+
+		tsk->io_context = ioc;
+	} else if (ioprio_valid(ioc->ioprio)) {
+		new_ioc = get_task_io_context(tsk, GFP_KERNEL, NUMA_NO_NODE);
+		if (unlikely(!new_ioc))
+			return -ENOMEM;
+
+		new_ioc->ioprio = ioc->ioprio;
+		put_io_context(new_ioc);
+	}
+
+	return 0;
+}
+
 /**
  * ioc_lookup_icq - lookup io_cq from ioc
  * @ioc: the associated io_context
diff --git a/include/linux/iocontext.h b/include/linux/iocontext.h
index 0a9dc40b7be84..bcd47d104d8e6 100644
--- a/include/linux/iocontext.h
+++ b/include/linux/iocontext.h
@@ -129,14 +129,6 @@ static inline void get_io_context_active(struct io_context *ioc)
 	atomic_inc(&ioc->active_ref);
 }
 
-static inline void ioc_task_link(struct io_context *ioc)
-{
-	get_io_context_active(ioc);
-
-	WARN_ON_ONCE(atomic_read(&ioc->nr_tasks) <= 0);
-	atomic_inc(&ioc->nr_tasks);
-}
-
 struct task_struct;
 #ifdef CONFIG_BLOCK
 void put_io_context(struct io_context *ioc);
@@ -144,10 +136,21 @@ void put_io_context_active(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)
+{
+	if (!current->io_context)
+		return 0;
+	return __copy_io(clone_flags, tsk);
+}
 #else
 struct io_context;
 static inline void put_io_context(struct io_context *ioc) { }
 static inline void exit_io_context(struct task_struct *task) { }
-#endif
+static inline int copy_io(unsigned long clone_flags, struct task_struct *tsk)
+{
+	return 0;
+}
+#endif /* CONFIG_BLOCK */
 
-#endif
+#endif /* IOCONTEXT_H */
diff --git a/kernel/fork.c b/kernel/fork.c
index 3244cc56b697d..3161d7980155e 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1556,32 +1556,6 @@ static int copy_files(unsigned long clone_flags, struct task_struct *tsk)
 	return error;
 }
 
-static int copy_io(unsigned long clone_flags, struct task_struct *tsk)
-{
-#ifdef CONFIG_BLOCK
-	struct io_context *ioc = current->io_context;
-	struct io_context *new_ioc;
-
-	if (!ioc)
-		return 0;
-	/*
-	 * Share io context with parent, if CLONE_IO is set
-	 */
-	if (clone_flags & CLONE_IO) {
-		ioc_task_link(ioc);
-		tsk->io_context = ioc;
-	} else if (ioprio_valid(ioc->ioprio)) {
-		new_ioc = get_task_io_context(tsk, GFP_KERNEL, NUMA_NO_NODE);
-		if (unlikely(!new_ioc))
-			return -ENOMEM;
-
-		new_ioc->ioprio = ioc->ioprio;
-		put_io_context(new_ioc);
-	}
-#endif
-	return 0;
-}
-
 static int copy_sighand(unsigned long clone_flags, struct task_struct *tsk)
 {
 	struct sighand_struct *sig;
-- 
2.30.2

[PATCH 03/14] bfq: simplify bfq_bic_lookup

[Christoph Hellwig]

Remove the unused bfqd argument, and hardcode ioc to current->io_context.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/bfq-iosched.c | 25 ++++++++++---------------
 1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 85554b8009703..c990c6409c119 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -433,26 +433,21 @@ static struct bfq_io_cq *icq_to_bic(struct io_cq *icq)
 
 /**
  * bfq_bic_lookup - search into @ioc a bic associated to @bfqd.
- * @bfqd: the lookup key.
- * @ioc: the io_context of the process doing I/O.
  * @q: the request queue.
  */
-static struct bfq_io_cq *bfq_bic_lookup(struct bfq_data *bfqd,
-					struct io_context *ioc,
-					struct request_queue *q)
+static struct bfq_io_cq *bfq_bic_lookup(struct request_queue *q)
 {
-	if (ioc) {
-		unsigned long flags;
-		struct bfq_io_cq *icq;
+	struct bfq_io_cq *icq;
+	unsigned long flags;
 
-		spin_lock_irqsave(&q->queue_lock, flags);
-		icq = icq_to_bic(ioc_lookup_icq(ioc, q));
-		spin_unlock_irqrestore(&q->queue_lock, flags);
+	if (!current->io_context)
+		return NULL;
 
-		return icq;
-	}
+	spin_lock_irqsave(&q->queue_lock, flags);
+	icq = icq_to_bic(ioc_lookup_icq(current->io_context, q));
+	spin_unlock_irqrestore(&q->queue_lock, flags);
 
-	return NULL;
+	return icq;
 }
 
 /*
@@ -2457,7 +2452,7 @@ static bool bfq_bio_merge(struct request_queue *q, struct bio *bio,
 	 * returned by bfq_bic_lookup does not go away before
 	 * bfqd->lock is taken.
 	 */
-	struct bfq_io_cq *bic = bfq_bic_lookup(bfqd, current->io_context, q);
+	struct bfq_io_cq *bic = bfq_bic_lookup(q);
 	bool ret;
 
 	spin_lock_irq(&bfqd->lock);
-- 
2.30.2

[PATCH 04/14] bfq: use bfq_bic_lookup in bfq_limit_depth

[Christoph Hellwig]

No need to create a new I/O context if there is none present yet in
->limit_depth.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/bfq-iosched.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index c990c6409c119..ecc2e57e68630 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -663,7 +663,7 @@ static bool bfqq_request_over_limit(struct bfq_queue *bfqq, int limit)
 static void bfq_limit_depth(unsigned int op, struct blk_mq_alloc_data *data)
 {
 	struct bfq_data *bfqd = data->q->elevator->elevator_data;
-	struct bfq_io_cq *bic = icq_to_bic(blk_mq_sched_get_icq(data->q));
+	struct bfq_io_cq *bic = bfq_bic_lookup(data->q);
 	struct bfq_queue *bfqq = bic ? bic_to_bfqq(bic, op_is_sync(op)) : NULL;
 	int depth;
 	unsigned limit = data->q->nr_requests;
-- 
2.30.2

[PATCH 05/14] Revert "block: Provide blk_mq_sched_get_icq()"

[Christoph Hellwig]

This reverts commit 4896c4e64ba5d5d5acdbcf68c5910dd4f6d8fa62.

The helper is not needed any more.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-mq-sched.c | 26 +++++++++++---------------
 block/blk-mq-sched.h |  1 -
 2 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
index 98c6a97729f24..b942b38000e53 100644
--- a/block/blk-mq-sched.c
+++ b/block/blk-mq-sched.c
@@ -18,8 +18,9 @@
 #include "blk-mq-tag.h"
 #include "blk-wbt.h"
 
-struct io_cq *blk_mq_sched_get_icq(struct request_queue *q)
+void blk_mq_sched_assign_ioc(struct request *rq)
 {
+	struct request_queue *q = rq->q;
 	struct io_context *ioc;
 	struct io_cq *icq;
 
@@ -27,27 +28,22 @@ struct io_cq *blk_mq_sched_get_icq(struct request_queue *q)
 	if (unlikely(!current->io_context))
 		create_task_io_context(current, GFP_ATOMIC, q->node);
 
-	/* May not have an IO context if context creation failed */
+	/*
+	 * May not have an IO context if it's a passthrough request
+	 */
 	ioc = current->io_context;
 	if (!ioc)
-		return NULL;
+		return;
 
 	spin_lock_irq(&q->queue_lock);
 	icq = ioc_lookup_icq(ioc, q);
 	spin_unlock_irq(&q->queue_lock);
-	if (icq)
-		return icq;
-	return ioc_create_icq(ioc, q, GFP_ATOMIC);
-}
-EXPORT_SYMBOL(blk_mq_sched_get_icq);
 
-void blk_mq_sched_assign_ioc(struct request *rq)
-{
-	struct io_cq *icq;
-
-	icq = blk_mq_sched_get_icq(rq->q);
-	if (!icq)
-		return;
+	if (!icq) {
+		icq = ioc_create_icq(ioc, q, GFP_ATOMIC);
+		if (!icq)
+			return;
+	}
 	get_io_context(icq->ioc);
 	rq->elv.icq = icq;
 }
diff --git a/block/blk-mq-sched.h b/block/blk-mq-sched.h
index add651ec06da7..25d1034952b65 100644
--- a/block/blk-mq-sched.h
+++ b/block/blk-mq-sched.h
@@ -8,7 +8,6 @@
 
 #define MAX_SCHED_RQ (16 * BLKDEV_DEFAULT_RQ)
 
-struct io_cq *blk_mq_sched_get_icq(struct request_queue *q);
 void blk_mq_sched_assign_ioc(struct request *rq);
 
 bool blk_mq_sched_try_merge(struct request_queue *q, struct bio *bio,
-- 
2.30.2

[PATCH 06/14] block: mark put_io_context_active static

[Christoph Hellwig]

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

diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index 3b31cfad4b75b..f3ff495756cb4 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -175,7 +175,7 @@ void put_io_context(struct io_context *ioc)
  * Undo get_io_context_active().  If active reference reaches zero after
  * put, @ioc can never issue further IOs and ioscheds are notified.
  */
-void put_io_context_active(struct io_context *ioc)
+static void put_io_context_active(struct io_context *ioc)
 {
 	struct io_cq *icq;
 
diff --git a/include/linux/iocontext.h b/include/linux/iocontext.h
index bcd47d104d8e6..3ba45953d5228 100644
--- a/include/linux/iocontext.h
+++ b/include/linux/iocontext.h
@@ -132,7 +132,6 @@ static inline void get_io_context_active(struct io_context *ioc)
 struct task_struct;
 #ifdef CONFIG_BLOCK
 void put_io_context(struct io_context *ioc);
-void put_io_context_active(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);
-- 
2.30.2

[PATCH 07/14] block: move blk_mq_sched_assign_ioc to blk-ioc.c

[Christoph Hellwig]

Move blk_mq_sched_assign_ioc so that many interfaces from the file can
be marked static.  Rename the function to ioc_find_get_icq as well and
return the icq to simplify the interface.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/bfq-iosched.c  |  2 +-
 block/blk-ioc.c      | 39 +++++++++++++++++++++++++++++++++++----
 block/blk-mq-sched.c | 31 -------------------------------
 block/blk-mq-sched.h |  2 --
 block/blk.h          |  6 +-----
 5 files changed, 37 insertions(+), 43 deletions(-)

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index ecc2e57e68630..2d484d3f7f22a 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -6666,7 +6666,7 @@ static struct bfq_queue *bfq_get_bfqq_handle_split(struct bfq_data *bfqd,
  */
 static void bfq_prepare_request(struct request *rq)
 {
-	blk_mq_sched_assign_ioc(rq);
+	rq->elv.icq = ioc_find_get_icq(rq->q);
 
 	/*
 	 * Regardless of whether we have an icq attached, we have to
diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index f3ff495756cb4..f4f84a2072be7 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -24,7 +24,7 @@ static struct kmem_cache *iocontext_cachep;
  *
  * Increment reference count to @ioc.
  */
-void get_io_context(struct io_context *ioc)
+static void get_io_context(struct io_context *ioc)
 {
 	BUG_ON(atomic_long_read(&ioc->refcount) <= 0);
 	atomic_long_inc(&ioc->refcount);
@@ -248,7 +248,8 @@ void ioc_clear_queue(struct request_queue *q)
 	__ioc_clear_queue(&icq_list);
 }
 
-int create_task_io_context(struct task_struct *task, gfp_t gfp_flags, int node)
+static int create_task_io_context(struct task_struct *task, gfp_t gfp_flags,
+		int node)
 {
 	struct io_context *ioc;
 	int ret;
@@ -397,8 +398,8 @@ EXPORT_SYMBOL(ioc_lookup_icq);
  * The caller is responsible for ensuring @ioc won't go away and @q is
  * alive and will stay alive until this function returns.
  */
-struct io_cq *ioc_create_icq(struct io_context *ioc, struct request_queue *q,
-			     gfp_t gfp_mask)
+static struct io_cq *ioc_create_icq(struct io_context *ioc,
+		struct request_queue *q, gfp_t gfp_mask)
 {
 	struct elevator_type *et = q->elevator->type;
 	struct io_cq *icq;
@@ -441,6 +442,36 @@ struct io_cq *ioc_create_icq(struct io_context *ioc, struct request_queue *q,
 	return icq;
 }
 
+struct io_cq *ioc_find_get_icq(struct request_queue *q)
+{
+	struct io_context *ioc;
+	struct io_cq *icq;
+
+	/* create task io_context, if we don't have one already */
+	if (unlikely(!current->io_context))
+		create_task_io_context(current, GFP_ATOMIC, q->node);
+
+	/*
+	 * May not have an IO context if it's a passthrough request
+	 */
+	ioc = current->io_context;
+	if (!ioc)
+		return NULL;
+
+	spin_lock_irq(&q->queue_lock);
+	icq = ioc_lookup_icq(ioc, q);
+	spin_unlock_irq(&q->queue_lock);
+
+	if (!icq) {
+		icq = ioc_create_icq(ioc, q, GFP_ATOMIC);
+		if (!icq)
+			return NULL;
+	}
+	get_io_context(icq->ioc);
+	return icq;
+}
+EXPORT_SYMBOL_GPL(ioc_find_get_icq);
+
 static int __init blk_ioc_init(void)
 {
 	iocontext_cachep = kmem_cache_create("blkdev_ioc",
diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
index b942b38000e53..0d7257848f7ef 100644
--- a/block/blk-mq-sched.c
+++ b/block/blk-mq-sched.c
@@ -18,37 +18,6 @@
 #include "blk-mq-tag.h"
 #include "blk-wbt.h"
 
-void blk_mq_sched_assign_ioc(struct request *rq)
-{
-	struct request_queue *q = rq->q;
-	struct io_context *ioc;
-	struct io_cq *icq;
-
-	/* create task io_context, if we don't have one already */
-	if (unlikely(!current->io_context))
-		create_task_io_context(current, GFP_ATOMIC, q->node);
-
-	/*
-	 * May not have an IO context if it's a passthrough request
-	 */
-	ioc = current->io_context;
-	if (!ioc)
-		return;
-
-	spin_lock_irq(&q->queue_lock);
-	icq = ioc_lookup_icq(ioc, q);
-	spin_unlock_irq(&q->queue_lock);
-
-	if (!icq) {
-		icq = ioc_create_icq(ioc, q, GFP_ATOMIC);
-		if (!icq)
-			return;
-	}
-	get_io_context(icq->ioc);
-	rq->elv.icq = icq;
-}
-EXPORT_SYMBOL_GPL(blk_mq_sched_assign_ioc);
-
 /*
  * Mark a hardware queue as needing a restart. For shared queues, maintain
  * a count of how many hardware queues are marked for restart.
diff --git a/block/blk-mq-sched.h b/block/blk-mq-sched.h
index 25d1034952b65..0250139724539 100644
--- a/block/blk-mq-sched.h
+++ b/block/blk-mq-sched.h
@@ -8,8 +8,6 @@
 
 #define MAX_SCHED_RQ (16 * BLKDEV_DEFAULT_RQ)
 
-void blk_mq_sched_assign_ioc(struct request *rq);
-
 bool blk_mq_sched_try_merge(struct request_queue *q, struct bio *bio,
 		unsigned int nr_segs, struct request **merged_request);
 bool blk_mq_sched_bio_merge(struct request_queue *q, struct bio *bio,
diff --git a/block/blk.h b/block/blk.h
index a57c84654d0a1..187cb2654ffde 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -363,14 +363,10 @@ static inline unsigned int bio_aligned_discard_max_sectors(
 /*
  * Internal io_context interface
  */
-void get_io_context(struct io_context *ioc);
+struct io_cq *ioc_find_get_icq(struct request_queue *q);
 struct io_cq *ioc_lookup_icq(struct io_context *ioc, struct request_queue *q);
-struct io_cq *ioc_create_icq(struct io_context *ioc, struct request_queue *q,
-			     gfp_t gfp_mask);
 void ioc_clear_queue(struct request_queue *q);
 
-int create_task_io_context(struct task_struct *task, gfp_t gfp_mask, int node);
-
 #ifdef CONFIG_BLK_DEV_THROTTLING_LOW
 extern ssize_t blk_throtl_sample_time_show(struct request_queue *q, char *page);
 extern ssize_t blk_throtl_sample_time_store(struct request_queue *q,
-- 
2.30.2

[PATCH 08/14] block: move the remaining elv.icq handling to the I/O scheduler

[Christoph Hellwig]

After the prepare side has been moved to the only I/O scheduler that
cares, do the same for the cleanup and the NULL initialization.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/bfq-iosched.c | 12 +++++++++++-
 block/blk-ioc.c     |  1 +
 block/blk-mq.c      | 14 +++-----------
 3 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 2d484d3f7f22a..8295b0f96cbfe 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -6569,6 +6569,16 @@ static void bfq_finish_requeue_request(struct request *rq)
 	rq->elv.priv[1] = NULL;
 }
 
+static void bfq_finish_request(struct request *rq)
+{
+	bfq_finish_requeue_request(rq);
+
+	if (rq->elv.icq) {
+		put_io_context(rq->elv.icq->ioc);
+		rq->elv.icq = NULL;
+	}
+}
+
 /*
  * Removes the association between the current task and bfqq, assuming
  * that bic points to the bfq iocontext of the task.
@@ -7388,7 +7398,7 @@ static struct elevator_type iosched_bfq_mq = {
 		.limit_depth		= bfq_limit_depth,
 		.prepare_request	= bfq_prepare_request,
 		.requeue_request        = bfq_finish_requeue_request,
-		.finish_request		= bfq_finish_requeue_request,
+		.finish_request		= bfq_finish_request,
 		.exit_icq		= bfq_exit_icq,
 		.insert_requests	= bfq_insert_requests,
 		.dispatch_request	= bfq_dispatch_request,
diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index f4f84a2072be7..3ba15c867dfa6 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -167,6 +167,7 @@ void put_io_context(struct io_context *ioc)
 	if (free_ioc)
 		kmem_cache_free(iocontext_cachep, ioc);
 }
+EXPORT_SYMBOL_GPL(put_io_context);
 
 /**
  * put_io_context_active - put active reference on ioc
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 82491ab676fb1..7bdef269a5d94 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -400,7 +400,6 @@ static struct request *blk_mq_rq_ctx_init(struct blk_mq_alloc_data *data,
 	if (rq->rq_flags & RQF_ELV) {
 		struct elevator_queue *e = data->q->elevator;
 
-		rq->elv.icq = NULL;
 		INIT_HLIST_NODE(&rq->hash);
 		RB_CLEAR_NODE(&rq->rb_node);
 
@@ -631,16 +630,9 @@ void blk_mq_free_request(struct request *rq)
 	struct request_queue *q = rq->q;
 	struct blk_mq_hw_ctx *hctx = rq->mq_hctx;
 
-	if (rq->rq_flags & RQF_ELVPRIV) {
-		struct elevator_queue *e = q->elevator;
-
-		if (e->type->ops.finish_request)
-			e->type->ops.finish_request(rq);
-		if (rq->elv.icq) {
-			put_io_context(rq->elv.icq->ioc);
-			rq->elv.icq = NULL;
-		}
-	}
+	if ((rq->rq_flags & RQF_ELVPRIV) &&
+	    q->elevator->type->ops.finish_request)
+		q->elevator->type->ops.finish_request(rq);
 
 	if (rq->rq_flags & RQF_MQ_INFLIGHT)
 		__blk_mq_dec_active_requests(hctx);
-- 
2.30.2

[PATCH 09/14] block: remove get_io_context_active

[Christoph Hellwig]

Fold it into it's only caller, and remove a lof of the debug checks
that are not needed.

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

diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index 3ba15c867dfa6..cc4eb2ba87f74 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -173,7 +173,7 @@ EXPORT_SYMBOL_GPL(put_io_context);
  * put_io_context_active - put active reference on ioc
  * @ioc: ioc of interest
  *
- * Undo get_io_context_active().  If active reference reaches zero after
+ * 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)
@@ -333,11 +333,9 @@ 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) {
-		get_io_context_active(ioc);
-
-		WARN_ON_ONCE(atomic_read(&ioc->nr_tasks) <= 0);
+		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)) {
 		new_ioc = get_task_io_context(tsk, GFP_KERNEL, NUMA_NO_NODE);
diff --git a/include/linux/iocontext.h b/include/linux/iocontext.h
index 3ba45953d5228..c1229fbd6691c 100644
--- a/include/linux/iocontext.h
+++ b/include/linux/iocontext.h
@@ -113,22 +113,6 @@ struct io_context {
 	struct work_struct release_work;
 };
 
-/**
- * get_io_context_active - get active reference on ioc
- * @ioc: ioc of interest
- *
- * Only iocs with active reference can issue new IOs.  This function
- * acquires an active reference on @ioc.  The caller must already have an
- * active reference on @ioc.
- */
-static inline void get_io_context_active(struct io_context *ioc)
-{
-	WARN_ON_ONCE(atomic_long_read(&ioc->refcount) <= 0);
-	WARN_ON_ONCE(atomic_read(&ioc->active_ref) <= 0);
-	atomic_long_inc(&ioc->refcount);
-	atomic_inc(&ioc->active_ref);
-}
-
 struct task_struct;
 #ifdef CONFIG_BLOCK
 void put_io_context(struct io_context *ioc);
-- 
2.30.2

[PATCH 10/14] block: factor out a alloc_io_context helper

[Christoph Hellwig]

Factor out a helper that just allocate an I/O context.

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

diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index cc4eb2ba87f74..b42fbb82d5c0c 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -249,18 +249,15 @@ void ioc_clear_queue(struct request_queue *q)
 	__ioc_clear_queue(&icq_list);
 }
 
-static int create_task_io_context(struct task_struct *task, gfp_t gfp_flags,
-		int node)
+static struct io_context *alloc_io_context(gfp_t gfp_flags, int node)
 {
 	struct io_context *ioc;
-	int ret;
 
 	ioc = kmem_cache_alloc_node(iocontext_cachep, gfp_flags | __GFP_ZERO,
 				    node);
 	if (unlikely(!ioc))
-		return -ENOMEM;
+		return NULL;
 
-	/* initialize */
 	atomic_long_set(&ioc->refcount, 1);
 	atomic_set(&ioc->nr_tasks, 1);
 	atomic_set(&ioc->active_ref, 1);
@@ -268,6 +265,18 @@ static int create_task_io_context(struct task_struct *task, gfp_t gfp_flags,
 	INIT_RADIX_TREE(&ioc->icq_tree, GFP_ATOMIC);
 	INIT_HLIST_HEAD(&ioc->icq_list);
 	INIT_WORK(&ioc->release_work, ioc_release_fn);
+	return ioc;
+}
+
+static int create_task_io_context(struct task_struct *task, gfp_t gfp_flags,
+		int node)
+{
+	struct io_context *ioc;
+	int ret;
+
+	ioc = alloc_io_context(gfp_flags, node);
+	if (!ioc)
+		return -ENOMEM;
 
 	/*
 	 * Try to install.  ioc shouldn't be installed if someone else
-- 
2.30.2

[PATCH 11/14] block: use alloc_io_context in __copy_io

[Christoph Hellwig]

In __copy_io we know that the newly allocate task_struct does not have
an I/O context yet and is not exiting.  So just allocate the I/O context
struct and install it directly.  There is no need to lock the task
either as it is just being created.

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

diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index b42fbb82d5c0c..f06d1040442c3 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -336,7 +336,6 @@ struct io_context *get_task_io_context(struct task_struct *task,
 int __copy_io(unsigned long clone_flags, struct task_struct *tsk)
 {
 	struct io_context *ioc = current->io_context;
-	struct io_context *new_ioc;
 
 	/*
 	 * Share io context with parent, if CLONE_IO is set
@@ -347,12 +346,10 @@ int __copy_io(unsigned long clone_flags, struct task_struct *tsk)
 		atomic_inc(&ioc->nr_tasks);
 		tsk->io_context = ioc;
 	} else if (ioprio_valid(ioc->ioprio)) {
-		new_ioc = get_task_io_context(tsk, GFP_KERNEL, NUMA_NO_NODE);
-		if (unlikely(!new_ioc))
+		tsk->io_context = alloc_io_context(GFP_KERNEL, NUMA_NO_NODE);
+		if (!tsk->io_context)
 			return -ENOMEM;
-
-		new_ioc->ioprio = ioc->ioprio;
-		put_io_context(new_ioc);
+		tsk->io_context->ioprio = ioc->ioprio;
 	}
 
 	return 0;
-- 
2.30.2

[PATCH 12/14] block: return the io_context from create_task_io_context

[Christoph Hellwig]

Grab a reference to the newly allocated or existing io_context in
create_task_io_context and return it.  This simplifies the callers and
removes the need for double lookups.

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

diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index f06d1040442c3..5bfe810496fca 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -268,15 +268,14 @@ static struct io_context *alloc_io_context(gfp_t gfp_flags, int node)
 	return ioc;
 }
 
-static int create_task_io_context(struct task_struct *task, gfp_t gfp_flags,
-		int node)
+static struct io_context *create_task_io_context(struct task_struct *task,
+		gfp_t gfp_flags, int node)
 {
 	struct io_context *ioc;
-	int ret;
 
 	ioc = alloc_io_context(gfp_flags, node);
 	if (!ioc)
-		return -ENOMEM;
+		return NULL;
 
 	/*
 	 * Try to install.  ioc shouldn't be installed if someone else
@@ -292,11 +291,11 @@ static int create_task_io_context(struct task_struct *task, gfp_t gfp_flags,
 	else
 		kmem_cache_free(iocontext_cachep, ioc);
 
-	ret = task->io_context ? 0 : -EBUSY;
-
+	ioc = task->io_context;
+	if (ioc)
+		get_io_context(ioc);
 	task_unlock(task);
-
-	return ret;
+	return ioc;
 }
 
 /**
@@ -319,18 +318,15 @@ struct io_context *get_task_io_context(struct task_struct *task,
 
 	might_sleep_if(gfpflags_allow_blocking(gfp_flags));
 
-	do {
-		task_lock(task);
-		ioc = task->io_context;
-		if (likely(ioc)) {
-			get_io_context(ioc);
-			task_unlock(task);
-			return ioc;
-		}
+	task_lock(task);
+	ioc = task->io_context;
+	if (unlikely(!ioc)) {
 		task_unlock(task);
-	} while (!create_task_io_context(task, gfp_flags, node));
-
-	return NULL;
+		return create_task_io_context(task, gfp_flags, node);
+	}
+	get_io_context(ioc);
+	task_unlock(task);
+	return ioc;
 }
 
 int __copy_io(unsigned long clone_flags, struct task_struct *tsk)
@@ -449,30 +445,28 @@ static struct io_cq *ioc_create_icq(struct io_context *ioc,
 
 struct io_cq *ioc_find_get_icq(struct request_queue *q)
 {
-	struct io_context *ioc;
-	struct io_cq *icq;
-
-	/* create task io_context, if we don't have one already */
-	if (unlikely(!current->io_context))
-		create_task_io_context(current, GFP_ATOMIC, q->node);
+	struct io_context *ioc = current->io_context;
+	struct io_cq *icq = NULL;
 
-	/*
-	 * May not have an IO context if it's a passthrough request
-	 */
-	ioc = current->io_context;
-	if (!ioc)
-		return NULL;
+	if (unlikely(!ioc)) {
+		ioc = create_task_io_context(current, GFP_ATOMIC, q->node);
+		if (!ioc)
+			return NULL;
+	} else {
+		get_io_context(ioc);
 
-	spin_lock_irq(&q->queue_lock);
-	icq = ioc_lookup_icq(ioc, q);
-	spin_unlock_irq(&q->queue_lock);
+		spin_lock_irq(&q->queue_lock);
+		icq = ioc_lookup_icq(ioc, q);
+		spin_unlock_irq(&q->queue_lock);
+	}
 
 	if (!icq) {
 		icq = ioc_create_icq(ioc, q, GFP_ATOMIC);
-		if (!icq)
+		if (!icq) {
+			put_io_context(ioc);
 			return NULL;
+		}
 	}
-	get_io_context(icq->ioc);
 	return icq;
 }
 EXPORT_SYMBOL_GPL(ioc_find_get_icq);
-- 
2.30.2

[PATCH 13/14] block: simplify ioc_create_icq

[Christoph Hellwig]

Remove the ioc and gfp_mask argument, which are hard coded by the caller.

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

diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index 5bfe810496fca..c56648f7cad47 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -389,9 +389,7 @@ EXPORT_SYMBOL(ioc_lookup_icq);
 
 /**
  * ioc_create_icq - create and link io_cq
- * @ioc: io_context of interest
  * @q: request_queue of interest
- * @gfp_mask: allocation mask
  *
  * Make sure io_cq linking @ioc and @q exists.  If icq doesn't exist, they
  * will be created using @gfp_mask.
@@ -399,19 +397,19 @@ EXPORT_SYMBOL(ioc_lookup_icq);
  * The caller is responsible for ensuring @ioc won't go away and @q is
  * alive and will stay alive until this function returns.
  */
-static struct io_cq *ioc_create_icq(struct io_context *ioc,
-		struct request_queue *q, gfp_t gfp_mask)
+static struct io_cq *ioc_create_icq(struct request_queue *q)
 {
+	struct io_context *ioc = current->io_context;
 	struct elevator_type *et = q->elevator->type;
 	struct io_cq *icq;
 
 	/* allocate stuff */
-	icq = kmem_cache_alloc_node(et->icq_cache, gfp_mask | __GFP_ZERO,
+	icq = kmem_cache_alloc_node(et->icq_cache, GFP_ATOMIC | __GFP_ZERO,
 				    q->node);
 	if (!icq)
 		return NULL;
 
-	if (radix_tree_maybe_preload(gfp_mask) < 0) {
+	if (radix_tree_maybe_preload(GFP_ATOMIC) < 0) {
 		kmem_cache_free(et->icq_cache, icq);
 		return NULL;
 	}
@@ -461,7 +459,7 @@ struct io_cq *ioc_find_get_icq(struct request_queue *q)
 	}
 
 	if (!icq) {
-		icq = ioc_create_icq(ioc, q, GFP_ATOMIC);
+		icq = ioc_create_icq(q);
 		if (!icq) {
 			put_io_context(ioc);
 			return NULL;
-- 
2.30.2

[PATCH 14/14] block: simplify ioc_lookup_icq

[Christoph Hellwig]

Remove the ioc argument as it always points to current->io_context.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/bfq-iosched.c | 2 +-
 block/blk-ioc.c     | 8 ++++----
 block/blk.h         | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 8295b0f96cbfe..0c612a9116967 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -444,7 +444,7 @@ static struct bfq_io_cq *bfq_bic_lookup(struct request_queue *q)
 		return NULL;
 
 	spin_lock_irqsave(&q->queue_lock, flags);
-	icq = icq_to_bic(ioc_lookup_icq(current->io_context, q));
+	icq = icq_to_bic(ioc_lookup_icq(q));
 	spin_unlock_irqrestore(&q->queue_lock, flags);
 
 	return icq;
diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index c56648f7cad47..536fb496ad763 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -353,14 +353,14 @@ int __copy_io(unsigned long clone_flags, struct task_struct *tsk)
 
 /**
  * ioc_lookup_icq - lookup io_cq from ioc
- * @ioc: the associated io_context
  * @q: the associated request_queue
  *
  * Look up io_cq associated with @ioc - @q pair from @ioc.  Must be called
  * with @q->queue_lock held.
  */
-struct io_cq *ioc_lookup_icq(struct io_context *ioc, struct request_queue *q)
+struct io_cq *ioc_lookup_icq(struct request_queue *q)
 {
+	struct io_context *ioc = current->io_context;
 	struct io_cq *icq;
 
 	lockdep_assert_held(&q->queue_lock);
@@ -430,7 +430,7 @@ static struct io_cq *ioc_create_icq(struct request_queue *q)
 			et->ops.init_icq(icq);
 	} else {
 		kmem_cache_free(et->icq_cache, icq);
-		icq = ioc_lookup_icq(ioc, q);
+		icq = ioc_lookup_icq(q);
 		if (!icq)
 			printk(KERN_ERR "cfq: icq link failed!\n");
 	}
@@ -454,7 +454,7 @@ struct io_cq *ioc_find_get_icq(struct request_queue *q)
 		get_io_context(ioc);
 
 		spin_lock_irq(&q->queue_lock);
-		icq = ioc_lookup_icq(ioc, q);
+		icq = ioc_lookup_icq(q);
 		spin_unlock_irq(&q->queue_lock);
 	}
 
diff --git a/block/blk.h b/block/blk.h
index 187cb2654ffde..3be0fdf76c9a5 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -364,7 +364,7 @@ static inline unsigned int bio_aligned_discard_max_sectors(
  * Internal io_context interface
  */
 struct io_cq *ioc_find_get_icq(struct request_queue *q);
-struct io_cq *ioc_lookup_icq(struct io_context *ioc, struct request_queue *q);
+struct io_cq *ioc_lookup_icq(struct request_queue *q);
 void ioc_clear_queue(struct request_queue *q);
 
 #ifdef CONFIG_BLK_DEV_THROTTLING_LOW
-- 
2.30.2

Re: cleanup I/O context handling

[Jens Axboe]

On Fri, 26 Nov 2021 12:58:03 +0100, Christoph Hellwig wrote:
> this series does a little spring cleaning of the I/O context handling/
> 
> Subject:
>  block/bfq-iosched.c                   |   41 ++++++------
>  block/blk-ioc.c                       |  115 +++++++++++++++++++++++++---------
>  block/blk-mq-sched.c                  |   35 ----------
>  block/blk-mq-sched.h                  |    3
>  block/blk-mq.c                        |   14 ----
>  block/blk.h                           |    8 --
>  drivers/infiniband/hw/qib/qib_verbs.c |    4 -
>  include/linux/iocontext.h             |   40 +++--------
>  kernel/fork.c                         |   26 -------
>  9 files changed, 128 insertions(+), 158 deletions(-)
> 
> [...]

Applied, thanks!

[01/14] RDMA/qib: rename copy_io to qib_copy_io
        commit: aa6c81e0dbe5ed782cc4cdb9274eaf1e14c07983
[02/14] fork: move copy_io to block/blk-ioc.c
        commit: 8a8d3786e0ea1793eca69d1e071141bff16d55d7
[03/14] bfq: simplify bfq_bic_lookup
        commit: 91d84d8eef716bfba98263493945897beff5e26a
[04/14] bfq: use bfq_bic_lookup in bfq_limit_depth
        commit: 4d6d46def2117d08edf72b080e768da8e3d36fe8
[05/14] Revert "block: Provide blk_mq_sched_get_icq()"
        commit: b2b522fb21b1a3dd10a1419884562114ab653bec
[06/14] block: mark put_io_context_active static
        commit: 6b939dcfa41384d18478ec34083ed64b3c485876
[07/14] block: move blk_mq_sched_assign_ioc to blk-ioc.c
        commit: 0afb8931998dad3d4ed125684e2dc74fca7b1714
[08/14] block: move the remaining elv.icq handling to the I/O scheduler
        commit: f390716138b4c5c32b883a047f9a1f38ef5b8c0f
[09/14] block: remove get_io_context_active
        commit: b9e117800715bad4920bc8ab8b286ffdedb22979
[10/14] block: factor out a alloc_io_context helper
        commit: a3335d4269a799c85395cb1a0712dd54b54f6497
[11/14] block: use alloc_io_context in __copy_io
        commit: 6767435a95a26560c2460e43aa26d00fb5b50e71
[12/14] block: return the io_context from create_task_io_context
        commit: af04d9b6c9037c4ff4312a8e1e58fd96a05c3ca5
[13/14] block: simplify ioc_create_icq
        commit: 22e0aa975c1fc52e05d9e9aa637e4833370eefb6
[14/14] block: simplify ioc_lookup_icq
        commit: c3ad7dd4999b6f4603dcdbbea0b7860c9c02bd86

Best regards,
-- 
Jens Axboe

Re: [PATCH 04/14] bfq: use bfq_bic_lookup in bfq_limit_depth

[Jan Kara]

On Fri 26-11-21 12:58:07, Christoph Hellwig wrote:
> No need to create a new I/O context if there is none present yet in
> ->limit_depth.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  block/bfq-iosched.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
> index c990c6409c119..ecc2e57e68630 100644
> --- a/block/bfq-iosched.c
> +++ b/block/bfq-iosched.c
> @@ -663,7 +663,7 @@ static bool bfqq_request_over_limit(struct bfq_queue *bfqq, int limit)
>  static void bfq_limit_depth(unsigned int op, struct blk_mq_alloc_data *data)
>  {
>  	struct bfq_data *bfqd = data->q->elevator->elevator_data;
> -	struct bfq_io_cq *bic = icq_to_bic(blk_mq_sched_get_icq(data->q));
> +	struct bfq_io_cq *bic = bfq_bic_lookup(data->q);

Maybe I'm missing something but bfq_limit_depth() needs to know to which
BFQ queue (and consequently blkcg) this IO is going to be added. And to be
able to lookup this queue we are using IO context. So AFAICT we need the
IO context allocated already in bfq_limit_depth()?

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 04/14] bfq: use bfq_bic_lookup in bfq_limit_depth

[Christoph Hellwig]

On Mon, Nov 29, 2021 at 05:09:25PM +0100, Jan Kara wrote:
> On Fri 26-11-21 12:58:07, Christoph Hellwig wrote:
> > No need to create a new I/O context if there is none present yet in
> > ->limit_depth.
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > ---
> >  block/bfq-iosched.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
> > index c990c6409c119..ecc2e57e68630 100644
> > --- a/block/bfq-iosched.c
> > +++ b/block/bfq-iosched.c
> > @@ -663,7 +663,7 @@ static bool bfqq_request_over_limit(struct bfq_queue *bfqq, int limit)
> >  static void bfq_limit_depth(unsigned int op, struct blk_mq_alloc_data *data)
> >  {
> >  	struct bfq_data *bfqd = data->q->elevator->elevator_data;
> > -	struct bfq_io_cq *bic = icq_to_bic(blk_mq_sched_get_icq(data->q));
> > +	struct bfq_io_cq *bic = bfq_bic_lookup(data->q);
> 
> Maybe I'm missing something but bfq_limit_depth() needs to know to which
> BFQ queue (and consequently blkcg) this IO is going to be added. And to be
> able to lookup this queue we are using IO context. So AFAICT we need the
> IO context allocated already in bfq_limit_depth()?

But by allocating it you won't now anything, as it will still be empty.

Re: [PATCH 04/14] bfq: use bfq_bic_lookup in bfq_limit_depth

[Jan Kara]

On Tue 30-11-21 07:39:55, Christoph Hellwig wrote:
> On Mon, Nov 29, 2021 at 05:09:25PM +0100, Jan Kara wrote:
> > On Fri 26-11-21 12:58:07, Christoph Hellwig wrote:
> > > No need to create a new I/O context if there is none present yet in
> > > ->limit_depth.
> > > 
> > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > > ---
> > >  block/bfq-iosched.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
> > > index c990c6409c119..ecc2e57e68630 100644
> > > --- a/block/bfq-iosched.c
> > > +++ b/block/bfq-iosched.c
> > > @@ -663,7 +663,7 @@ static bool bfqq_request_over_limit(struct bfq_queue *bfqq, int limit)
> > >  static void bfq_limit_depth(unsigned int op, struct blk_mq_alloc_data *data)
> > >  {
> > >  	struct bfq_data *bfqd = data->q->elevator->elevator_data;
> > > -	struct bfq_io_cq *bic = icq_to_bic(blk_mq_sched_get_icq(data->q));
> > > +	struct bfq_io_cq *bic = bfq_bic_lookup(data->q);
> > 
> > Maybe I'm missing something but bfq_limit_depth() needs to know to which
> > BFQ queue (and consequently blkcg) this IO is going to be added. And to be
> > able to lookup this queue we are using IO context. So AFAICT we need the
> > IO context allocated already in bfq_limit_depth()?
> 
> But by allocating it you won't now anything, as it will still be empty.

You're right, we'll create only IO context and corresponding bfq_io_cq but
we won't actually create bfqq in bfq_limit_depth() anyway and without that
bfq_limit_depth() isn't going to do more. So your patch indeed does not
change anything in that regard.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: cleanup I/O context handling

[Jan Kara]

On Sat 27-11-21 06:40:20, Jens Axboe wrote:
> On Fri, 26 Nov 2021 12:58:03 +0100, Christoph Hellwig wrote:
> > this series does a little spring cleaning of the I/O context handling/
> > 
> > Subject:
> >  block/bfq-iosched.c                   |   41 ++++++------
> >  block/blk-ioc.c                       |  115 +++++++++++++++++++++++++---------
> >  block/blk-mq-sched.c                  |   35 ----------
> >  block/blk-mq-sched.h                  |    3
> >  block/blk-mq.c                        |   14 ----
> >  block/blk.h                           |    8 --
> >  drivers/infiniband/hw/qib/qib_verbs.c |    4 -
> >  include/linux/iocontext.h             |   40 +++--------
> >  kernel/fork.c                         |   26 -------
> >  9 files changed, 128 insertions(+), 158 deletions(-)
> > 
> > [...]
> 
> Applied, thanks!

FWIW I've read the whole series now and it looks good to me so feel free to
add:

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

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR