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

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

[Jianlin Shi]

lowmem_reserve_ratio_sysctl_handler() ignores the return value of
proc_dointvec_minmax() and always sanitizes sysctl_lowmem_reserve_ratio
and calls setup_per_zone_lowmem_reserve(), even for read operations.

Reading /proc/sys/vm/lowmem_reserve_ratio should not recompute per-zone
lowmem_reserve[] and totalreserve_pages.  Only do so when the sysctl is
written, matching min_free_kbytes and watermark_scale_factor handlers.

Also propagate errors from proc_dointvec_minmax() instead of ignoring
them.

Compatibility note:
Previously a read also sanitized sysctl_lowmem_reserve_ratio[] and
called setup_per_zone_lowmem_reserve(), which rewrites each zone's
lowmem_reserve[] and recalculates pgdat->totalreserve_pages /
totalreserve_pages (visible via /proc/zoneinfo "protection" and used
by page allocation fallback and dirty-limit accounting).  After this
change only a write does that.  Documentation describes the meaning of
the ratio and the derived protection pages, but does not document any
read side-effect.

Worst case for odd userspace that treated a read as a refresh of those
derived values: lowmem_reserve[] and totalreserve_pages remain at their
last written/setup values until the next write of this sysctl, or until
another existing updater runs (e.g. adjust_managed_page_count() on
managed-page changes, or init/watermark setup paths).  Until then,
allocation fallback into lower zones and per-node dirtyable memory
(node_dirtyable_memory() subtracts pgdat->totalreserve_pages) may not
reflect a refresh that such userspace expected from the read alone.
Normal readers that only consume the ratio array are unaffected.

Signed-off-by: Jianlin Shi <shijianlin11@foxmail.com>
---
 mm/page_alloc.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 0387d2afd..ce33579ca 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6683,16 +6683,21 @@ 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;
+	int i, rc;
 
-	proc_dointvec_minmax(table, write, buffer, length, ppos);
+	rc = proc_dointvec_minmax(table, write, buffer, length, ppos);
+	if (rc)
+		return rc;
 
-	for (i = 0; i < MAX_NR_ZONES; i++) {
-		if (sysctl_lowmem_reserve_ratio[i] < 1)
-			sysctl_lowmem_reserve_ratio[i] = 0;
+	if (write) {
+		for (i = 0; i < MAX_NR_ZONES; i++) {
+			if (sysctl_lowmem_reserve_ratio[i] < 1)
+				sysctl_lowmem_reserve_ratio[i] = 0;
+		}
+
+		setup_per_zone_lowmem_reserve();
 	}
 
-	setup_per_zone_lowmem_reserve();
 	return 0;
 }
 
-- 
2.43.0

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

[Vlastimil Babka (SUSE)]

