Re: [PATCH] mm/damon/vaddr: remove redundant RCU lock

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] The removal of `rcu_read_lock()` breaks the KUnit test `damon_test_three_regions_in_vmas()`, causing a lockdep splat.

Pre-existing issues:
- [High] VMA tree iteration on an external mm_struct is performed without checking if the address space is stable, risking a crash on unstable MMs.
--

commit e3efcd87bda8a6d5dd6bcc064e6f5876f9980a31
Author: IgorpetinDev <igorpetindev@gmail.com>

mm/damon/vaddr: remove redundant RCU lock

This patch removes the rcu_read_lock() and rcu_read_unlock() calls from
__damon_va_three_regions(), assuming that the mmap_read_lock is always
held by the caller.

> __damon_va_three_regions() is called only by damon_va_three_regions(),
> which already holds mmap_read_lock(). Since mmap_read_lock() is held,

[Severity: High]
This is a pre-existing issue, but when damon_va_three_regions() acquires the
mmap_read_lock on an external task's mm_struct, is it safe to begin
traversing the VMA tree without first verifying address space stability?

If the target process is concurrently undergoing an OOM kill or failed a
dup_mmap(), the mm_struct could be flagged MMF_UNSTABLE and the maple tree
could contain XA_ZERO_ENTRY markers. Should damon_va_three_regions() call
check_stable_address_space() before __damon_va_three_regions() traverses
the tree, to prevent a potential kernel panic from dereferencing invalid
markers?

> diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
> index e73ec1ce016e5..882cc7cb09027 100644
> --- a/mm/damon/vaddr.c
> +++ b/mm/damon/vaddr.c
> @@ -82,7 +82,6 @@ static int __damon_va_three_regions(struct mm_struct *mm,
>  	 * If this is too slow, it can be optimised to examine the maple
>  	 * tree gaps.
>  	 */
> -	rcu_read_lock();
>  	for_each_vma(vmi, vma) {

[Severity: Medium]
Will the removal of this lock break the KUnit test
damon_test_three_regions_in_vmas() located in mm/damon/tests/vaddr-kunit.h?

The test framework calls __damon_va_three_regions() directly using a dummy
mm_struct and a maple tree initialized without the MT_FLAGS_LOCK_EXTERN flag.
Since the test environment doesn't hold the internal maple tree lock or an
external mmap_lock, won't removing this rcu_read_lock() cause a lockdep
warning (suspicious rcu_dereference_check usage) when for_each_vma()
attempts to traverse the tree?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629165349.4260-1-igorpetindev@gmail.com?part=1