Re: [PATCH v3] mm/page_alloc: only update lowmem_reserve_ratio on sysctl write
Joel Granados <[email protected]>
| Newsgroups | gmane.linux.kernel,gmane.linux.kernel.mm |
|---|---|
| Message-ID | <zz5kb26cwem7spqanv6mqe6fwonzclyi3t2wjk5hb5tmj4pdl2@kjkut65bziui> |
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 <[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. 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
signature.asc
(application/pgp-signature, 659 B)
-----BEGIN PGP SIGNATURE----- iQGzBAABCgAdFiEErkcJVyXmMSXOyyeQupfNUreWQU8FAmp0RYAACgkQupfNUreW QU+3xAv/Yx0a9im95SNkHMroF5dhMJPx3iGulMyTEt26QecBwhmOpFk7A/w8EIjr jPE3ChuriSjNxzp4Fw3IYvZMLbqjrw+dbt3ILuP7UD35wA7VIh7rVql2kelQ2Rpy BwP+1EyXMSK3xN5EwRdpdAL6oOSOVGiq96sYkO++neBp2zSvO0GTneCsd8KazRh4 i+h9+PuGZXEtzsjzkGYAbUAYPlzqrZPLctnnxKaOXx0Uc+t8+mRwoJN1SyGxwWSj yTx7rmaU8X/dhooNeEl+eGZs3k5I+9xdjqZQThx47nTBq06dmqN7kTNkqJ78oFXH y86RXSKjgaXYgEtm0vkQW7t5uwnAhB/vXnFPrtXhVGDyc9hr5l348ESzpk4F8mIg q7cphPU+NGLnqIf4396cTUmbxz4xHcXtV9f6LUKVSGjn1t3QZmwDHn4IIMYdRdQk wd2DEL560T5zJPmtpYm9X0nkg9s9YIptLeEBq1Dc0SAj2oJ4F8yeIGtS1SSB3OSc 9f8sH6Tb =5S3C -----END PGP SIGNATURE-----