Hi Jan,
On Tue, Nov 5, 2024 at 4:59 PM Jan Beulich <jbeul...@suse.com> wrote:
>
> On 25.10.2024 11:50, Carlo Nonato wrote:
> > PGC_static and PGC_extra needs to be preserved when assigning a page.
> > Define a new macro that groups those flags and use it instead of or'ing
> > every time.
> >
> > To make preserved flags even more meaningful, they are kept also when
> > switching state in mark_page_free().
> > Enforce the removal of PGC_extra before freeing new pages as this is
> > considered an error and can cause ASSERT violations.
>
> What does "new" here mean?
I believe it means "assigned to domains". So yes "new" it's definitely wrong.
> > Signed-off-by: Carlo Nonato <carlo.non...@minervasys.tech>
> > ---
> > v9:
> > - add PGC_broken to PGC_preserved
>
> Which then also wants reflecting in the description.
Yep.
> > @@ -2485,6 +2485,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;
> > + }
>
> The description doesn't cover this, only ...
PGC_extra must be cleared before freeing pages. Since here we're going to free
pages, PGC_extra must be removed.
> > @@ -2539,6 +2547,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;
> > }
> > }
>
> ... this is mentioned there. And it's not clear to me why it would need doing
> in both places.
>
> Jan
- Carlo
