[PATCH, RESEND 0/2] Simplify blk-mq implementation

[PATCH, RESEND 0/2] Simplify blk-mq implementation

[Bart Van Assche]

Hi Jens,

While reviewing the blk_mq_make_request() code I noticed that it is possible
to simplify the implementation of that function. Please consider these patches
for kernel v5.3.

Thanks,

Bart.

Bart Van Assche (2):
  blk-mq: Remove blk_mq_put_ctx()
  blk-mq: Simplify blk_mq_make_request()

 block/blk-mq-sched.c  |  5 +----
 block/blk-mq-tag.c    |  8 --------
 block/blk-mq.c        | 26 +++++---------------------
 block/blk-mq.h        |  7 +------
 block/kyber-iosched.c |  1 -
 5 files changed, 7 insertions(+), 40 deletions(-)

-- 
2.22.0.410.gd8fdbe21b5-goog

[PATCH 1/2] blk-mq: Remove blk_mq_put_ctx()

[Bart Van Assche]

No code that occurs between blk_mq_get_ctx() and blk_mq_put_ctx() depends
on preemption being disabled for its correctness. Since removing the CPU
preemption calls does not measurably affect performance, simplify the
blk-mq code by removing the blk_mq_put_ctx() function and also by not
disabling preemption in blk_mq_get_ctx().

Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ming Lei <ming.lei@redhat.com>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Omar Sandoval <osandov@fb.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 block/blk-mq-sched.c  |  5 +----
 block/blk-mq-tag.c    |  8 --------
 block/blk-mq.c        | 16 +++-------------
 block/blk-mq.h        |  7 +------
 block/kyber-iosched.c |  1 -
 5 files changed, 5 insertions(+), 32 deletions(-)

diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
index 956a7aa9a637..c9d183d6c499 100644
--- a/block/blk-mq-sched.c
+++ b/block/blk-mq-sched.c
@@ -330,10 +330,8 @@ bool __blk_mq_sched_bio_merge(struct request_queue *q, struct bio *bio,
 	bool ret = false;
 	enum hctx_type type;
 
-	if (e && e->type->ops.bio_merge) {
-		blk_mq_put_ctx(ctx);
+	if (e && e->type->ops.bio_merge)
 		return e->type->ops.bio_merge(hctx, bio, nr_segs);
-	}
 
 	type = hctx->type;
 	if ((hctx->flags & BLK_MQ_F_SHOULD_MERGE) &&
@@ -344,7 +342,6 @@ bool __blk_mq_sched_bio_merge(struct request_queue *q, struct bio *bio,
 		spin_unlock(&ctx->lock);
 	}
 
-	blk_mq_put_ctx(ctx);
 	return ret;
 }
 
diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
index 7513c8eaabee..da19f0bc8876 100644
--- a/block/blk-mq-tag.c
+++ b/block/blk-mq-tag.c
@@ -113,7 +113,6 @@ unsigned int blk_mq_get_tag(struct blk_mq_alloc_data *data)
 	struct sbq_wait_state *ws;
 	DEFINE_SBQ_WAIT(wait);
 	unsigned int tag_offset;
-	bool drop_ctx;
 	int tag;
 
 	if (data->flags & BLK_MQ_REQ_RESERVED) {
@@ -136,7 +135,6 @@ unsigned int blk_mq_get_tag(struct blk_mq_alloc_data *data)
 		return BLK_MQ_TAG_FAIL;
 
 	ws = bt_wait_ptr(bt, data->hctx);
-	drop_ctx = data->ctx == NULL;
 	do {
 		struct sbitmap_queue *bt_prev;
 
@@ -161,9 +159,6 @@ unsigned int blk_mq_get_tag(struct blk_mq_alloc_data *data)
 		if (tag != -1)
 			break;
 
-		if (data->ctx)
-			blk_mq_put_ctx(data->ctx);
-
 		bt_prev = bt;
 		io_schedule();
 
@@ -189,9 +184,6 @@ unsigned int blk_mq_get_tag(struct blk_mq_alloc_data *data)
 		ws = bt_wait_ptr(bt, data->hctx);
 	} while (1);
 
-	if (drop_ctx && data->ctx)
-		blk_mq_put_ctx(data->ctx);
-
 	sbitmap_finish_wait(bt, ws, &wait);
 
 found_tag:
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 2f0b14d2ecdc..4d661545ad1d 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -359,13 +359,13 @@ static struct request *blk_mq_get_request(struct request_queue *q,
 	struct elevator_queue *e = q->elevator;
 	struct request *rq;
 	unsigned int tag;
-	bool put_ctx_on_error = false;
+	bool clear_ctx_on_error = false;
 
 	blk_queue_enter_live(q);
 	data->q = q;
 	if (likely(!data->ctx)) {
 		data->ctx = blk_mq_get_ctx(q);
-		put_ctx_on_error = true;
+		clear_ctx_on_error = true;
 	}
 	if (likely(!data->hctx))
 		data->hctx = blk_mq_map_queue(q, data->cmd_flags,
@@ -391,10 +391,8 @@ static struct request *blk_mq_get_request(struct request_queue *q,
 
 	tag = blk_mq_get_tag(data);
 	if (tag == BLK_MQ_TAG_FAIL) {
-		if (put_ctx_on_error) {
-			blk_mq_put_ctx(data->ctx);
+		if (clear_ctx_on_error)
 			data->ctx = NULL;
-		}
 		blk_queue_exit(q);
 		return NULL;
 	}
@@ -431,8 +429,6 @@ struct request *blk_mq_alloc_request(struct request_queue *q, unsigned int op,
 	if (!rq)
 		return ERR_PTR(-EWOULDBLOCK);
 
-	blk_mq_put_ctx(alloc_data.ctx);
-
 	rq->__data_len = 0;
 	rq->__sector = (sector_t) -1;
 	rq->bio = rq->biotail = NULL;
@@ -1981,7 +1977,6 @@ static blk_qc_t blk_mq_make_request(struct request_queue *q, struct bio *bio)
 
 	plug = current->plug;
 	if (unlikely(is_flush_fua)) {
-		blk_mq_put_ctx(data.ctx);
 		blk_mq_bio_to_request(rq, bio, nr_segs);
 
 		/* bypass scheduler for flush rq */
@@ -1995,7 +1990,6 @@ static blk_qc_t blk_mq_make_request(struct request_queue *q, struct bio *bio)
 		unsigned int request_count = plug->rq_count;
 		struct request *last = NULL;
 
-		blk_mq_put_ctx(data.ctx);
 		blk_mq_bio_to_request(rq, bio, nr_segs);
 
 		if (!request_count)
@@ -2029,8 +2023,6 @@ static blk_qc_t blk_mq_make_request(struct request_queue *q, struct bio *bio)
 		blk_add_rq_to_plug(plug, rq);
 		trace_block_plug(q);
 
-		blk_mq_put_ctx(data.ctx);
-
 		if (same_queue_rq) {
 			data.hctx = same_queue_rq->mq_hctx;
 			trace_block_unplug(q, 1, true);
@@ -2039,11 +2031,9 @@ static blk_qc_t blk_mq_make_request(struct request_queue *q, struct bio *bio)
 		}
 	} else if ((q->nr_hw_queues > 1 && is_sync) || (!q->elevator &&
 			!data.hctx->dispatch_busy)) {
-		blk_mq_put_ctx(data.ctx);
 		blk_mq_bio_to_request(rq, bio, nr_segs);
 		blk_mq_try_issue_directly(data.hctx, rq, &cookie);
 	} else {
-		blk_mq_put_ctx(data.ctx);
 		blk_mq_bio_to_request(rq, bio, nr_segs);
 		blk_mq_sched_insert_request(rq, false, true, true);
 	}
diff --git a/block/blk-mq.h b/block/blk-mq.h
index 633a5a77ee8b..f4bf5161333e 100644
--- a/block/blk-mq.h
+++ b/block/blk-mq.h
@@ -151,12 +151,7 @@ static inline struct blk_mq_ctx *__blk_mq_get_ctx(struct request_queue *q,
  */
 static inline struct blk_mq_ctx *blk_mq_get_ctx(struct request_queue *q)
 {
-	return __blk_mq_get_ctx(q, get_cpu());
-}
-
-static inline void blk_mq_put_ctx(struct blk_mq_ctx *ctx)
-{
-	put_cpu();
+	return __blk_mq_get_ctx(q, raw_smp_processor_id());
 }
 
 struct blk_mq_alloc_data {
diff --git a/block/kyber-iosched.c b/block/kyber-iosched.c
index 3c2602601741..34dcea0ef637 100644
--- a/block/kyber-iosched.c
+++ b/block/kyber-iosched.c
@@ -575,7 +575,6 @@ static bool kyber_bio_merge(struct blk_mq_hw_ctx *hctx, struct bio *bio,
 	spin_lock(&kcq->lock);
 	merged = blk_mq_bio_list_merge(hctx->queue, rq_list, bio, nr_segs);
 	spin_unlock(&kcq->lock);
-	blk_mq_put_ctx(ctx);
 
 	return merged;
 }
-- 
2.22.0.410.gd8fdbe21b5-goog

Re: [PATCH 1/2] blk-mq: Remove blk_mq_put_ctx()

[Christoph Hellwig]

On Mon, Jul 01, 2019 at 08:47:29AM -0700, Bart Van Assche wrote:
> No code that occurs between blk_mq_get_ctx() and blk_mq_put_ctx() depends
> on preemption being disabled for its correctness. Since removing the CPU
> preemption calls does not measurably affect performance, simplify the
> blk-mq code by removing the blk_mq_put_ctx() function and also by not
> disabling preemption in blk_mq_get_ctx().

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 1/2] blk-mq: Remove blk_mq_put_ctx()

[Christoph Hellwig]

On Tue, Jul 02, 2019 at 03:25:44PM +0200, Christoph Hellwig wrote:
> On Mon, Jul 01, 2019 at 08:47:29AM -0700, Bart Van Assche wrote:
> > No code that occurs between blk_mq_get_ctx() and blk_mq_put_ctx() depends
> > on preemption being disabled for its correctness. Since removing the CPU
> > preemption calls does not measurably affect performance, simplify the
> > blk-mq code by removing the blk_mq_put_ctx() function and also by not
> > disabling preemption in blk_mq_get_ctx().
> 
> Looks good,
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 1/2] blk-mq: Remove blk_mq_put_ctx()

[Ming Lei]

On Mon, Jul 01, 2019 at 08:47:29AM -0700, Bart Van Assche wrote:
> No code that occurs between blk_mq_get_ctx() and blk_mq_put_ctx() depends
> on preemption being disabled for its correctness. Since removing the CPU
> preemption calls does not measurably affect performance, simplify the
> blk-mq code by removing the blk_mq_put_ctx() function and also by not
> disabling preemption in blk_mq_get_ctx().
> 
> Cc: Christoph Hellwig <hch@infradead.org>
> Cc: Ming Lei <ming.lei@redhat.com>
> Cc: Hannes Reinecke <hare@suse.com>
> Cc: Omar Sandoval <osandov@fb.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  block/blk-mq-sched.c  |  5 +----
>  block/blk-mq-tag.c    |  8 --------
>  block/blk-mq.c        | 16 +++-------------
>  block/blk-mq.h        |  7 +------
>  block/kyber-iosched.c |  1 -
>  5 files changed, 5 insertions(+), 32 deletions(-)
> 
> diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
> index 956a7aa9a637..c9d183d6c499 100644
> --- a/block/blk-mq-sched.c
> +++ b/block/blk-mq-sched.c
> @@ -330,10 +330,8 @@ bool __blk_mq_sched_bio_merge(struct request_queue *q, struct bio *bio,
>  	bool ret = false;
>  	enum hctx_type type;
>  
> -	if (e && e->type->ops.bio_merge) {
> -		blk_mq_put_ctx(ctx);
> +	if (e && e->type->ops.bio_merge)
>  		return e->type->ops.bio_merge(hctx, bio, nr_segs);
> -	}
>  
>  	type = hctx->type;
>  	if ((hctx->flags & BLK_MQ_F_SHOULD_MERGE) &&
> @@ -344,7 +342,6 @@ bool __blk_mq_sched_bio_merge(struct request_queue *q, struct bio *bio,
>  		spin_unlock(&ctx->lock);
>  	}
>  
> -	blk_mq_put_ctx(ctx);
>  	return ret;
>  }
>  
> diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
> index 7513c8eaabee..da19f0bc8876 100644
> --- a/block/blk-mq-tag.c
> +++ b/block/blk-mq-tag.c
> @@ -113,7 +113,6 @@ unsigned int blk_mq_get_tag(struct blk_mq_alloc_data *data)
>  	struct sbq_wait_state *ws;
>  	DEFINE_SBQ_WAIT(wait);
>  	unsigned int tag_offset;
> -	bool drop_ctx;
>  	int tag;
>  
>  	if (data->flags & BLK_MQ_REQ_RESERVED) {
> @@ -136,7 +135,6 @@ unsigned int blk_mq_get_tag(struct blk_mq_alloc_data *data)
>  		return BLK_MQ_TAG_FAIL;
>  
>  	ws = bt_wait_ptr(bt, data->hctx);
> -	drop_ctx = data->ctx == NULL;
>  	do {
>  		struct sbitmap_queue *bt_prev;
>  
> @@ -161,9 +159,6 @@ unsigned int blk_mq_get_tag(struct blk_mq_alloc_data *data)
>  		if (tag != -1)
>  			break;
>  
> -		if (data->ctx)
> -			blk_mq_put_ctx(data->ctx);
> -
>  		bt_prev = bt;
>  		io_schedule();
>  
> @@ -189,9 +184,6 @@ unsigned int blk_mq_get_tag(struct blk_mq_alloc_data *data)
>  		ws = bt_wait_ptr(bt, data->hctx);
>  	} while (1);
>  
> -	if (drop_ctx && data->ctx)
> -		blk_mq_put_ctx(data->ctx);
> -
>  	sbitmap_finish_wait(bt, ws, &wait);
>  
>  found_tag:
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 2f0b14d2ecdc..4d661545ad1d 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -359,13 +359,13 @@ static struct request *blk_mq_get_request(struct request_queue *q,
>  	struct elevator_queue *e = q->elevator;
>  	struct request *rq;
>  	unsigned int tag;
> -	bool put_ctx_on_error = false;
> +	bool clear_ctx_on_error = false;
>  
>  	blk_queue_enter_live(q);
>  	data->q = q;
>  	if (likely(!data->ctx)) {
>  		data->ctx = blk_mq_get_ctx(q);
> -		put_ctx_on_error = true;
> +		clear_ctx_on_error = true;
>  	}
>  	if (likely(!data->hctx))
>  		data->hctx = blk_mq_map_queue(q, data->cmd_flags,
> @@ -391,10 +391,8 @@ static struct request *blk_mq_get_request(struct request_queue *q,
>  
>  	tag = blk_mq_get_tag(data);
>  	if (tag == BLK_MQ_TAG_FAIL) {
> -		if (put_ctx_on_error) {
> -			blk_mq_put_ctx(data->ctx);
> +		if (clear_ctx_on_error)
>  			data->ctx = NULL;
> -		}
>  		blk_queue_exit(q);
>  		return NULL;
>  	}
> @@ -431,8 +429,6 @@ struct request *blk_mq_alloc_request(struct request_queue *q, unsigned int op,
>  	if (!rq)
>  		return ERR_PTR(-EWOULDBLOCK);
>  
> -	blk_mq_put_ctx(alloc_data.ctx);
> -
>  	rq->__data_len = 0;
>  	rq->__sector = (sector_t) -1;
>  	rq->bio = rq->biotail = NULL;
> @@ -1981,7 +1977,6 @@ static blk_qc_t blk_mq_make_request(struct request_queue *q, struct bio *bio)
>  
>  	plug = current->plug;
>  	if (unlikely(is_flush_fua)) {
> -		blk_mq_put_ctx(data.ctx);
>  		blk_mq_bio_to_request(rq, bio, nr_segs);
>  
>  		/* bypass scheduler for flush rq */
> @@ -1995,7 +1990,6 @@ static blk_qc_t blk_mq_make_request(struct request_queue *q, struct bio *bio)
>  		unsigned int request_count = plug->rq_count;
>  		struct request *last = NULL;
>  
> -		blk_mq_put_ctx(data.ctx);
>  		blk_mq_bio_to_request(rq, bio, nr_segs);
>  
>  		if (!request_count)
> @@ -2029,8 +2023,6 @@ static blk_qc_t blk_mq_make_request(struct request_queue *q, struct bio *bio)
>  		blk_add_rq_to_plug(plug, rq);
>  		trace_block_plug(q);
>  
> -		blk_mq_put_ctx(data.ctx);
> -
>  		if (same_queue_rq) {
>  			data.hctx = same_queue_rq->mq_hctx;
>  			trace_block_unplug(q, 1, true);
> @@ -2039,11 +2031,9 @@ static blk_qc_t blk_mq_make_request(struct request_queue *q, struct bio *bio)
>  		}
>  	} else if ((q->nr_hw_queues > 1 && is_sync) || (!q->elevator &&
>  			!data.hctx->dispatch_busy)) {
> -		blk_mq_put_ctx(data.ctx);
>  		blk_mq_bio_to_request(rq, bio, nr_segs);
>  		blk_mq_try_issue_directly(data.hctx, rq, &cookie);
>  	} else {
> -		blk_mq_put_ctx(data.ctx);
>  		blk_mq_bio_to_request(rq, bio, nr_segs);
>  		blk_mq_sched_insert_request(rq, false, true, true);
>  	}
> diff --git a/block/blk-mq.h b/block/blk-mq.h
> index 633a5a77ee8b..f4bf5161333e 100644
> --- a/block/blk-mq.h
> +++ b/block/blk-mq.h
> @@ -151,12 +151,7 @@ static inline struct blk_mq_ctx *__blk_mq_get_ctx(struct request_queue *q,
>   */
>  static inline struct blk_mq_ctx *blk_mq_get_ctx(struct request_queue *q)
>  {
> -	return __blk_mq_get_ctx(q, get_cpu());
> -}
> -
> -static inline void blk_mq_put_ctx(struct blk_mq_ctx *ctx)
> -{
> -	put_cpu();
> +	return __blk_mq_get_ctx(q, raw_smp_processor_id());
>  }

Then there isn't any difference between __blk_mq_get_ctx() and blk_mq_get_ctx(),
so could you kill __blk_mq_get_ctx()?

Otherwise, looks fine:

Reviewed-by: Ming Lei <ming.lei@redhat.com>

Thanks,
Ming

Re: [PATCH 1/2] blk-mq: Remove blk_mq_put_ctx()

[Bart Van Assche]

On 7/2/19 7:45 PM, Ming Lei wrote:
> Then there isn't any difference between __blk_mq_get_ctx() and blk_mq_get_ctx(),
> so could you kill __blk_mq_get_ctx()?

I will post a patch that does this unless someone else does this before me.

Bart.

[PATCH 2/2] blk-mq: Simplify blk_mq_make_request()

[Bart Van Assche]

Move the blk_mq_bio_to_request() call in front of the if-statement.

Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ming Lei <ming.lei@redhat.com>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Omar Sandoval <osandov@fb.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 block/blk-mq.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 4d661545ad1d..0fa03f524541 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1975,10 +1975,10 @@ static blk_qc_t blk_mq_make_request(struct request_queue *q, struct bio *bio)
 
 	cookie = request_to_qc_t(data.hctx, rq);
 
+	blk_mq_bio_to_request(rq, bio, nr_segs);
+
 	plug = current->plug;
 	if (unlikely(is_flush_fua)) {
-		blk_mq_bio_to_request(rq, bio, nr_segs);
-
 		/* bypass scheduler for flush rq */
 		blk_insert_flush(rq);
 		blk_mq_run_hw_queue(data.hctx, true);
@@ -1990,8 +1990,6 @@ static blk_qc_t blk_mq_make_request(struct request_queue *q, struct bio *bio)
 		unsigned int request_count = plug->rq_count;
 		struct request *last = NULL;
 
-		blk_mq_bio_to_request(rq, bio, nr_segs);
-
 		if (!request_count)
 			trace_block_plug(q);
 		else
@@ -2005,8 +2003,6 @@ static blk_qc_t blk_mq_make_request(struct request_queue *q, struct bio *bio)
 
 		blk_add_rq_to_plug(plug, rq);
 	} else if (plug && !blk_queue_nomerges(q)) {
-		blk_mq_bio_to_request(rq, bio, nr_segs);
-
 		/*
 		 * We do limited plugging. If the bio can be merged, do that.
 		 * Otherwise the existing request in the plug list will be
@@ -2031,10 +2027,8 @@ static blk_qc_t blk_mq_make_request(struct request_queue *q, struct bio *bio)
 		}
 	} else if ((q->nr_hw_queues > 1 && is_sync) || (!q->elevator &&
 			!data.hctx->dispatch_busy)) {
-		blk_mq_bio_to_request(rq, bio, nr_segs);
 		blk_mq_try_issue_directly(data.hctx, rq, &cookie);
 	} else {
-		blk_mq_bio_to_request(rq, bio, nr_segs);
 		blk_mq_sched_insert_request(rq, false, true, true);
 	}
 
-- 
2.22.0.410.gd8fdbe21b5-goog

Re: [PATCH 2/2] blk-mq: Simplify blk_mq_make_request()

[Christoph Hellwig]

On Mon, Jul 01, 2019 at 08:47:30AM -0700, Bart Van Assche wrote:
> Move the blk_mq_bio_to_request() call in front of the if-statement.

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 2/2] blk-mq: Simplify blk_mq_make_request()

[Minwoo Im]

On 19-07-01 08:47:30, Bart Van Assche wrote:
> Move the blk_mq_bio_to_request() call in front of the if-statement.

This looks good to me based on the first patch.

Reviewed-by: Minwoo Im <minwoo.im.dev@gmail.com>

Re: [PATCH 2/2] blk-mq: Simplify blk_mq_make_request()

[Ming Lei]

On Mon, Jul 01, 2019 at 08:47:30AM -0700, Bart Van Assche wrote:
> Move the blk_mq_bio_to_request() call in front of the if-statement.
> 
> Cc: Christoph Hellwig <hch@infradead.org>
> Cc: Ming Lei <ming.lei@redhat.com>
> Cc: Hannes Reinecke <hare@suse.com>
> Cc: Omar Sandoval <osandov@fb.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  block/blk-mq.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 4d661545ad1d..0fa03f524541 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -1975,10 +1975,10 @@ static blk_qc_t blk_mq_make_request(struct request_queue *q, struct bio *bio)
>  
>  	cookie = request_to_qc_t(data.hctx, rq);
>  
> +	blk_mq_bio_to_request(rq, bio, nr_segs);
> +
>  	plug = current->plug;
>  	if (unlikely(is_flush_fua)) {
> -		blk_mq_bio_to_request(rq, bio, nr_segs);
> -
>  		/* bypass scheduler for flush rq */
>  		blk_insert_flush(rq);
>  		blk_mq_run_hw_queue(data.hctx, true);
> @@ -1990,8 +1990,6 @@ static blk_qc_t blk_mq_make_request(struct request_queue *q, struct bio *bio)
>  		unsigned int request_count = plug->rq_count;
>  		struct request *last = NULL;
>  
> -		blk_mq_bio_to_request(rq, bio, nr_segs);
> -
>  		if (!request_count)
>  			trace_block_plug(q);
>  		else
> @@ -2005,8 +2003,6 @@ static blk_qc_t blk_mq_make_request(struct request_queue *q, struct bio *bio)
>  
>  		blk_add_rq_to_plug(plug, rq);
>  	} else if (plug && !blk_queue_nomerges(q)) {
> -		blk_mq_bio_to_request(rq, bio, nr_segs);
> -
>  		/*
>  		 * We do limited plugging. If the bio can be merged, do that.
>  		 * Otherwise the existing request in the plug list will be
> @@ -2031,10 +2027,8 @@ static blk_qc_t blk_mq_make_request(struct request_queue *q, struct bio *bio)
>  		}
>  	} else if ((q->nr_hw_queues > 1 && is_sync) || (!q->elevator &&
>  			!data.hctx->dispatch_busy)) {
> -		blk_mq_bio_to_request(rq, bio, nr_segs);
>  		blk_mq_try_issue_directly(data.hctx, rq, &cookie);
>  	} else {
> -		blk_mq_bio_to_request(rq, bio, nr_segs);
>  		blk_mq_sched_insert_request(rq, false, true, true);
>  	}
>  
> -- 
> 2.22.0.410.gd8fdbe21b5-goog
> 

Looks fine:

Reviewed-by: Ming Lei <ming.lei@redhat.com>

Thanks,
Ming

Re: [PATCH, RESEND 0/2] Simplify blk-mq implementation

[Jens Axboe]

On 7/1/19 9:47 AM, Bart Van Assche wrote:
> Hi Jens,
> 
> While reviewing the blk_mq_make_request() code I noticed that it is possible
> to simplify the implementation of that function. Please consider these patches
> for kernel v5.3.

Applied, thanks Bart.

-- 
Jens Axboe

[PATCH 0/2] Simplify blk-mq implementation

[Bart Van Assche]

Hi Jens,

While reviewing the blk_mq_make_request() code I noticed that it is possible
to simplify the implementation of that function. Please consider these patches
for kernel v5.3.

Thanks,

Bart.

Bart Van Assche (2):
  blk-mq: Remove blk_mq_put_ctx()
  blk-mq: Simplify blk_mq_make_request()

 block/blk-mq-sched.c  |  5 +----
 block/blk-mq-tag.c    |  8 --------
 block/blk-mq.c        | 26 +++++---------------------
 block/blk-mq.h        |  7 +------
 block/kyber-iosched.c |  1 -
 5 files changed, 7 insertions(+), 40 deletions(-)

-- 
2.22.0.rc3

[PATCH 2/2] blk-mq: Simplify blk_mq_make_request()

[Bart Van Assche]

Move the blk_mq_bio_to_request() call in front of the if-statement.

Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ming Lei <ming.lei@redhat.com>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Omar Sandoval <osandov@fb.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 block/blk-mq.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 00d572265aa3..8dd4a5cccf9a 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1969,10 +1969,10 @@ static blk_qc_t blk_mq_make_request(struct request_queue *q, struct bio *bio)
 
 	cookie = request_to_qc_t(data.hctx, rq);
 
+	blk_mq_bio_to_request(rq, bio);
+
 	plug = current->plug;
 	if (unlikely(is_flush_fua)) {
-		blk_mq_bio_to_request(rq, bio);
-
 		/* bypass scheduler for flush rq */
 		blk_insert_flush(rq);
 		blk_mq_run_hw_queue(data.hctx, true);
@@ -1984,8 +1984,6 @@ static blk_qc_t blk_mq_make_request(struct request_queue *q, struct bio *bio)
 		unsigned int request_count = plug->rq_count;
 		struct request *last = NULL;
 
-		blk_mq_bio_to_request(rq, bio);
-
 		if (!request_count)
 			trace_block_plug(q);
 		else
@@ -1999,8 +1997,6 @@ static blk_qc_t blk_mq_make_request(struct request_queue *q, struct bio *bio)
 
 		blk_add_rq_to_plug(plug, rq);
 	} else if (plug && !blk_queue_nomerges(q)) {
-		blk_mq_bio_to_request(rq, bio);
-
 		/*
 		 * We do limited plugging. If the bio can be merged, do that.
 		 * Otherwise the existing request in the plug list will be
@@ -2025,10 +2021,8 @@ static blk_qc_t blk_mq_make_request(struct request_queue *q, struct bio *bio)
 		}
 	} else if ((q->nr_hw_queues > 1 && is_sync) || (!q->elevator &&
 			!data.hctx->dispatch_busy)) {
-		blk_mq_bio_to_request(rq, bio);
 		blk_mq_try_issue_directly(data.hctx, rq, &cookie);
 	} else {
-		blk_mq_bio_to_request(rq, bio);
 		blk_mq_sched_insert_request(rq, false, true, true);
 	}
 
-- 
2.22.0.rc3