public inbox for linux-mm@kvack.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/2] Docs/admin-guide/mm/damon: warn commit_inputs vs other params race
@ 2026-03-28 17:24 SeongJae Park
  2026-03-28 17:24 ` [RFC PATCH 1/2] Docs/admin-guide/mm/damon/reclaim: warn commit_inputs vs param updates race SeongJae Park
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: SeongJae Park @ 2026-03-28 17:24 UTC (permalink / raw)
  Cc: SeongJae Park, Liam R. Howlett, # 5 . 19 . x, Andrew Morton,
	David Hildenbrand, Jonathan Corbet, Lorenzo Stoakes, Michal Hocko,
	Mike Rapoport, Shuah Khan, Suren Baghdasaryan, Vlastimil Babka,
	damon, linux-doc, linux-kernel, linux-mm

Writing 'Y' to the commit_inputs parameter of DAMON_RECLAIM and
DAMON_LRU_SORT, and writing other parameters before the commit_inputs
request is completely processed can cause race conditions.  While the
consequence can be bad, the documentation is not clearly describing
that.  Add clear warnings.

The issue was discovered [1,2] by sashiko.

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

SeongJae Park (2):
  Docs/admin-guide/mm/damon/reclaim: warn commit_inputs vs param updates
    race
  Docs/admin-guide/mm/damon/lru_sort: warn commit_inputs vs param
    updates race

 Documentation/admin-guide/mm/damon/lru_sort.rst | 4 ++++
 Documentation/admin-guide/mm/damon/reclaim.rst  | 4 ++++
 2 files changed, 8 insertions(+)


base-commit: 02617badb619e548c4489c371fec5a4ceb0c347e
-- 
2.47.3


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [RFC PATCH 1/2] Docs/admin-guide/mm/damon/reclaim: warn commit_inputs vs param updates race
  2026-03-28 17:24 [RFC PATCH 0/2] Docs/admin-guide/mm/damon: warn commit_inputs vs other params race SeongJae Park
@ 2026-03-28 17:24 ` SeongJae Park
  2026-03-28 17:34   ` (sashiko review) " SeongJae Park
  2026-03-28 17:24 ` [RFC PATCH 2/2] Docs/admin-guide/mm/damon/lru_sort: " SeongJae Park
  2026-03-28 17:34 ` (sashiko status) [RFC PATCH 0/2] Docs/admin-guide/mm/damon: warn commit_inputs vs other params race SeongJae Park
  2 siblings, 1 reply; 8+ messages in thread
From: SeongJae Park @ 2026-03-28 17:24 UTC (permalink / raw)
  Cc: SeongJae Park, Liam R. Howlett, # 5 . 19 . x, Andrew Morton,
	David Hildenbrand, Jonathan Corbet, Lorenzo Stoakes, Michal Hocko,
	Mike Rapoport, Shuah Khan, Suren Baghdasaryan, Vlastimil Babka,
	damon, linux-doc, linux-kernel, linux-mm

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..7e22cdfd277a 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 parametr, the user must not write to any
+parameters until reading ``commit_inputs`` again returns ``N``.  If users
+violate this rule, the kernel can do undefined behaviors.
+
 min_age
 -------
 
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [RFC PATCH 2/2] Docs/admin-guide/mm/damon/lru_sort: warn commit_inputs vs param updates race
  2026-03-28 17:24 [RFC PATCH 0/2] Docs/admin-guide/mm/damon: warn commit_inputs vs other params race SeongJae Park
  2026-03-28 17:24 ` [RFC PATCH 1/2] Docs/admin-guide/mm/damon/reclaim: warn commit_inputs vs param updates race SeongJae Park
@ 2026-03-28 17:24 ` SeongJae Park
  2026-03-28 17:35   ` (sashiko review) " SeongJae Park
  2026-03-28 17:34 ` (sashiko status) [RFC PATCH 0/2] Docs/admin-guide/mm/damon: warn commit_inputs vs other params race SeongJae Park
  2 siblings, 1 reply; 8+ messages in thread
From: SeongJae Park @ 2026-03-28 17:24 UTC (permalink / raw)
  Cc: SeongJae Park, Liam R. Howlett, # 6 . 0 . x, Andrew Morton,
	David Hildenbrand, Jonathan Corbet, Lorenzo Stoakes, Michal Hocko,
	Mike Rapoport, Shuah Khan, Suren Baghdasaryan, Vlastimil Babka,
	damon, linux-doc, linux-kernel, linux-mm

DAMON_LRU_SORT 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-2-objecting@objecting.org

Fixes: 6acfcd0d7524 ("Docs/admin-guide/damon: add a document for DAMON_LRU_SORT")
Cc: <stable@vger.kernel.org> # 6.0.x
Signed-off-by: SeongJae Park <sj@kernel.org>
---
 Documentation/admin-guide/mm/damon/lru_sort.rst | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/admin-guide/mm/damon/lru_sort.rst b/Documentation/admin-guide/mm/damon/lru_sort.rst
index 797962a459e6..5b8e16394228 100644
--- a/Documentation/admin-guide/mm/damon/lru_sort.rst
+++ b/Documentation/admin-guide/mm/damon/lru_sort.rst
@@ -79,6 +79,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_LRU_SORT will be disabled.
 
+Once ``Y`` is written to this parametr, the user must not write to any
+parameters until reading ``commit_inputs`` again returns ``N``.  If users
+violate this rule, the kernel can do undefined behaviors.
+
 active_mem_bp
 -------------
 
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: (sashiko status) [RFC PATCH 0/2] Docs/admin-guide/mm/damon: warn commit_inputs vs other params race
  2026-03-28 17:24 [RFC PATCH 0/2] Docs/admin-guide/mm/damon: warn commit_inputs vs other params race SeongJae Park
  2026-03-28 17:24 ` [RFC PATCH 1/2] Docs/admin-guide/mm/damon/reclaim: warn commit_inputs vs param updates race SeongJae Park
  2026-03-28 17:24 ` [RFC PATCH 2/2] Docs/admin-guide/mm/damon/lru_sort: " SeongJae Park
@ 2026-03-28 17:34 ` SeongJae Park
  2 siblings, 0 replies; 8+ messages in thread
