[Devel] [PATCH hci-8.1 v3 4/5] dm-ploop: reload metadata on resume

[Denis V. Lunev]

On 08.10.2026 16:03, Alexander Ivanov wrote:
> +static bool ploop_md_pages_are_clean(struct ploop *ploop)
> +{
> +	struct rb_node *node;
> +	struct md_page *md;
> +
> +	if (!llist_empty(&ploop->wb_batch_llist) ||
> +	    atomic_read(&ploop->inflight_md))
> +		return false;
> +
> +	ploop_for_each_md_page(ploop, md, node) {
> +		if (md->status & (MD_DIRTY | MD_WRITEBACK | MD_UPDATING) ||

MD_* are bit numbers, not masks. Everybody else does
set_bit(MD_UPDATING, &md->status) and test_bit(). MD_UPDATING is
(1U << 4), so set_bit() sets bit 16, while this expression tests
bits 1, 2 and 4. MD_DIRTY and MD_WRITEBACK are caught only because
their values happen to line up with the bits of other flags.
MD_UPDATING is never caught. Please use test_bit() for each flag.

[...]

> @@ -2771,6 +2777,16 @@ loff_t ploop_llseek_hole(struct dm_target *ti, loff_t offset, int whence)
[...]
> +	down_read(&ploop->ctl_rwsem);
> +	if (ploop->suspended) {
> +		result = -EAGAIN;
> +		goto unlock;
> +	}

ploop_message() holds ctl_rwsem for write for the whole command.
"merge" walks every cluster of the device in
ploop_process_merge_latest_snapshot() and "resize" relocates
clusters. SEEK_DATA/SEEK_HOLE used to return at once, now it sleeps
uninterruptibly until the merge is over. -EAGAIN is already the
"come back later" answer here, so down_read_trylock() returning
-EAGAIN fits better.

[...]

>  static void ploop_postsuspend(struct dm_target *ti)
>  {
> +	struct ploop *ploop = ti->private;
> +
> +	/*
> +	 * Let the next resumed ploop (maybe of reloaded table)
> +	 * see final size of the top delta.
> +	 */
> +	ploop_wait_prealloc(ploop);

This is the uninterruptible wait for the allocator. On a device in
standby (lease stolen or the mount gone) the allocator may be stuck
in fallocate on the dead file forever. Then postsuspend never
returns, under md->suspend_lock, and dm_replace() in libploop, which
is exactly the tool that recovers a standby device, hangs instead.
Before this patch postsuspend never touched the storage. Please skip
the wait when blk_queue_standby() is set. See also my reply to 5/5,
which has the same problem with the truncate.

[...]

> -	down_read(&ploop->ctl_rwsem);
> -	if (ploop->noresume)
> +	down_write(&ploop->ctl_rwsem);
> +	if (ploop->noresume) {
>  		ret = -EAGAIN;
> -	up_read(&ploop->ctl_rwsem);
> +	} else {
[...]
> +		ret = ploop_reload_metadata(ploop);

Until now preresume could fail only because of noresume. Now it fails
on -ENOMEM (a second copy of the whole BAT tree and holes_bitmap is
built under NOIO), on I/O errors and on -EPROTO/-EBADSLT from the
images, and the device stays suspended. The message has to say so,
and userspace has to be ready for it. It is not today:

- create_snapshot_online() ignores the result of
  ploop_resume_device() and reports success with the device left
  suspended. So do ploop_stop(), lib/di.c and the qcow2 path in
  lib/dm.c.

- ploop_tg_deinit() is worse. DM swaps the new table into dev before
  calling preresume, so when dm_resume(dev) fails, dev keeps the new
  ploop and the tracking table is already gone. The error path then
  resumes devtg and dev again, and if the retry succeeds two ploop
  instances are live on the same images. With 5/5 a later suspend or
  removal of devtg truncates the top delta under the live instance.

> +	old_hdr = ploop_md_first_entry(&old_bat_entries)->kmpage;
> +	hdr = ploop_md_first_entry(&ploop->bat_entries)->kmpage;
> +	hdr->m_DiskInUse = old_hdr->m_DiskInUse;

This is needed after flip_upper_deltas, but it is done on every
resume. save_cbt() in libploop writes SIGNATURE_DISK_CLOSED_V21 and
m_FormatExtensionOffset to the suspended device and then resumes it.
If BAT page 0 is written before the device is removed, IN_USE goes
back to disk. The check at umount then sees the image in use and trims
it at the last data cluster, which cuts off the CBT blocks. This needs
a write after the filesystem is unmounted, so it is unlikely. Still,
please restrict the override to the flip case or explain why it is
safe in general.

The patch does four things: the reload, moving prealloc to preresume
with the wait on postsuspend, wake_up_interruptible() -> wake_up() for
inflight_md_wq, and ctl_rwsem in ploop_llseek_hole(). The last two
stand on their own and would be easier to review as separate patches.
That would also cut the commit message down to the reload itself.