* [PATCH v2 1/8] blk-mq: Fix a comment in include/linux/blk-mq.h
2020-02-20 2:44 [PATCH v2 0/8] Eight patches related to changing the number of hardware queues Bart Van Assche
@ 2020-02-20 2:44 ` Bart Van Assche
2020-02-20 2:44 ` [PATCH v2 2/8] blk-mq: Keep set->nr_hw_queues and set->map[].nr_queues in sync Bart Van Assche
` (6 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Bart Van Assche @ 2020-02-20 2:44 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Christoph Hellwig, Bart Van Assche, André Almeida
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;
/**
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 2/8] blk-mq: Keep set->nr_hw_queues and set->map[].nr_queues in sync
2020-02-20 2:44 [PATCH v2 0/8] Eight patches related to changing the number of hardware queues Bart Van Assche
2020-02-20 2:44 ` [PATCH v2 1/8] blk-mq: Fix a comment in include/linux/blk-mq.h Bart Van Assche
@ 2020-02-20 2:44 ` Bart Van Assche
2020-02-20 10:05 ` Ming Lei
2020-02-20 2:44 ` [PATCH v2 3/8] blk-mq: Fix a recently introduced regression in blk_mq_realloc_hw_ctxs() Bart Van Assche
` (5 subsequent siblings)
7 siblings, 1 reply; 13+ messages in thread
From: Bart Van Assche @ 2020-02-20 2:44 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Christoph Hellwig, Bart Van Assche,
Christoph Hellwig, Ming Lei, Hannes Reinecke, Johannes Thumshirn,
syzbot+d44e1b26ce5c3e77458d
blk_mq_map_queues() and multiple .map_queues() implementations expect that
set->map[HCTX_TYPE_DEFAULT].nr_queues is set to the number of hardware
queues. Hence set .nr_queues before calling these functions. 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 | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/block/blk-mq.c b/block/blk-mq.c
index f298500e6dda..7f996e427733 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -3023,6 +3023,13 @@ static int blk_mq_alloc_rq_maps(struct blk_mq_tag_set *set)
static int blk_mq_update_queue_map(struct blk_mq_tag_set *set)
{
+ /*
+ * blk_mq_map_queues() and multiple .map_queues() implementations
+ * expect that set->map[HCTX_TYPE_DEFAULT].nr_queues is set to the
+ * number of hardware queues.
+ */
+ set->map[HCTX_TYPE_DEFAULT].nr_queues = set->nr_hw_queues;
+
if (set->ops->map_queues && !is_kdump_kernel()) {
int i;
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2 2/8] blk-mq: Keep set->nr_hw_queues and set->map[].nr_queues in sync
2020-02-20 2:44 ` [PATCH v2 2/8] blk-mq: Keep set->nr_hw_queues and set->map[].nr_queues in sync Bart Van Assche
@ 2020-02-20 10:05 ` Ming Lei
2020-02-20 16:14 ` Bart Van Assche
0 siblings, 1 reply; 13+ messages in thread
From: Ming Lei @ 2020-02-20 10:05 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, linux-block, Christoph Hellwig, Christoph Hellwig,
Hannes Reinecke, Johannes Thumshirn, syzbot+d44e1b26ce5c3e77458d
On Wed, Feb 19, 2020 at 06:44:35PM -0800, Bart Van Assche wrote:
> blk_mq_map_queues() and multiple .map_queues() implementations expect that
> set->map[HCTX_TYPE_DEFAULT].nr_queues is set to the number of hardware
Only single queue mapping expects set->map[HCTX_TYPE_DEFAULT].nr_queues
to be set->nr_hw_queues. For multiple mapping, set->nr_hw_queues should
be sum of each mapping's nr_queue.
Thanks,
Ming
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/8] blk-mq: Keep set->nr_hw_queues and set->map[].nr_queues in sync
2020-02-20 10:05 ` Ming Lei
@ 2020-02-20 16:14 ` Bart Van Assche
2020-02-20 20:47 ` Ming Lei
0 siblings, 1 reply; 13+ messages in thread
From: Bart Van Assche @ 2020-02-20 16:14 UTC (permalink / raw)
To: Ming Lei
Cc: Jens Axboe, linux-block, Christoph Hellwig, Christoph Hellwig,
Hannes Reinecke, Johannes Thumshirn, syzbot+d44e1b26ce5c3e77458d
On 2/20/20 2:05 AM, Ming Lei wrote:
> On Wed, Feb 19, 2020 at 06:44:35PM -0800, Bart Van Assche wrote:
>> blk_mq_map_queues() and multiple .map_queues() implementations expect that
>> set->map[HCTX_TYPE_DEFAULT].nr_queues is set to the number of hardware
>
> Only single queue mapping expects set->map[HCTX_TYPE_DEFAULT].nr_queues
> to be set->nr_hw_queues. For multiple mapping, set->nr_hw_queues should
> be sum of each mapping's nr_queue.
That's how it should work but that doesn't match what I found in the
kernel tree. Here is an example of a .map_queues() implementation that
depends on .nr_queues being set before it is called:
static int scsi_map_queues(struct blk_mq_tag_set *set)
{
struct Scsi_Host *shost = container_of(set, struct Scsi_Host,
tag_set);
if (shost->hostt->map_queues)
return shost->hostt->map_queues(shost);
return blk_mq_map_queues(&set->map[HCTX_TYPE_DEFAULT]);
}
If shost->hostt->map_queues == NULL, the above function only works
correctly if .nr_queues is set before this function is called.
Additionally, scsi_map_queues() may call e.g. the following function:
static int qla2xxx_map_queues(struct Scsi_Host *shost)
{
int rc;
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
rc = blk_mq_pci_map_queues(qmap, vha->hw->pdev,
vha->irq_offset);
return rc;
}
Both blk_mq_map_queues() and blk_mq_pci_map_queues() assume that
blk_mq_queue_map.nr_queues is set before these functions are called.
Bart.
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 2/8] blk-mq: Keep set->nr_hw_queues and set->map[].nr_queues in sync
2020-02-20 16:14 ` Bart Van Assche
@ 2020-02-20 20:47 ` Ming Lei
2020-02-21 2:50 ` Bart Van Assche
0 siblings, 1 reply; 13+ messages in thread
From: Ming Lei @ 2020-02-20 20:47 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, linux-block, Christoph Hellwig, Christoph Hellwig,
Hannes Reinecke, Johannes Thumshirn, syzbot+d44e1b26ce5c3e77458d
On Thu, Feb 20, 2020 at 08:14:11AM -0800, Bart Van Assche wrote:
> On 2/20/20 2:05 AM, Ming Lei wrote:
> > On Wed, Feb 19, 2020 at 06:44:35PM -0800, Bart Van Assche wrote:
> > > blk_mq_map_queues() and multiple .map_queues() implementations expect that
> > > set->map[HCTX_TYPE_DEFAULT].nr_queues is set to the number of hardware
> >
> > Only single queue mapping expects set->map[HCTX_TYPE_DEFAULT].nr_queues
> > to be set->nr_hw_queues. For multiple mapping, set->nr_hw_queues should
> > be sum of each mapping's nr_queue.
>
> That's how it should work but that doesn't match what I found in the kernel
> tree. Here is an example of a .map_queues() implementation that depends on
> .nr_queues being set before it is called:
>
> static int scsi_map_queues(struct blk_mq_tag_set *set)
> {
> struct Scsi_Host *shost = container_of(set, struct Scsi_Host,
> tag_set);
>
> if (shost->hostt->map_queues)
> return shost->hostt->map_queues(shost);
> return blk_mq_map_queues(&set->map[HCTX_TYPE_DEFAULT]);
> }
>
> If shost->hostt->map_queues == NULL, the above function only works correctly
> if .nr_queues is set before this function is called. Additionally,
> scsi_map_queues() may call e.g. the following function:
>
> static int qla2xxx_map_queues(struct Scsi_Host *shost)
> {
> int rc;
> 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
> rc = blk_mq_pci_map_queues(qmap, vha->hw->pdev,
> vha->irq_offset);
> return rc;
> }
>
> Both blk_mq_map_queues() and blk_mq_pci_map_queues() assume that
> blk_mq_queue_map.nr_queues is set before these functions are called.
Actually, I suggested to do the following way:
if (set->nr_maps == 1)
set->map[HCTX_TYPE_DEFAULT].nr_queues = set->nr_hw_queues;
then people won't be confused wrt. relation between set->nr_hw_queues
and .nr_queues of each mapping.
Thanks,
Ming
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 2/8] blk-mq: Keep set->nr_hw_queues and set->map[].nr_queues in sync
2020-02-20 20:47 ` Ming Lei
@ 2020-02-21 2:50 ` Bart Van Assche
0 siblings, 0 replies; 13+ messages in thread
From: Bart Van Assche @ 2020-02-21 2:50 UTC (permalink / raw)
To: Ming Lei
Cc: Jens Axboe, linux-block, Christoph Hellwig, Christoph Hellwig,
Hannes Reinecke, Johannes Thumshirn, syzbot+d44e1b26ce5c3e77458d
On 2020-02-20 12:47, Ming Lei wrote:
> Actually, I suggested to do the following way:
>
> if (set->nr_maps == 1)
> set->map[HCTX_TYPE_DEFAULT].nr_queues = set->nr_hw_queues;
>
> then people won't be confused wrt. relation between set->nr_hw_queues
> and .nr_queues of each mapping.
Ah, thanks for the clarification. I will make that change.
Bart.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 3/8] blk-mq: Fix a recently introduced regression in blk_mq_realloc_hw_ctxs()
2020-02-20 2:44 [PATCH v2 0/8] Eight patches related to changing the number of hardware queues Bart Van Assche
2020-02-20 2:44 ` [PATCH v2 1/8] blk-mq: Fix a comment in include/linux/blk-mq.h Bart Van Assche
2020-02-20 2:44 ` [PATCH v2 2/8] blk-mq: Keep set->nr_hw_queues and set->map[].nr_queues in sync Bart Van Assche
@ 2020-02-20 2:44 ` Bart Van Assche
2020-02-20 2:44 ` [PATCH v2 4/8] null_blk: Suppress an UBSAN complaint triggered when setting 'memory_backed' Bart Van Assche
` (4 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Bart Van Assche @ 2020-02-20 2:44 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Christoph Hellwig, Bart Van Assche, Ming Lei,
Christoph Hellwig, Hannes Reinecke, Johannes Thumshirn,
Keith Busch
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
Reviewed-by: Ming Lei <ming.lei@redhat.com>
Cc: Christoph Hellwig <hch@infradead.org>
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 7f996e427733..948523fbfa7f 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;
}
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 4/8] null_blk: Suppress an UBSAN complaint triggered when setting 'memory_backed'
2020-02-20 2:44 [PATCH v2 0/8] Eight patches related to changing the number of hardware queues Bart Van Assche
` (2 preceding siblings ...)
2020-02-20 2:44 ` [PATCH v2 3/8] blk-mq: Fix a recently introduced regression in blk_mq_realloc_hw_ctxs() Bart Van Assche
@ 2020-02-20 2:44 ` Bart Van Assche
2020-02-20 2:44 ` [PATCH v2 5/8] null_blk: Fix changing the number of hardware queues Bart Van Assche
` (3 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Bart Van Assche @ 2020-02-20 2:44 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Christoph Hellwig, Bart Van Assche,
Christoph Hellwig, Ming Lei, Hannes Reinecke, Johannes Thumshirn
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);\
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 5/8] null_blk: Fix changing the number of hardware queues
2020-02-20 2:44 [PATCH v2 0/8] Eight patches related to changing the number of hardware queues Bart Van Assche
` (3 preceding siblings ...)
2020-02-20 2:44 ` [PATCH v2 4/8] null_blk: Suppress an UBSAN complaint triggered when setting 'memory_backed' Bart Van Assche
@ 2020-02-20 2:44 ` Bart Van Assche
2020-02-20 2:44 ` [PATCH v2 6/8] null_blk: Fix the null_add_dev() error path Bart Van Assche
` (2 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Bart Van Assche @ 2020-02-20 2:44 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Christoph Hellwig, Bart Van Assche,
Christoph Hellwig, Ming Lei, Hannes Reinecke, Johannes Thumshirn
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) {
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 6/8] null_blk: Fix the null_add_dev() error path
2020-02-20 2:44 [PATCH v2 0/8] Eight patches related to changing the number of hardware queues Bart Van Assche
` (4 preceding siblings ...)
2020-02-20 2:44 ` [PATCH v2 5/8] null_blk: Fix changing the number of hardware queues Bart Van Assche
@ 2020-02-20 2:44 ` Bart Van Assche
2020-02-20 2:44 ` [PATCH v2 7/8] null_blk: Handle null_add_dev() failures properly Bart Van Assche
2020-02-20 2:44 ` [PATCH v2 8/8] null_blk: Add support for init_hctx() fault injection Bart Van Assche
7 siblings, 0 replies; 13+ messages in thread
From: Bart Van Assche @ 2020-02-20 2:44 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Christoph Hellwig, Bart Van Assche,
Christoph Hellwig, Ming Lei, Hannes Reinecke, Johannes Thumshirn
If null_add_dev() fails, clear dev->nullb.
This patch fixes the following KASAN complaint:
BUG: KASAN: use-after-free in nullb_device_submit_queues_store+0xcf/0x160 [null_blk]
Read of size 8 at addr ffff88803280fc30 by task check/8409
Call Trace:
dump_stack+0xa5/0xe6
print_address_description.constprop.0+0x26/0x260
__kasan_report.cold+0x7b/0x99
kasan_report+0x16/0x20
__asan_load8+0x58/0x90
nullb_device_submit_queues_store+0xcf/0x160 [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
RIP: 0033:0x7ff370926317
Code: 64 89 02 48 c7 c0 ff ff ff ff eb bb 0f 1f 80 00 00 00 00 f3 0f 1e fa 64 8b 04 25 18 00 00 00 85 c0 75 10 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 51 c3 48 83 ec 28 48 89 54 24 18 48 89 74 24
RSP: 002b:00007fff2dd2da48 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 0000000000000002 RCX: 00007ff370926317
RDX: 0000000000000002 RSI: 0000559437ef23f0 RDI: 0000000000000001
RBP: 0000559437ef23f0 R08: 000000000000000a R09: 0000000000000001
R10: 0000559436703471 R11: 0000000000000246 R12: 0000000000000002
R13: 00007ff370a006a0 R14: 00007ff370a014a0 R15: 00007ff370a008a0
Allocated by task 8409:
save_stack+0x23/0x90
__kasan_kmalloc.constprop.0+0xcf/0xe0
kasan_kmalloc+0xd/0x10
kmem_cache_alloc_node_trace+0x129/0x4c0
null_add_dev+0x24a/0xe90 [null_blk]
nullb_device_power_store+0x1b6/0x270 [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
Freed by task 8409:
save_stack+0x23/0x90
__kasan_slab_free+0x112/0x160
kasan_slab_free+0x12/0x20
kfree+0xdf/0x250
null_add_dev+0xaf3/0xe90 [null_blk]
nullb_device_power_store+0x1b6/0x270 [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>
Fixes: 2984c8684f96 ("nullb: factor disk parameters")
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
drivers/block/null_blk_main.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/block/null_blk_main.c b/drivers/block/null_blk_main.c
index 31678c9af43f..9846648780b6 100644
--- a/drivers/block/null_blk_main.c
+++ b/drivers/block/null_blk_main.c
@@ -1819,6 +1819,7 @@ static int null_add_dev(struct nullb_device *dev)
cleanup_queues(nullb);
out_free_nullb:
kfree(nullb);
+ dev->nullb = NULL;
out:
return rv;
}
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 7/8] null_blk: Handle null_add_dev() failures properly
2020-02-20 2:44 [PATCH v2 0/8] Eight patches related to changing the number of hardware queues Bart Van Assche
` (5 preceding siblings ...)
2020-02-20 2:44 ` [PATCH v2 6/8] null_blk: Fix the null_add_dev() error path Bart Van Assche
@ 2020-02-20 2:44 ` Bart Van Assche
2020-02-20 2:44 ` [PATCH v2 8/8] null_blk: Add support for init_hctx() fault injection Bart Van Assche
7 siblings, 0 replies; 13+ messages in thread
From: Bart Van Assche @ 2020-02-20 2:44 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Christoph Hellwig, Bart Van Assche,
Christoph Hellwig, Ming Lei, Hannes Reinecke, Johannes Thumshirn
If null_add_dev() fails then null_del_dev() is called with a NULL argument.
Make null_del_dev() handle this scenario correctly. This patch fixes the
following KASAN complaint:
null-ptr-deref in null_del_dev+0x28/0x280 [null_blk]
Read of size 8 at addr 0000000000000000 by task find/1062
Call Trace:
dump_stack+0xa5/0xe6
__kasan_report.cold+0x65/0x99
kasan_report+0x16/0x20
__asan_load8+0x58/0x90
null_del_dev+0x28/0x280 [null_blk]
nullb_group_drop_item+0x7e/0xa0 [null_blk]
client_drop_item+0x53/0x80 [configfs]
configfs_rmdir+0x395/0x4e0 [configfs]
vfs_rmdir+0xb6/0x220
do_rmdir+0x238/0x2c0
__x64_sys_unlinkat+0x75/0x90
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 | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/block/null_blk_main.c b/drivers/block/null_blk_main.c
index 9846648780b6..ba83fd0537ce 100644
--- a/drivers/block/null_blk_main.c
+++ b/drivers/block/null_blk_main.c
@@ -1469,7 +1469,12 @@ static const struct blk_mq_ops null_mq_ops = {
static void null_del_dev(struct nullb *nullb)
{
- struct nullb_device *dev = nullb->dev;
+ struct nullb_device *dev;
+
+ if (!nullb)
+ return;
+
+ dev = nullb->dev;
ida_simple_remove(&nullb_indexes, nullb->index);
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 8/8] null_blk: Add support for init_hctx() fault injection
2020-02-20 2:44 [PATCH v2 0/8] Eight patches related to changing the number of hardware queues Bart Van Assche
` (6 preceding siblings ...)
2020-02-20 2:44 ` [PATCH v2 7/8] null_blk: Handle null_add_dev() failures properly Bart Van Assche
@ 2020-02-20 2:44 ` Bart Van Assche
7 siblings, 0 replies; 13+ messages in thread
From: Bart Van Assche @ 2020-02-20 2:44 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Christoph Hellwig, Bart Van Assche,
Christoph Hellwig, Ming Lei, Hannes Reinecke, Johannes Thumshirn
This makes it possible to test the error path in blk_mq_realloc_hw_ctxs()
and also several error paths in null_blk.
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 | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/block/null_blk_main.c b/drivers/block/null_blk_main.c
index ba83fd0537ce..7fccf162b59b 100644
--- a/drivers/block/null_blk_main.c
+++ b/drivers/block/null_blk_main.c
@@ -23,6 +23,7 @@
#ifdef CONFIG_BLK_DEV_NULL_BLK_FAULT_INJECTION
static DECLARE_FAULT_ATTR(null_timeout_attr);
static DECLARE_FAULT_ATTR(null_requeue_attr);
+static DECLARE_FAULT_ATTR(null_init_hctx_attr);
#endif
static inline u64 mb_per_tick(int mbps)
@@ -101,6 +102,9 @@ module_param_string(timeout, g_timeout_str, sizeof(g_timeout_str), 0444);
static char g_requeue_str[80];
module_param_string(requeue, g_requeue_str, sizeof(g_requeue_str), 0444);
+
+static char g_init_hctx_str[80];
+module_param_string(init_hctx, g_init_hctx_str, sizeof(g_init_hctx_str), 0444);
#endif
static int g_queue_mode = NULL_Q_MQ;
@@ -1451,6 +1455,11 @@ static int null_init_hctx(struct blk_mq_hw_ctx *hctx, void *driver_data,
struct nullb *nullb = hctx->queue->queuedata;
struct nullb_queue *nq;
+#ifdef CONFIG_BLK_DEV_NULL_BLK_FAULT_INJECTION
+ if (g_init_hctx_str[0] && should_fail(&null_init_hctx_attr, 1))
+ return -EFAULT;
+#endif
+
nq = &nullb->queues[hctx_idx];
hctx->driver_data = nq;
null_init_queue(nullb, nq);
@@ -1685,6 +1694,8 @@ static bool null_setup_fault(void)
return false;
if (!__null_setup_fault(&null_requeue_attr, g_requeue_str))
return false;
+ if (!__null_setup_fault(&null_init_hctx_attr, g_init_hctx_str))
+ return false;
#endif
return true;
}
^ permalink raw reply related [flat|nested] 13+ messages in thread