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

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/[email protected]/
> 
> Signed-off-by: Jianlin Shi <[email protected]>

Code looks good to me. With the changelog fixed,

Acked-by: Johannes Weiner <[email protected]>