CONFIG_DM_MQ_DEFAULT makes my kernel unhappy..

CONFIG_DM_MQ_DEFAULT makes my kernel unhappy..

[Christoph Hellwig]

Running dm-mpath against a tcm_loop target with two ALUA paths makes
the kernel very unhappy when CONFIG_DM_MQ_DEFAULT is set.  Without it
it's perfectly happy.

[   12.865522] ------------[ cut here ]------------
[   12.866119] WARNING: CPU: 0 PID: 3736 at ../lib/debugobjects.c:263 debug_print_object+0x8c/0xb0()
[   12.866679] ODEBUG: init active (active state 0) object type: timer_list hint: blk_mq_rq_timer+0x0/0x100
[   12.866679] Modules linked in:
[   12.866679] CPU: 0 PID: 3736 Comm: multipathd Not tainted 4.0.0+ #335
[   12.866679] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007
[   12.866679]  ffffffff822b7186 ffff88007887b9e8 ffffffff81dfff1f 0000000000000000
[   12.866679]  ffff88007887ba38 ffff88007887ba28 ffffffff810c4dc2 0000000000000000
[   12.866679]  ffff88007b554b88 ffffffff82427f80 ffffffff823606cd 00000000000316c0
[   12.866679] Call Trace:
[   12.866679]  [<ffffffff81dfff1f>] dump_stack+0x45/0x57
[   12.866679]  [<ffffffff810c4dc2>] warn_slowpath_common+0x92/0xd0
[   12.866679]  [<ffffffff810c4ea1>] warn_slowpath_fmt+0x41/0x50
[   12.866679]  [<ffffffff817dcaac>] debug_print_object+0x8c/0xb0
[   12.866679]  [<ffffffff817a02a0>] ? blk_mq_free_request+0x40/0x40
[   12.866679]  [<ffffffff817dcbdc>] ? __debug_object_init+0x5c/0x430
[   12.866679]  [<ffffffff817dcdd3>] __debug_object_init+0x253/0x430
[   12.866679]  [<ffffffff817dcfcb>] debug_object_init+0x1b/0x20
[   12.866679]  [<ffffffff81131654>] init_timer_key+0x34/0xa0
[   12.866679]  [<ffffffff817a2b88>] blk_mq_init_allocated_queue+0x1b8/0x8f0
[   12.866679]  [<ffffffff81c7b6ef>] dm_setup_md_queue+0x1bf/0x2f0
[   12.866679]  [<ffffffff81c81060>] table_load+0x1b0/0x340
[   12.866679]  [<ffffffff81c80eb0>] ? table_clear+0xd0/0xd0
[   12.866679]  [<ffffffff81c81e2a>] ctl_ioctl+0x25a/0x4f0
[   12.866679]  [<ffffffff8110bf00>] ? match_held_lock+0x160/0x1f0
[   12.866679]  [<ffffffff81c820ce>] dm_ctl_ioctl+0xe/0x20
[   12.866679]  [<ffffffff811f2dc3>] do_vfs_ioctl+0x83/0x5b0
[   12.866679]  [<ffffffff811fe761>] ? __fget+0xb1/0x1e0
[   12.866679]  [<ffffffff811fe6b0>] ? put_unused_fd+0x60/0x60
[   12.866679]  [<ffffffff811fe8d5>] ? __fget_light+0x25/0x90
[   12.866679]  [<ffffffff811f3337>] SyS_ioctl+0x47/0x90
[   12.866679]  [<ffffffff81e0a7ae>] system_call_fastpath+0x12/0x76
[   12.866679] ---[ end trace f30ef5dd5b6c1afd ]---
[   12.888707] kobject (ffff88007b898df0): tried to init an initialized object, something is seriously wrong.
[   12.889790] CPU: 0 PID: 3736 Comm: multipathd Tainted: G        W       4.0.0+ #335
[   12.890704] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007
[   12.891355]  ffffffff824b5980 ffff88007887bba8 ffffffff81dfff1f ffff88007b534f10
[   12.892542]  ffff88007b898df0 ffff88007887bbc8 ffffffff817bf957 ffff88007b898690
[   12.893805]  0000000000000000 ffff88007887bc08 ffffffff817a4e54 ffff88007887bc38
[   12.894911] Call Trace:
[   12.895279]  [<ffffffff81dfff1f>] dump_stack+0x45/0x57
[   12.895888]  [<ffffffff817bf957>] kobject_init+0x87/0xa0
[   12.896502]  [<ffffffff817a4e54>] blk_mq_register_disk+0x34/0x160
[   12.897360]  [<ffffffff81c7b716>] dm_setup_md_queue+0x1e6/0x2f0
[   12.898151]  [<ffffffff81c81060>] table_load+0x1b0/0x340
[   12.898834]  [<ffffffff81c80eb0>] ? table_clear+0xd0/0xd0
[   12.899454]  [<ffffffff81c81e2a>] ctl_ioctl+0x25a/0x4f0
[   12.900075]  [<ffffffff8110bf00>] ? match_held_lock+0x160/0x1f0
[   12.900737]  [<ffffffff81c820ce>] dm_ctl_ioctl+0xe/0x20
[   12.901346]  [<ffffffff811f2dc3>] do_vfs_ioctl+0x83/0x5b0
[   12.901964]  [<ffffffff811fe761>] ? __fget+0xb1/0x1e0
[   12.902553]  [<ffffffff811fe6b0>] ? put_unused_fd+0x60/0x60
[   12.903184]  [<ffffffff811fe8d5>] ? __fget_light+0x25/0x90
[   12.903818]  [<ffffffff811f3337>] SyS_ioctl+0x47/0x90
[   12.904409]  [<ffffffff81e0a7ae>] system_call_fastpath+0x12/0x76
[   12.905168] ------------[ cut here ]------------
[   12.905730] WARNING: CPU: 0 PID: 3736 at ../fs/sysfs/dir.c:31 sysfs_warn_dup+0x6a/0x80()
[   12.906676] sysfs: cannot create duplicate filename '/devices/virtual/block/dm-0/mq'
[   12.907588] Modules linked in:
[   12.908071] CPU: 0 PID: 3736 Comm: multipathd Tainted: G        W       4.0.0+ #335
[   12.908973] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007
[   12.909621]  ffffffff8229a8ae ffff88007887b9e8 ffffffff81dfff1f ffff88007b534f10
[   12.910771]  ffff88007887ba38 ffff88007887ba28 ffffffff810c4dc2 ffff88007b298000
[   12.911907]  ffff88007b298000 ffff88007abe5be0 ffff88007b29a2e0 ffff88007b5a58a0
[   12.913024] Call Trace:
[   12.913408]  [<ffffffff81dfff1f>] dump_stack+0x45/0x57
[   12.914006]  [<ffffffff810c4dc2>] warn_slowpath_common+0x92/0xd0
[   12.914675]  [<ffffffff810c4ea1>] warn_slowpath_fmt+0x41/0x50
[   12.915323]  [<ffffffff8125bb00>] ? kernfs_path+0x50/0x70
[   12.915940]  [<ffffffff8125f3aa>] sysfs_warn_dup+0x6a/0x80
[   12.916564]  [<ffffffff8125f446>] sysfs_create_dir_ns+0x86/0x90
[   12.917242]  [<ffffffff817c0125>] kobject_add_internal+0xa5/0x2e0
[   12.917920]  [<ffffffff817c0577>] kobject_add+0x67/0xc0
[   12.918522]  [<ffffffff817a4efa>] blk_mq_register_disk+0xda/0x160
[   12.919201]  [<ffffffff81c7b716>] dm_setup_md_queue+0x1e6/0x2f0
[   12.919864]  [<ffffffff81c81060>] table_load+0x1b0/0x340
[   12.920482]  [<ffffffff81c80eb0>] ? table_clear+0xd0/0xd0
[   12.921101]  [<ffffffff81c81e2a>] ctl_ioctl+0x25a/0x4f0
[   12.921704]  [<ffffffff8110bf00>] ? match_held_lock+0x160/0x1f0
[   12.922365]  [<ffffffff81c820ce>] dm_ctl_ioctl+0xe/0x20
[   12.922963]  [<ffffffff811f2dc3>] do_vfs_ioctl+0x83/0x5b0
[   12.923696]  [<ffffffff811fe761>] ? __fget+0xb1/0x1e0
[   12.924390]  [<ffffffff811fe6b0>] ? put_unused_fd+0x60/0x60
[   12.925204]  [<ffffffff811fe8d5>] ? __fget_light+0x25/0x90
[   12.925838]  [<ffffffff811f3337>] SyS_ioctl+0x47/0x90
[   12.926430]  [<ffffffff81e0a7ae>] system_call_fastpath+0x12/0x76
[   12.927120] ---[ end trace f30ef5dd5b6c1afe ]---
[   12.927704] ------------[ cut here ]------------
[   12.928349] WARNING: CPU: 0 PID: 3736 at ../lib/kobject.c:240 kobject_add_internal+0x274/0x2e0()
[   12.929581] kobject_add_internal failed for mq with -EEXIST, don't try to register things with the same name in the same directory.
[   12.930926] Modules linked in:
[   12.931415] CPU: 0 PID: 3736 Comm: multipathd Tainted: G        W       4.0.0+ #335
[   12.932315] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007
[   12.932962]  ffffffff822b6a52 ffff88007887ba48 ffffffff81dfff1f ffff88007b534f10
[   12.934104]  ffff88007887ba98 ffff88007887ba88 ffffffff810c4dc2 ffff88007887ba88
[   12.935194]  ffff88007b898df0 0000000000000000 00000000ffffffef ffff88007b5a58a0
[   12.936286] Call Trace:
[   12.936651]  [<ffffffff81dfff1f>] dump_stack+0x45/0x57
[   12.937268]  [<ffffffff810c4dc2>] warn_slowpath_common+0x92/0xd0
[   12.937942]  [<ffffffff810c4ea1>] warn_slowpath_fmt+0x41/0x50
[   12.938591]  [<ffffffff8125f446>] ? sysfs_create_dir_ns+0x86/0x90
[   12.939271]  [<ffffffff817c02f4>] kobject_add_internal+0x274/0x2e0
[   12.939961]  [<ffffffff817c0577>] kobject_add+0x67/0xc0
[   12.940574]  [<ffffffff817a4efa>] blk_mq_register_disk+0xda/0x160
[   12.941268]  [<ffffffff81c7b716>] dm_setup_md_queue+0x1e6/0x2f0
[   12.941932]  [<ffffffff81c81060>] table_load+0x1b0/0x340
[   12.942540]  [<ffffffff81c80eb0>] ? table_clear+0xd0/0xd0
[   12.943148]  [<ffffffff81c81e2a>] ctl_ioctl+0x25a/0x4f0
[   12.951270]  [<ffffffff8110bf00>] ? match_held_lock+0x160/0x1f0
[   12.951936]  [<ffffffff81c820ce>] dm_ctl_ioctl+0xe/0x20
[   12.952542]  [<ffffffff811f2dc3>] do_vfs_ioctl+0x83/0x5b0
[   12.953166]  [<ffffffff811fe761>] ? __fget+0xb1/0x1e0
[   12.953838]  [<ffffffff811fe6b0>] ? put_unused_fd+0x60/0x60
[   12.954759]  [<ffffffff811fe8d5>] ? __fget_light+0x25/0x90
[   12.955566]  [<ffffffff811f3337>] SyS_ioctl+0x47/0x90
[   12.956174]  [<ffffffff81e0a7ae>] system_call_fastpath+0x12/0x76
[   12.956875] ---[ end trace f30ef5dd5b6c1aff ]---

