| Message ID | 20250309222904.449803-2-bmarzins@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | dm: fix issues with swapping dm tables | expand |
On 3/10/25 07:28, Benjamin Marzinski wrote: > __bind was changing the disk capacity, geometry and mempools of the > mapped device before calling dm_table_set_restrictions() which could > fail, forcing dm to drop the new table. Failing here would leave the > device using the old table but with the wrong capacity and mempools. > > Move dm_table_set_restrictions() earlier in __bind(). Since it needs the > capacity to be set, save the old version and restore it on failure. > > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> Does this need a "Fixes" tag maybe ? Otherwise looks good to me. Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
On Mon, Mar 10, 2025 at 08:18:31AM +0900, Damien Le Moal wrote: > On 3/10/25 07:28, Benjamin Marzinski wrote: > > __bind was changing the disk capacity, geometry and mempools of the > > mapped device before calling dm_table_set_restrictions() which could > > fail, forcing dm to drop the new table. Failing here would leave the > > device using the old table but with the wrong capacity and mempools. > > > > Move dm_table_set_restrictions() earlier in __bind(). Since it needs the > > capacity to be set, save the old version and restore it on failure. > > > > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> > > Does this need a "Fixes" tag maybe ? Yeah. I can go through and add the appropriate Fixes tags. -Ben > > Otherwise looks good to me. > > Reviewed-by: Damien Le Moal <dlemoal@kernel.org> > > -- > Damien Le Moal > Western Digital Research
diff --git a/drivers/md/dm.c b/drivers/md/dm.c index 5ab7574c0c76..f5c5ccb6f8d2 100644 --- a/drivers/md/dm.c +++ b/drivers/md/dm.c @@ -2421,21 +2421,29 @@ static struct dm_table *__bind(struct mapped_device *md, struct dm_table *t, struct queue_limits *limits) { struct dm_table *old_map; - sector_t size; + sector_t size, old_size; int ret; lockdep_assert_held(&md->suspend_lock); size = dm_table_get_size(t); + old_size = dm_get_size(md); + set_capacity(md->disk, size); + + ret = dm_table_set_restrictions(t, md->queue, limits); + if (ret) { + set_capacity(md->disk, old_size); + old_map = ERR_PTR(ret); + goto out; + } + /* * Wipe any geometry if the size of the table changed. */ - if (size != dm_get_size(md)) + if (size != old_size) memset(&md->geometry, 0, sizeof(md->geometry)); - set_capacity(md->disk, size); - dm_table_event_callback(t, event_callback, md); if (dm_table_request_based(t)) { @@ -2468,12 +2476,6 @@ static struct dm_table *__bind(struct mapped_device *md, struct dm_table *t, t->mempools = NULL; } - ret = dm_table_set_restrictions(t, md->queue, limits); - if (ret) { - old_map = ERR_PTR(ret); - goto out; - } - old_map = rcu_dereference_protected(md->map, lockdep_is_held(&md->suspend_lock)); rcu_assign_pointer(md->map, (void *)t); md->immutable_target_type = dm_table_get_immutable_target_type(t);
__bind was changing the disk capacity, geometry and mempools of the mapped device before calling dm_table_set_restrictions() which could fail, forcing dm to drop the new table. Failing here would leave the device using the old table but with the wrong capacity and mempools. Move dm_table_set_restrictions() earlier in __bind(). Since it needs the capacity to be set, save the old version and restore it on failure. Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> --- drivers/md/dm.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-)