Re: [PATCH v9 11/17] mm: replace vm_lock and detached flag with a reference count

[Wei Yang <richard.weiyang@gmail.com>]

On Fri, Jan 10, 2025 at 08:25:58PM -0800, Suren Baghdasaryan wrote:
>@@ -6354,7 +6422,6 @@ struct vm_area_struct *lock_vma_under_rcu(struct mm_struct *mm,
> 	struct vm_area_struct *vma;
> 
> 	rcu_read_lock();
>-retry:
> 	vma = mas_walk(&mas);
> 	if (!vma)
> 		goto inval;
>@@ -6362,13 +6429,6 @@ struct vm_area_struct *lock_vma_under_rcu(struct mm_struct *mm,
> 	if (!vma_start_read(vma))
> 		goto inval;
> 
>-	/* Check if the VMA got isolated after we found it */
>-	if (is_vma_detached(vma)) {
>-		vma_end_read(vma);
>-		count_vm_vma_lock_event(VMA_LOCK_MISS);
>-		/* The area was replaced with another one */
>-		goto retry;
>-	}

We have a little behavior change here.

Originally, if we found an detached vma, we may retry. But now, we would go to
the slow path directly. 

Maybe we can compare the event VMA_LOCK_MISS and VMA_LOCK_ABORT
to see the percentage of this case. If it shows this is a too rare
case to impact performance, we can ignore it.

Also the event VMA_LOCK_MISS recording is removed, but the definition is
there. We may record it in the vma_start_read() when oldcnt is 0.

BTW, the name of VMA_LOCK_SUCCESS confuse me a little. I thought it indicates
lock_vma_under_rcu() successfully get a valid vma. But seems not. Sounds we
don't have an overall success/failure statistic in vmstat.

> 	/*
> 	 * At this point, we have a stable reference to a VMA: The VMA is
> 	 * locked and we know it hasn't already been isolated.

-- 
Wei Yang
Help you, Help me