* [PATCH v2 0/3] xen/mm: fix fallout from populate_physmap() deferred scrub change
@ 2026-03-26 8:51 Roger Pau Monne
2026-03-26 8:51 ` [PATCH v2 1/3] xen/mm: don't unconditionally clear PGC_need_scrub in alloc_heap_pages() Roger Pau Monne
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Roger Pau Monne @ 2026-03-26 8:51 UTC (permalink / raw)
To: xen-devel
Cc: Roger Pau Monne, Andrew Cooper, Anthony PERARD, Michal Orzel,
Jan Beulich, Julien Grall, Stefano Stabellini
Hello,
Two fixes for the populate_physmap() deferred scrubbing changes, plus an
improvement.
Thanks, Roger.
Roger Pau Monne (3):
xen/mm: don't unconditionally clear PGC_need_scrub in
alloc_heap_pages()
xen/mm: do not assign pages to a domain until they are scrubbed
xen/mm: improve freeing of partially scrubbed pages
xen/common/memory.c | 13 +++++++---
xen/common/page_alloc.c | 56 +++++++++++++++++++++++++++++++----------
xen/include/xen/mm.h | 14 +++++++++++
3 files changed, 67 insertions(+), 16 deletions(-)
--
2.51.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 1/3] xen/mm: don't unconditionally clear PGC_need_scrub in alloc_heap_pages()
2026-03-26 8:51 [PATCH v2 0/3] xen/mm: fix fallout from populate_physmap() deferred scrub change Roger Pau Monne
@ 2026-03-26 8:51 ` Roger Pau Monne
2026-03-26 8:51 ` [PATCH v2 2/3] xen/mm: do not assign pages to a domain until they are scrubbed Roger Pau Monne
2026-03-26 8:51 ` [PATCH v2 3/3] xen/mm: improve freeing of partially scrubbed pages Roger Pau Monne
2 siblings, 0 replies; 8+ messages in thread
From: Roger Pau Monne @ 2026-03-26 8:51 UTC (permalink / raw)
To: xen-devel
Cc: Roger Pau Monne, Andrew Cooper, Anthony PERARD, Michal Orzel,
Jan Beulich, Julien Grall, Stefano Stabellini, Ayden Bottos
alloc_heap_pages() will unconditionally clear PGC_need_scrub, even when
MEMF_no_scrub is requested. This is kind of expected as otherwise some
callers will assert on seeing non-expected flags set on the count_info
field.
Introduce a new MEMF bit to signal to alloc_heap_pages() that non-scrubbed
pages should keep the PGC_need_scrub bit set. This fixes returning dirty
pages from alloc_domheap_pages() without the PGC_need_scrub bit set for
populate_physmap() to consume.
With the above change alloc_domheap_pages() needs an adjustment to cope
with allocated pages possibly having the PGC_need_scrub set.
Fixes: 83a784a15b47 ("xen/mm: allow deferred scrub of physmap populate allocated pages")
Reported-by: Ayden Bottos <aydenbottos12@gmail.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
xen/common/memory.c | 3 ++-
xen/common/page_alloc.c | 31 ++++++++++++++++++++++---------
xen/include/xen/mm.h | 3 +++
3 files changed, 27 insertions(+), 10 deletions(-)
diff --git a/xen/common/memory.c b/xen/common/memory.c
index 918510f287a0..f0ff1311881c 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -345,7 +345,8 @@ static void populate_physmap(struct memop_args *a)
unsigned int scrub_start = 0;
unsigned int memflags =
a->memflags | (d->creation_finished ? 0
- : MEMF_no_scrub);
+ : (MEMF_no_scrub |
+ MEMF_keep_scrub));
nodeid_t node =
(a->memflags & MEMF_exact_node) ? MEMF_get_node(a->memflags)
: NUMA_NO_NODE;
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 588b5b99cbc7..1316dfbd15ee 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -989,6 +989,8 @@ static struct page_info *alloc_heap_pages(
ASSERT(zone_lo <= zone_hi);
ASSERT(zone_hi < NR_ZONES);
+ ASSERT(!(memflags & MEMF_keep_scrub) || (memflags & MEMF_no_scrub));
+
if ( unlikely(order > MAX_ORDER) )
return NULL;
@@ -1110,17 +1112,26 @@ static struct page_info *alloc_heap_pages(
{
bool cold = d && d != current->domain;
- for ( i = 0; i < (1U << order); i++ )
+ if ( !(memflags & MEMF_no_scrub) )
{
- if ( test_and_clear_bit(_PGC_need_scrub, &pg[i].count_info) )
+ for ( i = 0; i < (1U << order); i++ )
{
- if ( !(memflags & MEMF_no_scrub) )
+ if ( test_and_clear_bit(_PGC_need_scrub, &pg[i].count_info) )
+ {
scrub_one_page(&pg[i], cold);
-
- dirty_cnt++;
+ dirty_cnt++;
+ }
+ else
+ check_one_page(&pg[i]);
}
- else if ( !(memflags & MEMF_no_scrub) )
- check_one_page(&pg[i]);
+ }
+ else
+ {
+ for ( i = 0; i < (1U << order); i++ )
+ if ( (memflags & MEMF_keep_scrub)
+ ? test_bit(_PGC_need_scrub, &pg[i].count_info)
+ : test_and_clear_bit(_PGC_need_scrub, &pg[i].count_info) )
+ dirty_cnt++;
}
if ( dirty_cnt )
@@ -2696,8 +2707,10 @@ struct page_info *alloc_domheap_pages(
for ( i = 0; i < (1UL << order); i++ )
{
- ASSERT(!pg[i].count_info);
- pg[i].count_info = PGC_extra;
+ ASSERT(!(pg[i].count_info &
+ ~((memflags & MEMF_keep_scrub) ? PGC_need_scrub
+ : 0UL)));
+ pg[i].count_info |= PGC_extra;
}
}
if ( assign_page(pg, order, d, memflags) )
diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index d80bfba6d393..5e786c874a73 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -208,6 +208,9 @@ struct npfec {
#define MEMF_no_refcount (1U<<_MEMF_no_refcount)
#define _MEMF_populate_on_demand 1
#define MEMF_populate_on_demand (1U<<_MEMF_populate_on_demand)
+/* MEMF_keep_scrub is only valid when specified together with MEMF_no_scrub. */
+#define _MEMF_keep_scrub 2
+#define MEMF_keep_scrub (1U << _MEMF_keep_scrub)
#define _MEMF_no_dma 3
#define MEMF_no_dma (1U<<_MEMF_no_dma)
#define _MEMF_exact_node 4
--
2.51.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 2/3] xen/mm: do not assign pages to a domain until they are scrubbed
2026-03-26 8:51 [PATCH v2 0/3] xen/mm: fix fallout from populate_physmap() deferred scrub change Roger Pau Monne
2026-03-26 8:51 ` [PATCH v2 1/3] xen/mm: don't unconditionally clear PGC_need_scrub in alloc_heap_pages() Roger Pau Monne
@ 2026-03-26 8:51 ` Roger Pau Monne
2026-03-26 11:51 ` Jan Beulich
2026-03-26 8:51 ` [PATCH v2 3/3] xen/mm: improve freeing of partially scrubbed pages Roger Pau Monne
2 siblings, 1 reply; 8+ messages in thread
From: Roger Pau Monne @ 2026-03-26 8:51 UTC (permalink / raw)
To: xen-devel
Cc: Roger Pau Monne, Andrew Cooper, Anthony PERARD, Michal Orzel,
Jan Beulich, Julien Grall, Stefano Stabellini
Assigning pages to a domain make them the possible target of hypercalls
like XENMEM_decrease_reservation ahead of such pages being scrubbed in
populate_physmap() when the guest is running in PV mode. This might allow
pages to be freed ahead of being scrubbed for example, as a stubdomain
already running could target them by guessing their MFNs. It's also
possible other action could set the page type ahead of scrubbing, which
would be problematic.
Prevent the pages pending scrub from being assigned to the domain, and only
do the assign once the scrubbing has finished. This has the disadvantage
that the allocated pages will be removed from the free pool, but not yet
accounted towards the domain consumed page quota. However there can only
be one stashed page in that state, and it's maximum size is bounded by the
memop-max-order option. This is not too different from the current logic,
where assigning pages to a domain (and thus checking whether such domain
doesn't overflow it's quota) is also done after the memory has been
allocated and removed from the pool of free pages.
Fixes: 83a784a15b47 ("xen/mm: allow deferred scrub of physmap populate allocated pages")
Reported-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
xen/common/memory.c | 6 ++++++
xen/common/page_alloc.c | 9 ++++++++-
xen/include/xen/mm.h | 7 ++++++-
3 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/xen/common/memory.c b/xen/common/memory.c
index f0ff1311881c..1ad4b51c5b02 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -388,6 +388,12 @@ static void populate_physmap(struct memop_args *a)
goto out;
}
}
+
+ if ( assign_page(page, a->extent_order, d, memflags) )
+ {
+ free_domheap_pages(page, a->extent_order);
+ goto out;
+ }
}
if ( unlikely(a->memflags & MEMF_no_tlbflush) )
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 1316dfbd15ee..b1edef87124f 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -2713,7 +2713,14 @@ struct page_info *alloc_domheap_pages(
pg[i].count_info |= PGC_extra;
}
}
- if ( assign_page(pg, order, d, memflags) )
+ /*
+ * Don't add pages with the PGC_need_scrub bit set to the domain, the
+ * caller must clean the bit and then manually call assign_pages().
+ * Otherwise pages still subject to scrubbing would be reachable using
+ * get_page().
+ */
+ if ( !(memflags & MEMF_keep_scrub) &&
+ assign_page(pg, order, d, memflags) )
{
free_heap_pages(pg, order, memflags & MEMF_no_scrub);
return NULL;
diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index 5e786c874a73..b80bec00c124 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -208,7 +208,12 @@ struct npfec {
#define MEMF_no_refcount (1U<<_MEMF_no_refcount)
#define _MEMF_populate_on_demand 1
#define MEMF_populate_on_demand (1U<<_MEMF_populate_on_demand)
-/* MEMF_keep_scrub is only valid when specified together with MEMF_no_scrub. */
+/*
+ * MEMF_keep_scrub is only valid when specified together with MEMF_no_scrub.
+ * Allocations with this flag never assign the pages to the domain, the caller
+ * must call assign_page() after the PGC_need_scrub bit is cleared if
+ * required.
+ */
#define _MEMF_keep_scrub 2
#define MEMF_keep_scrub (1U << _MEMF_keep_scrub)
#define _MEMF_no_dma 3
--
2.51.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 3/3] xen/mm: improve freeing of partially scrubbed pages
2026-03-26 8:51 [PATCH v2 0/3] xen/mm: fix fallout from populate_physmap() deferred scrub change Roger Pau Monne
2026-03-26 8:51 ` [PATCH v2 1/3] xen/mm: don't unconditionally clear PGC_need_scrub in alloc_heap_pages() Roger Pau Monne
2026-03-26 8:51 ` [PATCH v2 2/3] xen/mm: do not assign pages to a domain until they are scrubbed Roger Pau Monne
@ 2026-03-26 8:51 ` Roger Pau Monne
2026-03-26 11:50 ` Jan Beulich
2 siblings, 1 reply; 8+ messages in thread
From: Roger Pau Monne @ 2026-03-26 8:51 UTC (permalink / raw)
To: xen-devel
Cc: Roger Pau Monne, Andrew Cooper, Anthony PERARD, Michal Orzel,
Jan Beulich, Julien Grall, Stefano Stabellini
When freeing possibly partially scrubbed pages in populate_physmap() the
whole page is marked as dirty, but that's not fully accurate. Since the
PGC_need_scrub bit is preserved for the populate_physmap() allocation we
can use those when freeing to detect which pages need scrubbing instead of
marking the whole page as dirty.
This requires exposing free_heap_pages() globally, and switching
populate_physmap() to use it instead of free_domheap_pages().
Suggested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Jan: I'm not sure if that's what you suggested in the review of v1. I've
added your Suggested-by but I can drop it if that's not what you were
thinking of.
---
xen/common/memory.c | 6 +++---
xen/common/page_alloc.c | 16 +++++++++++++---
xen/include/xen/mm.h | 6 ++++++
3 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/xen/common/memory.c b/xen/common/memory.c
index 1ad4b51c5b02..68eef8291571 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -177,7 +177,7 @@ static void stash_allocation(struct domain *d, struct page_info *page,
* interface is designed to be used for single-threaded domain creation.
*/
if ( d->pending_scrub || d->is_dying )
- free_domheap_pages(page, order);
+ free_heap_pages(page, order, false);
else
{
d->pending_scrub_index = scrub_index;
@@ -210,7 +210,7 @@ static struct page_info *get_stashed_allocation(struct domain *d,
*scrub_index = d->pending_scrub_index;
}
else
- free_domheap_pages(d->pending_scrub, d->pending_scrub_order);
+ free_heap_pages(d->pending_scrub, d->pending_scrub_order, false);
/*
* The caller now owns the page or it has been freed, clear stashed
@@ -391,7 +391,7 @@ static void populate_physmap(struct memop_args *a)
if ( assign_page(page, a->extent_order, d, memflags) )
{
- free_domheap_pages(page, a->extent_order);
+ free_heap_pages(page, a->extent_order, false);
goto out;
}
}
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index b1edef87124f..8fc9b5a27f1b 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -1529,13 +1529,13 @@ static bool mark_page_free(struct page_info *pg, mfn_t mfn)
static void free_color_heap_page(struct page_info *pg, bool need_scrub);
/* Free 2^@order set of pages. */
-static void free_heap_pages(
- struct page_info *pg, unsigned int order, bool need_scrub)
+void free_heap_pages(struct page_info *pg, unsigned int order, bool need_scrub)
{
unsigned long mask;
mfn_t mfn = page_to_mfn(pg);
unsigned int i, node = mfn_to_nid(mfn);
unsigned int zone = page_to_zone(pg);
+ unsigned int first_dirty = INVALID_DIRTY_IDX, dirty_cnt = 0;
bool pg_offlined = false;
ASSERT(order <= MAX_ORDER);
@@ -1552,6 +1552,13 @@ static void free_heap_pages(
pg[i].count_info |= PGC_need_scrub;
poison_one_page(&pg[i]);
}
+ else if ( test_bit(_PGC_need_scrub, &pg[i].count_info) )
+ {
+ /* The caller might have returned pages pending scrub. */
+ if ( first_dirty == INVALID_DIRTY_IDX )
+ first_dirty = i;
+ dirty_cnt++;
+ }
if ( pg->count_info & PGC_colored )
{
@@ -1571,7 +1578,10 @@ static void free_heap_pages(
pg->u.free.first_dirty = 0;
}
else
- pg->u.free.first_dirty = INVALID_DIRTY_IDX;
+ {
+ node_need_scrub[node] += dirty_cnt;
+ pg->u.free.first_dirty = first_dirty;
+ }
/* Merge chunks as far as possible. */
while ( order < MAX_ORDER )
diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index b80bec00c124..0b192caa07bc 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -153,6 +153,12 @@ unsigned long avail_node_heap_pages(unsigned int nodeid);
} while ( false )
#define FREE_DOMHEAP_PAGE(p) FREE_DOMHEAP_PAGES(p, 0)
+/*
+ * Most callers should use free_{xen,dom}heap_pages() instead of directly
+ * calling free_heap_pages().
+ */
+void free_heap_pages(struct page_info *pg, unsigned int order, bool need_scrub);
+
void scrub_one_page(const struct page_info *pg, bool cold);
int online_page(mfn_t mfn, uint32_t *status);
--
2.51.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 3/3] xen/mm: improve freeing of partially scrubbed pages
2026-03-26 8:51 ` [PATCH v2 3/3] xen/mm: improve freeing of partially scrubbed pages Roger Pau Monne
@ 2026-03-26 11:50 ` Jan Beulich
2026-03-26 15:53 ` Roger Pau Monné
0 siblings, 1 reply; 8+ messages in thread
From: Jan Beulich @ 2026-03-26 11:50 UTC (permalink / raw)
To: Roger Pau Monne
Cc: Andrew Cooper, Anthony PERARD, Michal Orzel, Julien Grall,
Stefano Stabellini, xen-devel
On 26.03.2026 09:51, Roger Pau Monne wrote:
> When freeing possibly partially scrubbed pages in populate_physmap() the
> whole page is marked as dirty, but that's not fully accurate. Since the
> PGC_need_scrub bit is preserved for the populate_physmap() allocation we
> can use those when freeing to detect which pages need scrubbing instead of
> marking the whole page as dirty.
>
> This requires exposing free_heap_pages() globally, and switching
> populate_physmap() to use it instead of free_domheap_pages().
>
> Suggested-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> Jan: I'm not sure if that's what you suggested in the review of v1. I've
> added your Suggested-by but I can drop it if that's not what you were
> thinking of.
You're going quite a bit farther. In my comment I really only meant the one
new use you add in patch 2 (in which case no changes to the body of
free_heap_pages() would have been needed, and hence why I thought that it
could maybe be done right there). Up to you whether to keep the tag.
> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -177,7 +177,7 @@ static void stash_allocation(struct domain *d, struct page_info *page,
> * interface is designed to be used for single-threaded domain creation.
> */
> if ( d->pending_scrub || d->is_dying )
> - free_domheap_pages(page, order);
> + free_heap_pages(page, order, false);
> else
> {
> d->pending_scrub_index = scrub_index;
> @@ -210,7 +210,7 @@ static struct page_info *get_stashed_allocation(struct domain *d,
> *scrub_index = d->pending_scrub_index;
> }
> else
> - free_domheap_pages(d->pending_scrub, d->pending_scrub_order);
> + free_heap_pages(d->pending_scrub, d->pending_scrub_order, false);
>
> /*
> * The caller now owns the page or it has been freed, clear stashed
> @@ -391,7 +391,7 @@ static void populate_physmap(struct memop_args *a)
>
> if ( assign_page(page, a->extent_order, d, memflags) )
> {
> - free_domheap_pages(page, a->extent_order);
> + free_heap_pages(page, a->extent_order, false);
> goto out;
> }
> }
Along with all of these there's then also domain_pending_scrub_free().
> --- a/xen/include/xen/mm.h
> +++ b/xen/include/xen/mm.h
> @@ -153,6 +153,12 @@ unsigned long avail_node_heap_pages(unsigned int nodeid);
> } while ( false )
> #define FREE_DOMHEAP_PAGE(p) FREE_DOMHEAP_PAGES(p, 0)
>
> +/*
> + * Most callers should use free_{xen,dom}heap_pages() instead of directly
> + * calling free_heap_pages().
> + */
> +void free_heap_pages(struct page_info *pg, unsigned int order, bool need_scrub);
Might we better not put this here, but instead in a private header in common/?
Jan
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/3] xen/mm: do not assign pages to a domain until they are scrubbed
2026-03-26 8:51 ` [PATCH v2 2/3] xen/mm: do not assign pages to a domain until they are scrubbed Roger Pau Monne
@ 2026-03-26 11:51 ` Jan Beulich
0 siblings, 0 replies; 8+ messages in thread
From: Jan Beulich @ 2026-03-26 11:51 UTC (permalink / raw)
To: Roger Pau Monne
Cc: Andrew Cooper, Anthony PERARD, Michal Orzel, Julien Grall,
Stefano Stabellini, xen-devel
On 26.03.2026 09:51, Roger Pau Monne wrote:
> Assigning pages to a domain make them the possible target of hypercalls
> like XENMEM_decrease_reservation ahead of such pages being scrubbed in
> populate_physmap() when the guest is running in PV mode. This might allow
> pages to be freed ahead of being scrubbed for example, as a stubdomain
> already running could target them by guessing their MFNs. It's also
> possible other action could set the page type ahead of scrubbing, which
> would be problematic.
>
> Prevent the pages pending scrub from being assigned to the domain, and only
> do the assign once the scrubbing has finished. This has the disadvantage
> that the allocated pages will be removed from the free pool, but not yet
> accounted towards the domain consumed page quota. However there can only
> be one stashed page in that state, and it's maximum size is bounded by the
> memop-max-order option. This is not too different from the current logic,
> where assigning pages to a domain (and thus checking whether such domain
> doesn't overflow it's quota) is also done after the memory has been
> allocated and removed from the pool of free pages.
>
> Fixes: 83a784a15b47 ("xen/mm: allow deferred scrub of physmap populate allocated pages")
> Reported-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 3/3] xen/mm: improve freeing of partially scrubbed pages
2026-03-26 11:50 ` Jan Beulich
@ 2026-03-26 15:53 ` Roger Pau Monné
2026-03-26 16:05 ` Jan Beulich
0 siblings, 1 reply; 8+ messages in thread
From: Roger Pau Monné @ 2026-03-26 15:53 UTC (permalink / raw)
To: Jan Beulich
Cc: Andrew Cooper, Anthony PERARD, Michal Orzel, Julien Grall,
Stefano Stabellini, xen-devel
On Thu, Mar 26, 2026 at 12:50:27PM +0100, Jan Beulich wrote:
> On 26.03.2026 09:51, Roger Pau Monne wrote:
> > When freeing possibly partially scrubbed pages in populate_physmap() the
> > whole page is marked as dirty, but that's not fully accurate. Since the
> > PGC_need_scrub bit is preserved for the populate_physmap() allocation we
> > can use those when freeing to detect which pages need scrubbing instead of
> > marking the whole page as dirty.
> >
> > This requires exposing free_heap_pages() globally, and switching
> > populate_physmap() to use it instead of free_domheap_pages().
> >
> > Suggested-by: Jan Beulich <jbeulich@suse.com>
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > ---
> > Jan: I'm not sure if that's what you suggested in the review of v1. I've
> > added your Suggested-by but I can drop it if that's not what you were
> > thinking of.
>
> You're going quite a bit farther. In my comment I really only meant the one
> new use you add in patch 2 (in which case no changes to the body of
> free_heap_pages() would have been needed, and hence why I thought that it
> could maybe be done right there). Up to you whether to keep the tag.
I see, you meant to change the single usage in case assign_page()
fails. I think going a bit further is fine, seeing the adjustment to
free_heap_pages() is very minimal?
> > --- a/xen/common/memory.c
> > +++ b/xen/common/memory.c
> > @@ -177,7 +177,7 @@ static void stash_allocation(struct domain *d, struct page_info *page,
> > * interface is designed to be used for single-threaded domain creation.
> > */
> > if ( d->pending_scrub || d->is_dying )
> > - free_domheap_pages(page, order);
> > + free_heap_pages(page, order, false);
> > else
> > {
> > d->pending_scrub_index = scrub_index;
> > @@ -210,7 +210,7 @@ static struct page_info *get_stashed_allocation(struct domain *d,
> > *scrub_index = d->pending_scrub_index;
> > }
> > else
> > - free_domheap_pages(d->pending_scrub, d->pending_scrub_order);
> > + free_heap_pages(d->pending_scrub, d->pending_scrub_order, false);
> >
> > /*
> > * The caller now owns the page or it has been freed, clear stashed
> > @@ -391,7 +391,7 @@ static void populate_physmap(struct memop_args *a)
> >
> > if ( assign_page(page, a->extent_order, d, memflags) )
> > {
> > - free_domheap_pages(page, a->extent_order);
> > + free_heap_pages(page, a->extent_order, false);
> > goto out;
> > }
> > }
>
> Along with all of these there's then also domain_pending_scrub_free().
Yes, indeed.
> > --- a/xen/include/xen/mm.h
> > +++ b/xen/include/xen/mm.h
> > @@ -153,6 +153,12 @@ unsigned long avail_node_heap_pages(unsigned int nodeid);
> > } while ( false )
> > #define FREE_DOMHEAP_PAGE(p) FREE_DOMHEAP_PAGES(p, 0)
> >
> > +/*
> > + * Most callers should use free_{xen,dom}heap_pages() instead of directly
> > + * calling free_heap_pages().
> > + */
> > +void free_heap_pages(struct page_info *pg, unsigned int order, bool need_scrub);
>
> Might we better not put this here, but instead in a private header in common/?
No strong opinion. It could logically be used outside of common in
principle, hence we might end up moving it anyway. Would you prefer
me to introduce a common/memory.h header with just this prototype?
Thanks, Roger.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 3/3] xen/mm: improve freeing of partially scrubbed pages
2026-03-26 15:53 ` Roger Pau Monné
@ 2026-03-26 16:05 ` Jan Beulich
0 siblings, 0 replies; 8+ messages in thread
From: Jan Beulich @ 2026-03-26 16:05 UTC (permalink / raw)
To: Roger Pau Monné
Cc: Andrew Cooper, Anthony PERARD, Michal Orzel, Julien Grall,
Stefano Stabellini, xen-devel
On 26.03.2026 16:53, Roger Pau Monné wrote:
> On Thu, Mar 26, 2026 at 12:50:27PM +0100, Jan Beulich wrote:
>> On 26.03.2026 09:51, Roger Pau Monne wrote:
>>> When freeing possibly partially scrubbed pages in populate_physmap() the
>>> whole page is marked as dirty, but that's not fully accurate. Since the
>>> PGC_need_scrub bit is preserved for the populate_physmap() allocation we
>>> can use those when freeing to detect which pages need scrubbing instead of
>>> marking the whole page as dirty.
>>>
>>> This requires exposing free_heap_pages() globally, and switching
>>> populate_physmap() to use it instead of free_domheap_pages().
>>>
>>> Suggested-by: Jan Beulich <jbeulich@suse.com>
>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>>> ---
>>> Jan: I'm not sure if that's what you suggested in the review of v1. I've
>>> added your Suggested-by but I can drop it if that's not what you were
>>> thinking of.
>>
>> You're going quite a bit farther. In my comment I really only meant the one
>> new use you add in patch 2 (in which case no changes to the body of
>> free_heap_pages() would have been needed, and hence why I thought that it
>> could maybe be done right there). Up to you whether to keep the tag.
>
> I see, you meant to change the single usage in case assign_page()
> fails. I think going a bit further is fine, seeing the adjustment to
> free_heap_pages() is very minimal?
Oh, yes, sure. I was merely trying to address your remark.
>>> --- a/xen/include/xen/mm.h
>>> +++ b/xen/include/xen/mm.h
>>> @@ -153,6 +153,12 @@ unsigned long avail_node_heap_pages(unsigned int nodeid);
>>> } while ( false )
>>> #define FREE_DOMHEAP_PAGE(p) FREE_DOMHEAP_PAGES(p, 0)
>>>
>>> +/*
>>> + * Most callers should use free_{xen,dom}heap_pages() instead of directly
>>> + * calling free_heap_pages().
>>> + */
>>> +void free_heap_pages(struct page_info *pg, unsigned int order, bool need_scrub);
>>
>> Might we better not put this here, but instead in a private header in common/?
>
> No strong opinion. It could logically be used outside of common in
> principle, hence we might end up moving it anyway. Would you prefer
> me to introduce a common/memory.h header with just this prototype?
It would help if others could voice an opinion. To me exposing this
supposedly internal (to the page allocator) function feels a little
risky. Yet of course any undue use would likely be spotted and objected
to during review.
Jan
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-03-26 16:06 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-26 8:51 [PATCH v2 0/3] xen/mm: fix fallout from populate_physmap() deferred scrub change Roger Pau Monne
2026-03-26 8:51 ` [PATCH v2 1/3] xen/mm: don't unconditionally clear PGC_need_scrub in alloc_heap_pages() Roger Pau Monne
2026-03-26 8:51 ` [PATCH v2 2/3] xen/mm: do not assign pages to a domain until they are scrubbed Roger Pau Monne
2026-03-26 11:51 ` Jan Beulich
2026-03-26 8:51 ` [PATCH v2 3/3] xen/mm: improve freeing of partially scrubbed pages Roger Pau Monne
2026-03-26 11:50 ` Jan Beulich
2026-03-26 15:53 ` Roger Pau Monné
2026-03-26 16:05 ` Jan Beulich
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.