* [PATCH] mm/page_alloc: only update lowmem_reserve_ratio on sysctl write @ 2026-07-28 10:32 Jianlin Shi 2026-07-30 15:28 ` Vlastimil Babka (SUSE) 2026-07-31 3:42 ` [PATCH v2] " Jianlin Shi 0 siblings, 2 replies; 6+ messages in thread From: Jianlin Shi @ 2026-07-28 10:32 UTC (permalink / raw) To: linux-mm; +Cc: akpm, vbabka, linux-kernel 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] mm/page_alloc: only update lowmem_reserve_ratio on sysctl write 2026-07-28 10:32 [PATCH] mm/page_alloc: only update lowmem_reserve_ratio on sysctl write Jianlin Shi @ 2026-07-30 15:28 ` Vlastimil Babka (SUSE) 2026-07-31 3:36 ` Jianlin Shi 2026-07-31 3:42 ` [PATCH v2] " Jianlin Shi 1 sibling, 1 reply; 6+ messages in thread From: Vlastimil Babka (SUSE) @ 2026-07-30 15:28 UTC (permalink / raw) To: Jianlin Shi, linux-mm; +Cc: akpm, linux-kernel 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; > } > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm/page_alloc: only update lowmem_reserve_ratio on sysctl write 2026-07-30 15:28 ` Vlastimil Babka (SUSE) @ 2026-07-31 3:36 ` Jianlin Shi 0 siblings, 0 replies; 6+ messages in thread From: Jianlin Shi @ 2026-07-31 3:36 UTC (permalink / raw) To: vbabka; +Cc: linux-mm, akpm, linux-kernel 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] mm/page_alloc: only update lowmem_reserve_ratio on sysctl write 2026-07-28 10:32 [PATCH] mm/page_alloc: only update lowmem_reserve_ratio on sysctl write Jianlin Shi 2026-07-30 15:28 ` Vlastimil Babka (SUSE) @ 2026-07-31 3:42 ` Jianlin Shi 2026-07-31 8:06 ` Vlastimil Babka (SUSE) 2026-08-01 7:36 ` Johannes Weiner 1 sibling, 2 replies; 6+ messages in thread From: Jianlin Shi @ 2026-07-31 3:42 UTC (permalink / raw) To: linux-mm; +Cc: vbabka, akpm, linux-kernel 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] mm/page_alloc: only update lowmem_reserve_ratio on sysctl write 2026-07-31 3:42 ` [PATCH v2] " Jianlin Shi @ 2026-07-31 8:06 ` Vlastimil Babka (SUSE) 2026-08-01 7:36 ` Johannes Weiner 1 sibling, 0 replies; 6+ messages in thread From: Vlastimil Babka (SUSE) @ 2026-07-31 8:06 UTC (permalink / raw) To: Jianlin Shi, linux-mm Cc: akpm, linux-kernel, Suren Baghdasaryan, Michal Hocko, Brendan Jackman, Johannes Weiner, Zi Yan 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 > { ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] mm/page_alloc: only update lowmem_reserve_ratio on sysctl write 2026-07-31 3:42 ` [PATCH v2] " Jianlin Shi 2026-07-31 8:06 ` Vlastimil Babka (SUSE) @ 2026-08-01 7:36 ` Johannes Weiner 1 sibling, 0 replies; 6+ messages in thread From: Johannes Weiner @ 2026-08-01 7:36 UTC (permalink / raw) To: Jianlin Shi; +Cc: linux-mm, vbabka, akpm, linux-kernel 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> ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-08-01 7:36 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-28 10:32 [PATCH] mm/page_alloc: only update lowmem_reserve_ratio on sysctl write Jianlin Shi 2026-07-30 15:28 ` Vlastimil Babka (SUSE) 2026-07-31 3:36 ` Jianlin Shi 2026-07-31 3:42 ` [PATCH v2] " Jianlin Shi 2026-07-31 8:06 ` Vlastimil Babka (SUSE) 2026-08-01 7:36 ` Johannes Weiner
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox