[PATCH 0/2] blk-mq: fix hang in blk_mq_freeze_queue_wait

[PATCH 0/2] blk-mq: fix hang in blk_mq_freeze_queue_wait

[Ming Lei]

Hello,

The 1st patch fixes hang in blk_mq_freeze_queue_wait().

The 2nd one renames blk_attempt_bio_merge, so we can avoid duplicated
symbols in block layer. 


Ming Lei (2):
  blk-mq: don't grab ->q_usage_counter in blk_mq_sched_bio_merge
  blk-mq: rename blk_attempt_bio_merge

 block/blk-mq-sched.c |  4 ----
 block/blk-mq.c       | 10 ++++++----
 2 files changed, 6 insertions(+), 8 deletions(-)

-- 
2.31.1

[PATCH 1/2] blk-mq: don't grab ->q_usage_counter in blk_mq_sched_bio_merge

[Ming Lei]

blk_mq_sched_bio_merge is only called from blk-mq.c:blk_attempt_bio_merge(),
which is called when queue usage counter is grabbed already:

1) blk_mq_get_new_requests()

2) blk_mq_get_request()
- cached request in current plug owns one queue usage counter

So don't grab ->q_usage_counter in blk_mq_sched_bio_merge(), and more
importantly this nest way causes hang in blk_mq_freeze_queue_wait().

Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 block/blk-mq-sched.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
index 4be652fa38e7..ba21449439cc 100644
--- a/block/blk-mq-sched.c
+++ b/block/blk-mq-sched.c
@@ -370,9 +370,6 @@ bool blk_mq_sched_bio_merge(struct request_queue *q, struct bio *bio,
 	bool ret = false;
 	enum hctx_type type;
 
-	if (bio_queue_enter(bio))
-		return false;
-
 	if (e && e->type->ops.bio_merge) {
 		ret = e->type->ops.bio_merge(q, bio, nr_segs);
 		goto out_put;
@@ -397,7 +394,6 @@ bool blk_mq_sched_bio_merge(struct request_queue *q, struct bio *bio,
 
 	spin_unlock(&ctx->lock);
 out_put:
-	blk_queue_exit(q);
 	return ret;
 }
 
-- 
2.31.1

Re: [PATCH 1/2] blk-mq: don't grab ->q_usage_counter in blk_mq_sched_bio_merge

[Christoph Hellwig]

On Thu, Nov 11, 2021 at 04:51:33PM +0800, Ming Lei wrote:
> blk_mq_sched_bio_merge is only called from blk-mq.c:blk_attempt_bio_merge(),
> which is called when queue usage counter is grabbed already:
> 
> 1) blk_mq_get_new_requests()
> 
> 2) blk_mq_get_request()
> - cached request in current plug owns one queue usage counter
> 
> So don't grab ->q_usage_counter in blk_mq_sched_bio_merge(), and more
> importantly this nest way causes hang in blk_mq_freeze_queue_wait().

Looks good,

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

[PATCH 2/2] blk-mq: rename blk_attempt_bio_merge

[Ming Lei]

It is very annoying to have two block layer functions which share same
name, so rename blk_attempt_bio_merge in blk-mq.c as
blk_mq_attempt_bio_merge.

Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 block/blk-mq.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 629cf421417f..f511db395c7f 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -2495,8 +2495,9 @@ static inline unsigned short blk_plug_max_rq_count(struct blk_plug *plug)
 	return BLK_MAX_REQUEST_COUNT;
 }
 
-static bool blk_attempt_bio_merge(struct request_queue *q, struct bio *bio,
-				  unsigned int nr_segs, bool *same_queue_rq)
+static bool blk_mq_attempt_bio_merge(struct request_queue *q,
+				     struct bio *bio, unsigned int nr_segs,
+				     bool *same_queue_rq)
 {
 	if (!blk_queue_nomerges(q) && bio_mergeable(bio)) {
 		if (blk_attempt_plug_merge(q, bio, nr_segs, same_queue_rq))
@@ -2524,7 +2525,7 @@ static struct request *blk_mq_get_new_requests(struct request_queue *q,
 		return NULL;
 	if (unlikely(!submit_bio_checks(bio)))
 		goto put_exit;
-	if (blk_attempt_bio_merge(q, bio, nsegs, same_queue_rq))
+	if (blk_mq_attempt_bio_merge(q, bio, nsegs, same_queue_rq))
 		goto put_exit;
 
 	rq_qos_throttle(q, bio);
@@ -2560,7 +2561,8 @@ static inline struct request *blk_mq_get_request(struct request_queue *q,
 		if (rq && rq->q == q) {
 			if (unlikely(!submit_bio_checks(bio)))
 				return NULL;
-			if (blk_attempt_bio_merge(q, bio, nsegs, same_queue_rq))
+			if (blk_mq_attempt_bio_merge(q, bio, nsegs,
+						same_queue_rq))
 				return NULL;
 			plug->cached_rq = rq_list_next(rq);
 			INIT_LIST_HEAD(&rq->queuelist);
-- 
2.31.1

Re: [PATCH 2/2] blk-mq: rename blk_attempt_bio_merge

[Christoph Hellwig]

On Thu, Nov 11, 2021 at 04:51:34PM +0800, Ming Lei wrote:
> It is very annoying to have two block layer functions which share same
> name, so rename blk_attempt_bio_merge in blk-mq.c as
> blk_mq_attempt_bio_merge.

Looks good,

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

Re: [PATCH 0/2] blk-mq: fix hang in blk_mq_freeze_queue_wait

[Jens Axboe]

On Thu, 11 Nov 2021 16:51:32 +0800, Ming Lei wrote:
> The 1st patch fixes hang in blk_mq_freeze_queue_wait().
> 
> The 2nd one renames blk_attempt_bio_merge, so we can avoid duplicated
> symbols in block layer.
> 
> 
> Ming Lei (2):
>   blk-mq: don't grab ->q_usage_counter in blk_mq_sched_bio_merge
>   blk-mq: rename blk_attempt_bio_merge
> 
> [...]

Applied, thanks!

[1/2] blk-mq: don't grab ->q_usage_counter in blk_mq_sched_bio_merge
      commit: c20eb570e2c3aaf0890d85fc6d8ab84b8800c167
[2/2] blk-mq: rename blk_attempt_bio_merge
      commit: c7f87443cd935134b21fcc6b76693f45d15fccc0

Best regards,
-- 
Jens Axboe