From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-189.mta0.migadu.com (out-189.mta0.migadu.com [91.218.175.189]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 56556369996 for ; Fri, 12 Jun 2026 10:10:25 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=91.218.175.189 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781259027; cv=none; b=Z+tFTW/Fx2pTobO3rsCmLLugxqBv25PTK3uYrZju0LLYTnhtekBKkhuMFqGpuJ9bqBIcPpsOlJ5bZQTVeoAYLEUp7bb2RyU5hcIHj4GQOK/URhZSL6MR7MJ82kysSiaF89HbJCZrEnM2iQsU+e32+WTrJm3xC9YhAibbVtBxPd4= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781259027; c=relaxed/simple; bh=j7FqodmdvNJ4bvixmakCEEU6af0je9M4lyabZq5gWuM=; h=Mime-Version:Content-Type:Date:Message-Id:Cc:Subject:From:To: References:In-Reply-To; b=uwvVcMkLQHGRBe+mK2TBgZZ4Baxi6CVhzCrSh/wh1NvqzQTyN7pMDUd22Tx5Uc7nJ2L+hEOGLi4cGeNGJqeNO2TuYfWAvsrQKRu91HYAghv28kpIzDaSY3ud0VQ3DvLR4zWi2ssNeLrxTEaP99qfTYXc62RxG3+KdKhejvgMy/k= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=GSk4CN21; arc=none smtp.client-ip=91.218.175.189 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="GSk4CN21" Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1781259022; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=q519vfwp1vW5ZXI0F5glbgBRXP0Kz0Zm9ehHeL4ROgQ=; b=GSk4CN21/+x0DhgwLZXMmbEYTgJEStM0ZtUCWysnAXXh7WS0fw03tsYDUqz1WdvihuhrGb nBxH+iYYEaEFvpOuz5cE5kS4ZxIqyjkn2j/GS9s7p6kc/+x5U8V4MdpA3iuEBf4aVqJc5G +e/gd1FIzbAGElC99XoNaCVgOnfVkzk= Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Fri, 12 Jun 2026 10:10:20 +0000 Message-Id: Cc: "Brendan Jackman" , "Johannes Weiner" , "Zi Yan" , Subject: Re: [PATCH v2 1/1] mm: reduce NODE_RECLAIM_xxx and change to enum X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: "Brendan Jackman" To: "Petr Tesarik" , "Andrew Morton" , "David Hildenbrand" , "Lorenzo Stoakes" , "Liam R. Howlett" , "Vlastimil Babka" , "Mike Rapoport" , "Suren Baghdasaryan" , "Michal Hocko" , References: <20260612085052.59291-1-ptesarik@suse.com> In-Reply-To: <20260612085052.59291-1-ptesarik@suse.com> X-Migadu-Flow: FLOW_OUT On Fri Jun 12, 2026 at 8:50 AM UTC, 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 > > -- > > 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 */ > =20 > -#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, > +}; > =20 > #ifdef CONFIG_NUMA > extern int node_reclaim_mode; > =20 > -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 > =20 > -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_ma= sk) > { > 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; > =20 > @@ -3921,22 +3919,13 @@ get_page_from_freelist(gfp_t gfp_mask, unsigned i= nt order, int alloc_flags, > !zone_allows_reclaim(zonelist_zone(ac->preferred_zoneref), zone)) > continue; > =20 > - ret =3D 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) =3D=3D NODE_RECLA= IM_NONE) > continue; > - default: > - /* did we reclaim enough */ > - if (zone_watermark_ok(zone, order, mark, > - ac->highest_zoneidx, alloc_flags)) > - goto try_this_zone; > =20 > + /* did we reclaim enough */ > + if (!zone_watermark_ok(zone, order, mark, > + ac->highest_zoneidx, alloc_flags)) > continue; > - } > } > =20 > try_this_zone: > diff --git a/mm/vmscan.c b/mm/vmscan.c > index 3f3ff25e561ac..d5bd55620ee9a 100644 > --- a/mm/vmscan.c > +++ b/mm/vmscan.c > @@ -7786,9 +7786,9 @@ static unsigned long __node_reclaim(struct pglist_d= ata *pgdat, gfp_t gfp_mask, > return sc->nr_reclaimed; > } > =20 > -int node_reclaim(struct pglist_data *pgdat, gfp_t gfp_mask, unsigned int= order) > +enum node_reclaim node_reclaim(struct pglist_data *pgdat, gfp_t gfp_mask= , unsigned int order) > { > - int ret; > + unsigned long nr_reclaimed; > /* Minimum pages needed in order to stay on node */ > const unsigned long nr_pages =3D 1 << order; > struct scan_control sc =3D { > @@ -7815,13 +7815,13 @@ int node_reclaim(struct pglist_data *pgdat, gfp_t= gfp_mask, unsigned int order) > if (node_pagecache_reclaimable(pgdat) <=3D pgdat->min_unmapped_pages && > node_page_state_pages(pgdat, NR_SLAB_RECLAIMABLE_B) <=3D > pgdat->min_slab_pages) > - return NODE_RECLAIM_FULL; > + return NODE_RECLAIM_NONE; > =20 > /* > * Do not scan if the allocation should not be delayed. > */ > if (!gfpflags_allow_blocking(gfp_mask) || (current->flags & PF_MEMALLOC= )) > - return NODE_RECLAIM_NOSCAN; > + return NODE_RECLAIM_NONE; > =20 > /* > * Only run node reclaim on the local node or on nodes that do not > @@ -7830,20 +7830,20 @@ int node_reclaim(struct pglist_data *pgdat, gfp_t= gfp_mask, unsigned int order) > * as wide as possible. > */ > if (node_state(pgdat->node_id, N_CPU) && pgdat->node_id !=3D numa_node_= id()) > - return NODE_RECLAIM_NOSCAN; > + return NODE_RECLAIM_NONE; > =20 > if (test_and_set_bit_lock(PGDAT_RECLAIM_LOCKED, &pgdat->flags)) > - return NODE_RECLAIM_NOSCAN; > + return NODE_RECLAIM_NONE; > =20 > - ret =3D __node_reclaim(pgdat, gfp_mask, nr_pages, &sc) >=3D nr_pages; > + nr_reclaimed =3D __node_reclaim(pgdat, gfp_mask, nr_pages, &sc); > clear_bit_unlock(PGDAT_RECLAIM_LOCKED, &pgdat->flags); > =20 > - if (ret) > + if (nr_reclaimed >=3D nr_pages) > count_vm_event(PGSCAN_ZONE_RECLAIM_SUCCESS); > else > count_vm_event(PGSCAN_ZONE_RECLAIM_FAILED); > =20 > - return ret; > + return NODE_RECLAIM_SUCCESS; Should that be returning NODE_RECLAIM_NONE when !ret?