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

SeongJae Park posted 2 patches 3 days, 20 hours ago
[PATCH 1/2] Docs/admin-guide/mm/damon/reclaim: warn commit_inputs vs param updates race
Posted by SeongJae Park 3 days, 20 hours ago
DAMON_RECLAIM handles commit_inputs request inside kdamond thread,
reading the module parameters.  If the user updates the module
parameters while the kdamond thread is reading those, races can happen.
To avoid this, the commit_inputs parameter shows whether it is still in
the progress, assuming users wouldn't update parameters in the middle of
the work.  Some users might ignore that.  Add a warning about the
behavior.

The issue was discovered [1] by sashiko.

[1] https://lore.kernel.org/20260319161620.189392-3-objecting@objecting.org

Fixes: 81a84182c343 ("Docs/admin-guide/mm/damon/reclaim: document 'commit_inputs' parameter")
Cc: <stable@vger.kernel.org> # 5.19.x
Signed-off-by: SeongJae Park <sj@kernel.org>
---
 Documentation/admin-guide/mm/damon/reclaim.rst | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/admin-guide/mm/damon/reclaim.rst b/Documentation/admin-guide/mm/damon/reclaim.rst
index a100216f3a72..17e938c319e3 100644
--- a/Documentation/admin-guide/mm/damon/reclaim.rst
+++ b/Documentation/admin-guide/mm/damon/reclaim.rst
@@ -71,6 +71,10 @@ of parameters 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.
+
 min_age
 -------
 
-- 
2.47.3
Re: (sashiko review) [PATCH 1/2] Docs/admin-guide/mm/damon/reclaim: warn commit_inputs vs param updates race
Posted by SeongJae Park 3 days, 20 hours ago
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?


# end of sashiko.dev inline review
# review url: https://sashiko.dev/#/patchset/20260329153052.46657-2-sj@kernel.org
#
# hkml [1] generated a draft of this mail.  It can be regenerated
# using below command:
#
#     hkml patch sashiko_dev --for_forwarding \
#             20260329153052.46657-2-sj@kernel.org
#
# [1] https://github.com/sjp38/hackermail

Sent using hkml (https://github.com/sjp38/hackermail)
Re: (sashiko review) [PATCH 1/2] Docs/admin-guide/mm/damon/reclaim: warn commit_inputs vs param updates race
Posted by SeongJae Park 3 days, 20 hours ago
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

[...]