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

Joel Granados <[email protected]> Tue, 4 Aug 2026 14:20:22 +0200

Newsgroups	gmane.linux.kernel,gmane.linux.kernel.mm
Message-ID	<k53aljpl7v7naqs5fj4i4po6tu4grkzu5nui4h37orwt6v7bqo@sw4frf2ystmi>

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 <[email protected]> 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/[email protected]
> > 
> > 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.
> 
> 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

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