[PATCHSET 0/3] Misc block cleanups

[PATCHSET 0/3] Misc block cleanups

[Jens Axboe]

Hi,

First patch avoids setting up an io_context for the cases where we don't
need them, second patch optimizes re-writing to the bio if not needed, and
prunes a huge chunk of memory in the request_queue and makes it dynamic
instead.

-- 
Jens Axboe

[PATCH 1/3] block: move io_context creation into where it's needed

[Jens Axboe]

The only user of the io_context for IO is BFQ, yet we put the checking
and logic of it into the normal IO path.

Move the assignment and creation into BFQ.

Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 block/bfq-iosched.c  | 6 ++++++
 block/blk-core.c     | 9 ---------
 block/blk-ioc.c      | 1 +
 block/blk-mq-sched.c | 1 +
 block/blk-mq.c       | 3 ---
 5 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index fec18118dc30..a82475361a7e 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -6573,6 +6573,12 @@ static struct bfq_queue *bfq_get_bfqq_handle_split(struct bfq_data *bfqd,
  */
 static void bfq_prepare_request(struct request *rq)
 {
+	/* create task io_context, if we don't have one already */
+	if (unlikely(!current->io_context))
+		create_task_io_context(current, GFP_ATOMIC, rq->q->node);
+
+	blk_mq_sched_assign_ioc(rq);
+
 	/*
 	 * Regardless of whether we have an icq attached, we have to
 	 * clear the scheduler pointers, as they might point to
diff --git a/block/blk-core.c b/block/blk-core.c
index 6443f2dfe43e..6ae8297b033f 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -750,15 +750,6 @@ noinline_for_stack bool submit_bio_checks(struct bio *bio)
 		break;
 	}
 
-	/*
-	 * Various block parts want %current->io_context, so allocate it up
-	 * front rather than dealing with lots of pain to allocate it only
-	 * where needed. This may fail and the block layer knows how to live
-	 * with it.
-	 */
-	if (unlikely(!current->io_context))
-		create_task_io_context(current, GFP_ATOMIC, q->node);
-
 	if (blk_throtl_bio(bio))
 		return false;
 
diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index 57299f860d41..736e0280d76f 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -286,6 +286,7 @@ int create_task_io_context(struct task_struct *task, gfp_t gfp_flags, int node)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(create_task_io_context);
 
 /**
  * get_task_io_context - get io_context of a task
diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
index ba21449439cc..550e27189be2 100644
--- a/block/blk-mq-sched.c
+++ b/block/blk-mq-sched.c
@@ -43,6 +43,7 @@ void blk_mq_sched_assign_ioc(struct request *rq)
 	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
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 4c00b22590cc..20a6445f6a01 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -406,9 +406,6 @@ static struct request *blk_mq_rq_ctx_init(struct blk_mq_alloc_data *data,
 
 		if (!op_is_flush(data->cmd_flags) &&
 		    e->type->ops.prepare_request) {
-			if (e->type->icq_cache)
-				blk_mq_sched_assign_ioc(rq);
-
 			e->type->ops.prepare_request(rq);
 			rq->rq_flags |= RQF_ELVPRIV;
 		}
-- 
2.34.0

Re: [PATCH 1/3] block: move io_context creation into where it's needed

[Christoph Hellwig]

> +	/* create task io_context, if we don't have one already */
> +	if (unlikely(!current->io_context))
> +		create_task_io_context(current, GFP_ATOMIC, rq->q->node);

Wouldn't it be nicer to have an interface that hides the check
and the somewhat pointless current argument?  And name that with a
blk_ prefix?

> +
> +	blk_mq_sched_assign_ioc(rq);

But thinking about this a little more:

struct io_context has two uses, one is the unsigned short ioprio,
and the other is the whole bfq mess.

Can't we just split the ioprio out (we'll find a hole somewhere
in task_struct) and then just allocate the io_context in
blk_mq_sched_assign_ioc, which would also avoid the pointless
ioc_lookup_icq on a newly allocated io_context.  I'd also really
expect blk_mq_sched_assign_ioc to be implemented in blk-ioc.c.

While we're at it: bfq_bic_lookup is a really weird helper which
gets passed an unused bfqd argument.

Re: [PATCH 1/3] block: move io_context creation into where it's needed

[Jens Axboe]

On 11/23/21 9:39 AM, Christoph Hellwig wrote:
>> +	/* create task io_context, if we don't have one already */
>> +	if (unlikely(!current->io_context))
>> +		create_task_io_context(current, GFP_ATOMIC, rq->q->node);
> 
> Wouldn't it be nicer to have an interface that hides the check
> and the somewhat pointless current argument?  And name that with a
> blk_ prefix?

Yeah, we can do that.

>> +
>> +	blk_mq_sched_assign_ioc(rq);
> 
> But thinking about this a little more:
> 
> struct io_context has two uses, one is the unsigned short ioprio,
> and the other is the whole bfq mess.
> 
> Can't we just split the ioprio out (we'll find a hole somewhere
> in task_struct) and then just allocate the io_context in
> blk_mq_sched_assign_ioc, which would also avoid the pointless
> ioc_lookup_icq on a newly allocated io_context.  I'd also really
> expect blk_mq_sched_assign_ioc to be implemented in blk-ioc.c.

