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 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?

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