On 09.01.2026 15:46, Roger Pau Monné wrote:
> On Fri, Jan 09, 2026 at 11:22:39AM +0100, Jan Beulich wrote:
>> On 08.01.2026 18:55, Roger Pau Monne wrote:
>>> Such parameter allow requesting to scrub memory only from the specified
>>> node. If there's no memory to scrub from the requested node the function
>>> returns false. If the node is already being scrubbed from a different CPU
>>> the function returns true so the caller can differentiate whether there's
>>> still pending work to do.
>>
>> I'm really trying to understand both patches together, and peeking ahead I
>> don't understand the above, which looks to describe ...
>>
>>> --- a/xen/common/page_alloc.c
>>> +++ b/xen/common/page_alloc.c
>>> @@ -1339,16 +1339,27 @@ static void cf_check scrub_continue(void *data)
>>> }
>>> }
>>>
>>> -bool scrub_free_pages(void)
>>> +bool scrub_free_pages(nodeid_t node)
>>> {
>>> struct page_info *pg;
>>> unsigned int zone;
>>> unsigned int cpu = smp_processor_id();
>>> bool preempt = false;
>>> - nodeid_t node;
>>> unsigned int cnt = 0;
>>>
>>> - node = node_to_scrub(true);
>>> + if ( node != NUMA_NO_NODE )
>>> + {
>>> + if ( !node_need_scrub[node] )
>>> + /* Nothing to scrub. */
>>> + return false;
>>> +
>>> + if ( node_test_and_set(node, node_scrubbing) )
>>> + /* Another CPU is scrubbing it. */
>>> + return true;
>>
>> ... these two return-s. My problem being that patch 2 doesn't use the
>> return value (while existing callers don't take this path). Is this then
>> "just in case" for now (and making the meaning of the return values
>> somewhat inconsistent for the function as a whole)?
>
> I've added those so that the function return values are consistent,
> even if not consumed right now, it would make no sense for the return
> values to have different meaning when the node parameter is !=
> NUMA_NO_NODE. Or at least that was my impression.
>
> In fact an earlier version of patch 2 did consume those values. I've
> moved to a different approach, but I think it's good to keep the
> return values consistent regardless of the input parameters.
My point was though: The present "true" return doesn't mean "Another CPU
is scrubbing it." Instead it means "More work to do" aiui. That's similar
in a way, but not identical.
Jan