It would be nice to decouple ioprio from the io_context, and just
leave the io_context for BFQ essentially.

I'll give it a shot.

> While we're at it: bfq_bic_lookup is a really weird helper which
> gets passed an unused bfqd argument.

Unsurprising.

-- 
Jens Axboe

Re: [PATCH 1/3] block: move io_context creation into where it's needed

[Jens Axboe]

On 11/23/21 9:46 AM, Jens Axboe wrote:
> On 11/23/21 9:39 AM, Christoph Hellwig wrote:
>>> +	/* create task io_context, if we don't have one already */
>>> +	if (unlikely(!current->io_context))
>>> +		create_task_io_context(current, GFP_ATOMIC, rq->q->node);
>>
>> Wouldn't it be nicer to have an interface that hides the check
>> and the somewhat pointless current argument?  And name that with a
>> blk_ prefix?
> 
> Yeah, we can do that.
> 
>>> +
>>> +	blk_mq_sched_assign_ioc(rq);
>>
>> But thinking about this a little more:
>>
>> struct io_context has two uses, one is the unsigned short ioprio,
>> and the other is the whole bfq mess.
>>
>> Can't we just split the ioprio out (we'll find a hole somewhere
>> in task_struct) and then just allocate the io_context in
>> blk_mq_sched_assign_ioc, which would also avoid the pointless
>> ioc_lookup_icq on a newly allocated io_context.  I'd also really
>> expect blk_mq_sched_assign_ioc to be implemented in blk-ioc.c.
> 
> It would be nice to decouple ioprio from the io_context, and just
> leave the io_context for BFQ essentially.
> 
> I'll give it a shot.

Actually may not be that trivial - if a thread is cloned with CLONE_IO,
then it shares the io_context, and hence also the io priority. That will
auto-propagate if one linked task changes it, and putting it in the
task_struct would break that.

So I don't think we can do that - which is a shame, as it would be a
nice cleanup.

-- 
Jens Axboe

Re: [PATCH 1/3] block: move io_context creation into where it's needed

[Christoph Hellwig]

On Tue, Nov 23, 2021 at 09:53:58AM -0700, Jens Axboe wrote:
> Actually may not be that trivial - if a thread is cloned with CLONE_IO,
> then it shares the io_context, and hence also the io priority. That will
> auto-propagate if one linked task changes it, and putting it in the
> task_struct would break that.
> 
> So I don't think we can do that - which is a shame, as it would be a
> nice cleanup.

Indeed.  We should still be able to move the call to
create_task_io_context into blk_mq_sched_assign_ioc

Re: [PATCH 1/3] block: move io_context creation into where it's needed

[Jens Axboe]

On 11/23/21 9:56 AM, Christoph Hellwig wrote:
> On Tue, Nov 23, 2021 at 09:53:58AM -0700, Jens Axboe wrote:
>> Actually may not be that trivial - if a thread is cloned with CLONE_IO,
>> then it shares the io_context, and hence also the io priority. That will
>> auto-propagate if one linked task changes it, and putting it in the
>> task_struct would break that.
>>
>> So I don't think we can do that - which is a shame, as it would be a
>> nice cleanup.
> 
> Indeed.  We should still be able to move the call to
> create_task_io_context into blk_mq_sched_assign_ioc

Definitely, I'll make that change.

-- 
Jens Axboe

[PATCH 2/3] blk-ioprio: don't set bio priority if not needed

[Jens Axboe]

We don't need to write to the bio if:

