From: Damien Le Moal <dlemoal@kernel.org>
To: Benjamin Marzinski <bmarzins@redhat.com>,
Mikulas Patocka <mpatocka@redhat.com>,
Mike Snitzer <snitzer@redhat.com>, Jens Axboe <axboe@kernel.dk>
Cc: dm-devel@lists.linux.dev, linux-block@vger.kernel.org,
Christoph Hellwig <hch@lst.de>
Subject: Re: [RFC PATCH 0/7] dm: fix issues with swapping dm tables
Date: Mon, 10 Mar 2025 08:16:43 +0900 [thread overview]
Message-ID: <788a1ec4-ac86-40fb-a709-eba7e6d5535f@kernel.org> (raw)
In-Reply-To: <20250309222904.449803-1-bmarzins@redhat.com>
On 3/10/25 07:28, Benjamin Marzinski wrote:
> There were multiple places in dm's __bind() function where it could fail
> and not completely roll back, leaving the device using the the old
> table, but with device limits and resources from the new table.
> Additionally, unused mempools for request-based devices were not always
> freed immediately.
>
> Finally, there were a number of issues with switching zoned tables that
> emulate zone append (in other words, dm-crypt on top of zoned devices).
> dm_blk_report_zones() could be called while the device was suspended and
> modifying zoned resources or could possibly fail to end a srcu read
> section. More importantly, blk_revalidate_disk_zones() would never get
> called when updating a zoned table. This could cause the dm device to
> see the wrong zone write offsets, not have a large enough zwplugs
> reserved in its mempool, or read invalid memory when checking the
> conventional zones bitmap.
>
> This patchset fixes these issues. It does not make it so that
> device-mapper is able to load any zoned table from any other zoned
> table. Zoned dm-crypt devices can be safely grown and shrunk, but
> reloading a zoned dm-crypt device to, for instance, point at a
> completely different underlying device won't work correctly. IO might
> fail since the zone write offsets of the dm-crypt device will not be
> updated for all the existing zones with plugs. If the new device's zone
> offsets don't match the old device's offsets, IO to the zone will fail.
> If the ability to switch tables from a zoned dm-crypt device to an
> abritry other zoned dm-crypt device is important to people, it could be
> done as long as there are no plugged zones when dm suspends.
Thanks for fixing this.
Given that in the general case switching tables will always likely result in
unaligned write errors, I think we should just report a ENOTSUPP error if the
user attempts to swap tables.
> This patchset also doesn't touch the code for switching from a zoned to
> a non-zoned device. Switching from a zoned dm-crypt device to a
> non-zoned device is problematic if there are plugged zones, since the
> underlying device will not be prepared to handle these plugged writes.
> Switching to a target that does not pass down IOs, like the dm-error
> target, is fine. So is switching when there are no plugged zones, except
> that we do not free the zoned resources in this case, even though we
> safely could.
This is another case that does not make much sense in practice. So instead of
still allowing it knowing that it most likely will not work, we should return
ENOTSUPP here too I think.
> If people are interested in removing some of these limitations, I can
> send patches for them, but I'm not sure how much extra code we want,
> just to support niche zoned dm-crypt reloads.
I have never heard any complaints/bug reports from people attempting this. Which
likely means that no-one is needing/trying to do this. So as mentionned above,
we should make sure that this feature is not reported as not supported with a
ENOTSUPP error, and maybe a warning.
--
Damien Le Moal
Western Digital Research
next prev parent reply other threads:[~2025-03-09 23:16 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-03-09 22:28 [RFC PATCH 0/7] dm: fix issues with swapping dm tables Benjamin Marzinski
2025-03-09 22:28 ` [PATCH 1/7] dm: don't change md if dm_table_set_restrictions() fails Benjamin Marzinski
2025-03-09 23:18 ` Damien Le Moal
2025-03-10 16:39 ` Benjamin Marzinski
2025-03-09 22:28 ` [PATCH 2/7] dm: free table mempools if not used in __bind Benjamin Marzinski
2025-03-09 23:19 ` Damien Le Moal
2025-03-09 22:28 ` [PATCH 3/7] dm: handle failures in dm_table_set_restrictions Benjamin Marzinski
2025-03-09 23:25 ` Damien Le Moal
2025-03-10 17:37 ` Benjamin Marzinski
2025-03-10 18:15 ` Benjamin Marzinski
2025-03-10 23:27 ` Damien Le Moal
2025-03-14 13:38 ` Mikulas Patocka
2025-03-14 13:46 ` Mikulas Patocka
2025-03-10 23:16 ` Damien Le Moal
2025-03-09 22:29 ` [PATCH 4/7] dm: fix dm_blk_report_zones Benjamin Marzinski
2025-03-09 23:27 ` Damien Le Moal
2025-03-09 22:29 ` [PATCH 5/7] blk-zoned: clean up zone settings for devices without zwplugs Benjamin Marzinski
2025-03-09 23:31 ` Damien Le Moal
2025-03-09 22:29 ` [RFC PATCH 6/7] blk-zoned: modify blk_revalidate_disk_zones for bio-based drivers Benjamin Marzinski
2025-03-09 22:29 ` [RFC PATCH 7/7] dm: allow devices to revalidate existing zones Benjamin Marzinski
2025-03-09 23:59 ` Damien Le Moal
2025-03-10 17:43 ` Benjamin Marzinski
2025-03-10 23:19 ` Damien Le Moal
2025-03-10 23:42 ` Benjamin Marzinski
2025-03-11 0:00 ` Damien Le Moal
2025-03-09 23:16 ` Damien Le Moal [this message]
2025-03-10 16:38 ` [RFC PATCH 0/7] dm: fix issues with swapping dm tables Benjamin Marzinski
2025-03-10 23:13 ` Damien Le Moal
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=788a1ec4-ac86-40fb-a709-eba7e6d5535f@kernel.org \
--to=dlemoal@kernel.org \
--cc=axboe@kernel.dk \
--cc=bmarzins@redhat.com \
--cc=dm-devel@lists.linux.dev \
--cc=hch@lst.de \
--cc=linux-block@vger.kernel.org \
--cc=mpatocka@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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox