[PATCH 0/5] Five patches related to changing the number of hardware queues

[PATCH 0/5] Five patches related to changing the number of hardware queues

[Bart Van Assche]

Hi Jens,

These patches are what I came up with while analyzing syzbot and blktests
complaints related to dynamically changing the number of hardware queues.
Please consider these patches for the upstream kernel.

Thanks,

Bart.

Bart Van Assche (5):
  blk-mq: Fix a comment in include/linux/blk-mq.h
  blk-mq: Keep set->nr_hw_queues and set->map[].nr_queues in sync
  blk-mq: Fix a recently introduced regression in
    blk_mq_realloc_hw_ctxs()
  null_blk: Suppress an UBSAN complaint triggered when setting
    'memory_backed'
  null_blk: Fix changing the number of hardware queues

 block/blk-mq.c                |  17 ++++--
 drivers/block/null_blk_main.c | 105 ++++++++++++++++++++++------------
 include/linux/blk-mq.h        |   5 +-
 3 files changed, 84 insertions(+), 43 deletions(-)

[PATCH 1/5] blk-mq: Fix a comment in include/linux/blk-mq.h

[Bart Van Assche]

The 'hctx_list' member of struct blk_mq_hw_ctx is not a list head but
instead an entry in q->unused_hctx_list. Fix the comment above this
struct member.

