Re: [RFC PATCH 0/7] dm: fix issues with swapping dm tables

[Damien Le Moal <dlemoal@kernel.org>]

On 3/11/25 01:38, Benjamin Marzinski wrote:
> On Mon, Mar 10, 2025 at 08:16:43AM +0900, Damien Le Moal wrote:
>> 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.
> 
> If we don't think there's any interest in growing or shrinking zoned
> dm-crypt devices, that's fine.  I do think we should make an exception
> for switching to the dm-error target. We specifically call that out with
> DM_TARGET_WILDCARD so that we can always switch to it from any table if
> we just want to fail out all the IO.

Arg ! dm-error is used in xfstests so we need it (for btrfs at least since btrfs
supports zoned devices, and soon xfs as well). So I guess we should disallow
switching tables when the new table changes something to the zone configuration
(grow, shrink, zone size, zoned/non-zoned). dm-error does not change anything,
so we should still be able to allow it.



-- 
Damien Le Moal
Western Digital Research