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

["Brendan Jackman" <brendan.jackman@linux.dev>]

On Fri Jun 12, 2026 at 3:01 PM UTC, Zi Yan wrote:
> 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().

Yeah but in the more specific case of nr_reclaimed == 0, would it make
sense to assume !zone_watermark_ok()? It doesn't seem that important in
terms of behaviour but I think it would be a clarity win.