On 7/28/26 12:32, Jianlin Shi wrote:
> lowmem_reserve_ratio_sysctl_handler() ignores the return value of
> proc_dointvec_minmax() and always sanitizes sysctl_lowmem_reserve_ratio
> and calls setup_per_zone_lowmem_reserve(), even for read operations.
> 
> Reading /proc/sys/vm/lowmem_reserve_ratio should not recompute per-zone
> lowmem_reserve[] and totalreserve_pages.  Only do so when the sysctl is
> written, matching min_free_kbytes and watermark_scale_factor handlers.
> 
> Also propagate errors from proc_dointvec_minmax() instead of ignoring
> them.
> 
> Compatibility note:
> Previously a read also sanitized sysctl_lowmem_reserve_ratio[] and
> called setup_per_zone_lowmem_reserve(), which rewrites each zone's
> lowmem_reserve[] and recalculates pgdat->totalreserve_pages /
> totalreserve_pages (visible via /proc/zoneinfo "protection" and used
> by page allocation fallback and dirty-limit accounting).  After this
> change only a write does that.  Documentation describes the meaning of
> the ratio and the derived protection pages, but does not document any
> read side-effect.
> 
> Worst case for odd userspace that treated a read as a refresh of those
> derived values: lowmem_reserve[] and totalreserve_pages remain at their
> last written/setup values until the next write of this sysctl, or until
> another existing updater runs (e.g. adjust_managed_page_count() on
> managed-page changes, or init/watermark setup paths).  Until then,
> allocation fallback into lower zones and per-node dirtyable memory
> (node_dirtyable_memory() subtracts pgdat->totalreserve_pages) may not
> reflect a refresh that such userspace expected from the read alone.
> Normal readers that only consume the ratio array are unaffected.
> 
> Signed-off-by: Jianlin Shi <shijianlin11@foxmail.com>
> ---
>  mm/page_alloc.c | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 0387d2afd..ce33579ca 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -6683,16 +6683,21 @@ 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;
> +	int i, rc;
>  
> -	proc_dointvec_minmax(table, write, buffer, length, ppos);
> +	rc = proc_dointvec_minmax(table, write, buffer, length, ppos);
> +	if (rc)
> +		return rc;
>  
> -	for (i = 0; i < MAX_NR_ZONES; i++) {
> -		if (sysctl_lowmem_reserve_ratio[i] < 1)
> -			sysctl_lowmem_reserve_ratio[i] = 0;
> +	if (write) {
> +		for (i = 0; i < MAX_NR_ZONES; i++) {
> +			if (sysctl_lowmem_reserve_ratio[i] < 1)
> +				sysctl_lowmem_reserve_ratio[i] = 0;
> +		}

Could we also get rid of this adjustment with proc_dointvec_minmax's
handling of tbl->extra1 set to SYSCTL_ZERO?

> +
> +		setup_per_zone_lowmem_reserve();
>  	}
>  
> -	setup_per_zone_lowmem_reserve();
>  	return 0;
>  }
>  

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

[Jianlin Shi]

On Thu, 30 Jul 2026 15:28:00 +0000 Vlastimil Babka wrote:
> Could we also get rid of this adjustment with proc_dointvec_minmax's
> handling of tbl->extra1 set to SYSCTL_ZERO?

Hi Vlastimil,

Good point, thanks.  v2 drops the manual "< 1 -> 0" loop and adds
.extra1 = SYSCTL_ZERO to the lowmem_reserve_ratio ctl_table entry, so
proc_dointvec_minmax() enforces the minimum on write.

For integer values this is equivalent for 0 and positive writes; negative
values now return -EINVAL instead of being silently coerced to 0, which
seems more consistent with other vm sysctls.

I'll send [PATCH v2] in a separate mail shortly.

Thanks,
Jianlin

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

[Jianlin Shi]

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.

Reading /proc/sys/vm/lowmem_reserve_ratio should not recompute per-zone
lowmem_reserve[] and totalreserve_pages.  Only do so when the sysctl is
written, matching min_free_kbytes and watermark_scale_factor handlers.

Also propagate errors from proc_dointvec_minmax() instead of ignoring
them.

Drop the manual "< 1 -> 0" sanitization loop in the handler and set
.extra1 = SYSCTL_ZERO on the ctl_table entry so proc_dointvec_minmax()
enforces the minimum on write (suggested by Vlastimil Babka).

Compatibility note:
Previously a read also sanitized sysctl_lowmem_reserve_ratio[] and
called setup_per_zone_lowmem_reserve(), which rewrites each zone's
lowmem_reserve[] and recalculates pgdat->totalreserve_pages /
totalreserve_pages (visible via /proc/zoneinfo "protection" and used
by page allocation fallback and dirty-limit accounting).  After this
change only a write does that.  Documentation describes the meaning of
the ratio and the derived protection pages, but does not document any
read side-effect.

