All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] blk-mq: fix handling cpu hotplug
@ 2020-06-05 11:44 Ming Lei
  2020-06-05 11:44 ` [PATCH 1/2] blk-mq: split out a __blk_mq_get_driver_tag helper Ming Lei
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Ming Lei @ 2020-06-05 11:44 UTC (permalink / raw
  To: Jens Axboe
  Cc: linux-block, Ming Lei, Dongli Zhang, Hannes Reinecke,
	Daniel Wagner, Christoph Hellwig, John Garry

Hi Jens,

The 1st patch avoids to fail driver tag allocation because of inactive
hctx, so hang risk can be killed during cpu hotplug.

The 2nd patch fixes blk_mq_all_tag_iter so that we can drain all
requests before one hctx becomes inactive.

Both fixes bf0beec0607d ("blk-mq: drain I/O when all CPUs in a hctx are
offline").

John has verified that the two can fix his request timeout issue during
cpu hotplug.

Christoph Hellwig (1):
  blk-mq: split out a __blk_mq_get_driver_tag helper

Ming Lei (1):
  blk-mq: fix blk_mq_all_tag_iter

 block/blk-mq-tag.c | 39 ++++++++++++++++++++++++++++++++++++---
 block/blk-mq-tag.h |  8 ++++++++
 block/blk-mq.c     | 29 -----------------------------
 block/blk-mq.h     |  1 -
 4 files changed, 44 insertions(+), 33 deletions(-)

Cc: Dongli Zhang <dongli.zhang@oracle.com>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Daniel Wagner <dwagner@suse.de>
Cc: Christoph Hellwig <hch@lst.de>
Cc: John Garry <john.garry@huawei.com>


-- 
2.25.2


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] blk-mq: split out a __blk_mq_get_driver_tag helper
  2020-06-05 11:44 [PATCH 0/2] blk-mq: fix handling cpu hotplug Ming Lei
@ 2020-06-05 11:44 ` Ming Lei
  2020-06-05 11:44 ` [PATCH 2/2] blk-mq: fix blk_mq_all_tag_iter Ming Lei
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Ming Lei @ 2020-06-05 11:44 UTC (permalink / raw
  To: Jens Axboe
  Cc: linux-block, Christoph Hellwig, Dongli Zhang, Hannes Reinecke,
	Daniel Wagner, John Garry, Ming Lei

From: Christoph Hellwig <hch@lst.de>

Allocation of the driver tag in the case of using a scheduler shares very
little code with the "normal" tag allocation.  Split out a new helper to
streamline this path, and untangle it from the complex normal tag
allocation.

This way also avoids to fail driver tag allocation because of inactive hctx
during cpu hotplug, and fixes potential hang risk.

Cc: Dongli Zhang <dongli.zhang@oracle.com>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Daniel Wagner <dwagner@suse.de>
Tested-by: John Garry <john.garry@huawei.com>
Fixes: bf0beec0607d ("blk-mq: drain I/O when all CPUs in a hctx are offline")
Signed-off-by: Ming Lei <ming.lei@redhat.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-mq-tag.c | 27 +++++++++++++++++++++++++++
 block/blk-mq-tag.h |  8 ++++++++
 block/blk-mq.c     | 29 -----------------------------
 block/blk-mq.h     |  1 -
 4 files changed, 35 insertions(+), 30 deletions(-)

diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
index 96a39d0724a2..cded7fdcad8e 100644
--- a/block/blk-mq-tag.c
+++ b/block/blk-mq-tag.c
@@ -191,6 +191,33 @@ unsigned int blk_mq_get_tag(struct blk_mq_alloc_data *data)
 	return tag + tag_offset;
 }
 
+bool __blk_mq_get_driver_tag(struct request *rq)
+{
+	struct sbitmap_queue *bt = &rq->mq_hctx->tags->bitmap_tags;
+	unsigned int tag_offset = rq->mq_hctx->tags->nr_reserved_tags;
+	bool shared = blk_mq_tag_busy(rq->mq_hctx);
+	int tag;
+
+	if (blk_mq_tag_is_reserved(rq->mq_hctx->sched_tags, rq->internal_tag)) {
+		bt = &rq->mq_hctx->tags->breserved_tags;
+		tag_offset = 0;
+	}
+
+	if (!hctx_may_queue(rq->mq_hctx, bt))
+		return false;
+	tag = __sbitmap_queue_get(bt);
+	if (tag == BLK_MQ_NO_TAG)
+		return false;
+
+	rq->tag = tag + tag_offset;
+	if (shared) {
+		rq->rq_flags |= RQF_MQ_INFLIGHT;
+		atomic_inc(&rq->mq_hctx->nr_active);
+	}
+	rq->mq_hctx->tags->rqs[rq->tag] = rq;
+	return true;
+}
+
 void blk_mq_put_tag(struct blk_mq_tags *tags, struct blk_mq_ctx *ctx,
 		    unsigned int tag)
 {
diff --git a/block/blk-mq-tag.h b/block/blk-mq-tag.h
index d38e48f2a0a4..2e4ef51cdb32 100644
--- a/block/blk-mq-tag.h
+++ b/block/blk-mq-tag.h
@@ -51,6 +51,14 @@ enum {
 	BLK_MQ_TAG_MAX		= BLK_MQ_NO_TAG - 1,
 };
 
+bool __blk_mq_get_driver_tag(struct request *rq);
+static inline bool blk_mq_get_driver_tag(struct request *rq)
+{
+	if (rq->tag != BLK_MQ_NO_TAG)
+		return true;
+	return __blk_mq_get_driver_tag(rq);
+}
+
 extern bool __blk_mq_tag_busy(struct blk_mq_hw_ctx *);
 extern void __blk_mq_tag_idle(struct blk_mq_hw_ctx *);
 
diff --git a/block/blk-mq.c b/block/blk-mq.c
index a98a19353461..492ac216eac0 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1052,35 +1052,6 @@ static inline unsigned int queued_to_index(unsigned int queued)
 	return min(BLK_MQ_MAX_DISPATCH_ORDER - 1, ilog2(queued) + 1);
 }
 
-bool blk_mq_get_driver_tag(struct request *rq)
-{
-	struct blk_mq_alloc_data data = {
-		.q = rq->q,
-		.hctx = rq->mq_hctx,
-		.flags = BLK_MQ_REQ_NOWAIT,
-		.cmd_flags = rq->cmd_flags,
-	};
-	bool shared;
-
-	if (rq->tag != BLK_MQ_NO_TAG)
-		return true;
-
-	if (blk_mq_tag_is_reserved(data.hctx->sched_tags, rq->internal_tag))
-		data.flags |= BLK_MQ_REQ_RESERVED;
-
-	shared = blk_mq_tag_busy(data.hctx);
-	rq->tag = blk_mq_get_tag(&data);
-	if (rq->tag >= 0) {
-		if (shared) {
-			rq->rq_flags |= RQF_MQ_INFLIGHT;
-			atomic_inc(&data.hctx->nr_active);
-		}
-		data.hctx->tags->rqs[rq->tag] = rq;
-	}
-
-	return rq->tag != BLK_MQ_NO_TAG;
-}
-
 static int blk_mq_dispatch_wake(wait_queue_entry_t *wait, unsigned mode,
 				int flags, void *key)
 {
diff --git a/block/blk-mq.h b/block/blk-mq.h
index f3a93acfad03..f4c57c28ab40 100644
--- a/block/blk-mq.h
+++ b/block/blk-mq.h
@@ -45,7 +45,6 @@ bool blk_mq_dispatch_rq_list(struct blk_mq_hw_ctx *hctx, struct list_head *,
 void blk_mq_add_to_requeue_list(struct request *rq, bool at_head,
 				bool kick_requeue_list);
 void blk_mq_flush_busy_ctxs(struct blk_mq_hw_ctx *hctx, struct list_head *list);
-bool blk_mq_get_driver_tag(struct request *rq);
 struct request *blk_mq_dequeue_from_ctx(struct blk_mq_hw_ctx *hctx,
 					struct blk_mq_ctx *start);
 
-- 
2.25.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] blk-mq: fix blk_mq_all_tag_iter
  2020-06-05 11:44 [PATCH 0/2] blk-mq: fix handling cpu hotplug Ming Lei
  2020-06-05 11:44 ` [PATCH 1/2] blk-mq: split out a __blk_mq_get_driver_tag helper Ming Lei
