All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2 0/6] blk-mq: update_nr_hw_queues related improvement & bugfix
@ 2022-03-02 12:14 Ming Lei
  2022-03-02 12:14 ` [PATCH V2 1/6] blk-mq: figure out correct numa node for hw queue Ming Lei
                   ` (5 more replies)
  0 siblings, 6 replies; 14+ messages in thread
From: Ming Lei @ 2022-03-02 12:14 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block, Yu Kuai, Ming Lei

Hi,

The 1st patch figures out correct numa node for each kind of hw queue.

The 2nd patch simplifies reallocation of q->queue_hw_ctx a bit.

The 3rd patch re-configures poll capability after queue map is changed.

The 4th patch changes mtip32xx to avoid to refer to q->queue_hw_ctx
directly.

The 5th & 6th patches fix use-after-free on q->queue_hw_ctx.

V2:
	- use xa_for_each() to implement queue_for_each_hw_ctx(5/6 6/6) 
	- patch style change(1/6)
	- rename as suggested by Christoph(3/6)

Ming Lei (6):
  blk-mq: figure out correct numa node for hw queue
  blk-mq: simplify reallocation of hw ctxs a bit
  blk-mq: reconfigure poll after queue map is changed
  block: mtip32xx: don't touch q->queue_hw_ctx
  blk-mq: prepare for implementing hctx table via xarray
  blk-mq: manage hctx map via xarray

 block/blk-mq-debugfs.c            |   6 +-
 block/blk-mq-sched.c              |   9 +-
 block/blk-mq-sysfs.c              |  12 ++-
 block/blk-mq-tag.c                |   4 +-
 block/blk-mq.c                    | 158 +++++++++++++++++-------------
 block/blk-mq.h                    |   2 +-
 drivers/block/mtip32xx/mtip32xx.c |   4 +-
 drivers/block/rnbd/rnbd-clt.c     |   2 +-
 include/linux/blk-mq.h            |   3 +-
 include/linux/blkdev.h            |   2 +-
 10 files changed, 113 insertions(+), 89 deletions(-)

-- 
2.31.1


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

* [PATCH V2 1/6] blk-mq: figure out correct numa node for hw queue
  2022-03-02 12:14 [PATCH V2 0/6] blk-mq: update_nr_hw_queues related improvement & bugfix Ming Lei
@ 2022-03-02 12:14 ` Ming Lei
  2022-03-02 12:39   ` John Garry
  2022-03-02 12:14 ` [PATCH V2 2/6] blk-mq: simplify reallocation of hw ctxs a bit Ming Lei
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Ming Lei @ 2022-03-02 12:14 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block, Yu Kuai, Ming Lei, Christoph Hellwig

The current code always uses default queue map and hw queue index
for figuring out the numa node for hw queue, this way isn't correct
because blk-mq supports three queue maps, and the correct queue map
should be used for the specified hw queue.

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

diff --git a/block/blk-mq.c b/block/blk-mq.c
index a05ce7725031..5d25abf9e551 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -3107,15 +3107,41 @@ void blk_mq_free_rq_map(struct blk_mq_tags *tags)
 	blk_mq_free_tags(tags);
 }
 
+static enum hctx_type hctx_idx_to_type(struct blk_mq_tag_set *set,
+		unsigned int hctx_idx)
+{
+	int i;
+
+	for (i = 0; i < set->nr_maps; i++) {
+		unsigned int start = set->map[i].queue_offset;
+		unsigned int end = start + set->map[i].nr_queues;
+
+		if (hctx_idx >= start && hctx_idx < end)
+			break;
+	}
+
+	if (i >= set->nr_maps)
+		i = HCTX_TYPE_DEFAULT;
+
+	return i;
+}
+
+static int blk_mq_get_hctx_node(struct blk_mq_tag_set *set,
+		unsigned int hctx_idx)
+{
+	enum hctx_type type = hctx_idx_to_type(set, hctx_idx);
+
+	return blk_mq_hw_queue_to_node(&set->map[type], hctx_idx);
+}
+
 static struct blk_mq_tags *blk_mq_alloc_rq_map(struct blk_mq_tag_set *set,
 					       unsigned int hctx_idx,
 					       unsigned int nr_tags,
 					       unsigned int reserved_tags)
 {
+	int node = blk_mq_get_hctx_node(set, hctx_idx);
 	struct blk_mq_tags *tags;
-	int node;
 
-	node = blk_mq_hw_queue_to_node(&set->map[HCTX_TYPE_DEFAULT], hctx_idx);
 	if (node == NUMA_NO_NODE)
 		node = set->numa_node;
 
@@ -3164,10 +3190,9 @@ static int blk_mq_alloc_rqs(struct blk_mq_tag_set *set,
 			    unsigned int hctx_idx, unsigned int depth)
 {
 	unsigned int i, j, entries_per_page, max_order = 4;
+	int node = blk_mq_get_hctx_node(set, hctx_idx);
 	size_t rq_size, left;
-	int node;
 
-	node = blk_mq_hw_queue_to_node(&set->map[HCTX_TYPE_DEFAULT], hctx_idx);
 	if (node == NUMA_NO_NODE)
 		node = set->numa_node;
 
@@ -3941,10 +3966,9 @@ static void blk_mq_realloc_hw_ctxs(struct blk_mq_tag_set *set,
 	/* protect against switching io scheduler  */
 	mutex_lock(&q->sysfs_lock);
 	for (i = 0; i < set->nr_hw_queues; i++) {
-		int node;
+		int node = blk_mq_get_hctx_node(set, i);
 		struct blk_mq_hw_ctx *hctx;
 
-		node = blk_mq_hw_queue_to_node(&set->map[HCTX_TYPE_DEFAULT], i);
 		/*
 		 * If the hw queue has been mapped to another numa node,
 		 * we need to realloc the hctx. If allocation fails, fallback
-- 
2.31.1


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

* [PATCH V2 2/6] blk-mq: simplify reallocation of hw ctxs a bit
  2022-03-02 12:14 [PATCH V2 0/6] blk-mq: update_nr_hw_queues related improvement & bugfix Ming Lei
  2022-03-02 12:14 ` [PATCH V2 1/6] blk-mq: figure out correct numa node for hw queue Ming Lei
@ 2022-03-02 12:14 ` Ming Lei
  2022-03-02 12:14 ` [PATCH V2 3/6] blk-mq: reconfigure poll after queue map is changed Ming Lei
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 14+ messages in thread
From: Ming Lei @ 2022-03-02 12:14 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block, Yu Kuai, Ming Lei, Christoph Hellwig

