From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from out-182.mta0.migadu.com (out-182.mta0.migadu.com [91.218.175.182])
	(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 1572F31F998
	for <linux-kernel@vger.kernel.org>; Fri, 12 Jun 2026 14:28:44 +0000 (UTC)
Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=91.218.175.182
ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;
	t=1781274526; cv=none; b=SsO6HwYp4zokHOGQ1V7KGBnU8u7gs6UmA+2/BAvD5pdqSUJIm8Y0uRmY/1mgyXHcAyIOF09VVB2ypAJYbHwYmT2uqS9fRAcgWVdqFVezxj5oaADl184udx7CAoM9rkQKB2W/x0ORgKuRh70BYFz83Lfs9B2WegSGbYCa2rSM0AM=
ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org;
	s=arc-20240116; t=1781274526; c=relaxed/simple;
	bh=/V6SPZAshNosL/7L0qR4w94nwV9okXgnDSIBiQCkkmE=;
	h=Mime-Version:Content-Type:Date:Message-Id:Subject:From:To:Cc:
	 References:In-Reply-To; b=qGLvQNqi5RWzPiZ+CtYEAJMWghA51jHdqhVRPw2kJQ4yUORdKNLk7uQcGrAtjKURZuRcowi56hY4xJpLWBwtUeDc/2fekHplINdDJCxvDq06jUsZkFWSRrZ1PnKIz7zOgvVdz4Mgwj42LAeR5f4uF2p5m7Du7NjD0YuOxkpd7MY=
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=j1Il1Zuu; arc=none smtp.client-ip=91.218.175.182
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="j1Il1Zuu"
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
Mime-Version: 1.0
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1;
	t=1781274523;
	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=+uQFwXgZU0Ck9cr02Qktgh08VOPOTrz0vOOc1O5K1bE=;
	b=j1Il1Zuut8N1UBaK97YKnvlNSh/rcDwgA+Uc5Ar04/w7LegJIY0ddl00ePkUMRFJWaSzoF
	0bgyFsaLQSFmtiKOjgYgbJPVk7nUUdUKCpIXL9f3k5zelFeZUXX7r6m0i+ScvbQl2UWiyG
	ZWKl/sjhNq3Hh69HcGhxDIlX3+YrGtk=
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset=UTF-8
Date: Fri, 12 Jun 2026 14:28:40 +0000
Message-Id: <DJ7549TJF9I5.O3K9X2TP8VE1@linux.dev>
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" <brendan.jackman@linux.dev>
To: "Petr Tesarik" <ptesarik@suse.com>, "Brendan Jackman"
 <brendan.jackman@linux.dev>
Cc: "Andrew Morton" <akpm@linux-foundation.org>, "David Hildenbrand"
 <david@kernel.org>, "Lorenzo Stoakes" <ljs@kernel.org>, "Liam R. Howlett"
 <liam@infradead.org>, "Vlastimil Babka" <vbabka@kernel.org>, "Mike
 Rapoport" <rppt@kernel.org>, "Suren Baghdasaryan" <surenb@google.com>,
 "Michal Hocko" <mhocko@suse.com>, <linux-mm@kvack.org>, "Brendan Jackman"
 <jackmanb@google.com>, "Johannes Weiner" <hannes@cmpxchg.org>, "Zi Yan"
 <ziy@nvidia.com>, <linux-kernel@vger.kernel.org>
References: <20260612085052.59291-1-ptesarik@suse.com>
 <DJ6ZMH9VU0BE.6Q8EHJIL65DW@linux.dev> <20260612133155.71958e80@mordecai>
In-Reply-To: <20260612133155.71958e80@mordecai>
X-Migadu-Flow: FLOW_OUT

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 */
>> > =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,
>> > +};

>> > -	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; =20
>>=20
>> 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...