Re: (sashiko review) [PATCH 1/2] Docs/admin-guide/mm/damon/reclaim: warn commit_inputs vs param updates race

[SeongJae Park <sj@kernel.org>]

On Sun, 29 Mar 2026 08:49:36 -0700 SeongJae Park <sj@kernel.org> wrote:

> Forwarding Sashiko review for doing discussions via mails.
> 
> # review url: https://sashiko.dev/#/patchset/20260329153052.46657-2-sj@kernel.org
> # start of sashiko.dev inline review
> commit 140a828ed89f6ed59d7b68f9bb0f1ec9b051eec7
> Author: SeongJae Park <sj@kernel.org>
> 
> Docs/admin-guide/mm/damon/reclaim: warn commit_inputs vs param updates race
> 
> This commit adds documentation warning users not to update DAMON reclaim
> module parameters while a commit_inputs request is in progress. It notes
> that doing so can cause undefined behavior due to races with the kdamond
> thread reading those parameters.
> 
> Link: https://lore.kernel.org/20260319161620.189392-3-objecting@objecting.org
> 
> > diff --git a/Documentation/admin-guide/mm/damon/reclaim.rst b/Documentation/admin-guide/mm/damon/reclaim.rst
> > index 47854c461706..d7a0225b4950 100644
> > --- a/Documentation/admin-guide/mm/damon/reclaim.rst
> > +++ b/Documentation/admin-guide/mm/damon/reclaim.rst
> [ ... ]
> > @@ -71,6 +71,10 @@ of parametrs except ``enabled`` again.  Once the re-reading is done, this
> >  parameter is set as ``N``.  If invalid parameters are found while the
> >  re-reading, DAMON_RECLAIM will be disabled.
> >  
> > +Once ``Y`` is written to this parameter, the user must not write to any
> > +parameters until reading ``commit_inputs`` again returns ``N``.  If users
> > +violate this rule, the kernel may exhibit undefined behavior.
> 
> Is it acceptable to rely on documentation to prevent undefined behavior
> triggered by userspace?
> 
> If users write to sysfs parameters concurrently, can it still crash the
> kernel? For example, in damon_reclaim_apply_parameters(), the code verifies
> if (!damon_reclaim_mon_attrs.aggr_interval) to prevent division by zero. If
> a user concurrently writes 0 to the parameter via sysfs immediately after
> this check but before damon_reclaim_new_scheme() executes, wouldn't the
> subsequent .min_age_region = min_age / damon_reclaim_mon_attrs.aggr_interval
> calculation result in a divide-by-zero kernel panic?
> 
> Should this race be fixed in the code using synchronization primitives
> rather than adding a documentation warning?

I answered the questions on the previous version.  In short, I believe this is
ok for stable kernels.  For mainline, Liew's patch [1] that can also fix this
issue is on the way.

[1] https://lore.kernel.org/20260329075415.36775-1-aethernet65535@gmail.com


Thanks,
SJ

[...]