From: SeongJae Park @ 2026-03-28 17:34 UTC (permalink / raw)
  To: SeongJae Park
  Cc: Liam R. Howlett, # 5 . 19 . x, Andrew Morton, David Hildenbrand,
	Jonathan Corbet, Lorenzo Stoakes, Michal Hocko, Mike Rapoport,
	Shuah Khan, Suren Baghdasaryan, Vlastimil Babka, damon, linux-doc,
	linux-kernel, linux-mm

Forwarding sashiko.dev review status for this thread.

# review url: https://sashiko.dev/#/patchset/20260328172415.49940-1-sj@kernel.org

- [RFC PATCH 1/2] Docs/admin-guide/mm/damon/reclaim: warn commit_inputs vs param updates race
  - status: Reviewed
  - review: ISSUES MAY FOUND
- [RFC PATCH 2/2] Docs/admin-guide/mm/damon/lru_sort: warn commit_inputs vs param updates race
  - status: Reviewed
  - review: ISSUES MAY FOUND

# hkml [1] generated a draft of this mail.  It can be regenerated
# using below command:
#
#     hkml patch sashiko_dev --thread_status --for_forwarding \
#             20260328172415.49940-1-sj@kernel.org
#
# [1] https://github.com/sjp38/hackermail

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: (sashiko review) [RFC PATCH 1/2] Docs/admin-guide/mm/damon/reclaim: warn commit_inputs vs param updates race
  2026-03-28 17:24 ` [RFC PATCH 1/2] Docs/admin-guide/mm/damon/reclaim: warn commit_inputs vs param updates race SeongJae Park
@ 2026-03-28 17:34   ` SeongJae Park
  2026-03-28 17:48     ` SeongJae Park
  0 siblings, 1 reply; 8+ messages in thread
From: SeongJae Park @ 2026-03-28 17:34 UTC (permalink / raw)
  To: SeongJae Park
  Cc: Liam R. Howlett, # 5 . 19 . x, Andrew Morton, David Hildenbrand,
	Jonathan Corbet, Lorenzo Stoakes, Michal Hocko, Mike Rapoport,
	Shuah Khan, Suren Baghdasaryan, Vlastimil Babka, damon, linux-doc,
	linux-kernel, linux-mm

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)


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: (sashiko review) [RFC PATCH 2/2] Docs/admin-guide/mm/damon/lru_sort: warn commit_inputs vs param updates race
  2026-03-28 17:24 ` [RFC PATCH 2/2] Docs/admin-guide/mm/damon/lru_sort: " SeongJae Park
@ 2026-03-28 17:35   ` SeongJae Park
  2026-03-28 17:51     ` SeongJae Park
  0 siblings, 1 reply; 8+ messages in thread
From: SeongJae Park @ 2026-03-28 17:35 UTC (permalink / raw)
  To: SeongJae Park
  Cc: Liam R. Howlett, # 6 . 0 . x, Andrew Morton, David Hildenbrand,
	Jonathan Corbet, Lorenzo Stoakes, Michal Hocko, Mike Rapoport,
	Shuah Khan, Suren Baghdasaryan, Vlastimil Babka, damon, linux-doc,
	linux-kernel, linux-mm

Forwarding Sashiko review for doing discussions via mails.

# review url: https://sashiko.dev/#/patchset/20260328172415.49940-3-sj@kernel.org
# start of sashiko.dev inline review
commit 0a9a055d4225a92c337c76fef97bfcdca5d9508f
Author: SeongJae Park <sj@kernel.org>
Docs/admin-guide/mm/damon/lru_sort: warn commit_inputs vs param updates race

This patch adds documentation warning users about a data race in DAMON_LRU_SORT
when modifying module parameters while the kdamond thread is reading them. It
states that violating this rule can cause undefined behavior in the kernel.

