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

[Zi Yan <ziy@nvidia.com>]

On 12 Jun 2026, at 4:50, Petr Tesarik wrote:

> Change node_reclaim() to return an enum indicating whether any
> pages have been reclaimed, because that's all the information
> needed by the only caller, get_page_from_freelist().
>
> This leads to the following translation of the old macro
> identifiers to the new enum values:
>
> - NODE_RECLAIM_NOSCAN  -> NODE_RECLAIM_NONE
> - NODE_RECLAIM_FULL    -> NODE_RECLAIM_NONE
> - NODE_RECLAIM_SOME    -> NODE_RECLAIM_SUCCESS
> - NODE_RECLAIM_SUCCESS -> NODE_RECLAIM_SUCCESS
>
> Originally, I was looking for occurences of NODE_RECLAIM_SOME
> and NODE_RECLAIM_SUCCESS, but I couldn't find any. That's because
> they are typecast from the result of a relational operator. This
> seemed a bit fragile, so I dug a bit deeper and came up with this
> proposed cleanup.
>
> Signed-off-by: Petr Tesarik <ptesarik@suse.com>
>
> --
>
> Changes from v1:
> - use an enum instead of a bool
> ---
>  mm/internal.h   | 17 +++++++++--------
>  mm/page_alloc.c | 19 ++++---------------
>  mm/vmscan.c     | 18 +++++++++---------
>  3 files changed, 22 insertions(+), 32 deletions(-)
>
> diff --git a/mm/internal.h b/mm/internal.h
> index 181e79f1d6a20..89b0ea28051c1 100644
> --- 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,
> +};
>
>  #ifdef CONFIG_NUMA
>  extern int node_reclaim_mode;
>
> -extern int node_reclaim(struct pglist_data *, gfp_t, unsigned int);
> +extern enum node_reclaim node_reclaim(struct pglist_data *pgdat,
> +				      gfp_t gfp_mask, unsigned int order);
>  extern int find_next_best_node(int node, nodemask_t *used_node_mask);
>  #else
>  #define node_reclaim_mode 0
>
> -static inline int node_reclaim(struct pglist_data *pgdat, gfp_t mask,
> -				unsigned int order)
> +static inline enum node_reclaim node_reclaim(struct pglist_data *pgdat,
> +					     gfp_t mask, unsigned int order)
>  {
> -	return NODE_RECLAIM_NOSCAN;
> +	return NODE_RECLAIM_NONE;
>  }
>  static inline int find_next_best_node(int node, nodemask_t *used_node_mask)
>  {
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index f7db8f049bd23..83a1caac5ac9c 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -3899,8 +3899,6 @@ get_page_from_freelist(gfp_t gfp_mask, unsigned int order, int alloc_flags,
>  		if (!zone_watermark_fast(zone, order, mark,
>  				       ac->highest_zoneidx, alloc_flags,
>  				       gfp_mask)) {
> -			int ret;
> -
>  			if (cond_accept_memory(zone, order, alloc_flags))
>  				goto try_this_zone;
>
> @@ -3921,22 +3919,13 @@ get_page_from_freelist(gfp_t gfp_mask, unsigned int order, int alloc_flags,
>  			    !zone_allows_reclaim(zonelist_zone(ac->preferred_zoneref), zone))
>  				continue;
>
> -			ret = node_reclaim(zone->zone_pgdat, gfp_mask, order);
> -			switch (ret) {
> -			case NODE_RECLAIM_NOSCAN:
> -				/* did not scan */
> -				continue;
> -			case NODE_RECLAIM_FULL:
> -				/* scanned but unreclaimable */
> +			if (node_reclaim(zone->zone_pgdat, gfp_mask, order) == NODE_RECLAIM_NONE)
>  				continue;

NODE_RECLAIM_NOSCAN and NODE_RECLAIM_FULL both continue here and they appear
nowhere else, so they can be combined into NODE_RECLAIM_NONE like you did.

<snip>

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

I was wondering why ret is changed to nr_reclaimed >= nr_pages,
which seems to change the original semantics, until I see the “>= nr_pages”
at the end of __node_reclaim().

>  		count_vm_event(PGSCAN_ZONE_RECLAIM_SUCCESS);
>  	else
>  		count_vm_event(PGSCAN_ZONE_RECLAIM_FAILED);
>
> -	return ret;
> +	return NODE_RECLAIM_SUCCESS;
>  }
>
>  #else

The patch keeps the original semantics, so
Reviewed-by: Zi Yan <ziy@nvidia.com>

In terms of the question raised by Brendan about the return value by
__node_reclaim(), I will follow the discussion on the other thread.

Best Regards,
Yan, Zi