On Wed, 2026-10-07 at 21:21 +0200, Martin Wilck wrote:
> > [Ben wrote:] The issue that the second resume is solving is the same
> > one that lead us to add a second resume to dm_addmap_reload(). If the
> > device has an invalid inactive table, and you do a resume, dm will
> > attempt to swap the table, fail, and leave the device suspended with
> > the invalid table removed. Immediately issuing a second resume will
> > cause dm to actually resume with the old table.
>
> Right, thanks for explaining it. [...] it actually makes sense to
> retry the resume. [...] I'd appreciate a short comment explaining the
> problem scenario that the retry attempts to avoid (i.e. a short
> version of Ben's explanation here).
Thanks both, that explains it well. I'll restore the retry in v3 (it is
what v1 did) and add a short comment describing the scenario:
/*
* Resume the map suspended above. If it has an inactive table,
* the resume makes dm try to swap to it; if that swap fails dm
* leaves the map suspended with the invalid table dropped, and a
* second resume reactivates it with the old table (as
* dm_addmap_reload() does). Retry the resume once, and log if it
* is still not resumed so a leaked suspend is visible.
*/
On Ben's consistency point (always retry vs only after loading a new
table): the flush/remove path never loads an inactive table, so in
practice there should be none to swap here. But the retry is cheap and
harmless and mirrors dm_addmap_reload(), so v3 simply retries on this
path too. I kept the condlog(1) from v2 so a map still suspended after
the retry remains visible in the logs.
Re Martin's valid-inactive-table footnote: agreed -- if a valid inactive
table somehow existed, the second resume would swap to it rather than
remove, which would be wrong. But dm_flush_map__() loads no inactive
table, so that state should not arise on this path; it would be a
pre-existing anomaly outside this fix, so I've left it out of scope here.
Sending v3 now with the above.
Thanks,
Suresh