Worst case for odd userspace that treated a read as a refresh of those
derived values: lowmem_reserve[] and totalreserve_pages remain at their
last written/setup values until the next write of this sysctl, or until
another existing updater runs (e.g. adjust_managed_page_count() on
managed-page changes, or init/watermark setup paths).  Until then,
allocation fallback into lower zones and per-node dirtyable memory
(node_dirtyable_memory() subtracts pgdat->totalreserve_pages) may not
reflect a refresh that such userspace expected from the read alone.
Normal readers that only consume the ratio array are unaffected.

Changes in v2:
- Add .extra1 = SYSCTL_ZERO to the ctl_table entry
- Remove the manual sanitization loop (negative writes now return
  -EINVAL instead of being silently coerced to 0)

Link: https://lore.kernel.org/linux-mm/tencent_FFD4F4D728AAE8A8AE0AF277A59854A29A06@qq.com/

Signed-off-by: Jianlin Shi <shijianlin11@foxmail.com>
---
 mm/page_alloc.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 0387d2afd..a7381327d 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6683,16 +6683,15 @@ 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;
+	int rc;
 
-	proc_dointvec_minmax(table, write, buffer, length, ppos);
+	rc = proc_dointvec_minmax(table, write, buffer, length, ppos);
+	if (rc)
+		return rc;
 
-	for (i = 0; i < MAX_NR_ZONES; i++) {
-		if (sysctl_lowmem_reserve_ratio[i] < 1)
-			sysctl_lowmem_reserve_ratio[i] = 0;
-	}
+	if (write)
+		setup_per_zone_lowmem_reserve();
 
-	setup_per_zone_lowmem_reserve();
 	return 0;
 }
 
