From: Alasdair G Kergon <agk@redhat.com> To: Christoph Hellwig <hch@lst.de> Cc: Jens Axboe <axboe@kernel.dk>, Mike Snitzer <snitzer@redhat.com>, linux-block@vger.kernel.org, dm-devel@redhat.com, prajnoha@redhat.com Subject: Re: [dm-devel] [PATCH 7/8] dm: delay registering the gendisk Date: Tue, 10 Aug 2021 00:31:43 +0100 [thread overview] Message-ID: <20210809233143.GA101480@agk-cloud1.hosts.prod.upshift.rdu2.redhat.com> (raw) In-Reply-To: <20210804094147.459763-8-hch@lst.de> On Wed, Aug 04, 2021 at 11:41:46AM +0200, Christoph Hellwig wrote: > device mapper is currently the only outlier that tries to call > register_disk after add_disk, leading to fairly inconsistent state > of these block layer data structures. > Note that this introduces a user visible change: the dm kobject is > now only visible after the initial table has been loaded. Indeed. We should try to document the userspace implications of this change in a bit more detail. While lvm2 and any other tools that followed our recommendations about how to use dm should be OK, there's always the chance that some other less robustly-written code will need to make adjustments. Currently to make a dm device, 3 ioctls are called in sequence: 1. DM_DEV_CREATE - triggers 'add' uevents 2. DM_TABLE_LOAD 3. DM_SUSPEND - triggers 'change' uevent After this patch we have: 1. DM_DEV_CREATE 2. DM_TABLE_LOAD - triggers 'add' uevents 3. DM_SUSPEND - triggers 'change' uevent The equivalent dmsetup commands for a simple test device are 0. udevadm monitor --kernel --env & # View the uevents as they happen 1. dmsetup create dev1 --notable 2. dmsetup load --table "0 1 error" dev1 3. dmsetup resume dev1 => Anyone with a udev rule that relies on 'add' needs to check if they need to change their code. The udev rules that lvm2 uses to synchronise what it is doing rely only on the 'change' event - which is not moving. The 'add' event gets ignored. When loading tables, our tools also always refer to devices using the 'major:minor' format, which isn't affected, rather than using pathnames in /dev which might not exist now after this change if a table hasn't been loaded into a referenced device yet. Previously this was permissible but we always recommended against it to avoid a pointless pathname lookup that's subject to races and delays. So again, any tools that followed our recommendations ought to be unaffected. Here's an example of poor code that previously worked but will fail now: dmsetup create dev1 --notable dmsetup create dev2 --notable dmsetup ls <-- get the minor number of dev1 (say it's 1 corresponding to dm-1) dmsetup load dev2 --table '0 1 linear /dev/dm-1 0' ... Peter - have I missed anything? Alasdair
WARNING: multiple messages have this Message-ID (diff)
From: Alasdair G Kergon <agk@redhat.com> To: Christoph Hellwig <hch@lst.de> Cc: Jens Axboe <axboe@kernel.dk>, linux-block@vger.kernel.org, dm-devel@redhat.com, prajnoha@redhat.com, Mike Snitzer <snitzer@redhat.com> Subject: Re: [dm-devel] [PATCH 7/8] dm: delay registering the gendisk Date: Tue, 10 Aug 2021 00:31:43 +0100 [thread overview] Message-ID: <20210809233143.GA101480@agk-cloud1.hosts.prod.upshift.rdu2.redhat.com> (raw) In-Reply-To: <20210804094147.459763-8-hch@lst.de> On Wed, Aug 04, 2021 at 11:41:46AM +0200, Christoph Hellwig wrote: > device mapper is currently the only outlier that tries to call > register_disk after add_disk, leading to fairly inconsistent state > of these block layer data structures. > Note that this introduces a user visible change: the dm kobject is > now only visible after the initial table has been loaded. Indeed. We should try to document the userspace implications of this change in a bit more detail. While lvm2 and any other tools that followed our recommendations about how to use dm should be OK, there's always the chance that some other less robustly-written code will need to make adjustments. Currently to make a dm device, 3 ioctls are called in sequence: 1. DM_DEV_CREATE - triggers 'add' uevents 2. DM_TABLE_LOAD 3. DM_SUSPEND - triggers 'change' uevent After this patch we have: 1. DM_DEV_CREATE 2. DM_TABLE_LOAD - triggers 'add' uevents 3. DM_SUSPEND - triggers 'change' uevent The equivalent dmsetup commands for a simple test device are 0. udevadm monitor --kernel --env & # View the uevents as they happen 1. dmsetup create dev1 --notable 2. dmsetup load --table "0 1 error" dev1 3. dmsetup resume dev1 => Anyone with a udev rule that relies on 'add' needs to check if they need to change their code. The udev rules that lvm2 uses to synchronise what it is doing rely only on the 'change' event - which is not moving. The 'add' event gets ignored. When loading tables, our tools also always refer to devices using the 'major:minor' format, which isn't affected, rather than using pathnames in /dev which might not exist now after this change if a table hasn't been loaded into a referenced device yet. Previously this was permissible but we always recommended against it to avoid a pointless pathname lookup that's subject to races and delays. So again, any tools that followed our recommendations ought to be unaffected. Here's an example of poor code that previously worked but will fail now: dmsetup create dev1 --notable dmsetup create dev2 --notable dmsetup ls <-- get the minor number of dev1 (say it's 1 corresponding to dm-1) dmsetup load dev2 --table '0 1 linear /dev/dm-1 0' ... Peter - have I missed anything? Alasdair -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
next prev parent reply other threads:[~2021-08-09 23:31 UTC|newest] Thread overview: 68+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-08-04 9:41 use regular gendisk registration in device mapper v2 Christoph Hellwig 2021-08-04 9:41 ` [dm-devel] " Christoph Hellwig 2021-08-04 9:41 ` [PATCH 1/8] block: make the block holder code optional Christoph Hellwig 2021-08-04 9:41 ` [dm-devel] " Christoph Hellwig 2021-08-04 9:41 ` [PATCH 2/8] block: remove the extra kobject reference in bd_link_disk_holder Christoph Hellwig 2021-08-04 9:41 ` [dm-devel] " Christoph Hellwig 2021-08-04 9:41 ` [PATCH 3/8] block: look up holders by bdev Christoph Hellwig 2021-08-04 9:41 ` [dm-devel] " Christoph Hellwig 2021-08-04 9:41 ` [PATCH 4/8] block: support delayed holder registration Christoph Hellwig 2021-08-04 9:41 ` [dm-devel] " Christoph Hellwig [not found] ` <CGME20210810213058eucas1p109323e3c3ecaa76d37d8cf63b6d8ecfd@eucas1p1.samsung.com> 2021-08-10 21:30 ` Marek Szyprowski 2021-08-10 21:30 ` [dm-devel] " Marek Szyprowski 2021-08-14 21:13 ` Guenter Roeck 2021-08-14 21:13 ` Guenter Roeck 2021-08-15 7:07 ` Christoph Hellwig 2021-08-15 7:07 ` Christoph Hellwig 2021-08-15 14:27 ` Guenter Roeck 2021-08-15 14:27 ` Guenter Roeck 2021-08-16 7:21 ` Christoph Hellwig 2021-08-16 7:21 ` Christoph Hellwig 2021-08-16 14:17 ` Guenter Roeck 2021-08-16 14:17 ` Guenter Roeck 2021-08-20 15:08 ` Christoph Hellwig 2021-08-20 15:08 ` Christoph Hellwig 2021-08-21 3:17 ` Guenter Roeck 2021-08-21 3:17 ` Guenter Roeck 2021-08-18 2:51 ` Guenter Roeck 2021-08-18 2:51 ` Guenter Roeck 2021-08-04 9:41 ` [PATCH 5/8] dm: cleanup cleanup_mapped_device Christoph Hellwig 2021-08-04 9:41 ` [dm-devel] " Christoph Hellwig 2021-08-04 9:41 ` [PATCH 6/8] dm: move setting md->type into dm_setup_md_queue Christoph Hellwig 2021-08-04 9:41 ` [dm-devel] " Christoph Hellwig 2021-08-04 9:41 ` [PATCH 7/8] dm: delay registering the gendisk Christoph Hellwig 2021-08-04 9:41 ` [dm-devel] " Christoph Hellwig 2021-08-09 23:31 ` Alasdair G Kergon [this message] 2021-08-09 23:31 ` Alasdair G Kergon 2021-08-10 0:17 ` Alasdair G Kergon 2021-08-10 0:17 ` Alasdair G Kergon 2021-08-10 13:12 ` Peter Rajnoha 2021-08-10 13:12 ` Peter Rajnoha 2021-08-10 15:05 ` Alasdair G Kergon 2021-08-10 15:05 ` Alasdair G Kergon 2022-07-07 3:29 ` Yu Kuai 2022-07-07 3:29 ` [dm-devel] " Yu Kuai 2022-07-07 5:24 ` Christoph Hellwig 2022-07-07 5:24 ` [dm-devel] " Christoph Hellwig 2022-07-07 7:20 ` Yu Kuai 2022-07-07 7:20 ` [dm-devel] " Yu Kuai 2022-07-15 3:24 ` Yu Kuai 2022-07-15 3:24 ` [dm-devel] " Yu Kuai 2022-08-01 1:04 ` Yu Kuai 2022-08-01 1:04 ` [dm-devel] " Yu Kuai 2021-08-04 9:41 ` [PATCH 8/8] block: remove support for delayed queue registrations Christoph Hellwig 2021-08-04 9:41 ` [dm-devel] " Christoph Hellwig 2021-08-09 17:51 ` use regular gendisk registration in device mapper v2 Jens Axboe 2021-08-09 17:51 ` [dm-devel] " Jens Axboe 2021-08-10 0:36 ` Alasdair G Kergon 2021-08-10 0:36 ` Alasdair G Kergon 2021-08-10 14:41 ` Alasdair G Kergon 2021-08-10 14:41 ` Alasdair G Kergon 2021-08-19 15:58 ` holders not working properly, regression [was: Re: use regular gendisk registration in device mapper v2] Mike Snitzer 2021-08-19 15:58 ` [dm-devel] " Mike Snitzer 2021-08-19 18:05 ` Christoph Hellwig 2021-08-19 18:05 ` [dm-devel] " Christoph Hellwig 2021-08-19 22:08 ` Mike Snitzer 2021-08-19 22:08 ` [dm-devel] " Mike Snitzer -- strict thread matches above, loose matches on Subject: below -- 2021-07-25 5:54 use regular gendisk registration in device mapper Christoph Hellwig 2021-07-25 5:54 ` [dm-devel] [PATCH 7/8] dm: delay registering the gendisk Christoph Hellwig 2021-07-29 16:36 ` Mike Snitzer
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=20210809233143.GA101480@agk-cloud1.hosts.prod.upshift.rdu2.redhat.com \ --to=agk@redhat.com \ --cc=axboe@kernel.dk \ --cc=dm-devel@redhat.com \ --cc=hch@lst.de \ --cc=linux-block@vger.kernel.org \ --cc=prajnoha@redhat.com \ --cc=snitzer@redhat.com \ /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: linkBe 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.