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

[SJ Park <sj@kernel.org>]

On Mon, 29 Jun 2026 19:53:49 +0300 IgorpetinDev <igorpetindev@gmail.com> wrote:

> __damon_va_three_regions() is called only by damon_va_three_regions(),
> which already holds mmap_read_lock(). Since mmap_read_lock() is held,
> the maple tree and VMA list are protected from concurrent changes.
> Remove the unnecessary rcu_read_lock() and rcu_read_unlock() calls.
> 
> Signed-off-by: IgorpetinDev <igorpetindev@gmail.com>

We prefer using a real name.  I show you used 'Igor Putko' as your name before.
Any reason to change the name?

> ---
>  mm/damon/vaddr.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
> index d27147603..345b89b5d 100644
> --- a/mm/damon/vaddr.c
> +++ b/mm/damon/vaddr.c
> @@ -84,7 +84,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) {
>  		unsigned long gap;
>  
> @@ -105,7 +104,6 @@ static int __damon_va_three_regions(struct mm_struct *mm,
>  next:
>  		prev = vma;
>  	}
> -	rcu_read_unlock();

As Sashiko also commented, this may cause lockdep warning.  Actually this was
added to fix it.  Refer to the previous discussions [1] for more details.

How about moving rcu_read_[un]lock() to the caller test code?

[1] https://lore.kernel.org/20240904004534.1189-1-sj@kernel.org


Thanks,
SJ

[...]