blk_mq_alloc_and_init_hctx() has already taken reuse into account, so
no need to do it outside, then we can simplify blk_mq_realloc_hw_ctxs().

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

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 5d25abf9e551..64fddb36c93c 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -3966,29 +3966,24 @@ static void blk_mq_realloc_hw_ctxs(struct blk_mq_tag_set *set,
 	/* protect against switching io scheduler  */
 	mutex_lock(&q->sysfs_lock);
 	for (i = 0; i < set->nr_hw_queues; i++) {
+		int old_node;
 		int node = blk_mq_get_hctx_node(set, i);
-		struct blk_mq_hw_ctx *hctx;
+		struct blk_mq_hw_ctx *old_hctx = hctxs[i];
 
-		/*
-		 * If the hw queue has been mapped to another numa node,
-		 * we need to realloc the hctx. If allocation fails, fallback
-		 * to use the previous one.
-		 */
-		if (hctxs[i] && (hctxs[i]->numa_node == node))
-			continue;
+		if (old_hctx) {
+			old_node = old_hctx->numa_node;
+			blk_mq_exit_hctx(q, set, old_hctx, i);
+		}
 
-		hctx = blk_mq_alloc_and_init_hctx(set, q, i, node);
-		if (hctx) {
-			if (hctxs[i])
-				blk_mq_exit_hctx(q, set, hctxs[i], i);
-			hctxs[i] = hctx;
-		} else {
-			if (hctxs[i])
-				pr_warn("Allocate new hctx on node %d fails,\
-						fallback to previous one on node %d\n",
-						node, hctxs[i]->numa_node);
-			else
+		hctxs[i] = blk_mq_alloc_and_init_hctx(set, q, i, node);
+		if (!hctxs[i]) {
+			if (!old_hctx)
 				break;
+			pr_warn("Allocate new hctx on node %d fails, fallback to previous one on node %d\n",
+					node, old_node);
+			hctxs[i] = blk_mq_alloc_and_init_hctx(set, q, i,
+					old_node);
+			WARN_ON_ONCE(!hctxs[i]);
 		}
 	}
 	/*
-- 
2.31.1


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

* [PATCH V2 3/6] blk-mq: reconfigure poll after queue map is changed
  2022-03-02 12:14 [PATCH V2 0/6] blk-mq: update_nr_hw_queues related improvement & bugfix Ming Lei
  2022-03-02 12:14 ` [PATCH V2 1/6] blk-mq: figure out correct numa node for hw queue Ming Lei
  2022-03-02 12:14 ` [PATCH V2 2/6] blk-mq: simplify reallocation of hw ctxs a bit Ming Lei
@ 2022-03-02 12:14 ` Ming Lei
  2022-03-03 15:20   ` John Garry
  2022-03-02 12:14 ` [PATCH V2 4/6] block: mtip32xx: don't touch q->queue_hw_ctx Ming Lei
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Ming Lei @ 2022-03-02 12:14 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block, Yu Kuai, Ming Lei

queue map can be changed when updating nr_hw_queues, so we need to
reconfigure queue's poll capability. Add one helper for doing this job.

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

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 64fddb36c93c..57ae9df0f4dc 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -4010,6 +4010,17 @@ static void blk_mq_realloc_hw_ctxs(struct blk_mq_tag_set *set,
 	mutex_unlock(&q->sysfs_lock);
 }
 
+static void blk_mq_update_poll_flag(struct request_queue *q)
+{
+	struct blk_mq_tag_set *set = q->tag_set;
+
+	if (set->nr_maps > HCTX_TYPE_POLL &&
+	    set->map[HCTX_TYPE_POLL].nr_queues)
+		blk_queue_flag_set(QUEUE_FLAG_POLL, q);
+	else
+		blk_queue_flag_clear(QUEUE_FLAG_POLL, q);
+}
+
 int blk_mq_init_allocated_queue(struct blk_mq_tag_set *set,
 		struct request_queue *q)
 {
@@ -4044,9 +4055,7 @@ int blk_mq_init_allocated_queue(struct blk_mq_tag_set *set,
 	q->tag_set = set;
 
 	q->queue_flags |= QUEUE_FLAG_MQ_DEFAULT;
-	if (set->nr_maps > HCTX_TYPE_POLL &&
-	    set->map[HCTX_TYPE_POLL].nr_queues)
-		blk_queue_flag_set(QUEUE_FLAG_POLL, q);
+	blk_mq_update_poll_flag(q);
 
 	INIT_DELAYED_WORK(&q->requeue_work, blk_mq_requeue_work);
 	INIT_LIST_HEAD(&q->requeue_list);
@@ -4512,6 +4521,7 @@ static void __blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set,
 	blk_mq_update_queue_map(set);
 	list_for_each_entry(q, &set->tag_list, tag_set_list) {
 		blk_mq_realloc_hw_ctxs(set, q);
+		blk_mq_update_poll_flag(q);
 		if (q->nr_hw_queues != set->nr_hw_queues) {
 			int i = prev_nr_hw_queues;
 
-- 
2.31.1


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

* [PATCH V2 4/6] block: mtip32xx: don't touch q->queue_hw_ctx
  2022-03-02 12:14 [PATCH V2 0/6] blk-mq: update_nr_hw_queues related improvement & bugfix Ming Lei
                   ` (2 preceding siblings ...)
  2022-03-02 12:14 ` [PATCH V2 3/6] blk-mq: reconfigure poll after queue map is changed Ming Lei
@ 2022-03-02 12:14 ` Ming Lei
  2022-03-02 12:14 ` [PATCH V2 5/6] blk-mq: prepare for implementing hctx table via xarray Ming Lei
  2022-03-02 12:14 ` [PATCH V2 6/6] blk-mq: manage hctx map " Ming Lei
  5 siblings, 0 replies; 14+ messages in thread
From: Ming Lei @ 2022-03-02 12:14 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block, Yu Kuai, Ming Lei, Christoph Hellwig

q->queue_hw_ctx is really one blk-mq internal structure for retrieving
hctx via its index, not supposed to be used by drivers. Meantime drivers
can get the tags structure easily from tagset.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 drivers/block/mtip32xx/mtip32xx.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c
index cba956881d55..2d43ab5b5cc0 100644
--- a/drivers/block/mtip32xx/mtip32xx.c
+++ b/drivers/block/mtip32xx/mtip32xx.c
@@ -160,9 +160,7 @@ static bool mtip_check_surprise_removal(struct driver_data *dd)
 static struct mtip_cmd *mtip_cmd_from_tag(struct driver_data *dd,
 					  unsigned int tag)
 {
-	struct blk_mq_hw_ctx *hctx = dd->queue->queue_hw_ctx[0];
-
-	return blk_mq_rq_to_pdu(blk_mq_tag_to_rq(hctx->tags, tag));
+	return blk_mq_rq_to_pdu(blk_mq_tag_to_rq(dd->tags.tags[0], tag));
 }
 
 /*
-- 
2.31.1


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

* [PATCH V2 5/6] blk-mq: prepare for implementing hctx table via xarray
  2022-03-02 12:14 [PATCH V2 0/6] blk-mq: update_nr_hw_queues related improvement & bugfix Ming Lei
                   ` (3 preceding siblings ...)
  2022-03-02 12:14 ` [PATCH V2 4/6] block: mtip32xx: don't touch q->queue_hw_ctx Ming Lei
@ 2022-03-02 12:14 ` Ming Lei
  2022-03-03  9:08   ` kernel test robot
  2022-03-02 12:14 ` [PATCH V2 6/6] blk-mq: manage hctx map " Ming Lei
  5 siblings, 1 reply; 14+ messages in thread
From: Ming Lei @ 2022-03-02 12:14 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block, Yu Kuai, Ming Lei

It is inevitable to cause use-after-free on q->queue_hw_ctx between
queue_for_each_hw_ctx() and blk_mq_update_nr_hw_queues(). And converting
to xarray can fix the uaf, meantime code gets cleaner.

Prepare for converting q->queue_hctx_ctx into xarray, one thing is that
xa_for_each() can only accept 'unsigned long' as index, so changes type
of hctx index of queue_for_each_hw_ctx() into 'unsigned long'.

Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 block/blk-mq-debugfs.c        |  6 +++---
 block/blk-mq-sched.c          |  9 +++++----
 block/blk-mq-sysfs.c          | 10 ++++++----
 block/blk-mq-tag.c            |  2 +-
 block/blk-mq.c                | 30 ++++++++++++++++--------------
 drivers/block/rnbd/rnbd-clt.c |  2 +-
 6 files changed, 32 insertions(+), 27 deletions(-)

diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 3a790eb4995c..e2880f6deb34 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -707,7 +707,7 @@ static void debugfs_create_files(struct dentry *parent, void *data,
 void blk_mq_debugfs_register(struct request_queue *q)
 {
 	struct blk_mq_hw_ctx *hctx;
-	int i;
+	unsigned long i;
 
 	debugfs_create_files(q->debugfs_dir, q, blk_mq_debugfs_queue_attrs);
 
@@ -780,7 +780,7 @@ void blk_mq_debugfs_unregister_hctx(struct blk_mq_hw_ctx *hctx)
 void blk_mq_debugfs_register_hctxs(struct request_queue *q)
 {
 	struct blk_mq_hw_ctx *hctx;
-	int i;
+	unsigned long i;
 
 	queue_for_each_hw_ctx(q, hctx, i)
 		blk_mq_debugfs_register_hctx(q, hctx);
@@ -789,7 +789,7 @@ void blk_mq_debugfs_register_hctxs(struct request_queue *q)
 void blk_mq_debugfs_unregister_hctxs(struct request_queue *q)
 {
 	struct blk_mq_hw_ctx *hctx;
-	int i;
+	unsigned long i;
 
 	queue_for_each_hw_ctx(q, hctx, i)
 		blk_mq_debugfs_unregister_hctx(hctx);
diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
index 55488ba97823..e6ad8f761474 100644
--- a/block/blk-mq-sched.c
+++ b/block/blk-mq-sched.c
@@ -515,7 +515,7 @@ static void blk_mq_exit_sched_shared_tags(struct request_queue *queue)
 static void blk_mq_sched_tags_teardown(struct request_queue *q, unsigned int flags)
 {
 	struct blk_mq_hw_ctx *hctx;
-	int i;
+	unsigned long i;
 
 	queue_for_each_hw_ctx(q, hctx, i) {
 		if (hctx->sched_tags) {
@@ -550,9 +550,10 @@ static int blk_mq_init_sched_shared_tags(struct request_queue *queue)
 
 int blk_mq_init_sched(struct request_queue *q, struct elevator_type *e)
 {
-	unsigned int i, flags = q->tag_set->flags;
+	unsigned int flags = q->tag_set->flags;
 	struct blk_mq_hw_ctx *hctx;
 	struct elevator_queue *eq;
+	unsigned long i;
 	int ret;
 
 	if (!e) {
@@ -618,7 +619,7 @@ int blk_mq_init_sched(struct request_queue *q, struct elevator_type *e)
 void blk_mq_sched_free_rqs(struct request_queue *q)
 {
 	struct blk_mq_hw_ctx *hctx;
-	int i;
+	unsigned long i;
 
 	if (blk_mq_is_shared_tags(q->tag_set->flags)) {
 		blk_mq_free_rqs(q->tag_set, q->sched_shared_tags,
@@ -635,7 +636,7 @@ void blk_mq_sched_free_rqs(struct request_queue *q)
 void blk_mq_exit_sched(struct request_queue *q, struct elevator_queue *e)
 {
 	struct blk_mq_hw_ctx *hctx;
-	unsigned int i;
+	unsigned long i;
 	unsigned int flags = 0;
 
 	queue_for_each_hw_ctx(q, hctx, i) {
diff --git a/block/blk-mq-sysfs.c b/block/blk-mq-sysfs.c
index 674786574075..8d8c71c77ff8 100644
--- a/block/blk-mq-sysfs.c
+++ b/block/blk-mq-sysfs.c
@@ -206,7 +206,7 @@ static int blk_mq_register_hctx(struct blk_mq_hw_ctx *hctx)
 void blk_mq_unregister_dev(struct device *dev, struct request_queue *q)
 {
 	struct blk_mq_hw_ctx *hctx;
-	int i;
+	unsigned long i;
 
 	lockdep_assert_held(&q->sysfs_dir_lock);
 
@@ -255,7 +255,8 @@ void blk_mq_sysfs_init(struct request_queue *q)
 int __blk_mq_register_dev(struct device *dev, struct request_queue *q)
 {
 	struct blk_mq_hw_ctx *hctx;
-	int ret, i;
+	unsigned long i;
+	int ret;
 
 	WARN_ON_ONCE(!q->kobj.parent);
 	lockdep_assert_held(&q->sysfs_dir_lock);
@@ -290,7 +291,7 @@ int __blk_mq_register_dev(struct device *dev, struct request_queue *q)
 void blk_mq_sysfs_unregister(struct request_queue *q)
 {
 	struct blk_mq_hw_ctx *hctx;
-	int i;
+	unsigned long i;
 
 	mutex_lock(&q->sysfs_dir_lock);
 	if (!q->mq_sysfs_init_done)
@@ -306,7 +307,8 @@ void blk_mq_sysfs_unregister(struct request_queue *q)
 int blk_mq_sysfs_register(struct request_queue *q)
 {
 	struct blk_mq_hw_ctx *hctx;
-	int i, ret = 0;
+	unsigned long i;
+	int ret = 0;
 
 	mutex_lock(&q->sysfs_dir_lock);
 	if (!q->mq_sysfs_init_done)
diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
index 0fd409b8e86e..1850a4225e12 100644
--- a/block/blk-mq-tag.c
+++ b/block/blk-mq-tag.c
@@ -515,7 +515,7 @@ void blk_mq_queue_tag_busy_iter(struct request_queue *q, busy_tag_iter_fn *fn,
 		bt_for_each(NULL, q, btags, fn, priv, false);
 	} else {
 		struct blk_mq_hw_ctx *hctx;
-		int i;
+		unsigned long i;
 
 		queue_for_each_hw_ctx(q, hctx, i) {
 			struct blk_mq_tags *tags = hctx->tags;
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 57ae9df0f4dc..bffdd71c670d 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -312,7 +312,7 @@ EXPORT_SYMBOL_GPL(blk_mq_unquiesce_queue);
 void blk_mq_wake_waiters(struct request_queue *q)
 {
 	struct blk_mq_hw_ctx *hctx;
-	unsigned int i;
+	unsigned long i;
 
 	queue_for_each_hw_ctx(q, hctx, i)
 		if (blk_mq_hw_queue_mapped(hctx))
@@ -1442,7 +1442,7 @@ static void blk_mq_timeout_work(struct work_struct *work)
 		container_of(work, struct request_queue, timeout_work);
 	unsigned long next = 0;
 	struct blk_mq_hw_ctx *hctx;
-	int i;
+	unsigned long i;
 
 	/* A deadlock might occur if a request is stuck requiring a
 	 * timeout at the same time a queue freeze is waiting
@@ -2143,7 +2143,7 @@ static struct blk_mq_hw_ctx *blk_mq_get_sq_hctx(struct request_queue *q)
 void blk_mq_run_hw_queues(struct request_queue *q, bool async)
 {
 	struct blk_mq_hw_ctx *hctx, *sq_hctx;
-	int i;
+	unsigned long i;
 
 	sq_hctx = NULL;
 	if (blk_mq_has_sqsched(q))
@@ -2171,7 +2171,7 @@ EXPORT_SYMBOL(blk_mq_run_hw_queues);
 void blk_mq_delay_run_hw_queues(struct request_queue *q, unsigned long msecs)
 {
 	struct blk_mq_hw_ctx *hctx, *sq_hctx;
-	int i;
+	unsigned long i;
 
 	sq_hctx = NULL;
 	if (blk_mq_has_sqsched(q))
@@ -2209,7 +2209,7 @@ EXPORT_SYMBOL(blk_mq_delay_run_hw_queues);
 bool blk_mq_queue_stopped(struct request_queue *q)
 {
 	struct blk_mq_hw_ctx *hctx;
-	int i;
+	unsigned long i;
 
 	queue_for_each_hw_ctx(q, hctx, i)
 		if (blk_mq_hctx_stopped(hctx))
@@ -2248,7 +2248,7 @@ EXPORT_SYMBOL(blk_mq_stop_hw_queue);
 void blk_mq_stop_hw_queues(struct request_queue *q)
 {
 	struct blk_mq_hw_ctx *hctx;
-	int i;
+	unsigned long i;
 
 	queue_for_each_hw_ctx(q, hctx, i)
 		blk_mq_stop_hw_queue(hctx);
@@ -2266,7 +2266,7 @@ EXPORT_SYMBOL(blk_mq_start_hw_queue);
 void blk_mq_start_hw_queues(struct request_queue *q)
 {
 	struct blk_mq_hw_ctx *hctx;
-	int i;
+	unsigned long i;
 
 	queue_for_each_hw_ctx(q, hctx, i)
 		blk_mq_start_hw_queue(hctx);
@@ -2286,7 +2286,7 @@ EXPORT_SYMBOL_GPL(blk_mq_start_stopped_hw_queue);
 void blk_mq_start_stopped_hw_queues(struct request_queue *q, bool async)
 {
 	struct blk_mq_hw_ctx *hctx;
-	int i;
+	unsigned long i;
 
 	queue_for_each_hw_ctx(q, hctx, i)
 		blk_mq_start_stopped_hw_queue(hctx, async);
@@ -3446,7 +3446,7 @@ static void blk_mq_exit_hw_queues(struct request_queue *q,
 		struct blk_mq_tag_set *set, int nr_queue)
 {
 	struct blk_mq_hw_ctx *hctx;
-	unsigned int i;
+	unsigned long i;
 
 	queue_for_each_hw_ctx(q, hctx, i) {
 		if (i == nr_queue)
@@ -3637,7 +3637,8 @@ static void __blk_mq_free_map_and_rqs(struct blk_mq_tag_set *set,
 
 static void blk_mq_map_swqueue(struct request_queue *q)
 {
-	unsigned int i, j, hctx_idx;
+	unsigned int j, hctx_idx;
+	unsigned long i;
 	struct blk_mq_hw_ctx *hctx;
 	struct blk_mq_ctx *ctx;
 	struct blk_mq_tag_set *set = q->tag_set;
@@ -3744,7 +3745,7 @@ static void blk_mq_map_swqueue(struct request_queue *q)
 static void queue_set_hctx_shared(struct request_queue *q, bool shared)
 {
 	struct blk_mq_hw_ctx *hctx;
-	int i;
+	unsigned long i;
 
 	queue_for_each_hw_ctx(q, hctx, i) {
 		if (shared) {
@@ -3844,7 +3845,7 @@ static int blk_mq_alloc_ctxs(struct request_queue *q)
 void blk_mq_release(struct request_queue *q)
 {
 	struct blk_mq_hw_ctx *hctx, *next;
-	int i;
+	unsigned long i;
 
 	queue_for_each_hw_ctx(q, hctx, i)
 		WARN_ON_ONCE(hctx && list_empty(&hctx->hctx_list));
@@ -4362,7 +4363,8 @@ int blk_mq_update_nr_requests(struct request_queue *q, unsigned int nr)
 {
 	struct blk_mq_tag_set *set = q->tag_set;
 	struct blk_mq_hw_ctx *hctx;
-	int i, ret;
+	int ret;
+	unsigned long i;
 
 	if (!set)
 		return -EINVAL;
@@ -4738,7 +4740,7 @@ void blk_mq_cancel_work_sync(struct request_queue *q)
 {
 	if (queue_is_mq(q)) {
 		struct blk_mq_hw_ctx *hctx;
-		int i;
+		unsigned long i;
 
 		cancel_delayed_work_sync(&q->requeue_work);
 
diff --git a/drivers/block/rnbd/rnbd-clt.c b/drivers/block/rnbd/rnbd-clt.c
index c08971de369f..58304f978e10 100644
--- a/drivers/block/rnbd/rnbd-clt.c
+++ b/drivers/block/rnbd/rnbd-clt.c
@@ -1343,7 +1343,7 @@ static inline void rnbd_init_hw_queue(struct rnbd_clt_dev *dev,
 
 static void rnbd_init_mq_hw_queues(struct rnbd_clt_dev *dev)
 {
-	int i;
+	unsigned long i;
 	struct blk_mq_hw_ctx *hctx;
 	struct rnbd_queue *q;
 
-- 
2.31.1


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

* [PATCH V2 6/6] blk-mq: manage hctx map via xarray
  2022-03-02 12:14 [PATCH V2 0/6] blk-mq: update_nr_hw_queues related improvement & bugfix Ming Lei
                   ` (4 preceding siblings ...)
  2022-03-02 12:14 ` [PATCH V2 5/6] blk-mq: prepare for implementing hctx table via xarray Ming Lei
@ 2022-03-02 12:14 ` Ming Lei
  5 siblings, 0 replies; 14+ messages in thread
From: Ming Lei @ 2022-03-02 12:14 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block, Yu Kuai, Ming Lei

Firstly code becomes more clean by switching to xarray from plain array.

Secondly use-after-free on q->queue_hw_ctx can be fixed because
queue_for_each_hw_ctx() may be run when updating nr_hw_queues is
in-progress. With this patch, q->hctx_table is defined as xarray, and
this structure will share same lifetime with request queue, so
queue_for_each_hw_ctx() can use q->hctx_table to lookup hctx reliably.

Reported-by: Yu Kuai <yukuai3@huawei.com>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 block/blk-mq-sysfs.c   |  2 +-
 block/blk-mq-tag.c     |  2 +-
 block/blk-mq.c         | 55 ++++++++++++++++++------------------------
 block/blk-mq.h         |  2 +-
 include/linux/blk-mq.h |  3 +--
 include/linux/blkdev.h |  2 +-
 6 files changed, 29 insertions(+), 37 deletions(-)

diff --git a/block/blk-mq-sysfs.c b/block/blk-mq-sysfs.c
index 8d8c71c77ff8..6a7fb960e046 100644
--- a/block/blk-mq-sysfs.c
+++ b/block/blk-mq-sysfs.c
@@ -280,7 +280,7 @@ int __blk_mq_register_dev(struct device *dev, struct request_queue *q)
 
 unreg:
 	while (--i >= 0)
-		blk_mq_unregister_hctx(q->queue_hw_ctx[i]);
+		blk_mq_unregister_hctx(xa_load(&q->hctx_table, i));
 
 	kobject_uevent(q->mq_kobj, KOBJ_REMOVE);
 	kobject_del(q->mq_kobj);
diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
index 1850a4225e12..68ac23d0b640 100644
--- a/block/blk-mq-tag.c
+++ b/block/blk-mq-tag.c
@@ -498,7 +498,7 @@ void blk_mq_queue_tag_busy_iter(struct request_queue *q, busy_tag_iter_fn *fn,
 		void *priv)
 {
 	/*
-	 * __blk_mq_update_nr_hw_queues() updates nr_hw_queues and queue_hw_ctx
+	 * __blk_mq_update_nr_hw_queues() updates nr_hw_queues and hctx_table
 	 * while the queue is frozen. So we can use q_usage_counter to avoid
 	 * racing with it.
 	 */
diff --git a/block/blk-mq.c b/block/blk-mq.c
index bffdd71c670d..a15d12fb227c 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -71,7 +71,8 @@ static int blk_mq_poll_stats_bkt(const struct request *rq)
 static inline struct blk_mq_hw_ctx *blk_qc_to_hctx(struct request_queue *q,
 		blk_qc_t qc)
 {
-	return q->queue_hw_ctx[(qc & ~BLK_QC_T_INTERNAL) >> BLK_QC_T_SHIFT];
+	return xa_load(&q->hctx_table,
+			(qc & ~BLK_QC_T_INTERNAL) >> BLK_QC_T_SHIFT);
 }
 
 static inline struct request *blk_qc_to_rq(struct blk_mq_hw_ctx *hctx,
@@ -573,7 +574,7 @@ struct request *blk_mq_alloc_request_hctx(struct request_queue *q,
 	 * If not tell the caller that it should skip this queue.
 	 */
 	ret = -EXDEV;
-	data.hctx = q->queue_hw_ctx[hctx_idx];
+	data.hctx = xa_load(&q->hctx_table, hctx_idx);
 	if (!blk_mq_hw_queue_mapped(data.hctx))
 		goto out_queue_exit;
 	cpu = cpumask_first_and(data.hctx->cpumask, cpu_online_mask);
@@ -3437,6 +3438,8 @@ static void blk_mq_exit_hctx(struct request_queue *q,
 
 	blk_mq_remove_cpuhp(hctx);
 
+	xa_erase(&q->hctx_table, hctx_idx);
+
 	spin_lock(&q->unused_hctx_lock);
 	list_add(&hctx->hctx_list, &q->unused_hctx_list);
 	spin_unlock(&q->unused_hctx_lock);
@@ -3476,8 +3479,15 @@ static int blk_mq_init_hctx(struct request_queue *q,
 	if (blk_mq_init_request(set, hctx->fq->flush_rq, hctx_idx,
 				hctx->numa_node))
 		goto exit_hctx;
+
+	if (xa_insert(&q->hctx_table, hctx_idx, hctx, GFP_KERNEL))
+		goto exit_flush_rq;
+
 	return 0;
 
+ exit_flush_rq:
+	if (set->ops->exit_request)
+		set->ops->exit_request(set, hctx->fq->flush_rq, hctx_idx);
  exit_hctx:
 	if (set->ops->exit_hctx)
 		set->ops->exit_hctx(hctx, hctx_idx);
@@ -3856,7 +3866,7 @@ void blk_mq_release(struct request_queue *q)
 		kobject_put(&hctx->kobj);
 	}
 
-	kfree(q->queue_hw_ctx);
+	xa_destroy(&q->hctx_table);
 
 	/*
 	 * release .mq_kobj and sw queue's kobject now because
@@ -3946,45 +3956,28 @@ static void blk_mq_realloc_hw_ctxs(struct blk_mq_tag_set *set,
 						struct request_queue *q)
 {
 	int i, j, end;
-	struct blk_mq_hw_ctx **hctxs = q->queue_hw_ctx;
-
-	if (q->nr_hw_queues < set->nr_hw_queues) {
-		struct blk_mq_hw_ctx **new_hctxs;
-
-		new_hctxs = kcalloc_node(set->nr_hw_queues,
-				       sizeof(*new_hctxs), GFP_KERNEL,
-				       set->numa_node);
-		if (!new_hctxs)
-			return;
-		if (hctxs)
-			memcpy(new_hctxs, hctxs, q->nr_hw_queues *
-			       sizeof(*hctxs));
-		q->queue_hw_ctx = new_hctxs;
-		kfree(hctxs);
-		hctxs = new_hctxs;
-	}
 
 	/* protect against switching io scheduler  */
 	mutex_lock(&q->sysfs_lock);
 	for (i = 0; i < set->nr_hw_queues; i++) {
 		int old_node;
 		int node = blk_mq_get_hctx_node(set, i);
-		struct blk_mq_hw_ctx *old_hctx = hctxs[i];
+		struct blk_mq_hw_ctx *old_hctx = xa_load(&q->hctx_table, i);
 
 		if (old_hctx) {
 			old_node = old_hctx->numa_node;
 			blk_mq_exit_hctx(q, set, old_hctx, i);
 		}
 
-		hctxs[i] = blk_mq_alloc_and_init_hctx(set, q, i, node);
-		if (!hctxs[i]) {
+		if (!blk_mq_alloc_and_init_hctx(set, q, i, node)) {
+			struct blk_mq_hw_ctx *hctx;
+
 			if (!old_hctx)
 				break;
 			pr_warn("Allocate new hctx on node %d fails, fallback to previous one on node %d\n",
 					node, old_node);
-			hctxs[i] = blk_mq_alloc_and_init_hctx(set, q, i,
-					old_node);
-			WARN_ON_ONCE(!hctxs[i]);
+			hctx = blk_mq_alloc_and_init_hctx(set, q, i, old_node);
+			WARN_ON_ONCE(!hctx);
 		}
 	}
 	/*
@@ -4001,12 +3994,10 @@ static void blk_mq_realloc_hw_ctxs(struct blk_mq_tag_set *set,
 	}
 
 	for (; j < end; j++) {
-		struct blk_mq_hw_ctx *hctx = hctxs[j];
+		struct blk_mq_hw_ctx *hctx = xa_load(&q->hctx_table, j);
 
-		if (hctx) {
+		if (hctx)
 			blk_mq_exit_hctx(q, set, hctx, j);
-			hctxs[j] = NULL;
-		}
 	}
 	mutex_unlock(&q->sysfs_lock);
 }
@@ -4046,6 +4037,8 @@ int blk_mq_init_allocated_queue(struct blk_mq_tag_set *set,
 	INIT_LIST_HEAD(&q->unused_hctx_list);
 	spin_lock_init(&q->unused_hctx_lock);
 
+	xa_init(&q->hctx_table);
+
 	blk_mq_realloc_hw_ctxs(set, q);
 	if (!q->nr_hw_queues)
 		goto err_hctxs;
@@ -4075,7 +4068,7 @@ int blk_mq_init_allocated_queue(struct blk_mq_tag_set *set,
 	return 0;
 
 err_hctxs:
-	kfree(q->queue_hw_ctx);
+	xa_destroy(&q->hctx_table);
 	q->nr_hw_queues = 0;
 	blk_mq_sysfs_deinit(q);
 err_poll:
diff --git a/block/blk-mq.h b/block/blk-mq.h
index 948791ea2a3e..2615bd58bad3 100644
--- a/block/blk-mq.h
+++ b/block/blk-mq.h
@@ -83,7 +83,7 @@ static inline struct blk_mq_hw_ctx *blk_mq_map_queue_type(struct request_queue *
 							  enum hctx_type type,
 							  unsigned int cpu)
 {
-	return q->queue_hw_ctx[q->tag_set->map[type].mq_map[cpu]];
+	return xa_load(&q->hctx_table, q->tag_set->map[type].mq_map[cpu]);
 }
 
 static inline enum hctx_type blk_mq_get_hctx_type(unsigned int flags)
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 3a41d50b85d3..7aa5c54901a9 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -917,8 +917,7 @@ static inline void *blk_mq_rq_to_pdu(struct request *rq)
 }
 
 #define queue_for_each_hw_ctx(q, hctx, i)				\
-	for ((i) = 0; (i) < (q)->nr_hw_queues &&			\
-	     ({ hctx = (q)->queue_hw_ctx[i]; 1; }); (i)++)
+	xa_for_each(&(q)->hctx_table, (i), (hctx))
 
 #define hctx_for_each_ctx(hctx, ctx, i)					\
 	for ((i) = 0; (i) < (hctx)->nr_ctx &&				\
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index f757f9c2871f..a53ae40aaded 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -355,7 +355,7 @@ struct request_queue {
 	unsigned int		queue_depth;
 
 	/* hw dispatch queues */
-	struct blk_mq_hw_ctx	**queue_hw_ctx;
+	struct xarray		hctx_table;
 	unsigned int		nr_hw_queues;
 
 	/*
-- 
2.31.1


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

* Re: [PATCH V2 1/6] blk-mq: figure out correct numa node for hw queue
  2022-03-02 12:14 ` [PATCH V2 1/6] blk-mq: figure out correct numa node for hw queue Ming Lei
@ 2022-03-02 12:39   ` John Garry
  0 siblings, 0 replies; 14+ messages in thread
From: John Garry @ 2022-03-02 12:39 UTC (permalink / raw)
  To: Ming Lei, Jens Axboe; +Cc: linux-block, Yu Kuai, Christoph Hellwig

On 02/03/2022 12:14, Ming Lei wrote:
> The current code always uses default queue map and hw queue index
> for figuring out the numa node for hw queue, this way isn't correct
> because blk-mq supports three queue maps, and the correct queue map
> should be used for the specified hw queue.
> 
> Reviewed-by: Christoph Hellwig<hch@lst.de>
> Signed-off-by: Ming Lei<ming.lei@redhat.com>

FWIW:
Reviewed-by: John Garry <john.garry@huawei.com>

thanks

> ---
>   block/blk-mq.c | 36 ++++++++++++++++++++++++++++++------
>   1 file changed, 30 insertions(+), 6 deletions(-)


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

* Re: [PATCH V2 5/6] blk-mq: prepare for implementing hctx table via xarray
  2022-03-02 12:14 ` [PATCH V2 5/6] blk-mq: prepare for implementing hctx table via xarray Ming Lei
@ 2022-03-03  9:08   ` kernel test robot
  2022-03-03  9:31       ` Ming Lei
  0 siblings, 1 reply; 14+ messages in thread
From: kernel test robot @ 2022-03-03  9:08 UTC (permalink / raw)
  To: Ming Lei, Jens Axboe; +Cc: kbuild-all, linux-block, Yu Kuai, Ming Lei

Hi Ming,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on axboe-block/for-next]
[also build test WARNING on v5.17-rc6 next-20220302]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Ming-Lei/blk-mq-update_nr_hw_queues-related-improvement-bugfix/20220302-201636
base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
config: m68k-randconfig-m031-20220302 (https://download.01.org/0day-ci/archive/20220303/202203031651.z0z86F6E-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 11.2.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

smatch warnings:
block/blk-mq-sysfs.c:282 __blk_mq_register_dev() warn: always true condition '(--i >= 0) => (0-u32max >= 0)'
block/blk-mq-sysfs.c:282 __blk_mq_register_dev() warn: always true condition '(--i >= 0) => (0-u32max >= 0)'

vim +282 block/blk-mq-sysfs.c

67aec14ce87fe2 Jens Axboe      2014-05-30  254  
2d0364c8c1a97a Bart Van Assche 2017-04-26  255  int __blk_mq_register_dev(struct device *dev, struct request_queue *q)
320ae51feed5c2 Jens Axboe      2013-10-24  256  {
320ae51feed5c2 Jens Axboe      2013-10-24  257  	struct blk_mq_hw_ctx *hctx;
44849be579332c Ming Lei        2022-03-02  258  	unsigned long i;
44849be579332c Ming Lei        2022-03-02  259  	int ret;
320ae51feed5c2 Jens Axboe      2013-10-24  260  
2d0364c8c1a97a Bart Van Assche 2017-04-26  261  	WARN_ON_ONCE(!q->kobj.parent);
cecf5d87ff2035 Ming Lei        2019-08-27  262  	lockdep_assert_held(&q->sysfs_dir_lock);
4593fdbe7a2f44 Akinobu Mita    2015-09-27  263  
1db4909e76f64a Ming Lei        2018-11-20  264  	ret = kobject_add(q->mq_kobj, kobject_get(&dev->kobj), "%s", "mq");
320ae51feed5c2 Jens Axboe      2013-10-24  265  	if (ret < 0)
4593fdbe7a2f44 Akinobu Mita    2015-09-27  266  		goto out;
320ae51feed5c2 Jens Axboe      2013-10-24  267  
1db4909e76f64a Ming Lei        2018-11-20  268  	kobject_uevent(q->mq_kobj, KOBJ_ADD);
320ae51feed5c2 Jens Axboe      2013-10-24  269  
320ae51feed5c2 Jens Axboe      2013-10-24  270  	queue_for_each_hw_ctx(q, hctx, i) {
67aec14ce87fe2 Jens Axboe      2014-05-30  271  		ret = blk_mq_register_hctx(hctx);
320ae51feed5c2 Jens Axboe      2013-10-24  272  		if (ret)
f05d1ba7871a2c Bart Van Assche 2017-04-26  273  			goto unreg;
320ae51feed5c2 Jens Axboe      2013-10-24  274  	}
320ae51feed5c2 Jens Axboe      2013-10-24  275  
4593fdbe7a2f44 Akinobu Mita    2015-09-27  276  	q->mq_sysfs_init_done = true;
2d0364c8c1a97a Bart Van Assche 2017-04-26  277  
4593fdbe7a2f44 Akinobu Mita    2015-09-27  278  out:
2d0364c8c1a97a Bart Van Assche 2017-04-26  279  	return ret;
f05d1ba7871a2c Bart Van Assche 2017-04-26  280  
f05d1ba7871a2c Bart Van Assche 2017-04-26  281  unreg:
f05d1ba7871a2c Bart Van Assche 2017-04-26 @282  	while (--i >= 0)
f05d1ba7871a2c Bart Van Assche 2017-04-26  283  		blk_mq_unregister_hctx(q->queue_hw_ctx[i]);
f05d1ba7871a2c Bart Van Assche 2017-04-26  284  
1db4909e76f64a Ming Lei        2018-11-20  285  	kobject_uevent(q->mq_kobj, KOBJ_REMOVE);
1db4909e76f64a Ming Lei        2018-11-20  286  	kobject_del(q->mq_kobj);
f05d1ba7871a2c Bart Van Assche 2017-04-26  287  	kobject_put(&dev->kobj);
f05d1ba7871a2c Bart Van Assche 2017-04-26  288  	return ret;
2d0364c8c1a97a Bart Van Assche 2017-04-26  289  }
2d0364c8c1a97a Bart Van Assche 2017-04-26  290  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

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

* Re: [PATCH V2 5/6] blk-mq: prepare for implementing hctx table via xarray
  2022-03-03  9:08   ` kernel test robot
@ 2022-03-03  9:31       ` Ming Lei
  0 siblings, 0 replies; 14+ messages in thread
From: Ming Lei @ 2022-03-03  9:31 UTC (permalink / raw)
  To: kernel test robot; +Cc: Jens Axboe, kbuild-all, linux-block, Yu Kuai

On Thu, Mar 03, 2022 at 05:08:47PM +0800, kernel test robot wrote:
> Hi Ming,
> 
> Thank you for the patch! Perhaps something to improve:
> 
> [auto build test WARNING on axboe-block/for-next]
> [also build test WARNING on v5.17-rc6 next-20220302]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
> 
> url:    https://github.com/0day-ci/linux/commits/Ming-Lei/blk-mq-update_nr_hw_queues-related-improvement-bugfix/20220302-201636
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
> config: m68k-randconfig-m031-20220302 (https://download.01.org/0day-ci/archive/20220303/202203031651.z0z86F6E-lkp@intel.com/config)
> compiler: m68k-linux-gcc (GCC) 11.2.0
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> smatch warnings:
> block/blk-mq-sysfs.c:282 __blk_mq_register_dev() warn: always true condition '(--i >= 0) => (0-u32max >= 0)'
> block/blk-mq-sysfs.c:282 __blk_mq_register_dev() warn: always true condition '(--i >= 0) => (0-u32max >= 0)'

Good catch, will fix it by the following way in V3:

diff --git a/block/blk-mq-sysfs.c b/block/blk-mq-sysfs.c
index 6a7fb960e046..851faacc2f78 100644
--- a/block/blk-mq-sysfs.c
+++ b/block/blk-mq-sysfs.c
@@ -279,8 +279,8 @@ int __blk_mq_register_dev(struct device *dev, struct request_queue *q)
        return ret;

 unreg:
-       while (--i >= 0)
-               blk_mq_unregister_hctx(xa_load(&q->hctx_table, i));
+       queue_for_each_hw_ctx(q, hctx, i) {
+               blk_mq_unregister_hctx(hctx);

        kobject_uevent(q->mq_kobj, KOBJ_REMOVE);
        kobject_del(q->mq_kobj);



Thanks,
Ming


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

* Re: [PATCH V2 5/6] blk-mq: prepare for implementing hctx table via xarray
@ 2022-03-03  9:31       ` Ming Lei
  0 siblings, 0 replies; 14+ messages in thread
From: Ming Lei @ 2022-03-03  9:31 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 1850 bytes --]

On Thu, Mar 03, 2022 at 05:08:47PM +0800, kernel test robot wrote:
> Hi Ming,
> 
> Thank you for the patch! Perhaps something to improve:
> 
> [auto build test WARNING on axboe-block/for-next]
> [also build test WARNING on v5.17-rc6 next-20220302]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
> 
> url:    https://github.com/0day-ci/linux/commits/Ming-Lei/blk-mq-update_nr_hw_queues-related-improvement-bugfix/20220302-201636
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
> config: m68k-randconfig-m031-20220302 (https://download.01.org/0day-ci/archive/20220303/202203031651.z0z86F6E-lkp(a)intel.com/config)
> compiler: m68k-linux-gcc (GCC) 11.2.0
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> smatch warnings:
> block/blk-mq-sysfs.c:282 __blk_mq_register_dev() warn: always true condition '(--i >= 0) => (0-u32max >= 0)'
> block/blk-mq-sysfs.c:282 __blk_mq_register_dev() warn: always true condition '(--i >= 0) => (0-u32max >= 0)'

Good catch, will fix it by the following way in V3:

diff --git a/block/blk-mq-sysfs.c b/block/blk-mq-sysfs.c
index 6a7fb960e046..851faacc2f78 100644
--- a/block/blk-mq-sysfs.c
+++ b/block/blk-mq-sysfs.c
@@ -279,8 +279,8 @@ int __blk_mq_register_dev(struct device *dev, struct request_queue *q)
        return ret;

 unreg:
-       while (--i >= 0)
-               blk_mq_unregister_hctx(xa_load(&q->hctx_table, i));
+       queue_for_each_hw_ctx(q, hctx, i) {
+               blk_mq_unregister_hctx(hctx);

        kobject_uevent(q->mq_kobj, KOBJ_REMOVE);
        kobject_del(q->mq_kobj);



Thanks,
Ming

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

* Re: [PATCH V2 3/6] blk-mq: reconfigure poll after queue map is changed
  2022-03-02 12:14 ` [PATCH V2 3/6] blk-mq: reconfigure poll after queue map is changed Ming Lei
@ 2022-03-03 15:20   ` John Garry
  0 siblings, 0 replies; 14+ messages in thread
From: John Garry @ 2022-03-03 15:20 UTC (permalink / raw)
  To: Ming Lei, Jens Axboe; +Cc: linux-block, Yu Kuai

On 02/03/2022 12:14, Ming Lei wrote:
> queue map can be changed when updating nr_hw_queues, so we need to
> reconfigure queue's poll capability. Add one helper for doing this job.

nvme_mpath_alloc_disk() seems to have similar code to this helper, so 
maybe we can reuse this helper function there (obviously it would need 
to made public)

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

Regardless of comment:

Reviewed-by: John Garry <john.garry@huawei.com>

> ---
>   block/blk-mq.c | 16 +++++++++++++---
>   1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 64fddb36c93c..57ae9df0f4dc 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -4010,6 +4010,17 @@ static void blk_mq_realloc_hw_ctxs(struct blk_mq_tag_set *set,
>   	mutex_unlock(&q->sysfs_lock);
>   }
>   
> +static void blk_mq_update_poll_flag(struct request_queue *q)
> +{
> +	struct blk_mq_tag_set *set = q->tag_set;
> +
> +	if (set->nr_maps > HCTX_TYPE_POLL &&
> +	    set->map[HCTX_TYPE_POLL].nr_queues)
> +		blk_queue_flag_set(QUEUE_FLAG_POLL, q);
> +	else
> +		blk_queue_flag_clear(QUEUE_FLAG_POLL, q);
> +}
> +
>   int blk_mq_init_allocated_queue(struct blk_mq_tag_set *set,
>   		struct request_queue *q)
>   {
> @@ -4044,9 +4055,7 @@ int blk_mq_init_allocated_queue(struct blk_mq_tag_set *set,
>   	q->tag_set = set;
>   
>   	q->queue_flags |= QUEUE_FLAG_MQ_DEFAULT;
> -	if (set->nr_maps > HCTX_TYPE_POLL &&
> -	    set->map[HCTX_TYPE_POLL].nr_queues)
> -		blk_queue_flag_set(QUEUE_FLAG_POLL, q);
> +	blk_mq_update_poll_flag(q);
>   
>   	INIT_DELAYED_WORK(&q->requeue_work, blk_mq_requeue_work);
>   	INIT_LIST_HEAD(&q->requeue_list);
> @@ -4512,6 +4521,7 @@ static void __blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set,
>   	blk_mq_update_queue_map(set);
>   	list_for_each_entry(q, &set->tag_list, tag_set_list) {
>   		blk_mq_realloc_hw_ctxs(set, q);
> +		blk_mq_update_poll_flag(q);
>   		if (q->nr_hw_queues != set->nr_hw_queues) {
>   			int i = prev_nr_hw_queues;
>   


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

* Re: [PATCH V2 5/6] blk-mq: prepare for implementing hctx table via xarray
  2022-03-03 12:33 [PATCH V2 5/6] blk-mq: prepare for implementing hctx table " kernel test robot
@ 2022-03-09  3:01 ` kernel test robot
  0 siblings, 0 replies; 14+ messages in thread
From: kernel test robot @ 2022-03-09  3:01 UTC (permalink / raw)
  To: Ming Lei, Jens Axboe; +Cc: kbuild-all, linux-block, Yu Kuai, Ming Lei

Hi Ming,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on axboe-block/for-next]
[also build test WARNING on v5.17-rc6 next-20220302]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Ming-Lei/blk-mq-update_nr_hw_queues-related-improvement-bugfix/20220302-201636
base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
compiler: m68k-linux-gcc (GCC) 11.2.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <yujie.liu@intel.com>


cppcheck possible warnings: (new ones prefixed by >>, may not be real problems)

 >> block/blk-mq-sysfs.c:282:13: warning: Unsigned variable '--' can't be negative so it is unnecessary to test it. [unsignedPositive]
     while (--i >= 0)
                ^

vim +282 block/blk-mq-sysfs.c

67aec14ce87fe25 Jens Axboe      2014-05-30  254
2d0364c8c1a97a1 Bart Van Assche 2017-04-26  255  int __blk_mq_register_dev(struct device *dev, struct request_queue *q)
320ae51feed5c2f Jens Axboe      2013-10-24  256  {
320ae51feed5c2f Jens Axboe      2013-10-24  257  	struct blk_mq_hw_ctx *hctx;
44849be579332ce Ming Lei        2022-03-02 @258  	unsigned long i;
44849be579332ce Ming Lei        2022-03-02  259  	int ret;
320ae51feed5c2f Jens Axboe      2013-10-24  260
2d0364c8c1a97a1 Bart Van Assche 2017-04-26  261  	WARN_ON_ONCE(!q->kobj.parent);
cecf5d87ff20351 Ming Lei        2019-08-27  262  	lockdep_assert_held(&q->sysfs_dir_lock);
4593fdbe7a2f44d Akinobu Mita    2015-09-27  263
1db4909e76f64a8 Ming Lei        2018-11-20  264  	ret = kobject_add(q->mq_kobj, kobject_get(&dev->kobj), "%s", "mq");
320ae51feed5c2f Jens Axboe      2013-10-24  265  	if (ret < 0)
4593fdbe7a2f44d Akinobu Mita    2015-09-27  266  		goto out;
320ae51feed5c2f Jens Axboe      2013-10-24  267
1db4909e76f64a8 Ming Lei        2018-11-20  268  	kobject_uevent(q->mq_kobj, KOBJ_ADD);
320ae51feed5c2f Jens Axboe      2013-10-24  269
320ae51feed5c2f Jens Axboe      2013-10-24  270  	queue_for_each_hw_ctx(q, hctx, i) {
67aec14ce87fe25 Jens Axboe      2014-05-30  271  		ret = blk_mq_register_hctx(hctx);
320ae51feed5c2f Jens Axboe      2013-10-24  272  		if (ret)
f05d1ba7871a2c2 Bart Van Assche 2017-04-26  273  			goto unreg;
320ae51feed5c2f Jens Axboe      2013-10-24  274  	}
320ae51feed5c2f Jens Axboe      2013-10-24  275
4593fdbe7a2f44d Akinobu Mita    2015-09-27  276  	q->mq_sysfs_init_done = true;
2d0364c8c1a97a1 Bart Van Assche 2017-04-26  277
4593fdbe7a2f44d Akinobu Mita    2015-09-27  278  out:
2d0364c8c1a97a1 Bart Van Assche 2017-04-26  279  	return ret;
f05d1ba7871a2c2 Bart Van Assche 2017-04-26  280
f05d1ba7871a2c2 Bart Van Assche 2017-04-26  281  unreg:
f05d1ba7871a2c2 Bart Van Assche 2017-04-26 @282  	while (--i >= 0)
f05d1ba7871a2c2 Bart Van Assche 2017-04-26  283  		blk_mq_unregister_hctx(q->queue_hw_ctx[i]);
f05d1ba7871a2c2 Bart Van Assche 2017-04-26  284
1db4909e76f64a8 Ming Lei        2018-11-20  285  	kobject_uevent(q->mq_kobj, KOBJ_REMOVE);
1db4909e76f64a8 Ming Lei        2018-11-20  286  	kobject_del(q->mq_kobj);
f05d1ba7871a2c2 Bart Van Assche 2017-04-26  287  	kobject_put(&dev->kobj);
f05d1ba7871a2c2 Bart Van Assche 2017-04-26  288  	return ret;
2d0364c8c1a97a1 Bart Van Assche 2017-04-26  289  }
2d0364c8c1a97a1 Bart Van Assche 2017-04-26  290

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

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

* Re: [PATCH V2 5/6] blk-mq: prepare for implementing hctx table via xarray
@ 2022-03-03 12:33 kernel test robot
  2022-03-09  3:01 ` kernel test robot
  0 siblings, 1 reply; 14+ messages in thread
From: kernel test robot @ 2022-03-03 12:33 UTC (permalink / raw)
  To: kbuild

[-- Attachment #1: Type: text/plain, Size: 4095 bytes --]

CC: kbuild-all(a)lists.01.org
BCC: lkp(a)intel.com
In-Reply-To: <20220302121407.1361401-6-ming.lei@redhat.com>
References: <20220302121407.1361401-6-ming.lei@redhat.com>
TO: Ming Lei <ming.lei@redhat.com>
TO: Jens Axboe <axboe@kernel.dk>
CC: linux-block(a)vger.kernel.org
CC: Yu Kuai <yukuai3@huawei.com>
CC: Ming Lei <ming.lei@redhat.com>

Hi Ming,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on axboe-block/for-next]
[also build test WARNING on v5.17-rc6 next-20220302]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Ming-Lei/blk-mq-update_nr_hw_queues-related-improvement-bugfix/20220302-201636
base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
:::::: branch date: 24 hours ago
:::::: commit date: 24 hours ago
compiler: m68k-linux-gcc (GCC) 11.2.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


cppcheck possible warnings: (new ones prefixed by >>, may not real problems)

>> block/blk-mq-sysfs.c:282:13: warning: Unsigned variable '--' can't be negative so it is unnecessary to test it. [unsignedPositive]
    while (--i >= 0)
               ^

vim +282 block/blk-mq-sysfs.c

67aec14ce87fe25 Jens Axboe      2014-05-30  254  
2d0364c8c1a97a1 Bart Van Assche 2017-04-26  255  int __blk_mq_register_dev(struct device *dev, struct request_queue *q)
320ae51feed5c2f Jens Axboe      2013-10-24  256  {
320ae51feed5c2f Jens Axboe      2013-10-24  257  	struct blk_mq_hw_ctx *hctx;
44849be579332ce Ming Lei        2022-03-02  258  	unsigned long i;
44849be579332ce Ming Lei        2022-03-02  259  	int ret;
320ae51feed5c2f Jens Axboe      2013-10-24  260  
2d0364c8c1a97a1 Bart Van Assche 2017-04-26  261  	WARN_ON_ONCE(!q->kobj.parent);
cecf5d87ff20351 Ming Lei        2019-08-27  262  	lockdep_assert_held(&q->sysfs_dir_lock);
4593fdbe7a2f44d Akinobu Mita    2015-09-27  263  
1db4909e76f64a8 Ming Lei        2018-11-20  264  	ret = kobject_add(q->mq_kobj, kobject_get(&dev->kobj), "%s", "mq");
320ae51feed5c2f Jens Axboe      2013-10-24  265  	if (ret < 0)
4593fdbe7a2f44d Akinobu Mita    2015-09-27  266  		goto out;
320ae51feed5c2f Jens Axboe      2013-10-24  267  
1db4909e76f64a8 Ming Lei        2018-11-20  268  	kobject_uevent(q->mq_kobj, KOBJ_ADD);
320ae51feed5c2f Jens Axboe      2013-10-24  269  
320ae51feed5c2f Jens Axboe      2013-10-24  270  	queue_for_each_hw_ctx(q, hctx, i) {
67aec14ce87fe25 Jens Axboe      2014-05-30  271  		ret = blk_mq_register_hctx(hctx);
320ae51feed5c2f Jens Axboe      2013-10-24  272  		if (ret)
f05d1ba7871a2c2 Bart Van Assche 2017-04-26  273  			goto unreg;
320ae51feed5c2f Jens Axboe      2013-10-24  274  	}
320ae51feed5c2f Jens Axboe      2013-10-24  275  
4593fdbe7a2f44d Akinobu Mita    2015-09-27  276  	q->mq_sysfs_init_done = true;
2d0364c8c1a97a1 Bart Van Assche 2017-04-26  277  
4593fdbe7a2f44d Akinobu Mita    2015-09-27  278  out:
2d0364c8c1a97a1 Bart Van Assche 2017-04-26  279  	return ret;
f05d1ba7871a2c2 Bart Van Assche 2017-04-26  280  
f05d1ba7871a2c2 Bart Van Assche 2017-04-26  281  unreg:
f05d1ba7871a2c2 Bart Van Assche 2017-04-26 @282  	while (--i >= 0)
f05d1ba7871a2c2 Bart Van Assche 2017-04-26  283  		blk_mq_unregister_hctx(q->queue_hw_ctx[i]);
f05d1ba7871a2c2 Bart Van Assche 2017-04-26  284  
1db4909e76f64a8 Ming Lei        2018-11-20  285  	kobject_uevent(q->mq_kobj, KOBJ_REMOVE);
1db4909e76f64a8 Ming Lei        2018-11-20  286  	kobject_del(q->mq_kobj);
f05d1ba7871a2c2 Bart Van Assche 2017-04-26  287  	kobject_put(&dev->kobj);
f05d1ba7871a2c2 Bart Van Assche 2017-04-26  288  	return ret;
2d0364c8c1a97a1 Bart Van Assche 2017-04-26  289  }
2d0364c8c1a97a1 Bart Van Assche 2017-04-26  290  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

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

end of thread, other threads:[~2022-03-09  3:02 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-02 12:14 [PATCH V2 0/6] blk-mq: update_nr_hw_queues related improvement & bugfix Ming Lei
2022-03-02 12:14 ` [PATCH V2 1/6] blk-mq: figure out correct numa node for hw queue Ming Lei
2022-03-02 12:39   ` John Garry
2022-03-02 12:14 ` [PATCH V2 2/6] blk-mq: simplify reallocation of hw ctxs a bit Ming Lei
2022-03-02 12:14 ` [PATCH V2 3/6] blk-mq: reconfigure poll after queue map is changed Ming Lei
2022-03-03 15:20   ` John Garry
2022-03-02 12:14 ` [PATCH V2 4/6] block: mtip32xx: don't touch q->queue_hw_ctx Ming Lei
2022-03-02 12:14 ` [PATCH V2 5/6] blk-mq: prepare for implementing hctx table via xarray Ming Lei
2022-03-03  9:08   ` kernel test robot
2022-03-03  9:31     ` Ming Lei
2022-03-03  9:31       ` Ming Lei
2022-03-02 12:14 ` [PATCH V2 6/6] blk-mq: manage hctx map " Ming Lei
2022-03-03 12:33 [PATCH V2 5/6] blk-mq: prepare for implementing hctx table " kernel test robot
2022-03-09  3:01 ` kernel test robot

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.