Re: [PATCH v2 1/1] mm: reduce NODE_RECLAIM_xxx and change to enum

[Zi Yan]

On 12 Jun 2026, at 10:28, Brendan Jackman wrote:

> On Fri Jun 12, 2026 at 11:31 AM UTC, Petr Tesarik wrote:
>>>>
>>>> - NODE_RECLAIM_NOSCAN  -> NODE_RECLAIM_NONE
>>>> - NODE_RECLAIM_FULL    -> NODE_RECLAIM_NONE
>>>> - NODE_RECLAIM_SOME    -> NODE_RECLAIM_SUCCESS
>>>> - NODE_RECLAIM_SUCCESS -> NODE_RECLAIM_SUCCESS
>
>>>> --- a/mm/internal.h
>>>> +++ b/mm/internal.h
>>>> @@ -1373,23 +1373,24 @@ static inline void mminit_verify_zonelist(void)
>>>>  }
>>>>  #endif /* CONFIG_DEBUG_MEMORY_INIT */
>>>>
>>>> -#define NODE_RECLAIM_NOSCAN	-2
>>>> -#define NODE_RECLAIM_FULL	-1
>>>> -#define NODE_RECLAIM_SOME	0
>>>> -#define NODE_RECLAIM_SUCCESS	1
>>>> +enum node_reclaim {
>>>> +	NODE_RECLAIM_NONE,
>>>> +	NODE_RECLAIM_SUCCESS,
>>>> +};
>
>>>> -	ret = __node_reclaim(pgdat, gfp_mask, nr_pages, &sc) >= nr_pages;
>>>> +	nr_reclaimed = __node_reclaim(pgdat, gfp_mask, nr_pages, &sc);
>>>>  	clear_bit_unlock(PGDAT_RECLAIM_LOCKED, &pgdat->flags);
>>>>
>>>> -	if (ret)
>>>> +	if (nr_reclaimed >= nr_pages)
>>>>  		count_vm_event(PGSCAN_ZONE_RECLAIM_SUCCESS);
>>>>  	else
>>>>  		count_vm_event(PGSCAN_ZONE_RECLAIM_FAILED);
>>>>
>>>> -	return ret;
>>>> +	return NODE_RECLAIM_SUCCESS;
>>>
>>> Should that be returning NODE_RECLAIM_NONE when !ret?
>>
>> No. That's the thing. Before my patch, the return value here was either
>> NODE_RECLAIM_SUCCESS (when at least nr_pages were reclaimed), or
>> NODE_RECLAIM_SOME (if less than nr_pages were reclaimed), but the caller
>> makes no distinction, handling both cases in the default label of a
>> switch statement.
>
> Agh! Sorry. It's good that you are cleaning this up :D
>
> So in that case my question is: is it intended that we call
> zone_watermark_ok() in the case that we reclaimed 0 pages?

It seems to me that zone_watermark_ok() here is trying to
confirm the number of reclaimed pages is enough for the
allocation, although the return value of __node_reclaim()
might be able to tell the same thing like you suggested.
But nr_reclaimed >= nr_pages might not be equivalent
to zone_watermark_ok().

>
> If not, maybe what we want here is:
>
> - One patch to stop doing that.
>
> - Another patch to switch over to the enum.
>
> And then it _would_ return NODE_RECLAIM_NONE when !ret?
>
> Have a weird feeling I'm still being stupid here, let's see...


Best Regards,
Yan, Zi