@ 2020-06-05 11:44 ` Ming Lei
  2020-06-05 11:49 ` [PATCH 0/2] blk-mq: fix handling cpu hotplug John Garry
  2020-06-07 14:57 ` Jens Axboe
  3 siblings, 0 replies; 6+ messages in thread
From: Ming Lei @ 2020-06-05 11:44 UTC (permalink / raw
  To: Jens Axboe
  Cc: linux-block, Ming Lei, Dongli Zhang, Hannes Reinecke,
	Daniel Wagner, Christoph Hellwig, John Garry

blk_mq_all_tag_iter() is added to iterate all requests, so we should
fetch the request from ->static_rqs][] instead of ->rqs[] which is for
holding in-flight request only.

Fix it by adding flag of BT_TAG_ITER_STATIC_RQS.

Cc: Dongli Zhang <dongli.zhang@oracle.com>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Daniel Wagner <dwagner@suse.de>
Cc: Christoph Hellwig <hch@lst.de>
Tested-by: John Garry <john.garry@huawei.com>
Fixes: bf0beec0607d ("blk-mq: drain I/O when all CPUs in a hctx are offline")
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 block/blk-mq-tag.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
index cded7fdcad8e..44f3d0967cb4 100644
--- a/block/blk-mq-tag.c
+++ b/block/blk-mq-tag.c
@@ -296,6 +296,7 @@ struct bt_tags_iter_data {
 
 #define BT_TAG_ITER_RESERVED		(1 << 0)
 #define BT_TAG_ITER_STARTED		(1 << 1)
+#define BT_TAG_ITER_STATIC_RQS		(1 << 2)
 
 static bool bt_tags_iter(struct sbitmap *bitmap, unsigned int bitnr, void *data)
 {
@@ -309,9 +310,12 @@ static bool bt_tags_iter(struct sbitmap *bitmap, unsigned int bitnr, void *data)
 
 	/*
 	 * We can hit rq == NULL here, because the tagging functions
-	 * test and set the bit before assining ->rqs[].
+	 * test and set the bit before assigning ->rqs[].
 	 */
-	rq = tags->rqs[bitnr];
+	if (iter_data->flags & BT_TAG_ITER_STATIC_RQS)
+		rq = tags->static_rqs[bitnr];
+	else
+		rq = tags->rqs[bitnr];
 	if (!rq)
 		return true;
 	if ((iter_data->flags & BT_TAG_ITER_STARTED) &&
@@ -366,11 +370,13 @@ static void __blk_mq_all_tag_iter(struct blk_mq_tags *tags,
  *		indicates whether or not @rq is a reserved request. Return
  *		true to continue iterating tags, false to stop.
  * @priv:	Will be passed as second argument to @fn.
+ *
+ * Caller has to pass the tag map from which requests are allocated.
  */
 void blk_mq_all_tag_iter(struct blk_mq_tags *tags, busy_tag_iter_fn *fn,
 		void *priv)
 {
-	return __blk_mq_all_tag_iter(tags, fn, priv, 0);
+	return __blk_mq_all_tag_iter(tags, fn, priv, BT_TAG_ITER_STATIC_RQS);
 }
 
 /**
-- 
2.25.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/2] blk-mq: fix handling cpu hotplug
  2020-06-05 11:44 [PATCH 0/2] blk-mq: fix handling cpu hotplug Ming Lei
  2020-06-05 11:44 ` [PATCH 1/2] blk-mq: split out a __blk_mq_get_driver_tag helper Ming Lei
  2020-06-05 11:44 ` [PATCH 2/2] blk-mq: fix blk_mq_all_tag_iter Ming Lei
@ 2020-06-05 11:49 ` John Garry
  2020-06-05 16:08   ` John Garry
  2020-06-07 14:57 ` Jens Axboe
  3 siblings, 1 reply; 6+ messages in thread
