[PATCH v3] mm/page_alloc: simplify lowmem_reserve max calculation

[PATCH v3] mm/page_alloc: simplify lowmem_reserve max calculation

[Ye Liu]

From: Ye Liu <liuye@kylinos.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.

Signed-off-by: Ye Liu <liuye@kylinos.cn>
Acked-by: Johannes Weiner <hannes@cmpxchg.org>
Acked-by: Zi Yan <ziy@nvidia.com>

Changes in v3:
- max = min_t(unsigned long, max, managed_pages);
- Link to v2:https://lore.kernel.org/all/20250815023500.36893-1-ye.liu@linux.dev/

Changes in v2:
- Drop unnecessary braces
- Replace "if (max > managed_pages)" with min_t()
- Link to v1:https://lore.kernel.org/all/20250814090053.22241-1-ye.liu@linux.dev/
---
 mm/page_alloc.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 64872214bc7d..eff07323987c 100644
--- 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);
 
 			pgdat->totalreserve_pages += max;
 
-- 
2.43.0

Re: [PATCH v3] mm/page_alloc: simplify lowmem_reserve max calculation

[Wei Yang]

On Fri, Aug 15, 2025 at 10:45:09AM +0800, Ye Liu wrote:
>From: Ye Liu <liuye@kylinos.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.
>
>Signed-off-by: Ye Liu <liuye@kylinos.cn>
>Acked-by: Johannes Weiner <hannes@cmpxchg.org>
>Acked-by: Zi Yan <ziy@nvidia.com>

Reviewed-by: Wei Yang <richard.weiyang@gmail.com>

-- 
Wei Yang
Help you, Help me

Re: [PATCH v3] mm/page_alloc: simplify lowmem_reserve max calculation

[Andrew Morton]

On Fri, 15 Aug 2025 10:45:09 +0800 Ye Liu <ye.liu@linux.dev> wrote:

> From: Ye Liu <liuye@kylinos.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.

Re: [PATCH v3] mm/page_alloc: simplify lowmem_reserve max calculation

[Zi Yan]

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

> On Fri, 15 Aug 2025 10:45:09 +0800 Ye Liu <ye.liu@linux.dev> wrote:
>
>> From: Ye Liu <liuye@kylinos.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