All of lore.kernel.org
 help / color / mirror / Atom feed
From: Milan Broz <mbroz@redhat.com>
To: Tejun Heo <tj@kernel.org>
Cc: Jens Axboe <axboe@kernel.dk>,
	Jun'ichi Nomura <j-nomura@ce.jp.nec.com>,
	Valdis.Kletnieks@vt.edu, Alexander Viro <viro@zeniv.linux.org.uk>,
	Neil Brown <neilb@suse.de>,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-raid@vger.kernel.org,
	device-mapper development <dm-devel@redhat.com>,
	Kay Sievers <kay.sievers@vrfy.org>,
	Alasdair G Kergon <agk@redhat.com>
Subject: Re: [PATCH] block: restore multiple bd_link_disk_holder() support
Date: Thu, 13 Jan 2011 19:42:33 +0100	[thread overview]
Message-ID: <4D2F4799.5030901@redhat.com> (raw)
In-Reply-To: <20110113172133.GE14096@htj.dyndns.org>

On 01/13/2011 06:21 PM, Tejun Heo wrote:
> Commit e09b457b (block: simplify holder symlink handling) incorrectly
> assumed that there is only one holder at maximum.  dm may use multiple
> holders.  Remove the single holder assumption and automatic removal of
> the link.  Let the callers explicitly remove them.  This change makes
> it even more alien from the rest of the block layer.
> 
> While at it, note that this facility should not be used by anyone else
> than the current ones.  Sysfs symlinks shouldn't be abused like this
> and the whole thing doesn't belong in the block layer at all.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Reported-by: Milan Broz <mbroz@redhat.com>
> Cc: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>
> Cc: Neil Brown <neilb@suse.de>
> Cc: linux-raid@vger.kernel.org
> Cc: Kay Sievers <kay.sievers@vrfy.org>
> ---
> Milan, Jun, can you guys please verify this works correctly for the
> multi holder dm case?  Thank you.

Hi,

unfortunately not. And the problem is much worse,
it breaks lvm resize operation completely.

I cherry-picked you patch to my tree, it fails, reverting helps.

Following test is over linux-next with your last patch applied:

Test case (lvresize of volume over 3 disks sdb,c,d):

# pvcreate /dev/sd[bcd]
  Physical volume "/dev/sdb" successfully created
  Physical volume "/dev/sdc" successfully created
  Physical volume "/dev/sdd" successfully created

# vgcreate vg_test /dev/sd[bcd]
  Volume group "vg_test" successfully created

# lvcreate -l100%FREE -n lv vg_test
  Logical volume "lv" created

# dmsetup table
vg_test-lv: 0 409600 linear 8:16 2048
vg_test-lv: 409600 409600 linear 8:32 2048
vg_test-lv: 819200 409600 linear 8:48 2048

so we have active LV mapped to 3 devices now.

Now online resize it. I means that it will create inactive table,
then switch active (so the magic with claiming disks applies):

# lvresize -L-10M vg_test/lv
  Rounding up size to full physical extent 8.00 MiB
  WARNING: Reducing active logical volume to 592.00 MiB
  THIS MAY DESTROY YOUR DATA (filesystem etc.)
Do you really want to reduce lv? [y/n]: y
  Reducing logical volume lv to 592.00 MiB
  device-mapper: reload ioctl failed: Invalid argument
  Failed to suspend lv

