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

[Lance Yang]

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>
>---

Wow, pretty cool stuff. That's a nice speedup!

[...]
>+static void anon_rmap_tree_update_inplace(struct anon_vma_chain *avc)
>+{
>+#ifdef CONFIG_DEBUG_VM_RB
>+	avc->cached_vma_last = avc_last_pgoff(avc);
>+#endif
>+	/* Propagate all the way up the tree. */

Nit: propagate() can stop early when rb_subtree_last is unchanged ...

Maybe:

/* Update the subtree maximum and propagate any changes up the tree. */

>+	__anon_rmap_tree_augment.propagate(&avc->rb, NULL);
>+}
>+
[...]

Acked-by: Lance Yang <lance.yang@xxxxxxxxx>

Hammered it with VMA churn (split/merge/mremap/madvise/fork) + concurrent
rmap walks + hwpoison injection. Nothing complained :D

Tested-by: Lance Yang <lance.yang@xxxxxxxxx>

Cheers, Lance