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

[Joel Granados]

On Tue, Aug 04, 2026 at 02:20:22PM +0200, Joel Granados wrote:
> On Mon, Aug 03, 2026 at 10:27:46AM +0200, Vlastimil Babka (SUSE) wrote:
> > +Cc: sysctl maintainers
> > 
> > On 8/1/26 20:44, Andrew Morton wrote:
> > > On Sat,  1 Aug 2026 23:11:25 +0800 Jianlin Shi <shijianlin11@xxxxxxxxxxx> 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 two 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.
> > > 
> > > AI review suggest that this caused a new problem:
> > > 
> > > 	https://sashiko.dev/#/patchset/tencent_1BB7A5C4D5EEA67346634417753190E92A09@xxxxxx
> > > 
> > > Not sure what to do here.  Perhaps pass proc_dointvec_minmax() a
> > > temporary then copy that into sysctl_lowmem_reserve_ratio if all
> > > proc_dointvec_minmax() returns "OK".
> > 
> > Seeing the v4 [1] it seems easier to keep the current fixup code until
> > proc_dointvec_minmax() is fixed.
> 
> 1. V3 -vs- V4:
>   I would prefer V4 as it actually prevents the partial write of the
>   vector in case of an error & returns that error back to user space.
>   Whereas V3 returns the error back to user space but keeps the partial
>   write.
> 
> That patterns of using a temp ctltable entry is seldom used but not
> unheard of.
> 
> > 
> > > But really this is a flaw in proc_dointvec_minmax() isn't it?  It
> > > shouldn't update the table data until all the data has been validated.

Additionally, this will address the case where there is a partial write
to a vector where the input is erroneous. There is still a possibility
of having a "valid" partial write if you pass a set of valid values that
is less than the size of the vector.

> > 
> > I agree. What do the maintainers think?
> 
> I agree. The arrays being changed are not too big so we can easily have
> a staging variable that that gets written when all validations are done.
> The only "con" that I see for this solution is for when these
> proc_handlers get used with temp variables; in these cases we will be
> staging an already staging variable.
> 
> I have added this to my Todos
> 
> Best
> 
> -- 
> 
> Joel

Attachment: signature.asc
Description: PGP signature