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

[SeongJae Park <sj@kernel.org>]

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)