1) No ioprio value has ever been assigned to the blkcg
2) We wouldn't anyway, depending on bio and blkcg IO priority

Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 block/blk-ioprio.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/block/blk-ioprio.c b/block/blk-ioprio.c
index 332a07761bf8..2e7f10e1c03f 100644
--- a/block/blk-ioprio.c
+++ b/block/blk-ioprio.c
@@ -62,6 +62,7 @@ struct ioprio_blkg {
 struct ioprio_blkcg {
 	struct blkcg_policy_data cpd;
 	enum prio_policy	 prio_policy;
+	bool			 prio_set;
 };
 
 static inline struct ioprio_blkg *pd_to_ioprio(struct blkg_policy_data *pd)
@@ -112,7 +113,7 @@ static ssize_t ioprio_set_prio_policy(struct kernfs_open_file *of, char *buf,
 	if (ret < 0)
 		return ret;
 	blkcg->prio_policy = ret;
-
+	blkcg->prio_set = true;
 	return nbytes;
 }
 
@@ -190,6 +191,10 @@ static void blkcg_ioprio_track(struct rq_qos *rqos, struct request *rq,
 			       struct bio *bio)
 {
 	struct ioprio_blkcg *blkcg = ioprio_blkcg_from_bio(bio);
+	u16 prio;
+
+	if (!blkcg->prio_set)
+		return;
 
 	/*
 	 * Except for IOPRIO_CLASS_NONE, higher I/O priority numbers
@@ -199,8 +204,10 @@ static void blkcg_ioprio_track(struct rq_qos *rqos, struct request *rq,
 	 * bio I/O priority is not modified. If the bio I/O priority equals
 	 * IOPRIO_CLASS_NONE, the cgroup I/O priority is assigned to the bio.
 	 */
-	bio->bi_ioprio = max_t(u16, bio->bi_ioprio,
-			       IOPRIO_PRIO_VALUE(blkcg->prio_policy, 0));
+	prio = max_t(u16, bio->bi_ioprio,
+			IOPRIO_PRIO_VALUE(blkcg->prio_policy, 0));
+	if (prio > bio->bi_ioprio)
+		bio->bi_ioprio = prio;
 }
 
 static void blkcg_ioprio_exit(struct rq_qos *rqos)
-- 
2.34.0

[PATCH 3/3] block: only allocate poll_stats if there's a user of them

[Jens Axboe]

This is essentially never used, yet it's about 1/3rd of the total
queue size. Allocate it when needed, and don't embed it in the queue.

Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 block/blk-mq.c         | 20 ++++++++++++++++++--
 block/blk-stat.c       |  6 ------
 block/blk-sysfs.c      |  1 +
 include/linux/blkdev.h |  9 +++++++--
 4 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 20a6445f6a01..cb41c441aa8f 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -4577,9 +4577,25 @@ EXPORT_SYMBOL_GPL(blk_mq_update_nr_hw_queues);
 /* Enable polling stats and return whether they were already enabled. */
 static bool blk_poll_stats_enable(struct request_queue *q)
 {
-	if (test_bit(QUEUE_FLAG_POLL_STATS, &q->queue_flags) ||
-	    blk_queue_flag_test_and_set(QUEUE_FLAG_POLL_STATS, q))
+	struct blk_rq_stat *poll_stat;
+
+	if (test_bit(QUEUE_FLAG_POLL_STATS, &q->queue_flags))
 		return true;
+
+	poll_stat = kzalloc(BLK_MQ_POLL_STATS_BKTS * sizeof(*poll_stat),
+				GFP_ATOMIC);
+	if (!poll_stat)
+		return false;
+
+	spin_lock_irq(&q->stats->lock);
+	if (blk_queue_flag_test_and_set(QUEUE_FLAG_POLL_STATS, q)) {
+		spin_unlock_irq(&q->stats->lock);
+		kfree(poll_stat);
+		return true;
+	}
+	q->poll_stat = poll_stat;
+	spin_unlock_irq(&q->stats->lock);
+
 	blk_stat_add_callback(q, q->poll_cb);
 	return false;
 }
diff --git a/block/blk-stat.c b/block/blk-stat.c
index ae3dd1fb8e61..7ba504166d1b 100644
--- a/block/blk-stat.c
+++ b/block/blk-stat.c
@@ -12,12 +12,6 @@
 #include "blk-mq.h"
 #include "blk.h"
 
-struct blk_queue_stats {
-	struct list_head callbacks;
-	spinlock_t lock;
-	bool enable_accounting;
-};
-
 void blk_rq_stat_init(struct blk_rq_stat *stat)
 {
 	stat->min = -1ULL;
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index cd75b0f73dc6..e1b846ec58cb 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -790,6 +790,7 @@ static void blk_release_queue(struct kobject *kobj)
 	blk_stat_free_callback(q->poll_cb);
 
 	blk_free_queue_stats(q->stats);
+	kfree(q->poll_stat);
 
 	blk_exit_queue(q);
 
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index bd4370baccca..b46fd2a80062 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -28,10 +28,15 @@ struct blk_flush_queue;
 struct kiocb;
 struct pr_ops;
 struct rq_qos;
-struct blk_queue_stats;
 struct blk_stat_callback;
 struct blk_crypto_profile;
 
+struct blk_queue_stats {
+	struct list_head callbacks;
+	spinlock_t lock;
+	bool enable_accounting;
+};
+
 /* Must be consistent with blk_mq_poll_stats_bkt() */
 #define BLK_MQ_POLL_STATS_BKTS 16
 
@@ -267,7 +272,7 @@ struct request_queue {
 	int			poll_nsec;
 
 	struct blk_stat_callback	*poll_cb;
-	struct blk_rq_stat	poll_stat[BLK_MQ_POLL_STATS_BKTS];
+	struct blk_rq_stat	*poll_stat;
 
 	struct timer_list	timeout;
 	struct work_struct	timeout_work;
-- 
2.34.0

Re: [PATCH 3/3] block: only allocate poll_stats if there's a user of them

[Johannes Thumshirn]

On 23/11/2021 17:18, Jens Axboe wrote:
> +	poll_stat = kzalloc(BLK_MQ_POLL_STATS_BKTS * sizeof(*poll_stat),
> +				GFP_ATOMIC);

Why not kcalloc()?

Re: [PATCH 3/3] block: only allocate poll_stats if there's a user of them

[Jens Axboe]

On 11/23/21 9:21 AM, Johannes Thumshirn wrote:
> On 23/11/2021 17:18, Jens Axboe wrote:
>> +	poll_stat = kzalloc(BLK_MQ_POLL_STATS_BKTS * sizeof(*poll_stat),
>> +				GFP_ATOMIC);
> 
> Why not kcalloc()?

Sure, can do.

-- 
Jens Axboe

Re: [PATCH 3/3] block: only allocate poll_stats if there's a user of them

[Christoph Hellwig]

On Tue, Nov 23, 2021 at 09:18:13AM -0700, Jens Axboe wrote:
> This is essentially never used, yet it's about 1/3rd of the total
> queue size. Allocate it when needed, and don't embed it in the queue.
> 
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> ---
>  block/blk-mq.c         | 20 ++++++++++++++++++--
>  block/blk-stat.c       |  6 ------
>  block/blk-sysfs.c      |  1 +
>  include/linux/blkdev.h |  9 +++++++--
>  4 files changed, 26 insertions(+), 10 deletions(-)
> 
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 20a6445f6a01..cb41c441aa8f 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -4577,9 +4577,25 @@ EXPORT_SYMBOL_GPL(blk_mq_update_nr_hw_queues);
>  /* Enable polling stats and return whether they were already enabled. */
>  static bool blk_poll_stats_enable(struct request_queue *q)
>  {
> -	if (test_bit(QUEUE_FLAG_POLL_STATS, &q->queue_flags) ||
> -	    blk_queue_flag_test_and_set(QUEUE_FLAG_POLL_STATS, q))
> +	struct blk_rq_stat *poll_stat;
> +
> +	if (test_bit(QUEUE_FLAG_POLL_STATS, &q->queue_flags))
>  		return true;

Can't we replace the checks for QUEUE_FLAG_POLL_STATS with checks for
q->poll_stat now?

Re: [PATCH 3/3] block: only allocate poll_stats if there's a user of them

[Jens Axboe]

On 11/23/21 9:41 AM, Christoph Hellwig wrote:
> On Tue, Nov 23, 2021 at 09:18:13AM -0700, Jens Axboe wrote:
>> This is essentially never used, yet it's about 1/3rd of the total
>> queue size. Allocate it when needed, and don't embed it in the queue.
>>
>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>> ---
>>  block/blk-mq.c         | 20 ++++++++++++++++++--
>>  block/blk-stat.c       |  6 ------
>>  block/blk-sysfs.c      |  1 +
>>  include/linux/blkdev.h |  9 +++++++--
>>  4 files changed, 26 insertions(+), 10 deletions(-)
>>
>> diff --git a/block/blk-mq.c b/block/blk-mq.c
>> index 20a6445f6a01..cb41c441aa8f 100644
>> --- a/block/blk-mq.c
>> +++ b/block/blk-mq.c
>> @@ -4577,9 +4577,25 @@ EXPORT_SYMBOL_GPL(blk_mq_update_nr_hw_queues);
>>  /* Enable polling stats and return whether they were already enabled. */
>>  static bool blk_poll_stats_enable(struct request_queue *q)
>>  {
>> -	if (test_bit(QUEUE_FLAG_POLL_STATS, &q->queue_flags) ||
>> -	    blk_queue_flag_test_and_set(QUEUE_FLAG_POLL_STATS, q))
>> +	struct blk_rq_stat *poll_stat;
>> +
>> +	if (test_bit(QUEUE_FLAG_POLL_STATS, &q->queue_flags))
>>  		return true;
> 
> Can't we replace the checks for QUEUE_FLAG_POLL_STATS with checks for
> q->poll_stat now?

I think so:


diff --git a/block/blk-mq.c b/block/blk-mq.c
index f011fa3ebcc7..af4580bdf931 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -4593,7 +4593,7 @@ static bool blk_poll_stats_enable(struct request_queue *q)
 {
 	struct blk_rq_stat *poll_stat;
 
-	if (test_bit(QUEUE_FLAG_POLL_STATS, &q->queue_flags))
+	if (q->poll_stat)
 		return true;
 
 	poll_stat = kzalloc(BLK_MQ_POLL_STATS_BKTS * sizeof(*poll_stat),
@@ -4602,7 +4602,7 @@ static bool blk_poll_stats_enable(struct request_queue *q)
 		return false;
 
 	spin_lock_irq(&q->stats->lock);
-	if (blk_queue_flag_test_and_set(QUEUE_FLAG_POLL_STATS, q)) {
+	if (q->poll_stat) {
 		spin_unlock_irq(&q->stats->lock);
 		kfree(poll_stat);
 		return true;
@@ -4620,8 +4620,7 @@ static void blk_mq_poll_stats_start(struct request_queue *q)
 	 * We don't arm the callback if polling stats are not enabled or the
 	 * callback is already active.
 	 */
-	if (!test_bit(QUEUE_FLAG_POLL_STATS, &q->queue_flags) ||
-	    blk_stat_is_active(q->poll_cb))
+	if (!q->poll_stat || blk_stat_is_active(q->poll_cb))
 		return;
 
 	blk_stat_activate_msecs(q->poll_cb, 100);
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index e1b846ec58cb..c079be1c58a3 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -785,7 +785,7 @@ static void blk_release_queue(struct kobject *kobj)
 
 	might_sleep();
 
-	if (test_bit(QUEUE_FLAG_POLL_STATS, &q->queue_flags))
+	if (q->poll_stat)
 		blk_stat_remove_callback(q, q->poll_cb);
 	blk_stat_free_callback(q->poll_cb);
 
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 3d558cb397d5..20cf877d6627 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -414,7 +414,6 @@ struct request_queue {
 #define QUEUE_FLAG_FUA		18	/* device supports FUA writes */
 #define QUEUE_FLAG_DAX		19	/* device supports DAX */
 #define QUEUE_FLAG_STATS	20	/* track IO start and completion times */
-#define QUEUE_FLAG_POLL_STATS	21	/* collecting stats for hybrid polling */
 #define QUEUE_FLAG_REGISTERED	22	/* queue has been registered to a disk */
 #define QUEUE_FLAG_QUIESCED	24	/* queue has been quiesced */
 #define QUEUE_FLAG_PCI_P2PDMA	25	/* device supports PCI p2p requests */

-- 
Jens Axboe

Re: [PATCH 3/3] block: only allocate poll_stats if there's a user of them

[Christoph Hellwig]

On Tue, Nov 23, 2021 at 09:44:18AM -0700, Jens Axboe wrote:
> I think so:

I think my eternal enemy in blk-mq-debugfs.c will need an update for
the flag removal, but otherwise this looks good.

Re: [PATCH 3/3] block: only allocate poll_stats if there's a user of them

[Jens Axboe]

On 11/23/21 10:05 AM, Christoph Hellwig wrote:
> On Tue, Nov 23, 2021 at 09:44:18AM -0700, Jens Axboe wrote:
>> I think so:
> 
> I think my eternal enemy in blk-mq-debugfs.c will need an update for
> the flag removal, but otherwise this looks good.

Heh yes, it always evades grep.

-- 
Jens Axboe

[PATCHSET 0/3 v2] Misc block cleanups

[Jens Axboe]

Hi,

First patch avoids setting up an io_context for the cases where we don't
need them, second patch optimizes re-writing to the bio if not needed, and
prunes a huge chunk of memory in the request_queue and makes it dynamic
instead.

Since v1:
- Get rid of QUEUE_FLAG_POLL_STATS
- Use kcalloc() for poll_stats
- Move io_context creation into blk-mq-sched helper

-- 
Jens Axboe

[PATCH 1/3] block: move io_context creation into where it's needed

[Jens Axboe]

The only user of the io_context for IO is BFQ, yet we put the checking
and logic of it into the normal IO path.

Put the creation into blk_mq_sched_assign_ioc(), and have BFQ use that
helper.

Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 block/bfq-iosched.c  | 2 ++
 block/blk-core.c     | 9 ---------
 block/blk-ioc.c      | 1 +
 block/blk-mq-sched.c | 5 +++++
 block/blk-mq.c       | 3 ---
 5 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index fec18118dc30..1ce1a99a7160 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -6573,6 +6573,8 @@ 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);
+
 	/*
 	 * Regardless of whether we have an icq attached, we have to
 	 * clear the scheduler pointers, as they might point to
diff --git a/block/blk-core.c b/block/blk-core.c
index 6443f2dfe43e..6ae8297b033f 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -750,15 +750,6 @@ noinline_for_stack bool submit_bio_checks(struct bio *bio)
 		break;
 	}
 
-	/*
-	 * Various block parts want %current->io_context, so allocate it up
-	 * front rather than dealing with lots of pain to allocate it only
-	 * where needed. This may fail and the block layer knows how to live
-	 * with it.
-	 */
-	if (unlikely(!current->io_context))
-		create_task_io_context(current, GFP_ATOMIC, q->node);
-
 	if (blk_throtl_bio(bio))
 		return false;
 
diff --git a/block/blk-ioc.c b/block/blk-ioc.c
index 57299f860d41..736e0280d76f 100644
--- a/block/blk-ioc.c
+++ b/block/blk-ioc.c
@@ -286,6 +286,7 @@ int create_task_io_context(struct task_struct *task, gfp_t gfp_flags, int node)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(create_task_io_context);
 
 /**
  * get_task_io_context - get io_context of a task
diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
index ba21449439cc..b942b38000e5 100644
--- a/block/blk-mq-sched.c
+++ b/block/blk-mq-sched.c
@@ -24,6 +24,10 @@ void blk_mq_sched_assign_ioc(struct request *rq)
 	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
 	 */
@@ -43,6 +47,7 @@ void blk_mq_sched_assign_ioc(struct request *rq)
 	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
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 4c00b22590cc..20a6445f6a01 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -406,9 +406,6 @@ static struct request *blk_mq_rq_ctx_init(struct blk_mq_alloc_data *data,
 
 		if (!op_is_flush(data->cmd_flags) &&
 		    e->type->ops.prepare_request) {
-			if (e->type->icq_cache)
-				blk_mq_sched_assign_ioc(rq);
-
 			e->type->ops.prepare_request(rq);
 			rq->rq_flags |= RQF_ELVPRIV;
 		}
-- 
2.34.0

Re: [PATCH 1/3] block: move io_context creation into where it's needed

[Christoph Hellwig]

On Tue, Nov 23, 2021 at 10:10:56AM -0700, Jens Axboe wrote:
> --- a/block/blk-ioc.c
> +++ b/block/blk-ioc.c
> @@ -286,6 +286,7 @@ int create_task_io_context(struct task_struct *task, gfp_t gfp_flags, int node)
>  
>  	return ret;
>  }
> +EXPORT_SYMBOL_GPL(create_task_io_context);

No need to export this now.

Re: [PATCH 1/3] block: move io_context creation into where it's needed

[Jens Axboe]

On 11/23/21 11:46 AM, Christoph Hellwig wrote:
> On Tue, Nov 23, 2021 at 10:10:56AM -0700, Jens Axboe wrote:
>> --- a/block/blk-ioc.c
>> +++ b/block/blk-ioc.c
>> @@ -286,6 +286,7 @@ int create_task_io_context(struct task_struct *task, gfp_t gfp_flags, int node)
>>  
>>  	return ret;
>>  }
>> +EXPORT_SYMBOL_GPL(create_task_io_context);
> 
> No need to export this now.

Indeed, killed.

-- 
Jens Axboe