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

Joel Granados <[email protected]> Fri, 7 Aug 2026 11:45:11 +0200

Newsgroups	org.kvack.linux-mm,org.kernel.vger.linux-kernel
Message-ID	<pig4crs5bj5aiswzaozygjek6tj7csyhvjb6k6rb4cbro6efnf@3j7xsrrbmi7k>

On Thu, Aug 06, 2026 at 02:20:02PM -0700, Andrew Morton wrote:
> On Thu, 6 Aug 2026 09:50:16 -0400 Johannes Weiner <[email protected]> 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/[email protected]
> 
> 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

signature.asc (application/pgp-signature, 659 B)