mm/page_alloc.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-)
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.0On 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?
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
© 2016 - 2026 Red Hat, Inc.