@@ -6791,6 +6790,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
 	{
-- 
2.43.0

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

[Vlastimil Babka (SUSE)]

Hm I also noticed you didn't cc everyone from PAGE ALLOCATOR section in
MAINTAINERS. Please use scripts/get_maintainers.pl next time.

On 7/31/26 05:42, Jianlin Shi 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.
> 
> Reading /proc/sys/vm/lowmem_reserve_ratio should not recompute per-zone
> lowmem_reserve[] and totalreserve_pages.  Only do so when the sysctl is
> written, matching min_free_kbytes and watermark_scale_factor handlers.
> 
> Also propagate errors from proc_dointvec_minmax() instead of ignoring
> them.
> 
> Drop the manual "< 1 -> 0" sanitization loop in the handler and set
> .extra1 = SYSCTL_ZERO on the ctl_table entry so proc_dointvec_minmax()
> enforces the minimum on write (suggested by Vlastimil Babka).

It would be fair to explain the -EINVAL change in the compatibility note
too. FWIW I consider it low-risk.

> Compatibility note:
> Previously a read also sanitized sysctl_lowmem_reserve_ratio[] and
> called setup_per_zone_lowmem_reserve(), which rewrites each zone's
> lowmem_reserve[] and recalculates pgdat->totalreserve_pages /
> totalreserve_pages (visible via /proc/zoneinfo "protection" and used
> by page allocation fallback and dirty-limit accounting).  After this
> change only a write does that.  Documentation describes the meaning of
> the ratio and the derived protection pages, but does not document any
> read side-effect.
> 
> Worst case for odd userspace that treated a read as a refresh of those
> derived values: lowmem_reserve[] and totalreserve_pages remain at their
> last written/setup values until the next write of this sysctl, or until
> another existing updater runs (e.g. adjust_managed_page_count() on
> managed-page changes, or init/watermark setup paths).  Until then,
> allocation fallback into lower zones and per-node dirtyable memory
> (node_dirtyable_memory() subtracts pgdat->totalreserve_pages) may not
> reflect a refresh that such userspace expected from the read alone.
> Normal readers that only consume the ratio array are unaffected.

Well there should be no change to the totalreserve if nobody wrote new
ratios. Other paths that may change the result do call the update as you
describe. So this should have no observable effects anyway.

However, except watermark boosting hidden here in
calculate_totalreserve_pages():

    /* we treat the high watermark as reserved pages. */
    max += high_wmark_pages(zone);

The effect of boosting on this calculation was probably overlooked. I wonder
if we should also (as a separate patch) update it to use a value that
doesn't include boosting to make it stable and deterministic.

> Changes in v2:
> - Add .extra1 = SYSCTL_ZERO to the ctl_table entry
> - Remove the manual sanitization loop (negative writes now return
>   -EINVAL instead of being silently coerced to 0)

Changes normally go under to the diffstat area* and are not part of commit
log. Hence the suggestion for the note for -EINVAL to be elsewhere.

> 
> Link: https://lore.kernel.org/linux-mm/tencent_FFD4F4D728AAE8A8AE0AF277A59854A29A06@qq.com/
> 
> Signed-off-by: Jianlin Shi <shijianlin11@foxmail.com>
> ---

*here

Otherwise LGTM.
Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>


>  mm/page_alloc.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 0387d2afd..a7381327d 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -6683,16 +6683,15 @@ 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;
> +	int rc;
>  
> -	proc_dointvec_minmax(table, write, buffer, length, ppos);
> +	rc = proc_dointvec_minmax(table, write, buffer, length, ppos);
> +	if (rc)
> +		return rc;
>  
> -	for (i = 0; i < MAX_NR_ZONES; i++) {
> -		if (sysctl_lowmem_reserve_ratio[i] < 1)
> -			sysctl_lowmem_reserve_ratio[i] = 0;
> -	}
> +	if (write)
> +		setup_per_zone_lowmem_reserve();
>  
> -	setup_per_zone_lowmem_reserve();
>  	return 0;
>  }
>  
> @@ -6791,6 +6790,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
>  	{

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

[Johannes Weiner]

On Fri, Jul 31, 2026 at 11:42:26AM +0800, Jianlin Shi 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.
>
> Reading /proc/sys/vm/lowmem_reserve_ratio should not recompute per-zone
> lowmem_reserve[] and totalreserve_pages.  Only do so when the sysctl is
> written, matching min_free_kbytes and watermark_scale_factor handlers.
> 
> Also propagate errors from proc_dointvec_minmax() instead of ignoring
> them.

This appears to be the primary user-visible effect. You send in "birds
are real", function returns success, values are unchanged. After this
patch you get a proper error code on this lie.

> Drop the manual "< 1 -> 0" sanitization loop in the handler and set
> .extra1 = SYSCTL_ZERO on the ctl_table entry so proc_dointvec_minmax()
> enforces the minimum on write (suggested by Vlastimil Babka).

That's a nice cleanup.

> Compatibility note:
> Previously a read also sanitized sysctl_lowmem_reserve_ratio[] and
> called setup_per_zone_lowmem_reserve(), which rewrites each zone's
> lowmem_reserve[] and recalculates pgdat->totalreserve_pages /
> totalreserve_pages (visible via /proc/zoneinfo "protection" and used
> by page allocation fallback and dirty-limit accounting).  After this
> change only a write does that.  Documentation describes the meaning of
> the ratio and the derived protection pages, but does not document any
> read side-effect.
> 
> Worst case for odd userspace that treated a read as a refresh of those
> derived values: lowmem_reserve[] and totalreserve_pages remain at their
> last written/setup values until the next write of this sysctl, or until
> another existing updater runs (e.g. adjust_managed_page_count() on
> managed-page changes, or init/watermark setup paths).  Until then,
> allocation fallback into lower zones and per-node dirtyable memory
> (node_dirtyable_memory() subtracts pgdat->totalreserve_pages) may not
> reflect a refresh that such userspace expected from the read alone.
> Normal readers that only consume the ratio array are unaffected.

I don't understand this. Nothing actually changes? It just runs
through the calculations pointlessly, but using the same fixed
parameters (zone_managed_pages(), lowmem_reserve[], watermarks*).

* modulo the boost effect Vlastimil points out aside. Which seems
  worth fixing but it's a separate patch

So IMO the changelog should be:

1. Actually return error on bogus values - i.e. check if integers
   were parsed and bound to positive range instead of silent rounding
2. Don't pointlessly recalculate on read when inputs didn't change

> Changes in v2:
> - Add .extra1 = SYSCTL_ZERO to the ctl_table entry
> - Remove the manual sanitization loop (negative writes now return
>   -EINVAL instead of being silently coerced to 0)
> 
> Link: https://lore.kernel.org/linux-mm/tencent_FFD4F4D728AAE8A8AE0AF277A59854A29A06@qq.com/
> 
> Signed-off-by: Jianlin Shi <shijianlin11@foxmail.com>

Code looks good to me. With the changelog fixed,

Acked-by: Johannes Weiner <hannes@cmpxchg.org>

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

[Jianlin Shi]

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 two 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.  Reading /proc/sys/vm/lowmem_reserve_ratio should
   not recompute derived lowmem_reserve[] and totalreserve_pages when
   the inputs did not change, matching min_free_kbytes and
   watermark_scale_factor handlers.

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).