Cc: André Almeida <andrealmeid@collabora.com>
Fixes: d386732bc142 ("blk-mq: fill header with kernel-doc")
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 include/linux/blk-mq.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 11cfd6470b1a..31344d5f83e2 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -162,7 +162,10 @@ struct blk_mq_hw_ctx {
 	struct dentry		*sched_debugfs_dir;
 #endif
 
-	/** @hctx_list:	List of all hardware queues. */
+	/**
+	 * @hctx_list: if this hctx is not in use, this is an entry in
+	 * q->unused_hctx_list.
+	 */
 	struct list_head	hctx_list;
 
 	/**

[PATCH 2/5] blk-mq: Keep set->nr_hw_queues and set->map[].nr_queues in sync

[Bart Van Assche]

This patch fixes the following kernel warning:

WARNING: CPU: 0 PID: 2501 at include/linux/cpumask.h:137
Call Trace:
 blk_mq_run_hw_queue+0x19d/0x350 block/blk-mq.c:1508
 blk_mq_run_hw_queues+0x112/0x1a0 block/blk-mq.c:1525
 blk_mq_requeue_work+0x502/0x780 block/blk-mq.c:775
 process_one_work+0x9af/0x1740 kernel/workqueue.c:2269
 worker_thread+0x98/0xe40 kernel/workqueue.c:2415
 kthread+0x361/0x430 kernel/kthread.c:255

Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ming Lei <ming.lei@redhat.com>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Johannes Thumshirn <jth@kernel.org>
Reported-by: syzbot+d44e1b26ce5c3e77458d@syzkaller.appspotmail.com
Fixes: ed76e329d74a ("blk-mq: abstract out queue map") # v5.0
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 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 f298500e6dda..2b9f490f5a64 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -3050,6 +3050,16 @@ static int blk_mq_update_queue_map(struct blk_mq_tag_set *set)
 	}
 }
 
+static void blk_mq_set_nr_hw_queues(struct blk_mq_tag_set *set,
+				    int new_nr_hw_queues)
+{
+	int i;
+
+	set->nr_hw_queues = new_nr_hw_queues;
+	for (i = 0; i < set->nr_maps; i++)
+		set->map[i].nr_queues = new_nr_hw_queues;
+}
+
 static int blk_mq_realloc_tag_set_tags(struct blk_mq_tag_set *set,
 				  int cur_nr_hw_queues, int new_nr_hw_queues)
 {
@@ -3068,7 +3078,7 @@ static int blk_mq_realloc_tag_set_tags(struct blk_mq_tag_set *set,
 		       sizeof(*set->tags));
 	kfree(set->tags);
 	set->tags = new_tags;
-	set->nr_hw_queues = new_nr_hw_queues;
+	blk_mq_set_nr_hw_queues(set, new_nr_hw_queues);
 
 	return 0;
 }
@@ -3330,7 +3340,7 @@ static void __blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set,
 		goto reregister;
 
 	prev_nr_hw_queues = set->nr_hw_queues;
-	set->nr_hw_queues = nr_hw_queues;
+	blk_mq_set_nr_hw_queues(set, nr_hw_queues);
 	blk_mq_update_queue_map(set);
 fallback:
 	list_for_each_entry(q, &set->tag_list, tag_set_list) {
@@ -3338,7 +3348,7 @@ static void __blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set,
 		if (q->nr_hw_queues != set->nr_hw_queues) {
 			pr_warn("Increasing nr_hw_queues to %d fails, fallback to %d\n",
 					nr_hw_queues, prev_nr_hw_queues);
-			set->nr_hw_queues = prev_nr_hw_queues;
+			blk_mq_set_nr_hw_queues(set, prev_nr_hw_queues);
 			blk_mq_map_queues(&set->map[HCTX_TYPE_DEFAULT]);
 			goto fallback;
 		}

[PATCH 3/5] blk-mq: Fix a recently introduced regression in blk_mq_realloc_hw_ctxs()

[Bart Van Assche]

q->nr_hw_queues must only be updated once it is known that
blk_mq_realloc_hw_ctxs() has succeeded. Otherwise it can happen that
reallocation fails and that q->nr_hw_queues is larger than the number of
allocated hardware queues. This patch fixes the following crash if
increasing the number of hardware queues fails:

BUG: KASAN: null-ptr-deref in blk_mq_map_swqueue+0x775/0x810
Write of size 8 at addr 0000000000000118 by task check/977

CPU: 3 PID: 977 Comm: check Not tainted 5.6.0-rc1-dbg+ #8
Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
Call Trace:
 dump_stack+0xa5/0xe6
 __kasan_report.cold+0x65/0x99
 kasan_report+0x16/0x20
 check_memory_region+0x140/0x1b0
 memset+0x28/0x40
 blk_mq_map_swqueue+0x775/0x810
 blk_mq_update_nr_hw_queues+0x468/0x710
 nullb_device_submit_queues_store+0xf7/0x1a0 [null_blk]
 configfs_write_file+0x1c4/0x250 [configfs]
 __vfs_write+0x4c/0x90
 vfs_write+0x145/0x2c0
 ksys_write+0xd7/0x180
 __x64_sys_write+0x47/0x50
 do_syscall_64+0x6f/0x2f0
 entry_SYSCALL_64_after_hwframe+0x49/0xbe

Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ming Lei <ming.lei@redhat.com>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Johannes Thumshirn <jth@kernel.org>
Cc: Keith Busch <kbusch@kernel.org>
Fixes: ac0d6b926e74 ("block: Reduce the amount of memory required per request queue")
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 block/blk-mq.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 2b9f490f5a64..5408098b58f2 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -2824,7 +2824,6 @@ static void blk_mq_realloc_hw_ctxs(struct blk_mq_tag_set *set,
 			memcpy(new_hctxs, hctxs, q->nr_hw_queues *
 			       sizeof(*hctxs));
 		q->queue_hw_ctx = new_hctxs;
-		q->nr_hw_queues = set->nr_hw_queues;
 		kfree(hctxs);
 		hctxs = new_hctxs;
 	}

[PATCH 4/5] null_blk: Suppress an UBSAN complaint triggered when setting 'memory_backed'

[Bart Van Assche]

Although it is not clear to me why UBSAN complains when 'memory_backed'
is set, this patch suppresses the UBSAN complaint that is triggered when
setting that configfs attribute.

UBSAN: Undefined behaviour in drivers/block/null_blk_main.c:327:1
load of value 16 is not a valid value for type '_Bool'
CPU: 2 PID: 8396 Comm: check Not tainted 5.6.0-rc1-dbg+ #14
Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
Call Trace:
 dump_stack+0xa5/0xe6
 ubsan_epilogue+0x9/0x26
 __ubsan_handle_load_invalid_value+0x6d/0x76
 nullb_device_memory_backed_store.cold+0x2c/0x38 [null_blk]
 configfs_write_file+0x1c4/0x250 [configfs]
 __vfs_write+0x4c/0x90
 vfs_write+0x145/0x2c0
 ksys_write+0xd7/0x180
 __x64_sys_write+0x47/0x50
 do_syscall_64+0x6f/0x2f0
 entry_SYSCALL_64_after_hwframe+0x49/0xbe

Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ming Lei <ming.lei@redhat.com>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Johannes Thumshirn <jth@kernel.org>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/block/null_blk_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/block/null_blk_main.c b/drivers/block/null_blk_main.c
index 16510795e377..7cd31d4ef709 100644
--- a/drivers/block/null_blk_main.c
+++ b/drivers/block/null_blk_main.c
@@ -276,7 +276,7 @@ nullb_device_##NAME##_store(struct config_item *item, const char *page,	\
 {									\
 	int (*apply_fn)(struct nullb_device *dev, TYPE new_value) = APPLY;\
 	struct nullb_device *dev = to_nullb_device(item);		\
-	TYPE uninitialized_var(new_value);				\
+	TYPE new_value = 0;						\
 	int ret;							\
 									\
 	ret = nullb_device_##TYPE##_attr_store(&new_value, page, count);\

[PATCH 5/5] null_blk: Fix changing the number of hardware queues

[Bart Van Assche]

Instead of initializing null_blk hardware queues explicitly after the
request queue has been created, provide .init_hctx() and .exit_hctx()
callback functions. The latter functions are not only called during
request queue allocation but also when the number of hardware queues
changes. Allocate nr_cpu_ids queues during initialization to support
increasing the number of hardware queues above the initial hardware
queue count.

This change fixes increasing the number of hardware queues above the
initial number of hardware queues and also keeps nullb->nr_queues in
sync with the number of hardware queues.

Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ming Lei <ming.lei@redhat.com>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Johannes Thumshirn <jth@kernel.org>
Fixes: 45919fbfe1c4 ("null_blk: Enable modifying 'submit_queues' after an instance has been configured")
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/block/null_blk_main.c | 103 ++++++++++++++++++++++------------
 1 file changed, 66 insertions(+), 37 deletions(-)

diff --git a/drivers/block/null_blk_main.c b/drivers/block/null_blk_main.c
index 7cd31d4ef709..31678c9af43f 100644
--- a/drivers/block/null_blk_main.c
+++ b/drivers/block/null_blk_main.c
@@ -302,6 +302,12 @@ static int nullb_apply_submit_queues(struct nullb_device *dev,
 	if (!nullb)
 		return 0;
 
+	/*
+	 * Make sure that null_init_hctx() does not access nullb->queues[] past
+	 * the end of that array.
+	 */
+	if (submit_queues > nr_cpu_ids)
+		return -EINVAL;
 	set = nullb->tag_set;
 	blk_mq_update_nr_hw_queues(set, submit_queues);
 	return set->nr_hw_queues == submit_queues ? 0 : -ENOMEM;
@@ -1408,12 +1414,6 @@ static blk_status_t null_queue_rq(struct blk_mq_hw_ctx *hctx,
 	return null_handle_cmd(cmd, sector, nr_sectors, req_op(bd->rq));
 }
 
-static const struct blk_mq_ops null_mq_ops = {
-	.queue_rq       = null_queue_rq,
-	.complete	= null_complete_rq,
-	.timeout	= null_timeout_rq,
-};
-
 static void cleanup_queue(struct nullb_queue *nq)
 {
 	kfree(nq->tag_map);
@@ -1430,6 +1430,43 @@ static void cleanup_queues(struct nullb *nullb)
 	kfree(nullb->queues);
 }
 
+static void null_exit_hctx(struct blk_mq_hw_ctx *hctx, unsigned int hctx_idx)
+{
+	struct nullb_queue *nq = hctx->driver_data;
+	struct nullb *nullb = nq->dev->nullb;
+
+	nullb->nr_queues--;
+}
+
+static void null_init_queue(struct nullb *nullb, struct nullb_queue *nq)
+{
+	init_waitqueue_head(&nq->wait);
+	nq->queue_depth = nullb->queue_depth;
+	nq->dev = nullb->dev;
+}
+
+static int null_init_hctx(struct blk_mq_hw_ctx *hctx, void *driver_data,
+			  unsigned int hctx_idx)
+{
+	struct nullb *nullb = hctx->queue->queuedata;
+	struct nullb_queue *nq;
+
+	nq = &nullb->queues[hctx_idx];
+	hctx->driver_data = nq;
+	null_init_queue(nullb, nq);
+	nullb->nr_queues++;
+
+	return 0;
+}
+
+static const struct blk_mq_ops null_mq_ops = {
+	.queue_rq       = null_queue_rq,
+	.complete	= null_complete_rq,
+	.timeout	= null_timeout_rq,
+	.init_hctx	= null_init_hctx,
+	.exit_hctx	= null_exit_hctx,
+};
+
 static void null_del_dev(struct nullb *nullb)
 {
 	struct nullb_device *dev = nullb->dev;
@@ -1473,33 +1510,6 @@ static const struct block_device_operations null_ops = {
 	.report_zones	= null_report_zones,
 };
 
-static void null_init_queue(struct nullb *nullb, struct nullb_queue *nq)
-{
-	BUG_ON(!nullb);
-	BUG_ON(!nq);
-
-	init_waitqueue_head(&nq->wait);
-	nq->queue_depth = nullb->queue_depth;
-	nq->dev = nullb->dev;
-}
-
-static void null_init_queues(struct nullb *nullb)
-{
-	struct request_queue *q = nullb->q;
-	struct blk_mq_hw_ctx *hctx;
-	struct nullb_queue *nq;
-	int i;
-
-	queue_for_each_hw_ctx(q, hctx, i) {
-		if (!hctx->nr_ctx || !hctx->tags)
-			continue;
-		nq = &nullb->queues[i];
-		hctx->driver_data = nq;
-		null_init_queue(nullb, nq);
-		nullb->nr_queues++;
-	}
-}
-
 static int setup_commands(struct nullb_queue *nq)
 {
 	struct nullb_cmd *cmd;
@@ -1528,8 +1538,7 @@ static int setup_commands(struct nullb_queue *nq)
 
 static int setup_queues(struct nullb *nullb)
 {
-	nullb->queues = kcalloc(nullb->dev->submit_queues,
-				sizeof(struct nullb_queue),
+	nullb->queues = kcalloc(nr_cpu_ids, sizeof(struct nullb_queue),
 				GFP_KERNEL);
 	if (!nullb->queues)
 		return -ENOMEM;
@@ -1675,6 +1684,27 @@ static bool null_setup_fault(void)
 	return true;
 }
 
+/*
+ * This function is identical to blk_mq_init_queue() except that it sets
+ * queuedata before .init_hctx is called.
+ */
+static struct request_queue *nullb_alloc_queue(struct nullb *nullb)
+{
+	struct request_queue *uninit_q, *q;
+	struct blk_mq_tag_set *set = nullb->tag_set;
+
+	uninit_q = blk_alloc_queue_node(GFP_KERNEL, set->numa_node);
+	if (!uninit_q)
+		return ERR_PTR(-ENOMEM);
+
+	uninit_q->queuedata = nullb;
+	q = blk_mq_init_allocated_queue(set, uninit_q, false);
+	if (IS_ERR(q))
+		blk_cleanup_queue(uninit_q);
+
+	return q;
+}
+
 static int null_add_dev(struct nullb_device *dev)
 {
 	struct nullb *nullb;
@@ -1714,12 +1744,11 @@ static int null_add_dev(struct nullb_device *dev)
 			goto out_cleanup_queues;
 
 		nullb->tag_set->timeout = 5 * HZ;
-		nullb->q = blk_mq_init_queue(nullb->tag_set);
+		nullb->q = nullb_alloc_queue(nullb);
 		if (IS_ERR(nullb->q)) {
 			rv = -ENOMEM;
 			goto out_cleanup_tags;
 		}
-		null_init_queues(nullb);
 	} else if (dev->queue_mode == NULL_Q_BIO) {
 		nullb->q = blk_alloc_queue_node(GFP_KERNEL, dev->home_node);
 		if (!nullb->q) {

Re: [PATCH 1/5] blk-mq: Fix a comment in include/linux/blk-mq.h

[Ming Lei]

On Mon, Feb 17, 2020 at 01:08:35PM -0800, Bart Van Assche wrote:
> The 'hctx_list' member of struct blk_mq_hw_ctx is not a list head but
> instead an entry in q->unused_hctx_list. Fix the comment above this
> struct member.
> 
> Cc: André Almeida <andrealmeid@collabora.com>
> Fixes: d386732bc142 ("blk-mq: fill header with kernel-doc")
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  include/linux/blk-mq.h | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
> index 11cfd6470b1a..31344d5f83e2 100644
> --- a/include/linux/blk-mq.h
> +++ b/include/linux/blk-mq.h
> @@ -162,7 +162,10 @@ struct blk_mq_hw_ctx {
>  	struct dentry		*sched_debugfs_dir;
>  #endif
>  
> -	/** @hctx_list:	List of all hardware queues. */
> +	/**
> +	 * @hctx_list: if this hctx is not in use, this is an entry in
> +	 * q->unused_hctx_list.
> +	 */
>  	struct list_head	hctx_list;

The patch itself is correct, however, we may rename the field as
'unused_node' or whatever so that it can be self-documented.


Thanks,
Ming

Re: [PATCH 2/5] blk-mq: Keep set->nr_hw_queues and set->map[].nr_queues in sync

[Ming Lei]

On Mon, Feb 17, 2020 at 01:08:36PM -0800, Bart Van Assche wrote:
> This patch fixes the following kernel warning:
> 
> WARNING: CPU: 0 PID: 2501 at include/linux/cpumask.h:137
> Call Trace:
>  blk_mq_run_hw_queue+0x19d/0x350 block/blk-mq.c:1508
>  blk_mq_run_hw_queues+0x112/0x1a0 block/blk-mq.c:1525
>  blk_mq_requeue_work+0x502/0x780 block/blk-mq.c:775
>  process_one_work+0x9af/0x1740 kernel/workqueue.c:2269
>  worker_thread+0x98/0xe40 kernel/workqueue.c:2415
>  kthread+0x361/0x430 kernel/kthread.c:255
> 
> Cc: Christoph Hellwig <hch@infradead.org>
> Cc: Ming Lei <ming.lei@redhat.com>
> Cc: Hannes Reinecke <hare@suse.com>
> Cc: Johannes Thumshirn <jth@kernel.org>
> Reported-by: syzbot+d44e1b26ce5c3e77458d@syzkaller.appspotmail.com
> Fixes: ed76e329d74a ("blk-mq: abstract out queue map") # v5.0
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  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 f298500e6dda..2b9f490f5a64 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -3050,6 +3050,16 @@ static int blk_mq_update_queue_map(struct blk_mq_tag_set *set)
>  	}
>  }
>  
> +static void blk_mq_set_nr_hw_queues(struct blk_mq_tag_set *set,
> +				    int new_nr_hw_queues)
> +{
> +	int i;
> +
> +	set->nr_hw_queues = new_nr_hw_queues;
> +	for (i = 0; i < set->nr_maps; i++)
> +		set->map[i].nr_queues = new_nr_hw_queues;
> +}
> +
>  static int blk_mq_realloc_tag_set_tags(struct blk_mq_tag_set *set,
>  				  int cur_nr_hw_queues, int new_nr_hw_queues)
>  {
> @@ -3068,7 +3078,7 @@ static int blk_mq_realloc_tag_set_tags(struct blk_mq_tag_set *set,
>  		       sizeof(*set->tags));
>  	kfree(set->tags);
>  	set->tags = new_tags;
> -	set->nr_hw_queues = new_nr_hw_queues;
> +	blk_mq_set_nr_hw_queues(set, new_nr_hw_queues);
>  
>  	return 0;
>  }
> @@ -3330,7 +3340,7 @@ static void __blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set,
>  		goto reregister;
>  
>  	prev_nr_hw_queues = set->nr_hw_queues;
> -	set->nr_hw_queues = nr_hw_queues;
> +	blk_mq_set_nr_hw_queues(set, nr_hw_queues);
>  	blk_mq_update_queue_map(set);
>  fallback:
>  	list_for_each_entry(q, &set->tag_list, tag_set_list) {
> @@ -3338,7 +3348,7 @@ static void __blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set,
>  		if (q->nr_hw_queues != set->nr_hw_queues) {
>  			pr_warn("Increasing nr_hw_queues to %d fails, fallback to %d\n",
>  					nr_hw_queues, prev_nr_hw_queues);
> -			set->nr_hw_queues = prev_nr_hw_queues;
> +			blk_mq_set_nr_hw_queues(set, prev_nr_hw_queues);
>  			blk_mq_map_queues(&set->map[HCTX_TYPE_DEFAULT]);
>  			goto fallback;
>  		}
> 

I guess the issue can be fixed by the following change, and we do not
need to touch each queue map:

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 5f5c43ae3792..f7340afb89ec 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -3046,6 +3046,7 @@ static int blk_mq_update_queue_map(struct blk_mq_tag_set *set)
                return set->ops->map_queues(set);
        } else {
                BUG_ON(set->nr_maps > 1);
+               set->map[HCTX_TYPE_DEFAULT].nr_queues = set->nr_hw_queues;
                return blk_mq_map_queues(&set->map[HCTX_TYPE_DEFAULT]);
        }
 }


Thanks,
Ming

Re: [PATCH 3/5] blk-mq: Fix a recently introduced regression in blk_mq_realloc_hw_ctxs()

[Ming Lei]

On Mon, Feb 17, 2020 at 01:08:37PM -0800, Bart Van Assche wrote:
> q->nr_hw_queues must only be updated once it is known that
> blk_mq_realloc_hw_ctxs() has succeeded. Otherwise it can happen that
> reallocation fails and that q->nr_hw_queues is larger than the number of
> allocated hardware queues. This patch fixes the following crash if
> increasing the number of hardware queues fails:
> 
> BUG: KASAN: null-ptr-deref in blk_mq_map_swqueue+0x775/0x810
> Write of size 8 at addr 0000000000000118 by task check/977
> 
> CPU: 3 PID: 977 Comm: check Not tainted 5.6.0-rc1-dbg+ #8
> Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
> Call Trace:
>  dump_stack+0xa5/0xe6
>  __kasan_report.cold+0x65/0x99
>  kasan_report+0x16/0x20
>  check_memory_region+0x140/0x1b0
>  memset+0x28/0x40
>  blk_mq_map_swqueue+0x775/0x810
>  blk_mq_update_nr_hw_queues+0x468/0x710
>  nullb_device_submit_queues_store+0xf7/0x1a0 [null_blk]
>  configfs_write_file+0x1c4/0x250 [configfs]
>  __vfs_write+0x4c/0x90
>  vfs_write+0x145/0x2c0
>  ksys_write+0xd7/0x180
>  __x64_sys_write+0x47/0x50
>  do_syscall_64+0x6f/0x2f0
>  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> 
> Cc: Christoph Hellwig <hch@infradead.org>
> Cc: Ming Lei <ming.lei@redhat.com>
> Cc: Hannes Reinecke <hare@suse.com>
> Cc: Johannes Thumshirn <jth@kernel.org>
> Cc: Keith Busch <kbusch@kernel.org>
> Fixes: ac0d6b926e74 ("block: Reduce the amount of memory required per request queue")
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  block/blk-mq.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 2b9f490f5a64..5408098b58f2 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -2824,7 +2824,6 @@ static void blk_mq_realloc_hw_ctxs(struct blk_mq_tag_set *set,
>  			memcpy(new_hctxs, hctxs, q->nr_hw_queues *
>  			       sizeof(*hctxs));
>  		q->queue_hw_ctx = new_hctxs;
> -		q->nr_hw_queues = set->nr_hw_queues;
>  		kfree(hctxs);
>  		hctxs = new_hctxs;
>  	}

Looks correct, since ->nr_hw_queues needs to be updated after hctxs are
initialized successfully:

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


thanks,
Ming

Re: [PATCH 2/5] blk-mq: Keep set->nr_hw_queues and set->map[].nr_queues in sync

[Bart Van Assche]

On 2020-02-17 19:16, Ming Lei wrote:
> I guess the issue can be fixed by the following change, and we do not
> need to touch each queue map:
> 
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 5f5c43ae3792..f7340afb89ec 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -3046,6 +3046,7 @@ static int blk_mq_update_queue_map(struct blk_mq_tag_set *set)
>                 return set->ops->map_queues(set);
>         } else {
>                 BUG_ON(set->nr_maps > 1);
> +               set->map[HCTX_TYPE_DEFAULT].nr_queues = set->nr_hw_queues;
>                 return blk_mq_map_queues(&set->map[HCTX_TYPE_DEFAULT]);
>         }
>  }

Every caller of blk_mq_map_queues() needs to set the number of queues
in the default queue map so I would like to make the number of queues an
argument. How about the (untested) patch below?

Thanks,

Bart.

---
 block/blk-mq-cpumap.c                 |  5 +++--
 block/blk-mq-rdma.c                   |  2 +-
 block/blk-mq-virtio.c                 |  2 +-
 block/blk-mq.c                        |  6 ++++--
 drivers/block/virtio_blk.c            |  5 +++--
 drivers/nvme/host/pci.c               |  2 +-
 drivers/nvme/host/rdma.c              |  5 ++---
 drivers/nvme/host/tcp.c               | 11 ++++++-----
 drivers/scsi/qla2xxx/qla_os.c         |  8 +++++---
 drivers/scsi/scsi_lib.c               |  3 ++-
 drivers/scsi/smartpqi/smartpqi_init.c |  5 +++--
 drivers/scsi/virtio_scsi.c            |  1 +
 include/linux/blk-mq.h                |  2 +-
 13 files changed, 33 insertions(+), 24 deletions(-)

diff --git a/block/blk-mq-cpumap.c b/block/blk-mq-cpumap.c
index 0157f2b3485a..4c0d5768305a 100644
--- a/block/blk-mq-cpumap.c
+++ b/block/blk-mq-cpumap.c
@@ -32,12 +32,13 @@ static int get_first_sibling(unsigned int cpu)
 	return cpu;
 }

-int blk_mq_map_queues(struct blk_mq_queue_map *qmap)
+int blk_mq_map_queues(struct blk_mq_queue_map *qmap, unsigned int nr_queues)
 {
 	unsigned int *map = qmap->mq_map;
-	unsigned int nr_queues = qmap->nr_queues;
 	unsigned int cpu, first_sibling, q = 0;

+	qmap->nr_queues = nr_queues;
+
 	for_each_possible_cpu(cpu)
 		map[cpu] = -1;

diff --git a/block/blk-mq-rdma.c b/block/blk-mq-rdma.c
index 14f968e58b8f..c32af1231244 100644
--- a/block/blk-mq-rdma.c
+++ b/block/blk-mq-rdma.c
@@ -39,6 +39,6 @@ int blk_mq_rdma_map_queues(struct blk_mq_queue_map *map,
 	return 0;

 fallback:
-	return blk_mq_map_queues(map);
+	return blk_mq_map_queues(map, map->nr_queues);
 }
 EXPORT_SYMBOL_GPL(blk_mq_rdma_map_queues);
diff --git a/block/blk-mq-virtio.c b/block/blk-mq-virtio.c
index 488341628256..b5911cb43974 100644
--- a/block/blk-mq-virtio.c
+++ b/block/blk-mq-virtio.c
@@ -41,6 +41,6 @@ int blk_mq_virtio_map_queues(struct blk_mq_queue_map *qmap,

 	return 0;
 fallback:
-	return blk_mq_map_queues(qmap);
+	return blk_mq_map_queues(qmap, qmap->nr_queues);
 }
 EXPORT_SYMBOL_GPL(blk_mq_virtio_map_queues);
diff --git a/block/blk-mq.c b/block/blk-mq.c
index f298500e6dda..bbd9edde2b6f 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -3046,7 +3046,8 @@ static int blk_mq_update_queue_map(struct blk_mq_tag_set *set)
 		return set->ops->map_queues(set);
 	} else {
 		BUG_ON(set->nr_maps > 1);
-		return blk_mq_map_queues(&set->map[HCTX_TYPE_DEFAULT]);
+		return blk_mq_map_queues(&set->map[HCTX_TYPE_DEFAULT],
+					 set->nr_hw_queues);
 	}
 }