From: John Garry @ 2020-06-05 11:49 UTC (permalink / raw
  To: Ming Lei, Jens Axboe
  Cc: linux-block, Dongli Zhang, Hannes Reinecke, Daniel Wagner,
	Christoph Hellwig

On 05/06/2020 12:44, Ming Lei wrote:
> Hi Jens,
> 
> The 1st patch avoids to fail driver tag allocation because of inactive
> hctx, so hang risk can be killed during cpu hotplug.
> 
> The 2nd patch fixes blk_mq_all_tag_iter so that we can drain all
> requests before one hctx becomes inactive.
> 
> Both fixes bf0beec0607d ("blk-mq: drain I/O when all CPUs in a hctx are
> offline").
> 
> John has verified that the two can fix his request timeout issue during
> cpu hotplug.

But let me test it again my afternoon. My test branch earlier had some
debug stuff.

Cheers

> 
> Christoph Hellwig (1):
>    blk-mq: split out a __blk_mq_get_driver_tag helper
> 
> Ming Lei (1):
>    blk-mq: fix blk_mq_all_tag_iter
> 
>   block/blk-mq-tag.c | 39 ++++++++++++++++++++++++++++++++++++---
>   block/blk-mq-tag.h |  8 ++++++++
>   block/blk-mq.c     | 29 -----------------------------
>   block/blk-mq.h     |  1 -
>   4 files changed, 44 insertions(+), 33 deletions(-)
> 
> Cc: Dongli Zhang <dongli.zhang@oracle.com>
> Cc: Hannes Reinecke <hare@suse.de>
> Cc: Daniel Wagner <dwagner@suse.de>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: John Garry <john.garry@huawei.com>
> 
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/2] blk-mq: fix handling cpu hotplug
  2020-06-05 11:49 ` [PATCH 0/2] blk-mq: fix handling cpu hotplug John Garry
@ 2020-06-05 16:08   ` John Garry
  0 siblings, 0 replies; 6+ messages in thread
From: John Garry @ 2020-06-05 16:08 UTC (permalink / raw
  To: Ming Lei, Jens Axboe
  Cc: linux-block, Dongli Zhang, Hannes Reinecke, Daniel Wagner,
	Christoph Hellwig

On 05/06/2020 12:49, John Garry wrote:
> On 05/06/2020 12:44, Ming Lei wrote:
>> Hi Jens,
>>
>> The 1st patch avoids to fail driver tag allocation because of inactive
>> hctx, so hang risk can be killed during cpu hotplug.
>>
>> The 2nd patch fixes blk_mq_all_tag_iter so that we can drain all
>> requests before one hctx becomes inactive.
>>
>> Both fixes bf0beec0607d ("blk-mq: drain I/O when all CPUs in a hctx are
>> offline").
>>
>> John has verified that the two can fix his request timeout issue during
>> cpu hotplug.
> 
> But let me test it again my afternoon. My test branch earlier had some
> debug stuff.
> 

it looks ok, so the tested-by stands.

BTW, do you agree that the following comment from 
__blk_mq_run_hw_queue() is now stale:

"... we depend on blk-mq timeout handler to handle dispatched requests 
to this hctx"

Thanks,
John


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/2] blk-mq: fix handling cpu hotplug
  2020-06-05 11:44 [PATCH 0/2] blk-mq: fix handling cpu hotplug Ming Lei
                   ` (2 preceding siblings ...)
  2020-06-05 11:49 ` [PATCH 0/2] blk-mq: fix handling cpu hotplug John Garry
@ 2020-06-07 14:57 ` Jens Axboe
  3 siblings, 0 replies; 6+ messages in thread
From: Jens Axboe @ 2020-06-07 14:57 UTC (permalink / raw
  To: Ming Lei
  Cc: linux-block, Dongli Zhang, Hannes Reinecke, Daniel Wagner,
	Christoph Hellwig, John Garry

On 6/5/20 5:44 AM, Ming Lei wrote:
> Hi Jens,
> 
> The 1st patch avoids to fail driver tag allocation because of inactive
> hctx, so hang risk can be killed during cpu hotplug.
> 
> The 2nd patch fixes blk_mq_all_tag_iter so that we can drain all
> requests before one hctx becomes inactive.
> 
> Both fixes bf0beec0607d ("blk-mq: drain I/O when all CPUs in a hctx are
> offline").
> 
> John has verified that the two can fix his request timeout issue during
> cpu hotplug.

Thanks, applied.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-06-07 14:57 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-06-05 11:44 [PATCH 0/2] blk-mq: fix handling cpu hotplug Ming Lei
2020-06-05 11:44 ` [PATCH 1/2] blk-mq: split out a __blk_mq_get_driver_tag helper Ming Lei
2020-06-05 11:44 ` [PATCH 2/2] blk-mq: fix blk_mq_all_tag_iter Ming Lei
2020-06-05 11:49 ` [PATCH 0/2] blk-mq: fix handling cpu hotplug John Garry
2020-06-05 16:08   ` John Garry
2020-06-07 14:57 ` Jens Axboe

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.