Link: https://lore.kernel.org/20260319161620.189392-2-objecting@objecting.org

> diff --git a/Documentation/admin-guide/mm/damon/lru_sort.rst b/Documentation/admin-guide/mm/damon/lru_sort.rst
> index a7dea7c75a9b..3eaa3786c8ab 100644
> --- a/Documentation/admin-guide/mm/damon/lru_sort.rst
> +++ b/Documentation/admin-guide/mm/damon/lru_sort.rst
> @@ -79,6 +79,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_LRU_SORT 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 here (parametr).

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

Should this data race be fixed in the kernel code itself rather than just
documented?

The kernel generally should not exhibit undefined behavior, memory corruption,
or crashes due to concurrent sysfs or module parameter writes, even if those
writes are restricted to root users. Could the underlying code be updated
with proper synchronization to safely handle concurrent updates instead?

> +
>  active_mem_bp
>  -------------
>  


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

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: (sashiko review) [RFC PATCH 1/2] Docs/admin-guide/mm/damon/reclaim: warn commit_inputs vs param updates race
  2026-03-28 17:34   ` (sashiko review) " SeongJae Park
@ 2026-03-28 17:48     ` SeongJae Park
  0 siblings, 0 replies; 8+ messages in thread
From: SeongJae Park @ 2026-03-28 17:48 UTC (permalink / raw)
  To: SeongJae Park
  Cc: Liam R. Howlett, # 5 . 19 . x, Andrew Morton, David Hildenbrand,
	Jonathan Corbet, Lorenzo Stoakes, Michal Hocko, Mike Rapoport,
	Shuah Khan, Suren Baghdasaryan, Vlastimil Babka, damon, linux-doc,
	linux-kernel, linux-mm, Liew Rui Yan

On Sat, 28 Mar 2026 10:34:45 -0700 SeongJae Park <sj@kernel.org> wrote:

> 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.

Good eye, I will fix this in the next revision.

> 
> > +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?

Good callout.  In short, I agree for mainline kernel, but I'd argue this may be
enough for stable kernels.

That is, I was also thinking in a way similar to sashiko.  But, from the work I
realized the amount of changes for fixing those in stable-backporting-easy way
is not small.  Also, given the age of the broken commits (5.19!), I don't think
this is urgent.  And again arguably, as long as it is well documented, letting
users break kernel when they want (e.g., sysrq-trigger) is ok...?  Let me know
if other humans have different opinion, though.

But for future (mainline), surely this have to be fixed in the code.  And
Liew's patch [1] was initially started not for exactly this patch, but will fix
this together.  Cc-ing Liew.

So, I'm suggesting to add this documentation for stable kernels, and continue
helping Liew's patch for cleanly removing this awkward behavior in the mianline
and future kernels.

> 
> 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.

Good catch.  I will update so in the next version.

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


Thanks,
SJ

[...]


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: (sashiko review) [RFC PATCH 2/2] Docs/admin-guide/mm/damon/lru_sort: warn commit_inputs vs param updates race
  2026-03-28 17:35   ` (sashiko review) " SeongJae Park
@ 2026-03-28 17:51     ` SeongJae Park
  0 siblings, 0 replies; 8+ messages in thread
From: SeongJae Park @ 2026-03-28 17:51 UTC (permalink / raw)
  To: SeongJae Park
  Cc: Liam R. Howlett, # 6 . 0 . x, Andrew Morton, David Hildenbrand,
	Jonathan Corbet, Lorenzo Stoakes, Michal Hocko, Mike Rapoport,
	Shuah Khan, Suren Baghdasaryan, Vlastimil Babka, damon, linux-doc,
	linux-kernel, linux-mm

On Sat, 28 Mar 2026 10:35:04 -0700 SeongJae Park <sj@kernel.org> wrote:

> Forwarding Sashiko review for doing discussions via mails.

All comments are same to that for DAMON_RECLAIM.  Please refer to
https://lore.kernel.org/20260328174852.53338-1-sj@kernel.org for my response.


Thanks,
SJ

[...]


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-03-28 17:51 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-28 17:24 [RFC PATCH 0/2] Docs/admin-guide/mm/damon: warn commit_inputs vs other params race SeongJae Park
2026-03-28 17:24 ` [RFC PATCH 1/2] Docs/admin-guide/mm/damon/reclaim: warn commit_inputs vs param updates race SeongJae Park
2026-03-28 17:34   ` (sashiko review) " SeongJae Park
2026-03-28 17:48     ` SeongJae Park
2026-03-28 17:24 ` [RFC PATCH 2/2] Docs/admin-guide/mm/damon/lru_sort: " SeongJae Park
2026-03-28 17:35   ` (sashiko review) " SeongJae Park
2026-03-28 17:51     ` SeongJae Park
2026-03-28 17:34 ` (sashiko status) [RFC PATCH 0/2] Docs/admin-guide/mm/damon: warn commit_inputs vs other params race SeongJae Park

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox