Re: [PATCH v2] mm/vma: don't remove VMA from rmap if pgoff unchanged

[Pedro Falcato]

On Wed, Sep 30, 2026 at 06:53:36PM +0100, Lorenzo Stoakes (ARM) wrote:
> When updating a VMA, vma_prepare() unconditionally removes it from its rmap
> interval trees under the rmap lock, and vma_complete() reinserts it before
> releasing the lock.
> 
> This is wholly unnecessary if its page offset (file rmap) or anonymous page
> offset (anon rmap) is unchanged.
> 
> So, track whether they will change in the newly introduced
> vp->file_pgoff_unchanged and vp->anon_pgoff_unchanged fields, and use them
> to determine whether to remove the VMA or not.
> 
> The rmap lock keeps things safe as no rmap walks can concurrently occur
> during the operation.
> 
> Additionally, some architectures (arm, parisc, nios2, csky) have dcache
> flush rmap walkers which take only flush_dcache_mmap_lock(), which is
> likewise held across the operation.
> 
> If the VMA remains in the tree, it's necessary to keep the augmented
> rb_subtree_last field updated to reflect its changed range.
> 
> Provide mapping_rmap_tree_[pre, post]_update() and
> anon_rmap_tree_[pre, post]_update_vma() (replacing the existing logic in
> the anonymous case) to handle both the changed and unchanged cases.
> 
> For the anon rmap case, with CONFIG_DEBUG_VM_RB set, avc->cached_vma_last
> is also updated when propagating in place.
> 
> When performing a VMA shrink or a split where the VMA is the lower one, the
> page offset cannot change, so set the flags unconditionally in these cases.
> 
> When merging VMAs the page offset is unchanged only in some cases, so
> update init_multi_vma_prep() to set the flags only if the page offsets
> remain the same.
> 
> Finally, while we're here, also update expand_upwards() similarly.
> 
> These changes ultimately result in less rmap lock contention.
> 
> Pan Deng reported results using the UnixBench/excel benchmark on a 2-socket
> 192 core, 384 thread x86-64 system for v7.3-rc4 with/without the patch
> applied:
> 
> Execl Throughput, index score:
> 
>                     avg    %stdev      min        max
>   v7.3-rc4        3511.5     0.44%    3494.5    3543.4
>   + patch         4069.0     0.48%    4047.4    4109.5 (+15.9%)
> 
> Average wait on file rmap lock in ms, 5 runs per kernel:
> 
>                avg      %stdev     min       max
>   v7.3-rc4    9.470     2.91%     9.070     9.820
>   + patch     8.420     3.34%     8.100     8.770 (-11.1%)
> 
> Profiling data obtained during the operation highlighted the file rmap lock
> as the primary source of contention.
> 
> Suggested-by: Pan Deng <pan.deng@xxxxxxxxx>
> Reviewed-by: Rik van Riel <riel@xxxxxxxxxxx>
> Signed-off-by: Lorenzo Stoakes (ARM) <ljs@xxxxxxxxxx>

Looks great, thanks!

Reviewed-by: Pedro Falcato <pfalcato@xxxxxxx>

<snip>

> index 077e23694143..ad6be42cc65a 100644
> --- a/mm/vma.c
> +++ b/mm/vma.c
> @@ -201,8 +201,15 @@ static void init_multi_vma_prep(struct vma_prepare *vp,
>  	if (vp->file)
>  		vp->mapping = vma->vm_file->f_mapping;
>  
> -	if (vmg && vmg->skip_vma_uprobe)
> +	if (!vmg)
> +		return;
> +
> +	if (vmg->skip_vma_uprobe)
>  		vp->skip_vma_uprobe = true;
> +	if (vma_start_pgoff(vma) == vmg_start_pgoff(vmg))
> +		vp->file_pgoff_unchanged = true;
> +	if (vma_start_anon_pgoff(vma) == vmg_start_anon_pgoff(vmg))
> +		vp->anon_pgoff_unchanged = true;

FWIW I kinda would've liked to abstract this out to rmap code, but a quick look
shows that it is likely to become a header dependency hell if put in rmap.h. Could
perhaps have been separated out to a function call.

Anyway, your call :) This version already hides most of the ugly to
interval tree code.
 

-- 
Pedro