[PATCH v1 0/6] block: some misc changes

[PATCH v1 0/6] block: some misc changes

[Ming Lei]

Hi,

The 1st four patches are optimizations related with bio splitting.

The 5th patch is one fix for using trace_block_plug().

The 6th patch is to mark ctx as pending at batch in flush plug path.

V1:
	- 3/6: don't check bio_mergeable() in blk_mq_attempt_merge()
	- 4/6: check bio_mergeable() for non-mq too
	- 5/6: new patch for fixing using trace_block_plug()


 block/blk-merge.c | 32 +++++++++++++++++++++++++-------
 block/blk-mq.c    | 22 +++++++++++++++-------
 block/elevator.c  |  2 +-
 3 files changed, 41 insertions(+), 15 deletions(-)


Thanks,
Ming

[PATCH v1 1/6] block: setup bi_phys_segments after splitting

[Ming Lei]

The number of bio->bi_phys_segments is always obtained
during bio splitting, so it is natural to setup it
just after bio splitting, then we can avoid to compute
nr_segment again during merge.

Reviewed-by: Jeff Moyer <jmoyer@redhat.com>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 block/blk-merge.c | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/block/blk-merge.c b/block/blk-merge.c
index c4e9c37..22293fd 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -11,13 +11,16 @@
 
 static struct bio *blk_bio_discard_split(struct request_queue *q,
 					 struct bio *bio,
-					 struct bio_set *bs)
+					 struct bio_set *bs,
+					 unsigned *nsegs)
 {
 	unsigned int max_discard_sectors, granularity;
 	int alignment;
 	sector_t tmp;
 	unsigned split_sectors;
 
+	*nsegs = 1;
+
 	/* Zero-sector (unknown) and one-sector granularities are the same.  */
 	granularity = max(q->limits.discard_granularity >> 9, 1U);
 
@@ -51,8 +54,11 @@ static struct bio *blk_bio_discard_split(struct request_queue *q,
 
 static struct bio *blk_bio_write_same_split(struct request_queue *q,
 					    struct bio *bio,
-					    struct bio_set *bs)
+					    struct bio_set *bs,
+					    unsigned *nsegs)
 {
+	*nsegs = 1;
+
 	if (!q->limits.max_write_same_sectors)
 		return NULL;
 
@@ -64,7 +70,8 @@ static struct bio *blk_bio_write_same_split(struct request_queue *q,
 
 static struct bio *blk_bio_segment_split(struct request_queue *q,
 					 struct bio *bio,
-					 struct bio_set *bs)
+					 struct bio_set *bs,
+					 unsigned *segs)
 {
 	struct bio_vec bv, bvprv, *bvprvp = NULL;
 	struct bvec_iter iter;
@@ -106,22 +113,30 @@ new_segment:
 		sectors += bv.bv_len >> 9;
 	}
 
+	*segs = nsegs;
 	return NULL;
 split:
+	*segs = nsegs;
 	return bio_split(bio, sectors, GFP_NOIO, bs);
 }
 
 void blk_queue_split(struct request_queue *q, struct bio **bio,
 		     struct bio_set *bs)
 {
-	struct bio *split;
+	struct bio *split, *res;
+	unsigned nsegs;
 
 	if ((*bio)->bi_rw & REQ_DISCARD)
-		split = blk_bio_discard_split(q, *bio, bs);
+		split = blk_bio_discard_split(q, *bio, bs, &nsegs);
 	else if ((*bio)->bi_rw & REQ_WRITE_SAME)
-		split = blk_bio_write_same_split(q, *bio, bs);
+		split = blk_bio_write_same_split(q, *bio, bs, &nsegs);
 	else
-		split = blk_bio_segment_split(q, *bio, q->bio_split);
+		split = blk_bio_segment_split(q, *bio, q->bio_split, &nsegs);
+
+	/* physical segments can be figured out during splitting */
+	res = split ? split : *bio;
+	res->bi_phys_segments = nsegs;
+	bio_set_flag(res, BIO_SEG_VALID);
 
 	if (split) {
 		bio_chain(split, *bio);
-- 
1.9.1

[PATCH v1 2/6] block: avoid to merge splitted bio

[Ming Lei]

The splitted bio has been already too fat to merge, so mark it
as NOMERGE.

Reviewed-by: Jeff Moyer <jmoyer@redhat.com>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 block/blk-merge.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/block/blk-merge.c b/block/blk-merge.c
index 22293fd..de5716d8 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -139,6 +139,9 @@ void blk_queue_split(struct request_queue *q, struct bio **bio,
 	bio_set_flag(res, BIO_SEG_VALID);
 
 	if (split) {
+		/* there isn't chance to merge the splitted bio */
+		split->bi_rw |= REQ_NOMERGE;
+
 		bio_chain(split, *bio);
 		generic_make_request(*bio);
 		*bio = split;
-- 
1.9.1

[PATCH v1 3/6] blk-mq: check bio_mergeable() early before merging

[Ming Lei]

It isn't necessary to try to merge the bio which is marked
as NOMERGE.

Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 block/blk-mq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 546b3b8..d55d022 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1140,7 +1140,7 @@ static inline bool blk_mq_merge_queue_io(struct blk_mq_hw_ctx *hctx,
 					 struct blk_mq_ctx *ctx,
 					 struct request *rq, struct bio *bio)
 {
-	if (!hctx_allow_merges(hctx)) {
+	if (!hctx_allow_merges(hctx) || !bio_mergeable(bio)) {
 		blk_mq_bio_to_request(rq, bio);
 		spin_lock(&ctx->lock);
 insert_rq:
-- 
1.9.1

Re: [PATCH v1 3/6] blk-mq: check bio_mergeable() early before merging

[Jeff Moyer]

Ming Lei <ming.lei@canonical.com> writes:

> It isn't necessary to try to merge the bio which is marked
> as NOMERGE.
>
> Signed-off-by: Ming Lei <ming.lei@canonical.com>

Reviewed-by: Jeff Moyer <jmoyer@redhat.com>

> ---
>  block/blk-mq.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 546b3b8..d55d022 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -1140,7 +1140,7 @@ static inline bool blk_mq_merge_queue_io(struct blk_mq_hw_ctx *hctx,
>  					 struct blk_mq_ctx *ctx,
>  					 struct request *rq, struct bio *bio)
>  {
> -	if (!hctx_allow_merges(hctx)) {
> +	if (!hctx_allow_merges(hctx) || !bio_mergeable(bio)) {
>  		blk_mq_bio_to_request(rq, bio);
>  		spin_lock(&ctx->lock);
>  insert_rq:

[PATCH v1 4/6] block: check bio_mergeable() early before merging

[Ming Lei]

After bio splitting is introduced, one bio can be splitted
and it is marked as NOMERGE because it is too fat to be merged,
so check bio_mergeable() earlier to avoid to try to merge it
unnecessarily.

Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 block/elevator.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/elevator.c b/block/elevator.c
index 84d6394..c3555c9 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -420,7 +420,7 @@ int elv_merge(struct request_queue *q, struct request **req, struct bio *bio)
 	 * 	noxmerges: Only simple one-hit cache try
 	 * 	merges:	   All merge tries attempted
 	 */
-	if (blk_queue_nomerges(q))
+	if (blk_queue_nomerges(q) || !bio_mergeable(bio))
 		return ELEVATOR_NO_MERGE;
 
 	/*
-- 
1.9.1

Re: [PATCH v1 4/6] block: check bio_mergeable() early before merging

[Jeff Moyer]

Ming Lei <ming.lei@canonical.com> writes:

> After bio splitting is introduced, one bio can be splitted
> and it is marked as NOMERGE because it is too fat to be merged,
> so check bio_mergeable() earlier to avoid to try to merge it
> unnecessarily.
>
> Signed-off-by: Ming Lei <ming.lei@canonical.com>

Reviewed-by: Jeff Moyer <jmoyer@redhat.com>

Ming, do you think we should also add a check in blk_attempt_plug_merge?

-Jeff

> ---
>  block/elevator.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/block/elevator.c b/block/elevator.c
> index 84d6394..c3555c9 100644
> --- a/block/elevator.c
> +++ b/block/elevator.c
> @@ -420,7 +420,7 @@ int elv_merge(struct request_queue *q, struct request **req, struct bio *bio)
>  	 * 	noxmerges: Only simple one-hit cache try
>  	 * 	merges:	   All merge tries attempted
>  	 */
> -	if (blk_queue_nomerges(q))
> +	if (blk_queue_nomerges(q) || !bio_mergeable(bio))
>  		return ELEVATOR_NO_MERGE;
>  
>  	/*

Re: [PATCH v1 4/6] block: check bio_mergeable() early before merging

[Ming Lei]

On Mon, Oct 19, 2015 at 11:27 PM, Jeff Moyer <jmoyer@redhat.com> wrote:
> Ming Lei <ming.lei@canonical.com> writes:
>
>> After bio splitting is introduced, one bio can be splitted
>> and it is marked as NOMERGE because it is too fat to be merged,
>> so check bio_mergeable() earlier to avoid to try to merge it
>> unnecessarily.
>>
>> Signed-off-by: Ming Lei <ming.lei@canonical.com>
>
> Reviewed-by: Jeff Moyer <jmoyer@redhat.com>
>
> Ming, do you think we should also add a check in blk_attempt_plug_merge?

No, because 'request_count' need to be figured out for automatic flush plug.

Thanks,

>
> -Jeff
>
>> ---
>>  block/elevator.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/block/elevator.c b/block/elevator.c
>> index 84d6394..c3555c9 100644
>> --- a/block/elevator.c
>> +++ b/block/elevator.c
>> @@ -420,7 +420,7 @@ int elv_merge(struct request_queue *q, struct request **req, struct bio *bio)
>>        *      noxmerges: Only simple one-hit cache try
>>        *      merges:    All merge tries attempted
>>        */
>> -     if (blk_queue_nomerges(q))
>> +     if (blk_queue_nomerges(q) || !bio_mergeable(bio))
>>               return ELEVATOR_NO_MERGE;
>>
>>       /*
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v1 4/6] block: check bio_mergeable() early before merging

[Jeff Moyer]

Ming Lei <ming.lei@canonical.com> writes:

> On Mon, Oct 19, 2015 at 11:27 PM, Jeff Moyer <jmoyer@redhat.com> wrote:
>> Ming Lei <ming.lei@canonical.com> writes:
>>
>>> After bio splitting is introduced, one bio can be splitted
>>> and it is marked as NOMERGE because it is too fat to be merged,
>>> so check bio_mergeable() earlier to avoid to try to merge it
>>> unnecessarily.
>>>
>>> Signed-off-by: Ming Lei <ming.lei@canonical.com>
>>
>> Reviewed-by: Jeff Moyer <jmoyer@redhat.com>
>>
>> Ming, do you think we should also add a check in blk_attempt_plug_merge?
>
> No, because 'request_count' need to be figured out for automatic flush plug.

Interesting.  See my follow-up to patch 5.  :)

-Jeff

>
> Thanks,
>
>>
>> -Jeff
>>
>>> ---
>>>  block/elevator.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/block/elevator.c b/block/elevator.c
>>> index 84d6394..c3555c9 100644
>>> --- a/block/elevator.c
>>> +++ b/block/elevator.c
>>> @@ -420,7 +420,7 @@ int elv_merge(struct request_queue *q, struct request **req, struct bio *bio)
>>>        *      noxmerges: Only simple one-hit cache try
>>>        *      merges:    All merge tries attempted
>>>        */
>>> -     if (blk_queue_nomerges(q))
>>> +     if (blk_queue_nomerges(q) || !bio_mergeable(bio))
>>>               return ELEVATOR_NO_MERGE;
>>>
>>>       /*
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v1 5/6] blk-mq: fix for trace_block_plug()

[Ming Lei]

The trace point is for tracing plug event of each request
queue instead of each task, so we should check the request
count in the plug list from current queue instead of
current task.

Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 block/blk-mq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index d55d022..a5e33bc 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1377,7 +1377,7 @@ static void blk_sq_make_request(struct request_queue *q, struct bio *bio)
 	plug = current->plug;
 	if (plug) {
 		blk_mq_bio_to_request(rq, bio);
-		if (list_empty(&plug->mq_list))
+		if (!request_count)
 			trace_block_plug(q);
 		else if (request_count >= BLK_MAX_REQUEST_COUNT) {
 			blk_flush_plug_list(plug, false);
-- 
1.9.1

Re: [PATCH v1 5/6] blk-mq: fix for trace_block_plug()

[Jeff Moyer]

Ming Lei <ming.lei@canonical.com> writes:

> The trace point is for tracing plug event of each request
> queue instead of each task, so we should check the request
> count in the plug list from current queue instead of
> current task.

If blk_queue_nomerges returns true, request_count won't be updated, so
you've essentially broken plugging for those queues.

Cheers,
Jeff

>
> Signed-off-by: Ming Lei <ming.lei@canonical.com>
> ---
>  block/blk-mq.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index d55d022..a5e33bc 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -1377,7 +1377,7 @@ static void blk_sq_make_request(struct request_queue *q, struct bio *bio)
>  	plug = current->plug;
>  	if (plug) {
>  		blk_mq_bio_to_request(rq, bio);
> -		if (list_empty(&plug->mq_list))
> +		if (!request_count)
>  			trace_block_plug(q);
>  		else if (request_count >= BLK_MAX_REQUEST_COUNT) {
>  			blk_flush_plug_list(plug, false);

Re: [PATCH v1 5/6] blk-mq: fix for trace_block_plug()

[Ming Lei]

On Mon, Oct 19, 2015 at 11:38 PM, Jeff Moyer <jmoyer@redhat.com> wrote:
> Ming Lei <ming.lei@canonical.com> writes:
>
>> The trace point is for tracing plug event of each request
>> queue instead of each task, so we should check the request
>> count in the plug list from current queue instead of
>> current task.
>
> If blk_queue_nomerges returns true, request_count won't be updated, so
> you've essentially broken plugging for those queues.

Yes, you are right, and blk_queue_bio() has the same problem too,
and looks automatic flush plug can't work for nomerges queue at all.

Thanks,
Ming

>
> Cheers,
> Jeff
>
>>
>> Signed-off-by: Ming Lei <ming.lei@canonical.com>
>> ---
>>  block/blk-mq.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/block/blk-mq.c b/block/blk-mq.c
>> index d55d022..a5e33bc 100644
>> --- a/block/blk-mq.c
>> +++ b/block/blk-mq.c
>> @@ -1377,7 +1377,7 @@ static void blk_sq_make_request(struct request_queue *q, struct bio *bio)
>>       plug = current->plug;
>>       if (plug) {
>>               blk_mq_bio_to_request(rq, bio);
>> -             if (list_empty(&plug->mq_list))
>> +             if (!request_count)
>>                       trace_block_plug(q);
>>               else if (request_count >= BLK_MAX_REQUEST_COUNT) {
>>                       blk_flush_plug_list(plug, false);

Re: [PATCH v1 5/6] blk-mq: fix for trace_block_plug()

[Jeff Moyer]

Ming Lei <ming.lei@canonical.com> writes:

> On Mon, Oct 19, 2015 at 11:38 PM, Jeff Moyer <jmoyer@redhat.com> wrote:
>> Ming Lei <ming.lei@canonical.com> writes:
>>
>>> The trace point is for tracing plug event of each request
>>> queue instead of each task, so we should check the request
>>> count in the plug list from current queue instead of
>>> current task.
>>
>> If blk_queue_nomerges returns true, request_count won't be updated, so
>> you've essentially broken plugging for those queues.
>
> Yes, you are right, and blk_queue_bio() has the same problem too,
> and looks automatic flush plug can't work for nomerges queue at all.

Hi Ming,

Something like this would fix it.  Feel free to add it into your patch
set if you're going to do another spin.  Otherwise I'll just send it off
separately.

-Jeff

From: Jeff Moyer <jmoyer@redhat.com>
Subject: block: fix plug list flushing for nomerge queues

Request queues with merging disabled will not flush the plug list after
BLK_MAX_REQUEST_COUNT requests have been queued, since the code relies
on blk_attempt_plug_merge to compute the request_count.  Fix this by
computing the number of queued requests even for nomerge queues.

Signed-off-by: Jeff Moyer <jmoyer@redhat.com>

diff --git a/block/blk-core.c b/block/blk-core.c
index 2eb722d..f0ae087 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1594,6 +1594,30 @@ out:
 	return ret;
 }
 
+unsigned int blk_plug_queued_count(struct request_queue *q)
+{
+	struct blk_plug *plug;
+	struct request *rq;
+	struct list_head *plug_list;
+	unsigned int ret = 0;
+
+	plug = current->plug;
+	if (!plug)
+		goto out;
+
+	if (q->mq_ops)
+		plug_list = &plug->mq_list;
+	else
+		plug_list = &plug->list;
+
+	list_for_each_entry(rq, plug_list, queuelist) {
+		if (rq->q == q)
+			ret++;
+	}
+out:
+	return ret;
+}
+
 void init_request_from_bio(struct request *req, struct bio *bio)
 {
 	req->cmd_type = REQ_TYPE_FS;
@@ -1641,9 +1665,11 @@ static void blk_queue_bio(struct request_queue *q, struct bio *bio)
 	 * Check if we can merge with the plugged list before grabbing
 	 * any locks.
 	 */
-	if (!blk_queue_nomerges(q) &&
-	    blk_attempt_plug_merge(q, bio, &request_count, NULL))
-		return;
+	if (!blk_queue_nomerges(q)) {
+		if (blk_attempt_plug_merge(q, bio, &request_count, NULL))
+			return;
+	} else
+		request_count = blk_plug_queued_count(q);
 
 	spin_lock_irq(q->queue_lock);
 
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 7785ae9..604a1f3 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1267,9 +1267,12 @@ static void blk_mq_make_request(struct request_queue *q, struct bio *bio)
 
 	blk_queue_split(q, &bio, q->bio_split);
 
-	if (!is_flush_fua && !blk_queue_nomerges(q) &&
-	    blk_attempt_plug_merge(q, bio, &request_count, &same_queue_rq))
-		return;
+	if (!is_flush_fua && !blk_queue_nomerges(q)) {
+		if (blk_attempt_plug_merge(q, bio, &request_count,
+					   &same_queue_rq))
+			return;
+	} else
+		request_count = blk_plug_queued_count(q);
 
 	rq = blk_mq_map_request(q, bio, &data);
 	if (unlikely(!rq))
diff --git a/block/blk.h b/block/blk.h
index 98614ad..aa27d02 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -86,6 +86,7 @@ bool bio_attempt_back_merge(struct request_queue *q, struct request *req,
 bool blk_attempt_plug_merge(struct request_queue *q, struct bio *bio,
 			    unsigned int *request_count,
 			    struct request **same_queue_rq);
+unsigned int blk_plug_queued_count(struct request_queue *q);
 
 void blk_account_io_start(struct request *req, bool new_io);
 void blk_account_io_completion(struct request *req, unsigned int bytes);

Re: [PATCH v1 5/6] blk-mq: fix for trace_block_plug()

[Ming Lei]

On Tue, Oct 20, 2015 at 1:59 AM, Jeff Moyer <jmoyer@redhat.com> wrote:
> Ming Lei <ming.lei@canonical.com> writes:
>
>> On Mon, Oct 19, 2015 at 11:38 PM, Jeff Moyer <jmoyer@redhat.com> wrote:
>>> Ming Lei <ming.lei@canonical.com> writes:
>>>
>>>> The trace point is for tracing plug event of each request
>>>> queue instead of each task, so we should check the request
>>>> count in the plug list from current queue instead of
>>>> current task.
>>>
>>> If blk_queue_nomerges returns true, request_count won't be updated, so
>>> you've essentially broken plugging for those queues.
>>
>> Yes, you are right, and blk_queue_bio() has the same problem too,
>> and looks automatic flush plug can't work for nomerges queue at all.
>
> Hi Ming,
>
> Something like this would fix it.  Feel free to add it into your patch
> set if you're going to do another spin.  Otherwise I'll just send it off
> separately.

Looks fine for me, will add it in V2, thanks!


>
> -Jeff
>
> From: Jeff Moyer <jmoyer@redhat.com>
> Subject: block: fix plug list flushing for nomerge queues
>
> Request queues with merging disabled will not flush the plug list after
> BLK_MAX_REQUEST_COUNT requests have been queued, since the code relies
> on blk_attempt_plug_merge to compute the request_count.  Fix this by
> computing the number of queued requests even for nomerge queues.
>
> Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
>
> diff --git a/block/blk-core.c b/block/blk-core.c
> index 2eb722d..f0ae087 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -1594,6 +1594,30 @@ out:
>         return ret;
>  }
>
> +unsigned int blk_plug_queued_count(struct request_queue *q)
> +{
> +       struct blk_plug *plug;
> +       struct request *rq;
> +       struct list_head *plug_list;
> +       unsigned int ret = 0;
> +
> +       plug = current->plug;
> +       if (!plug)
> +               goto out;
> +
> +       if (q->mq_ops)
> +               plug_list = &plug->mq_list;
> +       else
> +               plug_list = &plug->list;
> +
> +       list_for_each_entry(rq, plug_list, queuelist) {
> +               if (rq->q == q)
> +                       ret++;
> +       }
> +out:
> +       return ret;
> +}
> +
>  void init_request_from_bio(struct request *req, struct bio *bio)
>  {
>         req->cmd_type = REQ_TYPE_FS;
> @@ -1641,9 +1665,11 @@ static void blk_queue_bio(struct request_queue *q, struct bio *bio)
>          * Check if we can merge with the plugged list before grabbing
>          * any locks.
>          */
> -       if (!blk_queue_nomerges(q) &&
> -           blk_attempt_plug_merge(q, bio, &request_count, NULL))
> -               return;
> +       if (!blk_queue_nomerges(q)) {
> +               if (blk_attempt_plug_merge(q, bio, &request_count, NULL))
> +                       return;
> +       } else
> +               request_count = blk_plug_queued_count(q);
>
>         spin_lock_irq(q->queue_lock);
>
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 7785ae9..604a1f3 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -1267,9 +1267,12 @@ static void blk_mq_make_request(struct request_queue *q, struct bio *bio)
>
>         blk_queue_split(q, &bio, q->bio_split);
>
> -       if (!is_flush_fua && !blk_queue_nomerges(q) &&
> -           blk_attempt_plug_merge(q, bio, &request_count, &same_queue_rq))
> -               return;
> +       if (!is_flush_fua && !blk_queue_nomerges(q)) {
> +               if (blk_attempt_plug_merge(q, bio, &request_count,
> +                                          &same_queue_rq))
> +                       return;
> +       } else
> +               request_count = blk_plug_queued_count(q);
>
>         rq = blk_mq_map_request(q, bio, &data);
>         if (unlikely(!rq))
> diff --git a/block/blk.h b/block/blk.h
> index 98614ad..aa27d02 100644
> --- a/block/blk.h
> +++ b/block/blk.h
> @@ -86,6 +86,7 @@ bool bio_attempt_back_merge(struct request_queue *q, struct request *req,
>  bool blk_attempt_plug_merge(struct request_queue *q, struct bio *bio,
>                             unsigned int *request_count,
>                             struct request **same_queue_rq);
> +unsigned int blk_plug_queued_count(struct request_queue *q);
>
>  void blk_account_io_start(struct request *req, bool new_io);
>  void blk_account_io_completion(struct request *req, unsigned int bytes);
>

[PATCH v1 6/6] blk-mq: mark ctx as pending at batch in flush plug path

[Ming Lei]

Most of times, flush plug should be the hottest I/O path,
so mark ctx as pending after all requests in the list are
inserted.

Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 block/blk-mq.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index a5e33bc..b169e2d 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -990,18 +990,25 @@ void blk_mq_delay_queue(struct blk_mq_hw_ctx *hctx, unsigned long msecs)
 }
 EXPORT_SYMBOL(blk_mq_delay_queue);
 
-static void __blk_mq_insert_request(struct blk_mq_hw_ctx *hctx,
-				    struct request *rq, bool at_head)
+static inline void __blk_mq_insert_req_list(struct blk_mq_hw_ctx *hctx,
+					    struct blk_mq_ctx *ctx,
+					    struct request *rq,
+					    bool at_head)
 {
-	struct blk_mq_ctx *ctx = rq->mq_ctx;
-
 	trace_block_rq_insert(hctx->queue, rq);
 
 	if (at_head)
 		list_add(&rq->queuelist, &ctx->rq_list);
 	else
 		list_add_tail(&rq->queuelist, &ctx->rq_list);
+}
+
+static void __blk_mq_insert_request(struct blk_mq_hw_ctx *hctx,
+				    struct request *rq, bool at_head)
+{
+	struct blk_mq_ctx *ctx = rq->mq_ctx;
 
+	__blk_mq_insert_req_list(hctx, ctx, rq, at_head);
 	blk_mq_hctx_mark_pending(hctx, ctx);
 }
 
@@ -1057,8 +1064,9 @@ static void blk_mq_insert_requests(struct request_queue *q,
 		rq = list_first_entry(list, struct request, queuelist);
 		list_del_init(&rq->queuelist);
 		rq->mq_ctx = ctx;
-		__blk_mq_insert_request(hctx, rq, false);
+		__blk_mq_insert_req_list(hctx, ctx, rq, false);
 	}
+	blk_mq_hctx_mark_pending(hctx, ctx);
 	spin_unlock(&ctx->lock);
 
 	blk_mq_run_hw_queue(hctx, from_schedule);
-- 
1.9.1

Re: [PATCH v1 6/6] blk-mq: mark ctx as pending at batch in flush plug path

[Jeff Moyer]

Ming Lei <ming.lei@canonical.com> writes:

> Most of times, flush plug should be the hottest I/O path,
> so mark ctx as pending after all requests in the list are
> inserted.
>
> Signed-off-by: Ming Lei <ming.lei@canonical.com>

I agree this looks like a better approach.  Whether it makes a
difference or not, I can't say.

Reviewed-by: Jeff Moyer <jmoyer@redhat.com>

> ---
>  block/blk-mq.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index a5e33bc..b169e2d 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -990,18 +990,25 @@ void blk_mq_delay_queue(struct blk_mq_hw_ctx *hctx, unsigned long msecs)
>  }
>  EXPORT_SYMBOL(blk_mq_delay_queue);
>  
> -static void __blk_mq_insert_request(struct blk_mq_hw_ctx *hctx,
> -				    struct request *rq, bool at_head)
> +static inline void __blk_mq_insert_req_list(struct blk_mq_hw_ctx *hctx,
> +					    struct blk_mq_ctx *ctx,
> +					    struct request *rq,
> +					    bool at_head)
>  {
> -	struct blk_mq_ctx *ctx = rq->mq_ctx;
> -
>  	trace_block_rq_insert(hctx->queue, rq);
>  
>  	if (at_head)
>  		list_add(&rq->queuelist, &ctx->rq_list);
>  	else
>  		list_add_tail(&rq->queuelist, &ctx->rq_list);
> +}
> +
> +static void __blk_mq_insert_request(struct blk_mq_hw_ctx *hctx,
> +				    struct request *rq, bool at_head)
> +{
> +	struct blk_mq_ctx *ctx = rq->mq_ctx;
>  
> +	__blk_mq_insert_req_list(hctx, ctx, rq, at_head);
>  	blk_mq_hctx_mark_pending(hctx, ctx);
>  }
>  
> @@ -1057,8 +1064,9 @@ static void blk_mq_insert_requests(struct request_queue *q,
>  		rq = list_first_entry(list, struct request, queuelist);
>  		list_del_init(&rq->queuelist);
>  		rq->mq_ctx = ctx;
> -		__blk_mq_insert_request(hctx, rq, false);
> +		__blk_mq_insert_req_list(hctx, ctx, rq, false);
>  	}
> +	blk_mq_hctx_mark_pending(hctx, ctx);
>  	spin_unlock(&ctx->lock);
>  
>  	blk_mq_run_hw_queue(hctx, from_schedule);