| Message ID | 20241202165921.249585-9-carlo.nonato@minervasys.tech (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Arm cache coloring | expand |
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.
On 02.12.2024 18:33, Carlo Nonato wrote: > Sorry guys, this patch is wrong. > Here's the correct one. Which looks okay to me now, just that imo ... > --- 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) ... this new #define now wants a comment, to clarify where the constant is to be used (or specifically not to be used). Unlike for PGC_no_buddy_merge this can't be easily deduced from the name. Jan
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; } }
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(-)