Hi Muchun,
I have been testing this series for microVMs that mount a read-only
rootfs over virtio-pmem with FS-DAX. In this setup, most file data is
accessed through read/write rather than mmap, so most PFNs never need
private writable metadata. This optimization is very useful for
reducing guest memory overhead and running more VMs on a host.
While testing, I ran into two issues, both in patch 3, including a
crash when initializing the shared template page during probe, and the
128 MiB alignment check preventing our 2 MiB-aligned rootfs images from
using the optimization, even though the check can be relaxed without
changing the registration or teardown paths. I have included a patch
for both at the end of this email.
First, regarding the shared template initialization in
memmap_init_zone_device():
> @@ -1133,6 +1134,15 @@ void __ref memmap_init_zone_device(struct zone *zone,
> if (!nr_pages)
> return;
>
> + if (pgmap_vmemmap_optimizable(pgmap)) {
> + struct page *page = page_address(pgmap->vmemmap_shared_page);
> +
> + for (int i = 0; i < PAGE_SIZE / sizeof(struct page); i++)
> + __init_zone_device_page(page + i, start_pfn + i,
> + ZONE_DEVICE, nid, pgmap);
> + goto pageblock_init;
> + }
pgmap->vmemmap_shared_page is allocated from normal kernel memory
(direct map). It is just a template buffer, not a real struct page
inside the vmemmap space, so passing it to __init_zone_device_page()
causes page_to_pfn(page) to compute an invalid, out-of-bounds PFN.
I first hit this when backporting the series to a 6.6.117 guest kernel,
where set_pageblock_migratetype() uses that invalid PFN and crashes
during device probe.
On your base commit (next-20260902), a normal boot does not crash
because pageblock init was moved to the end of the function. However,
set_page_count() still calls page_to_pfn(page) inside the page_ref_set
tracepoint. Booting with CONFIG_DEBUG_PAGE_REF=y and
trace_event=page_ref:page_ref_set triggers the same NULL pointer
dereference during probe:
BUG: kernel NULL pointer dereference, address: 0000000000000008
#PF: supervisor read access in kernel mode
#PF: error_code(0x0000) - not-present page
Call Trace:
trace_event_raw_event_page_ref_set+0x4e/0xa0
__init_zone_device_page+0x98/0xd0
memmap_init_zone_device+0x1c3/0x2e0
To fix this, I added init_fsdax_page_template() to initialize the
template fields (ZONE_DEVICE links, Reserved flag, pgmap, and refcount/
mapcount via atomic_set()) directly without calling page_to_pfn(). It
also preserves the WANT_PAGE_VIRTUAL handling from __init_single_page(),
since patch 1 enables this optimization on LoongArch where KFENCE
selects WANT_PAGE_VIRTUAL. I only have x86-64 to test on, so please let
me know if that part looks right to you.
Second, regarding the alignment check in
pgmap_vmemmap_shared_page_alloc():
> The initial implementation is deliberately limited to a single
> memory-block-aligned range. That is not a fundamental requirement, but keeps
> the registration and teardown paths simple; support for multiple ranges or
> less strict alignment can be added later.
...
> + if (pgmap->nr_range != 1 ||
> + !IS_ALIGNED(range->start | range_len(range), MIN_MEMORY_BLOCK_SIZE))
> + return 0;
After fixing the crash above, the optimization still did not take
effect for my 130 MiB test rootfs image. Because MIN_MEMORY_BLOCK_SIZE
is 128 MiB on x86-64, the 130 MiB device failed this check and fell
back to allocating all 520 vmemmap pages.
In our microVM setup, rootfs images are sized to fit the filesystem and
aligned to 2 MiB rather than 128 MiB. In fact, 2 MiB (SUBSECTION_SIZE)
is already the native alignment for FS-DAX (via memremap_compat_align())
and for ZONE_DEVICE vmemmap population and teardown. To keep those paths
simple, we only need to make sure a shared vmemmap page never crosses a
pgmap boundary, which 2 MiB alignment already guarantees on 4K-page
kernels.
So I relaxed the check from MIN_MEMORY_BLOCK_SIZE to SUBSECTION_SIZE.
I also added an alignment check on (PAGE_SIZE / sizeof(struct page))
PFNs, so the same boundary guarantee holds on large page kernels,
where one vmemmap page covers more than 2 MiB of device memory (such
as 16K/64K pages on LoongArch).
With both changes applied on next-20260902 (QEMU x86-64), probing with
CONFIG_DEBUG_PAGE_REF=y and trace_event=page_ref:page_ref_set works
cleanly. The 130 MiB test device now uses one shared vmemmap backing
page instead of 520 private pages, and DAX faults materialize private
metadata on demand as expected. Mounting two 2 MiB-aligned virtio-pmem
devices side by side also works properly, and unbinding/rebinding either
device does not affect the other.
If these changes look good to you, could you fold them into patch 3 for
v2? (Note that the patch is against v1 and still uses the
PGMAP_VMEMMAP_OPTIMIZATION flag.)
I also noticed that next-20261023 has refactored
memmap_init_zone_device() to use zone_device_page_init_from_template().
The mm/mm_init.c change will need to be adapted when rebasing the
series, while the mm/memremap.c change should remain applicable. I am
happy to help retest v2.
Signed-off-by: Yilei Zheng <[email protected]>
---
mm/memremap.c | 10 +++++++++-
mm/mm_init.c | 29 +++++++++++++++++++++++++++--
2 files changed, 36 insertions(+), 3 deletions(-)
diff --git a/mm/memremap.c b/mm/memremap.c
index a53d09b..34d8d89 100644
--- a/mm/memremap.c
+++ b/mm/memremap.c
@@ -93,8 +93,16 @@ static int pgmap_vmemmap_shared_page_alloc(struct
dev_pagemap *pgmap, int nid)
!(pgmap->flags & PGMAP_VMEMMAP_OPTIMIZATION))
return 0;
+ /*
+ * Subsections can be populated and removed independently, but a
+ * shared vmemmap page must never span across different pgmaps
+ * (which can exceed a subsection on large-page kernels). Check
+ * both boundaries instead of requiring a full memory block.
+ */
if (pgmap->nr_range != 1 ||
- !IS_ALIGNED(range->start | range_len(range), MIN_MEMORY_BLOCK_SIZE))
+ !IS_ALIGNED(range->start | range_len(range), SUBSECTION_SIZE) ||
+ !IS_ALIGNED(PHYS_PFN(range->start | range_len(range)),
+ PAGE_SIZE / sizeof(struct page)))
return 0;
pgmap->vmemmap_shared_page = alloc_pages_node(nid, GFP_KERNEL, 0);
diff --git a/mm/mm_init.c b/mm/mm_init.c
index 7dd03b8..1c3e269 100644
--- a/mm/mm_init.c
+++ b/mm/mm_init.c
@@ -1030,6 +1030,31 @@ static void __ref __init_zone_device_page(struct page
*page, unsigned long pfn,
}
}
+/*
+ * Initialize struct page slots in the shared backing page. Because this
+ * page is a direct-map template buffer rather than a real vmemmap entry,
+ * set_page_count() must not be used here as its tracepoint calls
+ * page_to_pfn(). Keep the initialized state in sync with
+ * __init_single_page() and __init_zone_device_page().
+ */
+static void __ref init_fsdax_page_template(struct page *page,
+ unsigned long pfn, int nid,
+ struct dev_pagemap *pgmap)
+{
+ mm_zero_struct_page(page);
+ set_page_links(page, ZONE_DEVICE, nid, pfn);
+#ifdef WANT_PAGE_VIRTUAL
+ if (!is_highmem_idx(ZONE_DEVICE))
+ set_page_address(page, __va(pfn << PAGE_SHIFT));
+#endif
+ atomic_set(&page->_refcount, 0);
+ atomic_set(&page->_mapcount, -1);
+ page_cpupid_reset_last(page);
+ page_kasan_tag_reset(page);
+ __SetPageReserved(page);
+ page_folio(page)->pgmap = pgmap;
+}
+
static void zone_device_page_init_from_template(struct page *page,
unsigned long pfn, struct page *template)
{
@@ -1138,7 +1163,7 @@ void __ref memmap_init_zone_device(struct zone *zone,
struct page *page = page_address(pgmap->vmemmap_shared_page);
for (int i = 0; i < PAGE_SIZE / sizeof(struct page); i++)
- __init_zone_device_page(page + i, start_pfn + i,
- ZONE_DEVICE, nid, pgmap);
+ init_fsdax_page_template(page + i, start_pfn + i,
+ nid, pgmap);
goto pageblock_init;
}
--
2.39.5
