* [PATCH] dm: Refuse to load an sq table on an mq device
@ 2016-12-08 0:56 Bart Van Assche
2016-12-08 2:11 ` Mike Snitzer
0 siblings, 1 reply; 6+ messages in thread
From: Bart Van Assche @ 2016-12-08 0:56 UTC (permalink / raw)
To: Mike Snitzer; +Cc: device-mapper development
This patch avoids that the following call trace can be triggered:
------------[ cut here ]------------
WARNING: CPU: 10 PID: 19102 at drivers/md/dm-mpath.c:543 __multipath_map.isra.17+0x23a/0x370 [dm_multipath]
CPU: 10 PID: 19102 Comm: mkfs.ext4 Not tainted 4.9.0-rc8-dbg+ #4
Call Trace:
[<ffffffff81329cd5>] dump_stack+0x68/0x93
[<ffffffff810650e6>] __warn+0xc6/0xe0
[<ffffffff810651b8>] warn_slowpath_null+0x18/0x20
[<ffffffffa004603a>] __multipath_map.isra.17+0x23a/0x370 [dm_multipath]
[<ffffffffa0046188>] multipath_clone_and_map+0x18/0x20 [dm_multipath]
[<ffffffffa002a54d>] map_request+0xed/0x300 [dm_mod]
[<ffffffffa002a807>] dm_mq_queue_rq+0x77/0x110 [dm_mod]
[<ffffffff81310fbf>] blk_mq_process_rq_list+0x23f/0x340
[<ffffffff813111e2>] __blk_mq_run_hw_queue+0x122/0x1c0
[<ffffffff81310d5f>] blk_mq_run_hw_queue+0x9f/0xc0
[<ffffffff81311ef5>] blk_mq_insert_requests+0x245/0x320
[<ffffffff813133f5>] blk_mq_flush_plug_list+0x125/0x140
[<ffffffff81306b12>] blk_flush_plug_list+0xa2/0x210
[<ffffffff81307157>] blk_finish_plug+0x27/0x40
[<ffffffff8116c119>] __do_page_cache_readahead+0x279/0x2f0
[<ffffffff8116c728>] force_page_cache_readahead+0xa8/0x100
[<ffffffff8116c7b9>] page_cache_sync_readahead+0x39/0x40
[<ffffffff8115c964>] generic_file_read_iter+0x234/0x780
[<ffffffff8121f8e0>] blkdev_read_iter+0x30/0x40
[<ffffffff811dd2db>] __vfs_read+0xbb/0x130
[<ffffffff811de341>] vfs_read+0x91/0x130
[<ffffffff811df6e4>] SyS_read+0x44/0xa0
[<ffffffff816401aa>] entry_SYSCALL_64_fastpath+0x18/0xad
---[ end trace c5c6591c32eae6dd ]---
after having inserted the following code in __multipath_map():
WARN_ON_ONCE(clone && q->mq_ops);
WARN_ON_ONCE(!clone && !q->mq_ops);
Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: <stable@vger.kernel.org>
---
drivers/md/dm-ioctl.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index c72a77048b73..918e8b363015 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -1322,6 +1322,11 @@ static int table_load(struct dm_ioctl *param, size_t param_size)
DMWARN("can't change device type after initial table load.");
r = -EINVAL;
goto err_unlock_md_type;
+ } else if (dm_get_md_type(md) == DM_TYPE_MQ_REQUEST_BASED &&
+ !dm_table_all_blk_mq_devices(t)) {
+ DMWARN("can't load a sq table on a blk-mq dm device.");
+ r = -EINVAL;
+ goto err_unlock_md_type;
}
dm_unlock_md_type(md);
--
2.11.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: dm: Refuse to load an sq table on an mq device
2016-12-08 0:56 [PATCH] dm: Refuse to load an sq table on an mq device Bart Van Assche
@ 2016-12-08 2:11 ` Mike Snitzer
2016-12-08 16:52 ` Bart Van Assche
0 siblings, 1 reply; 6+ messages in thread
From: Mike Snitzer @ 2016-12-08 2:11 UTC (permalink / raw)
To: Bart Van Assche; +Cc: device-mapper development
On Wed, Dec 07 2016 at 7:56P -0500,
Bart Van Assche <bart.vanassche@sandisk.com> wrote:
> This patch avoids that the following call trace can be triggered:
>
> ------------[ cut here ]------------
> WARNING: CPU: 10 PID: 19102 at drivers/md/dm-mpath.c:543 __multipath_map.isra.17+0x23a/0x370 [dm_multipath]
> CPU: 10 PID: 19102 Comm: mkfs.ext4 Not tainted 4.9.0-rc8-dbg+ #4
> Call Trace:
> [<ffffffff81329cd5>] dump_stack+0x68/0x93
> [<ffffffff810650e6>] __warn+0xc6/0xe0
> [<ffffffff810651b8>] warn_slowpath_null+0x18/0x20
> [<ffffffffa004603a>] __multipath_map.isra.17+0x23a/0x370 [dm_multipath]
> [<ffffffffa0046188>] multipath_clone_and_map+0x18/0x20 [dm_multipath]
> [<ffffffffa002a54d>] map_request+0xed/0x300 [dm_mod]
> [<ffffffffa002a807>] dm_mq_queue_rq+0x77/0x110 [dm_mod]
> [<ffffffff81310fbf>] blk_mq_process_rq_list+0x23f/0x340
> [<ffffffff813111e2>] __blk_mq_run_hw_queue+0x122/0x1c0
> [<ffffffff81310d5f>] blk_mq_run_hw_queue+0x9f/0xc0
> [<ffffffff81311ef5>] blk_mq_insert_requests+0x245/0x320
> [<ffffffff813133f5>] blk_mq_flush_plug_list+0x125/0x140
> [<ffffffff81306b12>] blk_flush_plug_list+0xa2/0x210
> [<ffffffff81307157>] blk_finish_plug+0x27/0x40
> [<ffffffff8116c119>] __do_page_cache_readahead+0x279/0x2f0
> [<ffffffff8116c728>] force_page_cache_readahead+0xa8/0x100
> [<ffffffff8116c7b9>] page_cache_sync_readahead+0x39/0x40
> [<ffffffff8115c964>] generic_file_read_iter+0x234/0x780
> [<ffffffff8121f8e0>] blkdev_read_iter+0x30/0x40
> [<ffffffff811dd2db>] __vfs_read+0xbb/0x130
> [<ffffffff811de341>] vfs_read+0x91/0x130
> [<ffffffff811df6e4>] SyS_read+0x44/0xa0
> [<ffffffff816401aa>] entry_SYSCALL_64_fastpath+0x18/0xad
> ---[ end trace c5c6591c32eae6dd ]---
>
> after having inserted the following code in __multipath_map():
>
> WARN_ON_ONCE(clone && q->mq_ops);
> WARN_ON_ONCE(!clone && !q->mq_ops);
>
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Cc: <stable@vger.kernel.org>
> ---
> drivers/md/dm-ioctl.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
> index c72a77048b73..918e8b363015 100644
> --- a/drivers/md/dm-ioctl.c
> +++ b/drivers/md/dm-ioctl.c
> @@ -1322,6 +1322,11 @@ static int table_load(struct dm_ioctl *param, size_t param_size)
> DMWARN("can't change device type after initial table load.");
> r = -EINVAL;
> goto err_unlock_md_type;
> + } else if (dm_get_md_type(md) == DM_TYPE_MQ_REQUEST_BASED &&
> + !dm_table_all_blk_mq_devices(t)) {
> + DMWARN("can't load a sq table on a blk-mq dm device.");
> + r = -EINVAL;
> + goto err_unlock_md_type;
> }
>
> dm_unlock_md_type(md);
Hi Bart,
Thanks for catching this lack of table type verification.
I'm not a fan of a patch header including a stack trace for debugging
was temporarily introduced loclaly for debugging purposes; especially
not for a patch that will cc stable. As such I'll be re-writing the
patch header.
Also, I prefer the following variant of your fix (results in failing the
table load a bit sooner and doesn't allow a table->type inconsistency to
escape dm_table_determine_type()).
Should I still attribute authorship of this change to you? Or would you
prefer I switch to you being the Reporter (via Reported-by)?
diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 8ce81d0..0a427de 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -976,6 +976,11 @@ static int dm_table_determine_type(struct dm_table *t)
}
t->all_blk_mq = mq_count > 0;
+ if (t->type == DM_TYPE_MQ_REQUEST_BASED && !t->all_blk_mq) {
+ DMERR("table load rejected: all devices are not blk-mq request-stackable");
+ return -EINVAL;
+ }
+
return 0;
}
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-12-09 21:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-08 0:56 [PATCH] dm: Refuse to load an sq table on an mq device Bart Van Assche
2016-12-08 2:11 ` Mike Snitzer
2016-12-08 16:52 ` Bart Van Assche
2016-12-08 17:18 ` Mike Snitzer
2016-12-08 19:54 ` Mike Snitzer
2016-12-09 21:01 ` Bart Van Assche
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.