linux-kernel - Re: [PATCH v3] mm/page_alloc: simplify lowmem

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <8A6CFFC7-36D2-4E79-87C4-580C31694F78@nvidia.com>
Date: Sat, 16 Aug 2025 07:42:33 -0400
From: Zi Yan <ziy@...dia.com>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: Ye Liu <ye.liu@...ux.dev>, Vlastimil Babka <vbabka@...e.cz>,
 Ye Liu <liuye@...inos.cn>, Johannes Weiner <hannes@...xchg.org>,
 Suren Baghdasaryan <surenb@...gle.com>, Michal Hocko <mhocko@...e.com>,
 Brendan Jackman <jackmanb@...gle.com>, linux-mm@...ck.org,
 linux-kernel@...r.kernel.org, Mel Gorman <mgorman@...hsingularity.net>
Subject: Re: [PATCH v3] mm/page_alloc: simplify lowmem_reserve max calculation

On 16 Aug 2025, at 3:27, Andrew Morton wrote:

> On Fri, 15 Aug 2025 10:45:09 +0800 Ye Liu <ye.liu@...ux.dev> wrote:
>
>> From: Ye Liu <liuye@...inos.cn>
>>
>> Use max() to find the maximum lowmem_reserve value and min_t() to
>> cap it to managed_pages in calculate_totalreserve_pages(), instead
>> of open-coding the comparisons. No functional change.
>>
>> ...
>>
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -6235,16 +6235,13 @@ static void calculate_totalreserve_pages(void)
>>  			unsigned long managed_pages = zone_managed_pages(zone);
>>
>>  			/* 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];
>> -			}
>> +			for (j = i; j < MAX_NR_ZONES; j++)
>> +				max = max(max, zone->lowmem_reserve[j]);
>>
>>  			/* we treat the high watermark as reserved pages. */
>>  			max += high_wmark_pages(zone);
>>
>> -			if (max > managed_pages)
>> -				max = managed_pages;
>> +			max = min_t(unsigned long, max, managed_pages);
>>
>
> Use of max_t/min_t is usually a sign that we messed up the type choices
> somewhere.
>
> In this case, I'd say that zone.lowmem_reserve[] should have been
> ulong.  Oh well.

It was, but then changed into long because in __zone_watermark_ok() it is compared
to can-be-negative free_pages (see commit 3484b2de9499). Maybe it is better to revert
that change and convert ulong lowmem_reserve to long in __zone_watermark_ok()
for the comparison.

+Mel, for his opinion on it, since he made the change in 3484b2de9499.

--
Best Regards,
Yan, Zi