On Thu, Jan 29, 2026 at 08:53:05AM +0100, Jan Beulich wrote:
> On 28.01.2026 20:06, Roger Pau Monné wrote:
> > On Wed, Jan 28, 2026 at 03:46:04PM +0100, Jan Beulich wrote:
> >> On 28.01.2026 13:03, Roger Pau Monne wrote:
> >>> @@ -275,7 +339,18 @@ static void populate_physmap(struct memop_args *a)
> >>> }
> >>> else
> >>> {
> >>> - page = alloc_domheap_pages(d, a->extent_order,
> >>> a->memflags);
> >>> + unsigned int scrub_start = 0;
> >>> + nodeid_t node =
> >>> + (a->memflags & MEMF_exact_node) ?
> >>> MEMF_get_node(a->memflags)
> >>> + : NUMA_NO_NODE;
> >>> +
> >>> + page = get_stashed_allocation(d, a->extent_order, node,
> >>> + &scrub_start);
> >>> +
> >>> + if ( !page )
> >>> + page = alloc_domheap_pages(d, a->extent_order,
> >>> + a->memflags | (d->creation_finished ? 0
> >>> + :
> >>> MEMF_no_scrub));
> >>
> >> I fear there's a more basic issue here that so far we didn't pay attention
> >> to:
> >> alloc_domheap_pages() is what invokes assign_page(), which in turn resets
> >> ->count_info for each of the pages. This reset includes setting
> >> PGC_allocated,
> >> which ...
> >>
> >>> @@ -286,6 +361,30 @@ static void populate_physmap(struct memop_args *a)
> >>> goto out;
> >>> }
> >>>
> >>> + if ( !d->creation_finished )
> >>> + {
> >>> + unsigned int dirty_cnt = 0;
> >>> +
> >>> + /* Check if there's anything to scrub. */
> >>> + for ( j = scrub_start; j < (1U << a->extent_order);
> >>> j++ )
> >>> + {
> >>> + if ( !test_and_clear_bit(_PGC_need_scrub,
> >>> + &page[j].count_info) )
> >>> + continue;
> >>
> >> ... means we will now scrub every page in the block, not just those which
> >> weren't
> >> scrubbed yet, and we end up clearing PGC_allocated. All because of
> >> PGC_need_scrub
> >> aliasing PGC_allocated. I wonder how this didn't end up screwing any
> >> testing you
> >> surely will have done. Or maybe I'm completely off here?
> >
> > Thanks for spotting this! No, I didn't see any issues. I don't see
> > any check for PGC_allocated in free_domheap_pages(), which could
> > explain the lack of failures?
>
> Maybe. PGC_allocated consumes a page ref, so I would have expected accounting
> issues.
>
> > I will have to allocate with MEMF_no_owner and then do the
> > assign_pages() call from populate_physmap() after the scrubbing is
> > done. Maybe that would work. Memory allocated using MEMF_no_owner
> > still consumes the claim pool if a domain parameter is passed to
> > alloc_heap_pages().
>
> Technically this looks like it might work, but it's feeling as if this was
> getting increasingly fragile. I'm also not quite sure whether MEMF_no_owner
> allocations should consume claimed pages. Imo there are arguments both in
> favor and against such behavior.
>
> We may want to explore the alternative of un-aliasing the two PGC_*.
I expected the PGC_ bits would be all consumed, but I see there's a
range that are still empty, so it might indeed be easier to remove the
alias. Let me give that a try.
Thanks, Roger.
