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] Removing `rcu_read_lock()` breaks the KUnit test `damon_test_three_regions_in_vmas()`, causing a lockdep splat.

Pre-existing issues:
- [High] VMA iteration on an external mm_struct is performed without checking for address space stability.
--

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

mm/damon/vaddr: remove redundant RCU lock

This commit removes the rcu_read_lock() and rcu_read_unlock() calls from
__damon_va_three_regions(). The commit message states that since
mmap_read_lock() is already held by the caller, the maple tree and VMA list
are protected from concurrent changes, making the RCU locks redundant.

> 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 removing this rcu_read_lock() cause a lockdep splat in the KUnit tests?

Looking at damon_test_three_regions_in_vmas() in mm/damon/tests/vaddr-kunit.h,
it calls __damon_va_three_regions() directly on a mocked maple tree:

	__damon_va_three_regions(&mm, regions);

Because this KUnit test doesn't hold mmap_read_lock() or configure an external
tree lock, removing the localized rcu_read_lock() here will cause for_each_vma()
to trigger RCU lockdep assertions for suspicious RCU usage.

[Severity: High]
This isn't a bug introduced by this patch, but does this function safely iterate
over an external mm_struct?

Looking at the caller damon_va_three_regions() in mm/damon/vaddr.c, it acquires
mmap_read_lock() on an external mm, but check_stable_address_space(mm) is never
called before iterating the VMAs.

If the target process fails dup_mmap() during fork or is processed by the OOM
reaper, its mm is flagged MMF_UNSTABLE and the maple tree slots may contain
XA_ZERO_ENTRY markers. Iterating over these markers without checking address
space stability can cause the iterator to return invalid VMA pointers, which
will cause a panic when dereferenced.

[ ... ]

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