Re: (sashiko review) [RFC 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/20260328172415.49940-2-sj@kernel.org
# start of sashiko.dev inline review
commit 8b189801fc104c5ecc412454a3d8db6a3faf27d6
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 write to module
parameters while commit_inputs is in progress. This warning is intended to
help users avoid a race condition that can cause undefined kernel behavior.

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
> --- 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 parametr, the user must not write to any

This isn't a bug, but there is a typo ("parametr") in this line.

> +parameters until reading ``commit_inputs`` again returns ``N``.  If users
> +violate this rule, the kernel can do undefined behaviors.

Is it acceptable to document a race condition that leads to undefined
behavior instead of fixing it?

Even if the module parameter interface is restricted to root, user space
shouldn't be able to trigger kernel undefined behavior (which could lead to
memory corruption or crashes) simply by writing to module parameters
concurrently.

Should the underlying race condition in DAMON_RECLAIM be fixed with proper
synchronization rather than relying on userspace adherence to a documentation
warning?

This isn't a bug, but the phrasing "can do undefined behaviors" is slightly
awkward. Rephrasing to "may exhibit undefined behavior" might be more natural.


# end of sashiko.dev inline review
# review url: https://sashiko.dev/#/patchset/20260328172415.49940-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 \
#             20260328172415.49940-2-sj@kernel.org
#
# [1] https://github.com/sjp38/hackermail

Sent using hkml (https://github.com/sjp38/hackermail)