Re: CONFIG_DM_MQ_DEFAULT makes my kernel unhappy..

[Mike Snitzer]

On Sat, Apr 25 2015 at  5:23am -0400,
Christoph Hellwig <hch@infradead.org> wrote:

> Running dm-mpath against a tcm_loop target with two ALUA paths makes
> the kernel very unhappy when CONFIG_DM_MQ_DEFAULT is set.  Without it
> it's perfectly happy.

Wonder if commit 63a4f065ec ("dm: fix add_disk() NULL pointer due to
race with free_dev()") left something not quite right relative to
blk-mq?  Or maybe this is a problem confined to blk-mq itself.

What were you actually testing when this happened?  Were you failing
paths under IO load or something?

Also, which upstream commit was your tree based on?  Did you have any
non-upstream patches applied that I should be aware of?

Thanks,
Mike

Re: CONFIG_DM_MQ_DEFAULT makes my kernel unhappy..

[Christoph Hellwig]

On Sat, Apr 25, 2015 at 11:28:57AM -0400, Mike Snitzer wrote:
> On Sat, Apr 25 2015 at  5:23am -0400,
> Christoph Hellwig <hch@infradead.org> wrote:
> 
> > Running dm-mpath against a tcm_loop target with two ALUA paths makes
> > the kernel very unhappy when CONFIG_DM_MQ_DEFAULT is set.  Without it
> > it's perfectly happy.
> 
> Wonder if commit 63a4f065ec ("dm: fix add_disk() NULL pointer due to
> race with free_dev()") left something not quite right relative to
> blk-mq?  Or maybe this is a problem confined to blk-mq itself.

I'll give it a spin.

> What were you actually testing when this happened?  Were you failing
> paths under IO load or something?

This happened on the initial udev-direct scan of the devices.

> Also, which upstream commit was your tree based on?  Did you have any
> non-upstream patches applied that I should be aware of?

Latest Linus tree as of this morning.

Re: CONFIG_DM_MQ_DEFAULT makes my kernel unhappy..

[Christoph Hellwig]

On Sat, Apr 25, 2015 at 08:37:32AM -0700, Christoph Hellwig wrote:
> I'll give it a spin.
> 
> > What were you actually testing when this happened?  Were you failing
> > paths under IO load or something?
> 
> This happened on the initial udev-direct scan of the devices.
> 
> > Also, which upstream commit was your tree based on?  Did you have any
> > non-upstream patches applied that I should be aware of?
> 
> Latest Linus tree as of this morning.

Seems like the first warning goes away without CONFIG_DEBUG_OBJECTS.
Note that in either way I/O over dm-mpath actually works fine.

Btw, this is the script used to reproduce it, you need
CONFIG_TARGET_CORE, CONFIG_TCM_IBLOCK and CONFIG_LOOPBACK_TARGET
enabled.  It jsut creates two different targets with a lun each,
pointing to the same device.  The target code automatically supports
ALUA in this case.

---
#!/bin/bash

# add your own device
#DEV=/dev/sda

mount -t configfs none /sys/kernel/config/

tcm_node --block iblock_0/array /dev/sda

line=$(tcm_loop --createnexus=0)
wwn=$(echo $line | awk '{print $15}')
tcm_loop --addlun=$wwn 0 0 iblock_0/array

line=$(tcm_loop --createnexus=1)
wwn=$(echo $line | awk '{print $15}')
tcm_loop --addlun=$wwn 1 0 iblock_0/array

Re: CONFIG_DM_MQ_DEFAULT makes my kernel unhappy..

[Mike Snitzer]

On Sun, Apr 26 2015 at 10:16am -0400,
Christoph Hellwig <hch@infradead.org> wrote:

> On Sat, Apr 25, 2015 at 08:37:32AM -0700, Christoph Hellwig wrote:
> > I'll give it a spin.
> > 
> > > What were you actually testing when this happened?  Were you failing
> > > paths under IO load or something?
> > 
> > This happened on the initial udev-direct scan of the devices.
> > 
> > > Also, which upstream commit was your tree based on?  Did you have any
> > > non-upstream patches applied that I should be aware of?
> > 
> > Latest Linus tree as of this morning.
> 
> Seems like the first warning goes away without CONFIG_DEBUG_OBJECTS.
> Note that in either way I/O over dm-mpath actually works fine.
> 
> Btw, this is the script used to reproduce it, you need
> CONFIG_TARGET_CORE, CONFIG_TCM_IBLOCK and CONFIG_LOOPBACK_TARGET
> enabled.  It jsut creates two different targets with a lun each,
> pointing to the same device.  The target code automatically supports
> ALUA in this case.
> 
> ---
> #!/bin/bash
> 
> # add your own device
> #DEV=/dev/sda
> 
> mount -t configfs none /sys/kernel/config/
> 
> tcm_node --block iblock_0/array /dev/sda
> 
> line=$(tcm_loop --createnexus=0)
> wwn=$(echo $line | awk '{print $15}')
> tcm_loop --addlun=$wwn 0 0 iblock_0/array
> 
> line=$(tcm_loop --createnexus=1)
> wwn=$(echo $line | awk '{print $15}')
> tcm_loop --addlun=$wwn 1 0 iblock_0/array
> 

So you're still using lio-utils?  Seems "lio-utils are deprecated and
have been superseded by targetcli".. what am I missing?

What lio-utils repo should I be using to get the utils that'll work with
your script?

Re: CONFIG_DM_MQ_DEFAULT makes my kernel unhappy..

[Christoph Hellwig]

On Mon, Apr 27, 2015 at 11:59:15AM -0400, Mike Snitzer wrote:
> So you're still using lio-utils?  Seems "lio-utils are deprecated and
> have been superseded by targetcli".. what am I missing?

That targetcli is a complete fucking mess, and due to it's stateful
nature entirely useless for reproducer scripts or volatile test setups.

> What lio-utils repo should I be using to get the utils that'll work with
> your script?

I'm using the stock Debian wheezy packages. But AFAIK lio-utils never
changed much, so any version should be fine.

You can also try to recreate this situation using targetcli, I'm pretty
sure the result will be the same.

Re: CONFIG_DM_MQ_DEFAULT makes my kernel unhappy..

[Mike Snitzer]

On Sun, Apr 26 2015 at 10:16am -0400,
Christoph Hellwig <hch@infradead.org> wrote:

> On Sat, Apr 25, 2015 at 08:37:32AM -0700, Christoph Hellwig wrote:
> > I'll give it a spin.
> > 
> > > What were you actually testing when this happened?  Were you failing
> > > paths under IO load or something?
> > 
> > This happened on the initial udev-direct scan of the devices.
> > 
> > > Also, which upstream commit was your tree based on?  Did you have any
> > > non-upstream patches applied that I should be aware of?
> > 
> > Latest Linus tree as of this morning.
> 
> Seems like the first warning goes away without CONFIG_DEBUG_OBJECTS.
> Note that in either way I/O over dm-mpath actually works fine.

I'm not seeing any warnings with use_blk_mq=Y but I don't have
CONFIG_DEBUG_OBJECTS set.  I expected to still see the later errors you
saw though.  Unfortunately (or fortunately? ;) I'm not... with 4.1-rc1
on a pretty fast bare-metal system.

Since you can easily reproduce you might need to chase down the problem.

BTW, I'm liking LIO and these lio-utils for quick test setups.. much
more useful than scsi_debug!

For now I'll move on to reviewing/testing your v2 patch.

[PATCH] dm: fix blk-mq request-based DM queue initialization

[Mike Snitzer]

Commit bfebd1cdb4 ("dm: add full blk-mq support to request-based DM")
didn't properly account for the need to short-circuit re-initializing
DM's blk-mq request_queue if it was already initialized.

Fix dm_init_request_based_blk_mq_queue() to return early if the
mapped_device's tag_set already has its driver_data set to the
mapped_device pointer.

Otherwise, reloading a blk-mq request-based DM table (either manually or
via multipathd) resulted in errors like the following:

WARNING: CPU: 9 PID: 2836 at lib/list_debug.c:36 __list_add+0x92/0xc0()
...
kobject (ffff880035625fd0): tried to init an initialized object, something is seriously wrong.
...
sysfs: cannot create duplicate filename '/devices/virtual/block/dm-13/mq'
...
kobject_add_internal failed for mq with -EEXIST, don't try to register things with the same name in the same directory.

Reported-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
---
 drivers/md/dm.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 1567f6c..c834a94 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -2704,6 +2704,9 @@ static int dm_init_request_based_blk_mq_queue(struct mapped_device *md)
 	struct request_queue *q;
 	int err;
 
+	if (md->tag_set.driver_data == md)
+		return 0;
+
 	memset(&md->tag_set, 0, sizeof(md->tag_set));
 	md->tag_set.ops = &dm_mq_ops;
 	md->tag_set.queue_depth = BLKDEV_MAX_RQ;
@@ -2719,7 +2722,7 @@ static int dm_init_request_based_blk_mq_queue(struct mapped_device *md)
 
 	err = blk_mq_alloc_tag_set(&md->tag_set);
 	if (err)
-		return err;
+		goto out;
 
 	q = blk_mq_init_allocated_queue(&md->tag_set, md->queue);
 	if (IS_ERR(q)) {
@@ -2739,6 +2742,8 @@ static int dm_init_request_based_blk_mq_queue(struct mapped_device *md)
 
 out_tag_set:
 	blk_mq_free_tag_set(&md->tag_set);
+out:
+	memset(&md->tag_set, 0, sizeof(md->tag_set));
 	return err;
 }
 
-- 
2.3.2 (Apple Git-55)

Re: [PATCH] dm: fix blk-mq request-based DM queue initialization

[Christoph Hellwig]

On Tue, Apr 28, 2015 at 08:59:13PM -0400, Mike Snitzer wrote:
> Commit bfebd1cdb4 ("dm: add full blk-mq support to request-based DM")
> didn't properly account for the need to short-circuit re-initializing
> DM's blk-mq request_queue if it was already initialized.
> 
> Fix dm_init_request_based_blk_mq_queue() to return early if the
> mapped_device's tag_set already has its driver_data set to the
> mapped_device pointer.

I'm not quite up to speed with this area of code, but shouldn't
we always tear the queue fully down before trying to reinitialize it?

Re: dm: fix blk-mq request-based DM queue initialization

[Mike Snitzer]

On Wed, Apr 29 2015 at  9:00am -0400,
Christoph Hellwig <hch@lst.de> wrote:

> On Tue, Apr 28, 2015 at 08:59:13PM -0400, Mike Snitzer wrote:
> > Commit bfebd1cdb4 ("dm: add full blk-mq support to request-based DM")
> > didn't properly account for the need to short-circuit re-initializing
> > DM's blk-mq request_queue if it was already initialized.
> > 
> > Fix dm_init_request_based_blk_mq_queue() to return early if the
> > mapped_device's tag_set already has its driver_data set to the
> > mapped_device pointer.
> 
> I'm not quite up to speed with this area of code, but shouldn't
> we always tear the queue fully down before trying to reinitialize it?

No, the same request_queue should be used for the lifetime of a DM
device.

I'm not interested in reinitializing the queue.  A DM table reload has
no need to destroy and recreate the request_queue associated with 
the DM device.  It would be bad to do so too because we actually use the
queue across table reloads in the case of dm-multipath with no paths
(see: map_request's handling of DM_MAPIO_REQUEUE which triggers
dm_requeue_request).

Re: dm: fix blk-mq request-based DM queue initialization

[Christoph Hellwig]

On Wed, Apr 29, 2015 at 09:41:52PM -0400, Mike Snitzer wrote:
> > I'm not quite up to speed with this area of code, but shouldn't
> > we always tear the queue fully down before trying to reinitialize it?
> 
> No, the same request_queue should be used for the lifetime of a DM
> device.
> 
> I'm not interested in reinitializing the queue.  A DM table reload has
> no need to destroy and recreate the request_queue associated with 
> the DM device.  It would be bad to do so too because we actually use the
> queue across table reloads in the case of dm-multipath with no paths
> (see: map_request's handling of DM_MAPIO_REQUEUE which triggers
> dm_requeue_request).

Well - we're obviously trying to reinitialize it here and only error
out very low level with your patch.  What I mean is that we shouldn't
even try to reinitialize it at a much higher level, so we don't need
this blk-mq specific hack down here.

Re: dm: fix blk-mq request-based DM queue initialization

[Christoph Hellwig]

On Thu, Apr 30, 2015 at 09:22:00AM +0200, Christoph Hellwig wrote:
> Well - we're obviously trying to reinitialize it here and only error
> out very low level with your patch.  What I mean is that we shouldn't
> even try to reinitialize it at a much higher level, so we don't need
> this blk-mq specific hack down here.

FYI, this fixes the isssue for me and seems to be "more correct" at
a higher level:

---
From: Christoph Hellwig <hch@lst.de>
Subject: [PATCH] dm: only initialize request_queue once

We should only inіnitialize the request_queue on the initial table load when
we assing the device type.

Signed-off-by: Christoph Hellwig <hch@lst.de>

diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index c8a18e4..de92662 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -1298,21 +1298,22 @@ static int table_load(struct dm_ioctl *param, size_t param_size)
 		goto err_unlock_md_type;
 	}
 
-	if (dm_get_md_type(md) == DM_TYPE_NONE)
+	if (dm_get_md_type(md) == DM_TYPE_NONE) {
 		/* Initial table load: acquire type of table. */
 		dm_set_md_type(md, dm_table_get_type(t));
-	else if (dm_get_md_type(md) != dm_table_get_type(t)) {
+	
+		/* setup md->queue to reflect md's type (may block) */
+		r = dm_setup_md_queue(md);
+		if (r) {
+			DMWARN("unable to set up device queue for new table.");
+			goto err_unlock_md_type;
+		}
+	} else if (dm_get_md_type(md) != dm_table_get_type(t)) {
 		DMWARN("can't change device type after initial table load.");
 		r = -EINVAL;
 		goto err_unlock_md_type;
 	}
 
-	/* setup md->queue to reflect md's type (may block) */
-	r = dm_setup_md_queue(md);
-	if (r) {
-		DMWARN("unable to set up device queue for new table.");
-		goto err_unlock_md_type;
-	}
 	dm_unlock_md_type(md);
 
 	/* stage inactive table */

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: dm: fix blk-mq request-based DM queue initialization

[Mike Snitzer]

On Thu, Apr 30 2015 at  4:23am -0400,
Christoph Hellwig <hch@lst.de> wrote:

> On Thu, Apr 30, 2015 at 09:22:00AM +0200, Christoph Hellwig wrote:
> > Well - we're obviously trying to reinitialize it here and only error
> > out very low level with your patch.  What I mean is that we shouldn't
> > even try to reinitialize it at a much higher level, so we don't need
> > this blk-mq specific hack down here.

OK, understood.

> FYI, this fixes the isssue for me and seems to be "more correct" at
> a higher level:

Yeah, I like this.  Means the early return in
dm_init_request_based_queue() can be removed too.

> ---
> From: Christoph Hellwig <hch@lst.de>
> Subject: [PATCH] dm: only initialize request_queue once
> 
> We should only inіnitialize the request_queue on the initial table load when
> we assing the device type.

"assing" ;)  Will fix up and also add that this patch fixes the issue
you reported, thanks.

> Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
> index c8a18e4..de92662 100644
> --- a/drivers/md/dm-ioctl.c
> +++ b/drivers/md/dm-ioctl.c
> @@ -1298,21 +1298,22 @@ static int table_load(struct dm_ioctl *param, size_t param_size)
>  		goto err_unlock_md_type;
>  	}
>  
> -	if (dm_get_md_type(md) == DM_TYPE_NONE)
> +	if (dm_get_md_type(md) == DM_TYPE_NONE) {
>  		/* Initial table load: acquire type of table. */
>  		dm_set_md_type(md, dm_table_get_type(t));
> -	else if (dm_get_md_type(md) != dm_table_get_type(t)) {
> +	
> +		/* setup md->queue to reflect md's type (may block) */
> +		r = dm_setup_md_queue(md);
> +		if (r) {
> +			DMWARN("unable to set up device queue for new table.");
> +			goto err_unlock_md_type;
> +		}
> +	} else if (dm_get_md_type(md) != dm_table_get_type(t)) {
>  		DMWARN("can't change device type after initial table load.");
>  		r = -EINVAL;
>  		goto err_unlock_md_type;
>  	}
>  
> -	/* setup md->queue to reflect md's type (may block) */
> -	r = dm_setup_md_queue(md);
> -	if (r) {
> -		DMWARN("unable to set up device queue for new table.");
> -		goto err_unlock_md_type;
> -	}
>  	dm_unlock_md_type(md);
>  
>  	/* stage inactive table */

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel