linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Zi Yan <ziy@nvidia.com>
To: Ye Liu <ye.liu@linux.dev>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Vlastimil Babka <vbabka@suse.cz>, Ye Liu <liuye@kylinos.cn>,
	Suren Baghdasaryan <surenb@google.com>,
	Michal Hocko <mhocko@suse.com>,
	Brendan Jackman <jackmanb@google.com>,
	Johannes Weiner <hannes@cmpxchg.org>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mm/page_alloc: simplify lowmem_reserve max calculation
Date: Thu, 14 Aug 2025 21:59:22 -0400	[thread overview]
Message-ID: <78E63F8C-7A6F-4B6B-AD92-25477CF59E4E@nvidia.com> (raw)
In-Reply-To: <0032c975-1e37-484f-9b7b-a3f0fea0401a@linux.dev>

On 14 Aug 2025, at 21:38, Ye Liu wrote:

> 在 2025/8/14 22:47, Zi Yan 写道:
>> On 14 Aug 2025, at 5:00, Ye Liu wrote:
>>
>>> From: Ye Liu <liuye@kylinos.cn>
>>>
>>> Use max() macro to simplify the calculation of maximum lowmem_reserve
>>> value in calculate_totalreserve_pages(), instead of open-coding the
>>> comparison. The functionality remains identical.
>>>
>>> Signed-off-by: Ye Liu <liuye@kylinos.cn>
>>> ---
>>>  mm/page_alloc.c | 3 +--
>>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>>> index 64872214bc7d..8a55a4951d19 100644
>>> --- a/mm/page_alloc.c
>>> +++ b/mm/page_alloc.c
>>> @@ -6236,8 +6236,7 @@ static void calculate_totalreserve_pages(void)
>>>
>>>  			/* Find valid and maximum lowmem_reserve in the zone */
>>>  			for (j = i; j < MAX_NR_ZONES; j++) {
>>> -				if (zone->lowmem_reserve[j] > max)
>>> -					max = zone->lowmem_reserve[j];
>>> +				max = max(max, zone->lowmem_reserve[j]);
>>>  			}
>>
>> There is a “if (max > managed_pages)” below. Maybe convert that as well?
>
> I should use min() here, but I noticed the two variables have different types:
> one is 'long' and the other is 'unsigned long'. So, I should use min_t().
> But then again, why is lowmem_reserve of type 'long'?

Based on commit 3484b2de9499 ("mm: rearrange zone fields into read-only, page
alloc, statistics and page reclaim lines”), lowmem_reserve type is changed
due to the watermark calculation. Looking at current __zone_watermark_ok(),
the comment above free_pages -= ... says free_pages may go negative and
later free_pages is compared with lowmem_reserve.

> It should be a non-negative number, right?
> Is it possible to change the type of lowmem_reserve to 'unsigned long' and
> change all uses of it at the same time? Is it necessary?

In calculate_totalreserve_pages(), max cannot be negative, so you
probably can use min_t(unsigned long, max, managed_pages) and without
changing lowmem_reserve type.


>
>>
>> Feel free to add Acked-by: Zi Yan <ziy@nvidia.com>.
>>
>> Best Regards,
>> Yan, Zi
>
> -- 
> Thanks,
> Ye Liu


Best Regards,
Yan, Zi


      reply	other threads:[~2025-08-15  1:59 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-14  9:00 Ye Liu
2025-08-14 14:07 ` Johannes Weiner
2025-08-15  1:59   ` Ye Liu
2025-08-15  2:01     ` Zi Yan
2025-08-14 14:47 ` Zi Yan
2025-08-15  1:38   ` Ye Liu
2025-08-15  1:59     ` Zi Yan [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=78E63F8C-7A6F-4B6B-AD92-25477CF59E4E@nvidia.com \
    --to=ziy@nvidia.com \
    --cc=akpm@linux-foundation.org \
    --cc=hannes@cmpxchg.org \
    --cc=jackmanb@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=liuye@kylinos.cn \
    --cc=mhocko@suse.com \
    --cc=surenb@google.com \
    --cc=vbabka@suse.cz \
    --cc=ye.liu@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox