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 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.0On 8/6/26 10:27, 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> Acked-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
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>
On Thu, 6 Aug 2026 09:50:16 -0400 Johannes Weiner <hannes@cmpxchg.org> wrote:
> > - 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.
> */
>
I added a fixlet:
--- a/mm/page_alloc.c~mm-page_alloc-only-update-lowmem_reserve_ratio-on-sysctl-write-fix
+++ a/mm/page_alloc.c
@@ -6932,6 +6932,10 @@ static int lowmem_reserve_ratio_sysctl_h
if (!write)
return proc_dointvec_minmax(table, write, buffer, length, ppos);
+ /*
+ * 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;
_
Sashiko got all upset about concurrency issues:
https://sashiko.dev/#/patchset/tencent_A860C873956A52E26AD8D309A308A241BA08@qq.com
But I think I'm hearing from Joel is that the longer-term plan is to fix
all this up within proc_dointvec_minmax()?
On Thu, Aug 06, 2026 at 02:20:02PM -0700, Andrew Morton wrote:
> On Thu, 6 Aug 2026 09:50:16 -0400 Johannes Weiner <hannes@cmpxchg.org> wrote:
>
> > > - 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.
> > */
> >
>
> I added a fixlet:
>
> --- a/mm/page_alloc.c~mm-page_alloc-only-update-lowmem_reserve_ratio-on-sysctl-write-fix
> +++ a/mm/page_alloc.c
> @@ -6932,6 +6932,10 @@ static int lowmem_reserve_ratio_sysctl_h
> if (!write)
> return proc_dointvec_minmax(table, write, buffer, length, ppos);
>
> + /*
> + * 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;
>
> _
>
> Sashiko got all upset about concurrency issues:
>
> https://sashiko.dev/#/patchset/tencent_A860C873956A52E26AD8D309A308A241BA08@qq.com
>
> But I think I'm hearing from Joel is that the longer-term plan is to fix
> all this up within proc_dointvec_minmax()?
Indeed. From this I have two todos:
1. Address the partial write for sysctl vectors
2 Address the Potential concurrency issue
Best
Joel
© 2016 - 2026 Red Hat, Inc.