Link: https://lore.kernel.org/linux-mm/tencent_FFD4F4D728AAE8A8AE0AF277A59854A29A06@qq.com/

Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
Acked-by: Johannes Weiner <hannes@cmpxchg.org>
Signed-off-by: Jianlin Shi <shijianlin11@foxmail.com>
---
Changes in v3:
- Rewrite commit log to focus on the two tangible fixes as suggested by
  Johannes Weiner.
- Code remains unchanged versus v2; retain previously obtained tags.

Changes in v2:
- Add .extra1 = SYSCTL_ZERO to ctl_table entry
- Remove manual sanitization loop; negative writes now return -EINVAL

v1: https://lore.kernel.org/linux-mm/tencent_FFD4F4D728AAE8A8AE0AF277A59854A29A06@qq.com/

 mm/page_alloc.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 0387d2afd..a7381327d 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6683,16 +6683,15 @@ 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;
+	int rc;
 
-	proc_dointvec_minmax(table, write, buffer, length, ppos);
+	rc = proc_dointvec_minmax(table, write, buffer, length, ppos);
+	if (rc)
+		return rc;
 
-	for (i = 0; i < MAX_NR_ZONES; i++) {
-		if (sysctl_lowmem_reserve_ratio[i] < 1)
-			sysctl_lowmem_reserve_ratio[i] = 0;
-	}
+	if (write)
+		setup_per_zone_lowmem_reserve();
 
-	setup_per_zone_lowmem_reserve();
 	return 0;
 }
 
