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@xxxxxxxxxxx>
> ---
>  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;
>  }
>