Re: [PATCH v4 2/3] xen/mm: allow deferred scrub of physmap populate allocated pages

[Jan Beulich <jbeulich@xxxxxxxx>]

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_*.

Jan