and in syslog:
[  291.221081] ------------[ cut here ]------------
[  291.221111] WARNING: at fs/sysfs/dir.c:455 sysfs_add_one+0x6b/0x80()
[  291.221130] Hardware name: VMware Virtual Platform
[  291.221140] sysfs: cannot create duplicate filename '/devices/virtual/block/dm-0/slaves/sdb'
[  291.221162] Modules linked in: usbcore dm_mod
[  291.221265] Pid: 4074, comm: lvresize Tainted: G        W   2.6.37-next-20110113+ #2
[  291.221269] Call Trace:
[  291.221286]  [<c102e58f>] ? warn_slowpath_common+0x65/0x7a
[  291.221290]  [<c10f8911>] ? sysfs_add_one+0x6b/0x80
[  291.221295]  [<c102e608>] ? warn_slowpath_fmt+0x26/0x2a
[  291.221299]  [<c10f8911>] ? sysfs_add_one+0x6b/0x80
[  291.221304]  [<c10f9482>] ? sysfs_do_create_link+0xda/0x164
[  291.221308]  [<c10f9522>] ? sysfs_create_link+0xa/0xc
[  291.221314]  [<c10db85c>] ? bd_link_disk_holder+0x66/0xad
[  291.221484]  [<d08289ea>] ? open_dev+0x47/0x67 [dm_mod]
[  291.221492]  [<d0828aff>] ? dm_get_device+0xf5/0x1d5 [dm_mod]
[  291.221502]  [<c11bf168>] ? vsscanf+0x364/0x3ee
[  291.221510]  [<c10b250a>] ? cache_alloc_debugcheck_after+0xf/0x180
[  291.221523]  [<d0829812>] ? linear_ctr+0x86/0xc0 [dm_mod]
[  291.221531]  [<d0829287>] ? dm_table_add_target+0x153/0x1d3 [dm_mod]
[  291.221538]  [<d082adea>] ? table_load+0x1fd/0x21e [dm_mod]
[  291.221545]  [<d082b9f8>] ? dm_ctl_ioctl+0x188/0x1c8 [dm_mod]
[  291.221551]  [<d082abed>] ? table_load+0x0/0x21e [dm_mod]
[  291.221558]  [<d082b870>] ? dm_ctl_ioctl+0x0/0x1c8 [dm_mod]
[  291.221565]  [<c10c38ab>] ? do_vfs_ioctl+0x493/0x4d8
[  291.221573]  [<c1313511>] ? do_page_fault+0x3ee/0x418
[  291.221578]  [<c10c391e>] ? sys_ioctl+0x2e/0x48
[  291.221583]  [<c1002853>] ? sysenter_do_call+0x12/0x32
[  291.221609] ---[ end trace c21a5bad605a4a87 ]---
[  291.221760] device-mapper: table: 254:0: linear: dm-linear: Device lookup failed
[  291.221782] device-mapper: ioctl: error adding target to table

Milan

  reply	other threads:[~2011-01-13 18:42 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-01-12 17:34 linux-next - WARNING: at fs/block_dev.c:824 bd_link_disk_holder+0x92/0x1ac() Valdis.Kletnieks
2011-01-13  0:23 ` Milan Broz
2011-01-13  2:19   ` Jun'ichi Nomura
2011-01-13 11:06     ` Tejun Heo
2011-01-13 11:26       ` [dm-devel] " Milan Broz
2011-01-13 12:27         ` Karel Zak
2011-01-13 13:12           ` Tejun Heo
2011-01-13 13:26             ` Karel Zak
2011-01-13 13:37               ` Tejun Heo
2011-01-13 13:52                 ` Tejun Heo
2011-01-13 13:58                 ` Milan Broz
2011-01-13 14:11                   ` Tejun Heo
2011-01-13 14:25                     ` Milan Broz
2011-01-13 14:30                       ` Tejun Heo
2011-01-13 14:43                         ` Kay Sievers
2011-01-13 15:03                           ` Milan Broz
2011-01-14  7:38                             ` Jun'ichi Nomura
2011-01-13 15:59                           ` Karel Zak
2011-01-13 15:59                             ` [dm-devel] " Karel Zak
2011-01-13 16:10                             ` Kay Sievers
2011-01-14 15:07                               ` Karel Zak
2011-01-14 15:07                                 ` Karel Zak
2011-01-14 15:23                                 ` Kay Sievers
2011-01-14 15:23                                   ` Kay Sievers
2011-01-13 14:45                         ` Theodore Tso
2011-01-13 20:18                           ` NeilBrown
2011-01-13 20:41                             ` Ted Ts'o
2011-01-14 16:20                               ` Tejun Heo
2011-01-14 16:20                                 ` Tejun Heo
2011-01-14 17:59                                 ` Ted Ts'o
2011-01-14 18:23                                   ` Tejun Heo
2011-01-14 18:23                                   ` Tejun Heo
2011-01-14 18:23                                   ` Tejun Heo
2011-01-14 18:23                                   ` [dm-devel] " Tejun Heo
2011-01-14 16:20                               ` Tejun Heo
2011-01-13 14:49                         ` Milan Broz
2011-01-14 16:35                           ` Tejun Heo
2011-01-13 17:21     ` [PATCH] block: restore multiple bd_link_disk_holder() support Tejun Heo
2011-01-13 18:42       ` Milan Broz [this message]
2011-01-14  7:31         ` Jun'ichi Nomura
2011-01-14 16:10           ` [PATCH UPDATED] " Tejun Heo
2011-01-14 16:10             ` Tejun Heo
2011-01-14 21:09             ` Milan Broz
2011-01-17  0:18               ` Jun'ichi Nomura

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4D2F4799.5030901@redhat.com \
    --to=mbroz@redhat.com \
    --cc=Valdis.Kletnieks@vt.edu \
    --cc=agk@redhat.com \
    --cc=axboe@kernel.dk \
    --cc=dm-devel@redhat.com \
    --cc=j-nomura@ce.jp.nec.com \
    --cc=kay.sievers@vrfy.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-raid@vger.kernel.org \
    --cc=neilb@suse.de \
    --cc=tj@kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.