@@ -6791,6 +6790,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
 	{
-- 
2.43.0

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

[Andrew Morton]

On Sat,  1 Aug 2026 23:11:25 +0800 Jianlin Shi <shijianlin11@foxmail.com> 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 two 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.

AI review suggest that this caused a new problem:

	https://sashiko.dev/#/patchset/tencent_1BB7A5C4D5EEA67346634417753190E92A09@qq.com

Not sure what to do here.  Perhaps pass proc_dointvec_minmax() a
temporary then copy that into sysctl_lowmem_reserve_ratio if all
proc_dointvec_minmax() returns "OK".

But really this is a flaw in proc_dointvec_minmax() isn't it?  It
shouldn't update the table data until all the data has been validated.

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

[Jianlin Shi]

On Sat, 1 Aug 2026 11:44:34 -0700 Andrew Morton wrote:
> Not sure what to do here.  Perhaps pass proc_dointvec_minmax() a
> temporary then copy that into sysctl_lowmem_reserve_ratio if all
> proc_dointvec_minmax() returns "OK".

Hi Andrew,

Thanks for the suggestion.  v4 uses a temporary ratio[] on write and
only copies into sysctl_lowmem_reserve_ratio[] and calls
setup_per_zone_lowmem_reserve() after the full parse succeeds.

I'll send [PATCH v4] in a separate mail shortly.

Thanks,
Jianlin

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

[Vlastimil Babka (SUSE)]

+Cc: sysctl maintainers

On 8/1/26 20:44, Andrew Morton wrote:
> On Sat,  1 Aug 2026 23:11:25 +0800 Jianlin Shi <shijianlin11@foxmail.com> 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 two 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.
> 
> AI review suggest that this caused a new problem:
> 
> 	https://sashiko.dev/#/patchset/tencent_1BB7A5C4D5EEA67346634417753190E92A09@qq.com
> 
> Not sure what to do here.  Perhaps pass proc_dointvec_minmax() a
> temporary then copy that into sysctl_lowmem_reserve_ratio if all
> proc_dointvec_minmax() returns "OK".

Seeing the v4 [1] it seems easier to keep the current fixup code until
proc_dointvec_minmax() is fixed.

> But really this is a flaw in proc_dointvec_minmax() isn't it?  It
> shouldn't update the table data until all the data has been validated.

I agree. What do the maintainers think?

[1]
https://lore.kernel.org/all/tencent_B9556590D4B65ACF74C06897689A6E39F206@qq.com/

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

[Joel Granados]

[-- Attachment #1: Type: text/plain, Size: 2066 bytes --]

On Mon, Aug 03, 2026 at 10:27:46AM +0200, Vlastimil Babka (SUSE) wrote:
> +Cc: sysctl maintainers
> 
> On 8/1/26 20:44, Andrew Morton wrote:
> > On Sat,  1 Aug 2026 23:11:25 +0800 Jianlin Shi <shijianlin11@foxmail.com> 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 two 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.
> > 
> > AI review suggest that this caused a new problem:
> > 
> > 	https://sashiko.dev/#/patchset/tencent_1BB7A5C4D5EEA67346634417753190E92A09@qq.com
> > 
> > Not sure what to do here.  Perhaps pass proc_dointvec_minmax() a
> > temporary then copy that into sysctl_lowmem_reserve_ratio if all
> > proc_dointvec_minmax() returns "OK".
> 
> Seeing the v4 [1] it seems easier to keep the current fixup code until
> proc_dointvec_minmax() is fixed.

1. V3 -vs- V4:
  I would prefer V4 as it actually prevents the partial write of the
  vector in case of an error & returns that error back to user space.
  Whereas V3 returns the error back to user space but keeps the partial
  write.

That patterns of using a temp ctltable entry is seldom used but not
unheard of.

> 
> > But really this is a flaw in proc_dointvec_minmax() isn't it?  It
> > shouldn't update the table data until all the data has been validated.
> 
> I agree. What do the maintainers think?

I agree. The arrays being changed are not too big so we can easily have
a staging variable that that gets written when all validations are done.
The only "con" that I see for this solution is for when these
proc_handlers get used with temp variables; in these cases we will be
staging an already staging variable.

I have added this to my Todos

Best

-- 

Joel

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

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

[Vlastimil Babka (SUSE)]

On 8/4/26 14:20, Joel Granados wrote:
> On Mon, Aug 03, 2026 at 10:27:46AM +0200, Vlastimil Babka (SUSE) wrote:
>> +Cc: sysctl maintainers
>> 
>> On 8/1/26 20:44, Andrew Morton wrote:
>> > On Sat,  1 Aug 2026 23:11:25 +0800 Jianlin Shi <shijianlin11@foxmail.com> 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 two 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.
>> > 
>> > AI review suggest that this caused a new problem:
>> > 
>> > 	https://sashiko.dev/#/patchset/tencent_1BB7A5C4D5EEA67346634417753190E92A09@qq.com
>> > 
>> > Not sure what to do here.  Perhaps pass proc_dointvec_minmax() a
>> > temporary then copy that into sysctl_lowmem_reserve_ratio if all
>> > proc_dointvec_minmax() returns "OK".
>> 
>> Seeing the v4 [1] it seems easier to keep the current fixup code until
>> proc_dointvec_minmax() is fixed.
> 
> 1. V3 -vs- V4:
>   I would prefer V4 as it actually prevents the partial write of the
>   vector in case of an error & returns that error back to user space.
>   Whereas V3 returns the error back to user space but keeps the partial
>   write.
> 
> That patterns of using a temp ctltable entry is seldom used but not
> unheard of.
> 
>> 
>> > But really this is a flaw in proc_dointvec_minmax() isn't it?  It
>> > shouldn't update the table data until all the data has been validated.
>> 
>> I agree. What do the maintainers think?
> 
> I agree. The arrays being changed are not too big so we can easily have
> a staging variable that that gets written when all validations are done.
> The only "con" that I see for this solution is for when these
> proc_handlers get used with temp variables; in these cases we will be
> staging an already staging variable.

Maybe have a variant of the function e.g. proc_dointvec_minmax() that takes
an extra parameter pointing to the staging array? That way the caller can
allocate it on stack according to its needs (like v4 does) but there's no
fiddling with a temp ctltable entry. That way the core code doesn't need a
staging variable big enough for everyone, and users can be converted to the
new variant, and there's not double staging at any point.

> I have added this to my Todos
> 
> Best
> 

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

[Joel Granados]

[-- Attachment #1: Type: text/plain, Size: 2507 bytes --]

On Tue, Aug 04, 2026 at 02:20:22PM +0200, Joel Granados wrote:
> On Mon, Aug 03, 2026 at 10:27:46AM +0200, Vlastimil Babka (SUSE) wrote:
> > +Cc: sysctl maintainers
> > 
> > On 8/1/26 20:44, Andrew Morton wrote:
> > > On Sat,  1 Aug 2026 23:11:25 +0800 Jianlin Shi <shijianlin11@foxmail.com> 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 two 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.
> > > 
> > > AI review suggest that this caused a new problem:
> > > 
> > > 	https://sashiko.dev/#/patchset/tencent_1BB7A5C4D5EEA67346634417753190E92A09@qq.com
> > > 
> > > Not sure what to do here.  Perhaps pass proc_dointvec_minmax() a
> > > temporary then copy that into sysctl_lowmem_reserve_ratio if all
> > > proc_dointvec_minmax() returns "OK".
> > 
> > Seeing the v4 [1] it seems easier to keep the current fixup code until
> > proc_dointvec_minmax() is fixed.
> 
> 1. V3 -vs- V4:
>   I would prefer V4 as it actually prevents the partial write of the
>   vector in case of an error & returns that error back to user space.
>   Whereas V3 returns the error back to user space but keeps the partial
>   write.
> 
> That patterns of using a temp ctltable entry is seldom used but not
> unheard of.
> 
> > 
> > > But really this is a flaw in proc_dointvec_minmax() isn't it?  It
> > > shouldn't update the table data until all the data has been validated.

Additionally, this will address the case where there is a partial write
to a vector where the input is erroneous. There is still a possibility
of having a "valid" partial write if you pass a set of valid values that
is less than the size of the vector.

> > 
> > I agree. What do the maintainers think?
> 
> I agree. The arrays being changed are not too big so we can easily have
> a staging variable that that gets written when all validations are done.
> The only "con" that I see for this solution is for when these
> proc_handlers get used with temp variables; in these cases we will be
> staging an already staging variable.
> 
> I have added this to my Todos
> 
> Best
> 
> -- 
> 
> Joel



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

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

[Jianlin Shi]

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).

Link: https://lore.kernel.org/linux-mm/tencent_FFD4F4D728AAE8A8AE0AF277A59854A29A06@qq.com/

Signed-off-by: Jianlin Shi <shijianlin11@foxmail.com>
---
Changes in v4:
- Parse writes into a temporary ratio[] array; commit and setup only on
  full success (Andrew Morton)
- Update handler comment: proc_dointvec_minmax() and write-only setup

Changes in v3:
- Rewrite commit log to focus on the two tangible fixes as suggested by
  Johannes Weiner.

Changes in v2:
- Add .extra1 = SYSCTL_ZERO to ctl_table entry
- Remove manual sanitization loop; negative writes now return -EINVAL

v1: https://lore.kernel.org/linux-mm/tencent_FFD4F4D728AAE8A8AE0AF277A59854A29A06@qq.com/

mm/page_alloc.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 0387d2afd..13fabb062 100644
--- 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
 	{
-- 
2.43.0

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@foxmail.com> 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?

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

[Jianlin Shi]

On Mon, 3 Aug 2026 17:48:17 -0700 Andrew Morton wrote:
> 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?

Agreed, that is nicer.  The temporary buffer is meant to mirror
sysctl_lowmem_reserve_ratio[], so sizing it with ARRAY_SIZE() makes
that relationship clearer than MAX_NR_ZONES.

I'll fold it into v5 and send shortly.

Thanks,
Jianlin

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

[Jianlin Shi]

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).

Link: https://lore.kernel.org/linux-mm/tencent_FFD4F4D728AAE8A8AE0AF277A59854A29A06@qq.com/

Signed-off-by: Jianlin Shi <shijianlin11@foxmail.com>
---
Changes in v5:
- Size temporary ratio[] with ARRAY_SIZE(sysctl_lowmem_reserve_ratio)
  (Andrew Morton)

Changes in v4:
- Parse writes into a temporary ratio[] array; commit and setup only on
  full success (Andrew Morton)
- Update handler comment: proc_dointvec_minmax() and write-only setup

Changes in v3:
- Rewrite commit log to focus on the two tangible fixes as suggested by
  Johannes Weiner.

Changes in v2:
- Add .extra1 = SYSCTL_ZERO to ctl_table entry
- Remove manual sanitization loop; negative writes now return -EINVAL

v1: https://lore.kernel.org/linux-mm/tencent_FFD4F4D728AAE8A8AE0AF277A59854A29A06@qq.com/

mm/page_alloc.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 0387d2afd535..51f35e44132a 100644
--- 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[ARRAY_SIZE(sysctl_lowmem_reserve_ratio)];
+	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
 	{
-- 
2.43.0

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

[Johannes Weiner]

On Thu, Aug 06, 2026 at 04:27:34PM +0800, Jianlin Shi 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).
> 
> Link: https://lore.kernel.org/linux-mm/tencent_FFD4F4D728AAE8A8AE0AF277A59854A29A06@qq.com/
> 
> Signed-off-by: Jianlin Shi <shijianlin11@foxmail.com>
> ---
> Changes in v5:
> - Size temporary ratio[] with ARRAY_SIZE(sysctl_lowmem_reserve_ratio)
>   (Andrew Morton)
> 
> Changes in v4:
> - Parse writes into a temporary ratio[] array; commit and setup only on
>   full success (Andrew Morton)
> - Update handler comment: proc_dointvec_minmax() and write-only setup
> 
> Changes in v3:
> - Rewrite commit log to focus on the two tangible fixes as suggested by
>   Johannes Weiner.
> 
> Changes in v2:
> - Add .extra1 = SYSCTL_ZERO to ctl_table entry
> - Remove manual sanitization loop; negative writes now return -EINVAL
> 
> v1: https://lore.kernel.org/linux-mm/tencent_FFD4F4D728AAE8A8AE0AF277A59854A29A06@qq.com/
> 
> mm/page_alloc.c | 24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 0387d2afd535..51f35e44132a 100644
> --- 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[ARRAY_SIZE(sysctl_lowmem_reserve_ratio)];
> +	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;
> -	}

This could use a comment. How about:

	/*
	 * proc_dointvec_max() works incrementally. Use a buffer
	 * and only set the values if all of them parse cleanly.
	 */

> +	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();

But otherwise, looks good to me now.

Acked-by: Johannes Weiner <hannes@cmpxchg.org>