@@ -3339,7 +3340,8 @@ static void __blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set,
 			pr_warn("Increasing nr_hw_queues to %d fails, fallback to %d\n",
 					nr_hw_queues, prev_nr_hw_queues);
 			set->nr_hw_queues = prev_nr_hw_queues;
-			blk_mq_map_queues(&set->map[HCTX_TYPE_DEFAULT]);
+			blk_mq_map_queues(&set->map[HCTX_TYPE_DEFAULT],
+					  set->nr_hw_queues);
 			goto fallback;
 		}
 		blk_mq_map_swqueue(q);
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 54158766334b..61d6482a0b9e 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -595,9 +595,10 @@ static int virtblk_init_request(struct blk_mq_tag_set *set, struct request *rq,
 static int virtblk_map_queues(struct blk_mq_tag_set *set)
 {
 	struct virtio_blk *vblk = set->driver_data;
+	struct blk_mq_queue_map *qmap = &set->map[HCTX_TYPE_DEFAULT];

-	return blk_mq_virtio_map_queues(&set->map[HCTX_TYPE_DEFAULT],
-					vblk->vdev, 0);
+	qmap->nr_queues = set->nr_hw_queues;
+	return blk_mq_virtio_map_queues(qmap, vblk->vdev, 0);
 }

 static const struct blk_mq_ops virtio_mq_ops = {
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index da392b50f73e..7531a6950736 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -438,7 +438,7 @@ static int nvme_pci_map_queues(struct blk_mq_tag_set *set)
 		if (i != HCTX_TYPE_POLL && offset)
 			blk_mq_pci_map_queues(map, to_pci_dev(dev->dev), offset);
 		else
-			blk_mq_map_queues(map);
+			blk_mq_map_queues(map, map->nr_queues);
 		qoff += map->nr_queues;
 		offset += map->nr_queues;
 	}
diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index 2a47c6c5007e..27d00bc7a746 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -1844,12 +1844,11 @@ static int nvme_rdma_map_queues(struct blk_mq_tag_set *set)

 	if (opts->nr_poll_queues && ctrl->io_queues[HCTX_TYPE_POLL]) {
 		/* map dedicated poll queues only if we have queues left */
-		set->map[HCTX_TYPE_POLL].nr_queues =
-				ctrl->io_queues[HCTX_TYPE_POLL];
 		set->map[HCTX_TYPE_POLL].queue_offset =
 			ctrl->io_queues[HCTX_TYPE_DEFAULT] +
 			ctrl->io_queues[HCTX_TYPE_READ];
-		blk_mq_map_queues(&set->map[HCTX_TYPE_POLL]);
+		blk_mq_map_queues(&set->map[HCTX_TYPE_POLL],
+				  ctrl->io_queues[HCTX_TYPE_POLL]);
 	}

 	dev_info(ctrl->ctrl.device,
diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 6d43b23a0fc8..c05ca7dec661 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -2192,17 +2192,18 @@ static int nvme_tcp_map_queues(struct blk_mq_tag_set *set)
 			ctrl->io_queues[HCTX_TYPE_DEFAULT];
 		set->map[HCTX_TYPE_READ].queue_offset = 0;
 	}
-	blk_mq_map_queues(&set->map[HCTX_TYPE_DEFAULT]);
-	blk_mq_map_queues(&set->map[HCTX_TYPE_READ]);
+	blk_mq_map_queues(&set->map[HCTX_TYPE_DEFAULT],
+			  set->map[HCTX_TYPE_DEFAULT].nr_queues);
+	blk_mq_map_queues(&set->map[HCTX_TYPE_READ],
+			  set->map[HCTX_TYPE_READ].nr_queues);

 	if (opts->nr_poll_queues && ctrl->io_queues[HCTX_TYPE_POLL]) {
 		/* map dedicated poll queues only if we have queues left */
-		set->map[HCTX_TYPE_POLL].nr_queues =
-				ctrl->io_queues[HCTX_TYPE_POLL];
 		set->map[HCTX_TYPE_POLL].queue_offset =
 			ctrl->io_queues[HCTX_TYPE_DEFAULT] +
 			ctrl->io_queues[HCTX_TYPE_READ];
-		blk_mq_map_queues(&set->map[HCTX_TYPE_POLL]);
+		blk_mq_map_queues(&set->map[HCTX_TYPE_POLL],
+				  ctrl->io_queues[HCTX_TYPE_POLL]);
 	}

 	dev_info(ctrl->ctrl.device,
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index b520a980d1dc..71058bec7d98 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -7114,10 +7114,12 @@ static int qla2xxx_map_queues(struct Scsi_Host *shost)
 	scsi_qla_host_t *vha = (scsi_qla_host_t *)shost->hostdata;
 	struct blk_mq_queue_map *qmap = &shost->tag_set.map[HCTX_TYPE_DEFAULT];

-	if (USER_CTRL_IRQ(vha->hw) || !vha->hw->mqiobase)
-		rc = blk_mq_map_queues(qmap);
-	else
+	if (USER_CTRL_IRQ(vha->hw) || !vha->hw->mqiobase) {
+		rc = blk_mq_map_queues(qmap, shost->tag_set.nr_hw_queues);
+	} else {
+		qmap->nr_queues = shost->tag_set.nr_hw_queues;
 		rc = blk_mq_pci_map_queues(qmap, vha->hw->pdev, vha->irq_offset);
+	}
 	return rc;
 }

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 610ee41fa54c..45a4b115dadf 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1775,7 +1775,8 @@ static int scsi_map_queues(struct blk_mq_tag_set *set)

 	if (shost->hostt->map_queues)
 		return shost->hostt->map_queues(shost);
-	return blk_mq_map_queues(&set->map[HCTX_TYPE_DEFAULT]);
+	return blk_mq_map_queues(&set->map[HCTX_TYPE_DEFAULT],
+				 set->nr_hw_queues);
 }

 void __scsi_init_queue(struct Scsi_Host *shost, struct request_queue *q)
diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c
index b7492568e02f..18ae5ce129b2 100644
--- a/drivers/scsi/smartpqi/smartpqi_init.c
+++ b/drivers/scsi/smartpqi/smartpqi_init.c
@@ -5826,9 +5826,10 @@ static int pqi_slave_alloc(struct scsi_device *sdev)
 static int pqi_map_queues(struct Scsi_Host *shost)
 {
 	struct pqi_ctrl_info *ctrl_info = shost_to_hba(shost);
+	struct blk_mq_queue_map *qmap = &shost->tag_set.map[HCTX_TYPE_DEFAULT];

-	return blk_mq_pci_map_queues(&shost->tag_set.map[HCTX_TYPE_DEFAULT],
-					ctrl_info->pci_dev, 0);
+	qmap->nr_queues = shost->tag_set.nr_hw_queues;
+	return blk_mq_pci_map_queues(qmap, ctrl_info->pci_dev, 0);
 }

 static int pqi_getpciinfo_ioctl(struct pqi_ctrl_info *ctrl_info,
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index bfec84aacd90..9b9da0409643 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -705,6 +705,7 @@ static int virtscsi_map_queues(struct Scsi_Host *shost)
 	struct virtio_scsi *vscsi = shost_priv(shost);
 	struct blk_mq_queue_map *qmap = &shost->tag_set.map[HCTX_TYPE_DEFAULT];

+	qmap->nr_queues = shost->tag_set.nr_hw_queues;
 	return blk_mq_virtio_map_queues(qmap, vscsi->vdev, 2);
 }

diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 31344d5f83e2..c63b315ed0c8 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -516,7 +516,7 @@ void blk_mq_freeze_queue_wait(struct request_queue *q);
 int blk_mq_freeze_queue_wait_timeout(struct request_queue *q,
 				     unsigned long timeout);

-int blk_mq_map_queues(struct blk_mq_queue_map *qmap);
+int blk_mq_map_queues(struct blk_mq_queue_map *qmap, unsigned int nr_queues);
 void blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set, int nr_hw_queues);

 void blk_mq_quiesce_queue_nowait(struct request_queue *q);