Hi all,
On Mon, Dec 2, 2024 at 5:59 PM Carlo Nonato
<carlo.nonato@minervasys.tech> wrote:
>
> PGC_static and PGC_extra need to be preserved when assigning a page.
> Define a new macro that groups those flags and use it instead of or'ing
> every time.
>
> Signed-off-by: Carlo Nonato <carlo.nonato@minervasys.tech>
> ---
> v11:
> - removed PGC_broken from PGC_preserved
> - removed PGC preservation from mark_page_free()
> v10:
> - fixed commit message
> v9:
> - add PGC_broken to PGC_preserved
> - clear PGC_extra in alloc_domheap_pages() only if MEMF_no_refcount is set
> v8:
> - fixed PGC_extra ASSERT fail in alloc_domheap_pages() by removing PGC_extra
> before freeing
> v7:
> - PGC_preserved used also in mark_page_free()
> v6:
> - preserved_flags renamed to PGC_preserved
> - PGC_preserved is used only in assign_pages()
> v5:
> - new patch
> ---
> xen/common/page_alloc.c | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
> index 55d561e93c..e73b404169 100644
> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -161,6 +161,7 @@
> #endif
>
> #define PGC_no_buddy_merge PGC_static
> +#define PGC_preserved (PGC_extra | PGC_static)
>
> #ifndef PGT_TYPE_INFO_INITIALIZER
> #define PGT_TYPE_INFO_INITIALIZER 0
> @@ -1447,8 +1448,7 @@ static bool mark_page_free(struct page_info *pg, mfn_t mfn)
> break;
>
> case PGC_state_offlining:
> - pg->count_info = (pg->count_info & PGC_broken) |
> - PGC_state_offlined;
> + pg->count_info = (pg->count_info & PGC_broken) | PGC_state_offlined;
> pg_offlined = true;
> break;
>
> @@ -2382,7 +2382,7 @@ int assign_pages(
>
> for ( i = 0; i < nr; i++ )
> {
> - ASSERT(!(pg[i].count_info & ~(PGC_extra | PGC_static)));
> + ASSERT(!(pg[i].count_info & ~PGC_preserved));
> if ( pg[i].count_info & PGC_extra )
> extra_pages++;
> }
> @@ -2442,7 +2442,7 @@ int assign_pages(
> page_set_owner(&pg[i], d);
> smp_wmb(); /* Domain pointer must be visible before updating refcnt. */
> pg[i].count_info =
> - (pg[i].count_info & (PGC_extra | PGC_static)) | PGC_allocated | 1;
> + (pg[i].count_info & PGC_preserved) | PGC_allocated | 1;
>
> page_list_add_tail(&pg[i], page_to_list(d, &pg[i]));
> }
> @@ -2501,6 +2501,14 @@ struct page_info *alloc_domheap_pages(
> }
> if ( assign_page(pg, order, d, memflags) )
> {
> + if ( memflags & MEMF_no_refcount )
> + {
> + unsigned long i;
> +
> + for ( i = 0; i < (1UL << order); i++ )
> + pg[i].count_info &= ~PGC_extra;
> + }
> +
> free_heap_pages(pg, order, memflags & MEMF_no_scrub);
> return NULL;
> }
> @@ -2555,6 +2563,7 @@ void free_domheap_pages(struct page_info *pg, unsigned int order)
> {
> ASSERT(d->extra_pages);
> d->extra_pages--;
> + pg[i].count_info &= ~PGC_extra;
> }
> }
>
> --
> 2.43.0
>
Sorry guys, this patch is wrong.
Here's the correct one.
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 55d561e93c..1c0990b180 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -161,6 +161,7 @@
#endif
#define PGC_no_buddy_merge PGC_static
+#define PGC_preserved (PGC_extra | PGC_static)
#ifndef PGT_TYPE_INFO_INITIALIZER
#define PGT_TYPE_INFO_INITIALIZER 0
@@ -2382,7 +2383,7 @@ int assign_pages(
for ( i = 0; i < nr; i++ )
{
- ASSERT(!(pg[i].count_info & ~(PGC_extra | PGC_static)));
+ ASSERT(!(pg[i].count_info & ~PGC_preserved));
if ( pg[i].count_info & PGC_extra )
extra_pages++;
}
@@ -2442,7 +2443,7 @@ int assign_pages(
page_set_owner(&pg[i], d);
smp_wmb(); /* Domain pointer must be visible before updating refcnt. */
pg[i].count_info =
- (pg[i].count_info & (PGC_extra | PGC_static)) | PGC_allocated | 1;
+ (pg[i].count_info & PGC_preserved) | PGC_allocated | 1;
page_list_add_tail(&pg[i], page_to_list(d, &pg[i]));
}
Sorry about that.
Thanks.