Re: [PATCH v4] mm/page_alloc: only update lowmem_reserve_ratio on sysctl write

[Andrew Morton]

On Sun,  2 Aug 2026 22:57:07 +0800 Jianlin Shi <shijianlin11@xxxxxxxxxxx> wrote:

> lowmem_reserve_ratio_sysctl_handler() ignores the return value of
> proc_dointvec_minmax() and always calls setup_per_zone_lowmem_reserve(),
> even for read operations.
> 
> Fix three issues:
> 
> 1. Propagate errors from proc_dointvec_minmax() instead of always
>    returning success.  For example, writing non-integer garbage to the
>    sysctl now returns an error instead of silently succeeding with
>    unchanged values.
> 
> 2. Only call setup_per_zone_lowmem_reserve() when the sysctl is
>    actually written, matching the write-only refresh pattern of
>    min_free_kbytes and watermark_scale_factor handlers.
> 
> 3. On write, parse into a temporary ratio[] array and only copy into
>    sysctl_lowmem_reserve_ratio[] and refresh derived state after the
>    full vector is validated.  This avoids leaving the ratio array
>    partially updated while skipping setup when proc_dointvec_minmax()
>    returns an error on a later element (suggested by Andrew Morton).
> 
> Drop the manual "< 1 -> 0" sanitization loop and set .extra1 =
> SYSCTL_ZERO on the ctl_table entry so proc_dointvec_minmax() enforces
> the minimum on write; negative values now return -EINVAL instead of
> being silently coerced to 0 (suggested by Vlastimil Babka).
> 
> ...
>
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -6673,8 +6673,8 @@ static int sysctl_min_slab_ratio_sysctl_handler(const struct ctl_table *table, i
>  
>  /*
>   * lowmem_reserve_ratio_sysctl_handler - just a wrapper around
> - *	proc_dointvec() so that we can call setup_per_zone_lowmem_reserve()
> - *	whenever sysctl_lowmem_reserve_ratio changes.
> + *	proc_dointvec_minmax() so that we can call
> + *	setup_per_zone_lowmem_reserve() when the sysctl is written.
>   *
>   * The reserve ratio obviously has absolutely no relation with the
>   * minimum watermarks. The lowmem reserve ratio can only make sense
> @@ -6683,16 +6683,23 @@ static int sysctl_min_slab_ratio_sysctl_handler(const struct ctl_table *table, i
>  static int lowmem_reserve_ratio_sysctl_handler(const struct ctl_table *table,
>  		int write, void *buffer, size_t *length, loff_t *ppos)
>  {
> -	int i;
> +	struct ctl_table tmp = *table;
> +	int ratio[MAX_NR_ZONES];
> +	int rc;
>  
> -	proc_dointvec_minmax(table, write, buffer, length, ppos);
> +	if (!write)
> +		return proc_dointvec_minmax(table, write, buffer, length, ppos);
>  
> -	for (i = 0; i < MAX_NR_ZONES; i++) {
> -		if (sysctl_lowmem_reserve_ratio[i] < 1)
> -			sysctl_lowmem_reserve_ratio[i] = 0;
> -	}
> +	memcpy(ratio, sysctl_lowmem_reserve_ratio, sizeof(ratio));
> +	tmp.data = ratio;
>  
> +	rc = proc_dointvec_minmax(&tmp, write, buffer, length, ppos);
> +	if (rc)
> +		return rc;
> +
> +	memcpy(sysctl_lowmem_reserve_ratio, ratio, sizeof(ratio));
>  	setup_per_zone_lowmem_reserve();
> +
>  	return 0;
>  }
>  
> @@ -6791,6 +6798,7 @@ static const struct ctl_table page_alloc_sysctl_table[] = {
>  		.maxlen		= sizeof(sysctl_lowmem_reserve_ratio),
>  		.mode		= 0644,
>  		.proc_handler	= lowmem_reserve_ratio_sysctl_handler,
> +		.extra1		= SYSCTL_ZERO,
>  	},
>  #ifdef CONFIG_NUMA

lgtm, thanks.

I find this a little tidier:

--- a/mm/page_alloc.c~mm-page_alloc-only-update-lowmem_reserve_ratio-on-sysctl-write-fix
+++ a/mm/page_alloc.c
@@ -6926,7 +6926,7 @@ static int lowmem_reserve_ratio_sysctl_h
 		int write, void *buffer, size_t *length, loff_t *ppos)
 {
 	struct ctl_table tmp = *table;
-	int ratio[MAX_NR_ZONES];
+	int ratio[ARRAY_SIZE(sysctl_lowmem_reserve_ratio)];
 	int rc;
 
 	if (!write)

A bit more self-documenting and future-proof.  What do you think?