[Devel] [PATCH hci-8.1 v3 4/5] dm-ploop: reload metadata on resume
Alexander Ivanov
alexander.ivanov at virtuozzo.com
Thu Oct 8 17:03:32 MSK 2026
On "dmsetup reload" the new ploop reads metadata of the images in
ploop_ctr(), but IO continues through the old ploop until the device
is suspended. So the old ploop may change BAT and sizes of the delta
files, and after resume the new ploop works with outdated metadata,
e.g. allocates clusters already used by the old one or relies on a
wrong file size. Also userspace may modify images of suspended device
(e.g. merge zeroes BAT entries of the base delta, snapshot creation
trims the tail of the top delta).
Reload all the metadata in ploop_preresume(): re-read header of the top
delta, BAT of every delta and sizes of delta files, and rebuild BAT
tree and holes_bitmap from scratch. On error the old metadata is kept
and resume fails.
Skip the in-use signature check on reload, as the image is already
checked in ploop_ctr(), and userspace doesn't mark the delta, which
becomes top after flip_upper_deltas, as in use. Instead it relies that
the in-use signature of the old top delta from in-memory header will
be written to the new top delta with BAT page 0. So keep the in-use
signature of the old in-memory header in the reloaded one, otherwise
the next table load fails with -ENOLCK after online merge.
Metadata of the device which has already been running can't be dirty
after suspend, since all IO is completed. If it is dirty anyway, keep
it as is, as md pages may still be referenced. Note that data bios
are completed before BAT writeback is finished, so wait for inflight_md
on postsuspend too. Use wake_up() for inflight_md_wq, as the waiter is
uninterruptible.
The new ploop must not modify images before resume either, so request
preallocation in ploop_preresume() after the reload instead of
ploop_ctr(). Otherwise it may truncate the top delta back to the stale
file size. And wait for pending preallocation on postsuspend, so that
the reloaded ploop sees the final size of the top delta.
Reload frees the old md pages tree, while ploop_llseek_hole() walks it
locklessly: dm_llseek_hole() checks for suspend only once on entry and
isn't synchronized with it. So take ctl_rwsem for read in
ploop_llseek_hole() (reload is done under it for write) and return
-EAGAIN if the device is suspended. This also protects the walk from
resize, which changes the tree under ctl_rwsem too.
https://virtuozzo.atlassian.net/browse/VSTOR-148171
Feature: dm-ploop: ploop target driver
Signed-off-by: Alexander Ivanov <alexander.ivanov at virtuozzo.com>
---
drivers/md/dm-ploop-bat.c | 139 ++++++++++++++++++++++++++++++++++-
drivers/md/dm-ploop-map.c | 30 ++++++--
drivers/md/dm-ploop-target.c | 72 +++++++++++-------
drivers/md/dm-ploop.h | 5 +-
4 files changed, 213 insertions(+), 33 deletions(-)
diff --git a/drivers/md/dm-ploop-bat.c b/drivers/md/dm-ploop-bat.c
index 57a45119f0c4..2e7949a1b154 100644
--- a/drivers/md/dm-ploop-bat.c
+++ b/drivers/md/dm-ploop-bat.c
@@ -184,7 +184,8 @@ static int ploop_setup_holes_bitmap(struct ploop *ploop, u32 bat_clusters)
}
ALLOW_ERROR_INJECTION(ploop_setup_holes_bitmap, ERRNO);
-int ploop_setup_metadata(struct ploop *ploop, struct page *page)
+int ploop_setup_metadata(struct ploop *ploop, struct page *page,
+ bool check_in_use)
{
struct ploop_pvd_header *m_hdr = NULL;
u32 bat_clusters, offset_clusters;
@@ -199,7 +200,8 @@ int ploop_setup_metadata(struct ploop *ploop, struct page *page)
goto out;
ret = -ENOLCK;
- if (m_hdr->m_DiskInUse != cpu_to_le32(SIGNATURE_DISK_IN_USE) &&
+ if (check_in_use &&
+ m_hdr->m_DiskInUse != cpu_to_le32(SIGNATURE_DISK_IN_USE) &&
!ploop_is_ro(ploop) && !ignore_signature_disk_in_use)
goto out;
@@ -558,6 +560,139 @@ int ploop_add_delta(struct ploop *ploop, u32 level, struct file *file, bool is_r
}
ALLOW_ERROR_INJECTION(ploop_add_delta, ERRNO);
+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->piwb || !llist_empty(&md->wait_llist))
+ return false;
+ }
+
+ return true;
+}
+
+/*
+ * Build in-memory metadata from scratch: re-read header of top delta,
+ * BAT of every delta and sizes of delta files. This is called on resume
+ * of suspended device, since after "dmsetup reload" the metadata of the
+ * new ploop may be outdated: the images could be changed by the old
+ * ploop after the new one was constructed. On error the old metadata
+ * is kept.
+ */
+int ploop_reload_metadata(struct ploop *ploop)
+{
+ struct rb_root old_bat_entries = ploop->bat_entries;
+ void *old_holes_bitmap = ploop->holes_bitmap;
+ u32 old_nr_bat_entries = ploop->nr_bat_entries;
+ u32 old_hb_nr = ploop->hb_nr;
+ u32 old_m_Type = ploop->m_Type;
+ u32 old_m_Sectors = ploop->m_Sectors;
+ struct ploop_pvd_header *hdr, *old_hdr;
+ u8 old_m_Sig[sizeof(ploop->m_Sig)];
+ struct ploop_delta *delta, *old_deltas;
+ struct page *page;
+ loff_t file_size;
+ u32 nr_be;
+ int level, ret;
+
+ /*
+ * Metadata of the device that has already been running is up to
+ * date, so we only need to reload clean metadata. Dirty metadata
+ * must not appear after suspend, as all IO and BAT writeback are
+ * completed (see ploop_postsuspend()), but we can't free md pages,
+ * which are still in use, in any case.
+ */
+ if (WARN_ON_ONCE(!ploop_md_pages_are_clean(ploop)))
+ return 0;
+
+ memcpy(old_m_Sig, ploop->m_Sig, sizeof(old_m_Sig));
+
+ old_deltas = kmemdup(ploop->deltas,
+ sizeof(*old_deltas) * ploop->nr_deltas,
+ GFP_KERNEL);
+ page = alloc_page(GFP_KERNEL);
+ ret = -ENOMEM;
+ if (!old_deltas || !page)
+ goto out;
+
+ ret = ploop_rw_page_sync(READ, ploop_top_delta(ploop)->file, 0, page);
+ if (ret)
+ goto out;
+
+ ploop->bat_entries = RB_ROOT;
+ ploop->holes_bitmap = NULL;
+
+ /* Image is already checked for being in use in ploop_ctr() */
+ ret = ploop_setup_metadata(ploop, page, false);
+ if (ret)
+ goto restore;
+
+ ret = ploop_prealloc_md_pages(&ploop->bat_entries, 0,
+ ploop->nr_bat_entries);
+ if (ret)
+ goto restore;
+
+ /*
+ * Same order as in ploop_add_deltas_stack(): from top to base.
+ * ploop_read_delta() relies on it, see the comment there. nr_be of
+ * the delta is updated right away, as the lower delta is checked
+ * against it on the next iteration.
+ */
+ for (level = ploop_top_level(ploop); level >= 0; level--) {
+ delta = &ploop->deltas[level];
+
+ ret = ploop_read_delta(ploop, level, delta->file,
+ &file_size, &nr_be);
+ if (ret)
+ goto restore;
+
+ delta->file_size = file_size;
+ delta->file_preallocated_area_start = file_size;
+ delta->nr_be = nr_be;
+ }
+
+ /*
+ * Header of top delta is written with BAT page 0. Userspace doesn't
+ * mark the delta, which becomes top after flip_upper_deltas, as in
+ * use, and relies that the in-use signature of the old top delta
+ * will be written to it. So keep the signature we had.
+ */
+ 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;
+
+ ploop_free_md_pages_tree(&old_bat_entries);
+ kvfree(old_holes_bitmap);
+ goto out;
+restore:
+ ploop_free_md_pages_tree(&ploop->bat_entries);
+ kvfree(ploop->holes_bitmap);
+ ploop->bat_entries = old_bat_entries;
+ ploop->holes_bitmap = old_holes_bitmap;
+ ploop->nr_bat_entries = old_nr_bat_entries;
+ ploop->hb_nr = old_hb_nr;
+ memcpy(ploop->m_Sig, old_m_Sig, sizeof(ploop->m_Sig));
+ ploop->m_Type = old_m_Type;
+ ploop->m_Sectors = old_m_Sectors;
+ memcpy(ploop->deltas, old_deltas,
+ sizeof(*old_deltas) * ploop->nr_deltas);
+out:
+ if (page)
+ put_page(page);
+ kfree(old_deltas);
+ if (ret)
+ PL_ERR("Failed to reload metadata: %d", ret);
+ return ret;
+}
+ALLOW_ERROR_INJECTION(ploop_reload_metadata, ERRNO);
+
void ploop_put_delta(struct ploop *ploop, struct ploop_delta *delta)
{
int i;
diff --git a/drivers/md/dm-ploop-map.c b/drivers/md/dm-ploop-map.c
index a3a5393e533d..9a892ad71863 100644
--- a/drivers/md/dm-ploop-map.c
+++ b/drivers/md/dm-ploop-map.c
@@ -926,7 +926,7 @@ static void ploop_put_piwb(struct ploop_index_wb *piwb)
*/
if (atomic_dec_and_test(&piwb->ploop->inflight_md))
- wake_up_interruptible(&piwb->ploop->inflight_md_wq);
+ wake_up(&piwb->ploop->inflight_md_wq);
ploop_free_piwb(piwb);
}
}
@@ -1106,7 +1106,7 @@ void ploop_break_bat_update(struct ploop *ploop, struct md_page *md,
spin_unlock_irqrestore(&ploop->bat_lock, flags);
if (atomic_dec_and_test(&ploop->inflight_md))
- wake_up_interruptible(&ploop->inflight_md_wq);
+ wake_up(&ploop->inflight_md_wq);
ploop_free_piwb(piwb);
}
@@ -1245,6 +1245,12 @@ static int ploop_no_pending_prealloc(struct ploop *ploop)
return ret;
}
+void ploop_wait_prealloc(struct ploop *ploop)
+{
+ wait_event(ploop->dispatcher_wq_prealloc,
+ ploop_no_pending_prealloc(ploop));
+}
+
static int ploop_allocate_cluster(struct ploop *ploop, u32 *dst_clu, struct file *file)
{
struct ploop_delta *top = ploop_top_delta(ploop);
@@ -2377,7 +2383,7 @@ int ploop_allocator(void *data)
PL_FMT("failed to allocate space: %d\n"),
ploop_device_name(ploop), ret);
}
- wake_up_interruptible(&ploop->dispatcher_wq_prealloc);
+ wake_up(&ploop->dispatcher_wq_prealloc);
current_restore_flags(old_flags, PLOOP_PFLAGS);
}
@@ -2771,6 +2777,16 @@ loff_t ploop_llseek_hole(struct dm_target *ti, loff_t offset, int whence)
struct md_page *md;
loff_t result;
+ /*
+ * Protect md pages tree from being freed or replaced under us
+ * by metadata reload in ploop_preresume() or by resize.
+ */
+ down_read(&ploop->ctl_rwsem);
+ if (ploop->suspended) {
+ result = -EAGAIN;
+ goto unlock;
+ }
+
clu = SEC_TO_CLU(ploop, to_sector(offset) + ploop->skip_off);
id = U32_MAX;
@@ -2778,8 +2794,10 @@ loff_t ploop_llseek_hole(struct dm_target *ti, loff_t offset, int whence)
if (id != ploop_bat_clu_to_page_nr(clu)) {
id = ploop_bat_clu_to_page_nr(clu);
md = ploop_md_page_find(ploop, id);
- if (!md)
- return -EINVAL;
+ if (!md) {
+ result = -EINVAL;
+ goto unlock;
+ }
}
bat = md->kmpage;
@@ -2794,5 +2812,7 @@ loff_t ploop_llseek_hole(struct dm_target *ti, loff_t offset, int whence)
}
result = to_bytes(CLU_TO_SEC(ploop, clu) - ploop->skip_off);
+unlock:
+ up_read(&ploop->ctl_rwsem);
return result;
}
diff --git a/drivers/md/dm-ploop-target.c b/drivers/md/dm-ploop-target.c
index 3b589579741c..1fbcb48cff7a 100644
--- a/drivers/md/dm-ploop-target.c
+++ b/drivers/md/dm-ploop-target.c
@@ -21,6 +21,7 @@
#include <linux/uio.h>
#include <linux/timer.h>
#include <linux/error-injection.h>
+#include <linux/sched/mm.h>
#include "dm-ploop.h"
#define DM_MSG_PREFIX "ploop"
@@ -288,7 +289,7 @@ static int ploop_check_top_delta(struct ploop *ploop, struct file *file)
/* flush dirty pages before changing direct IO */
vfs_fsync(file, 1);
- ret = ploop_setup_metadata(ploop, page);
+ ret = ploop_setup_metadata(ploop, page, true);
if (ret)
goto out;
@@ -419,8 +420,7 @@ static struct ploop_worker *ploop_worker_create(struct ploop *ploop,
* suspended and resumed. This behavior is intentional to allow for atomic
* updates to the mapping table without disrupting ongoing I/O operations.
*
- * But dm-ploop is not fully compatible with this design. Ploop does not do a
- * "metadata" reload on resume. So there can be situations like:
+ * So there can be situations like:
*
* 1. dmsetup reload is called, and new ploop is created with ploop_ctr for
* the same delta image file as an old ploop and ploop_add_deltas_stack reads
@@ -431,23 +431,19 @@ static struct ploop_worker *ploop_worker_create(struct ploop *ploop,
* the new one, new ploop has outdated metadata and thus may corrupt the
* images by relying on it.
*
- * For instance we thought to truncate the preallocations to the delta image
- * file on ploop_dtr, but that can't be done as ploop_dtr is called on the old
- * ploop after the new one is resumed, so we can't possibly modify the image
- * from the old ploop context at this point, as new ploop may be already doing
- * its own IO or preallocation.
+ * To handle this, ploop_preresume reloads all the metadata from the images
+ * (see ploop_reload_metadata), so that the new ploop "sees" all previous
+ * updates from the old one. For the same reason the new ploop must not
+ * modify the images before resume, so preallocation is started only after
+ * the metadata reload, and the old ploop waits for its preallocation to
+ * complete on postsuspend.
*
- * The better way to do this truncation is on ploop_*suspend, and then on resume
- * to reload the metadata in the new ploop so that the new ploop "sees" all
- * previous updates from the old one.
- *
- * For now we can leave with it, as we chose not to do this truncation on
- * destroy and we rely that userspace always does dmsetup reload on already
- * suspended device.
- *
- * FIXME: We must implement metadata reload on resume. After that we can
- * implement preallocation truncation on suspend (see dm-qcow2 as it already
- * implements that in qcow2_parse_metadata and qcow2_truncate_preallocations).
+ * Still, we can't truncate the preallocations to the delta image file on
+ * ploop_dtr, as ploop_dtr is called on the old ploop after the new one is
+ * resumed, so we can't possibly modify the image from the old ploop context
+ * at this point, as new ploop may be already doing its own IO or
+ * preallocation. This truncation should be done on ploop_*suspend (see
+ * dm-qcow2 as it already implements that in qcow2_truncate_preallocations).
*/
static int ploop_ctr(struct dm_target *ti, unsigned int argc, char **argv)
{
@@ -637,9 +633,6 @@ static int ploop_ctr(struct dm_target *ti, unsigned int argc, char **argv)
ti->num_discard_bios = 1;
ti->discards_supported = true;
- if (!ploop_is_ro(ploop) && ploop->nr_deltas > 0)
- ploop_should_prealloc(ploop);
-
return 0;
err:
@@ -743,19 +736,48 @@ static void ploop_presuspend_undo(struct dm_target *ti)
}
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);
+ /*
+ * Data bios are completed before BAT writeback is finished
+ * (see ploop_put_piwb()), so wait for it explicitly to make
+ * md pages clean for ploop_reload_metadata().
+ */
+ wait_event(ploop->inflight_md_wq, !atomic_read(&ploop->inflight_md));
ploop_set_suspended(ti, true);
}
static int ploop_preresume(struct dm_target *ti)
{
struct ploop *ploop = ti->private;
+ unsigned int noio_flags;
int ret = 0;
- down_read(&ploop->ctl_rwsem);
- if (ploop->noresume)
+ down_write(&ploop->ctl_rwsem);
+ if (ploop->noresume) {
ret = -EAGAIN;
- up_read(&ploop->ctl_rwsem);
+ } else {
+ /*
+ * The device is suspended, so reclaim must not wait
+ * for IO to it. Otherwise we deadlock with ourselves.
+ */
+ noio_flags = memalloc_noio_save();
+ ret = ploop_reload_metadata(ploop);
+ memalloc_noio_restore(noio_flags);
+ }
+ up_write(&ploop->ctl_rwsem);
if (ret == 0) {
+ /*
+ * Preallocate only after the metadata is reloaded,
+ * as prealloc relies on the actual top delta size.
+ */
+ if (!ploop_is_ro(ploop))
+ ploop_should_prealloc(ploop);
/*
* We are singleton target. There will be
* no more reasons to break resume.
diff --git a/drivers/md/dm-ploop.h b/drivers/md/dm-ploop.h
index 23662943d56a..1cf0d5e0cea8 100644
--- a/drivers/md/dm-ploop.h
+++ b/drivers/md/dm-ploop.h
@@ -631,7 +631,9 @@ extern struct pio *ploop_alloc_pio_with_pages(struct ploop *ploop);
extern void ploop_free_pio_with_pages(struct ploop *ploop, struct pio *pio);
extern void ploop_pio_prepare_offsets(struct ploop *ploop, struct pio *pio, u32 clu);
-extern int ploop_setup_metadata(struct ploop *ploop, struct page *page);
+extern int ploop_setup_metadata(struct ploop *ploop, struct page *page,
+ bool check_in_use);
+extern int ploop_reload_metadata(struct ploop *ploop);
extern int ploop_read_delta_metadata(struct ploop *ploop, struct file *file,
struct rb_root *md_root, u32 *delta_nr_be);
extern void ploop_index_wb_init(struct ploop_index_wb *piwb,
@@ -649,6 +651,7 @@ extern void ploop_disable_writeback_delay(struct ploop *ploop);
extern void ploop_enable_writeback_delay(struct ploop *ploop);
extern void ploop_should_prealloc(struct ploop *ploop);
+extern void ploop_wait_prealloc(struct ploop *ploop);
extern void ploop_resubmit_enospc_pios(struct ploop *ploop);
#endif /* __DM_PLOOP_H */
--
2.53.0
More information about the Devel
mailing list