* [PATCH] mm/vma: don't remove VMA from rmap if pgoff unchanged
@ 2026-09-25 18:32 Lorenzo Stoakes (ARM)
2026-09-25 18:59 ` Rik van Riel
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-09-25 18:32 UTC (permalink / raw)
To: Andrew Morton, David Hildenbrand, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Rik van Riel, Harry Yoo, Jann Horn, Lance Yang, Pedro Falcato
Cc: linux-mm, linux-kernel, Pan Deng, Lorenzo Stoakes, Lorenzo Stoakes (ARM)
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->anon_pgoff_unchanged and vp->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.
It's also necessary to keep the rb_subtree_last field updated in the
interval tree so implement anon_rmap_tree_update_inplace() and
mapping_rmap_tree_update_inplace() to do that.
This is done in vma_complete(), after the VMA's range has been updated, so
in the interim the field may be invalid. However, given the locks described
above, this cannot be observed until after the state is corrected.
The anonymous rmap is keyed on anon_vma_chains not VMAs, so in those
instances anon_rmap_tree_update_vma_inplace() iterates over
vma->anon_vma_chain, invoking anon_rmap_tree_update_inplace() on each one.
For the anon rmap case, with CONFIG_DEBUG_VM_RB set, avc->cached_vma_last
is also updated in anon_rmap_tree_update_inplace().
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.
These changes ultimately result in less rmap lock contention.
Reported-by: Pan Deng <pan.deng@intel.com>
Closes: https://lore.kernel.org/linux-mm/20260924054301.2330822-1-pan.deng@intel.com/
Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
---
Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
---
include/linux/mm.h | 3 +++
mm/interval_tree.c | 33 ++++++++++++++++++++++++++++++++
mm/vma.c | 40 ++++++++++++++++++++++++++++++++++-----
mm/vma.h | 2 ++
tools/testing/vma/include/stubs.h | 8 ++++++++
5 files changed, 81 insertions(+), 5 deletions(-)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 6e71eaa4af3f..94c2eb055716 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -4357,6 +4357,8 @@ void mapping_rmap_tree_insert_after(struct vm_area_struct *vma,
struct address_space *mapping);
void mapping_rmap_tree_remove(struct vm_area_struct *vma,
struct address_space *mapping);
+void mapping_rmap_tree_update_inplace(struct vm_area_struct *vma);
+
struct vm_area_struct *
mapping_rmap_tree_iter_first(struct address_space *mapping,
pgoff_t pgoff_start, pgoff_t pgoff_last);
@@ -4374,6 +4376,7 @@ void anon_rmap_tree_insert(struct anon_vma_chain *avc,
struct anon_vma *anon_vma);
void anon_rmap_tree_remove(struct anon_vma_chain *avc,
struct anon_vma *anon_vma);
+void anon_rmap_tree_update_inplace(struct anon_vma_chain *avc);
struct anon_vma_chain *
anon_rmap_tree_iter_first(struct anon_vma *anon_vma,
pgoff_t pgoff_start, pgoff_t pgoff_last);
diff --git a/mm/interval_tree.c b/mm/interval_tree.c
index 7bbbf15cfbf0..eafde5d12ef5 100644
--- a/mm/interval_tree.c
+++ b/mm/interval_tree.c
@@ -64,6 +64,21 @@ void mapping_rmap_tree_remove(struct vm_area_struct *vma,
__mapping_rmap_tree_remove(vma, &mapping->i_mmap);
}
+/**
+ * mapping_rmap_tree_update_inplace() - Update file rmap tree to reflect an
+ * in-place change in a VMA's size.
+ * @vma: The VMA whose size has changed.
+ *
+ * The file rmap lock must be held.
+ *
+ * Invalid to do so if @vma->vm_pgoff has changed.
+ */
+void mapping_rmap_tree_update_inplace(struct vm_area_struct *vma)
+{
+ /* Propagate all the way up the tree. */
+ __mapping_rmap_tree_augment.propagate(&vma->shared.rb, NULL);
+}
+
struct vm_area_struct *
mapping_rmap_tree_iter_first(struct address_space *mapping,
pgoff_t pgoff_start, pgoff_t pgoff_last)
@@ -111,6 +126,24 @@ void anon_rmap_tree_remove(struct anon_vma_chain *avc,
__anon_rmap_tree_remove(avc, &anon_vma->rb_root);
}
+/**
+ * anon_rmap_tree_update_inplace() - Update anon rmap tree to reflect an
+ * in-place change in the size of @avc's VMA.
+ * @avc: The anon_vma_chain whose VMA's size has changed.
+ *
+ * The anon rmap root lock must be held.
+ *
+ * Invalid to do so if the VMA's anonymous pgoff has changed.
+ */
+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. */
+ __anon_rmap_tree_augment.propagate(&avc->rb, NULL);
+}
+
struct anon_vma_chain *
anon_rmap_tree_iter_first(struct anon_vma *anon_vma,
pgoff_t pgoff_start, pgoff_t pgoff_last)
diff --git a/mm/vma.c b/mm/vma.c
index 077e23694143..8b333ec0c958 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->pgoff_unchanged = true;
+ if (vma_start_anon_pgoff(vma) == vmg_start_anon_pgoff(vmg))
+ vp->anon_pgoff_unchanged = true;
}
/*
@@ -331,6 +338,15 @@ anon_rmap_tree_post_update_vma(struct vm_area_struct *vma)
anon_rmap_tree_insert(avc, avc->anon_vma);
}
+static void
+anon_rmap_tree_update_vma_inplace(struct vm_area_struct *vma)
+{
+ struct anon_vma_chain *avc;
+
+ list_for_each_entry(avc, &vma->anon_vma_chain, same_vma)
+ anon_rmap_tree_update_inplace(avc);
+}
+
/*
* vma_prepare() - Helper function for handling locking VMAs prior to altering
* @vp: The initialized vma_prepare struct
@@ -359,14 +375,16 @@ static void vma_prepare(struct vma_prepare *vp)
if (vp->anon_vma) {
anon_vma_lock_write(vp->anon_vma);
- anon_rmap_tree_pre_update_vma(vp->vma);
+ if (!vp->anon_pgoff_unchanged)
+ anon_rmap_tree_pre_update_vma(vp->vma);
if (vp->adj_next)
anon_rmap_tree_pre_update_vma(vp->adj_next);
}
if (vp->file) {
flush_dcache_mmap_lock(vp->mapping);
- mapping_rmap_tree_remove(vp->vma, vp->mapping);
+ if (!vp->pgoff_unchanged)
+ mapping_rmap_tree_remove(vp->vma, vp->mapping);
if (vp->adj_next)
mapping_rmap_tree_remove(vp->adj_next, vp->mapping);
}
@@ -387,7 +405,11 @@ static void vma_complete(struct vma_prepare *vp, struct vma_iterator *vmi,
if (vp->file) {
if (vp->adj_next)
mapping_rmap_tree_insert(vp->adj_next, vp->mapping);
- mapping_rmap_tree_insert(vp->vma, vp->mapping);
+ /* Need only propagate the change inplace. */
+ if (vp->pgoff_unchanged)
+ mapping_rmap_tree_update_inplace(vp->vma);
+ else
+ mapping_rmap_tree_insert(vp->vma, vp->mapping);
flush_dcache_mmap_unlock(vp->mapping);
}
@@ -406,7 +428,11 @@ static void vma_complete(struct vma_prepare *vp, struct vma_iterator *vmi,
}
if (vp->anon_vma) {
- anon_rmap_tree_post_update_vma(vp->vma);
+ /* Need only propagate the change inplace. */
+ if (vp->anon_pgoff_unchanged)
+ anon_rmap_tree_update_vma_inplace(vp->vma);
+ else
+ anon_rmap_tree_post_update_vma(vp->vma);
if (vp->adj_next)
anon_rmap_tree_post_update_vma(vp->adj_next);
anon_vma_unlock_write(vp->anon_vma);
@@ -593,6 +619,8 @@ __split_vma(struct vma_iterator *vmi, struct vm_area_struct *vma,
init_vma_prep(&vp, vma);
vp.insert = new;
+ vp.pgoff_unchanged = !new_below;
+ vp.anon_pgoff_unchanged = !new_below;
vma_prepare(&vp);
/*
@@ -1346,6 +1374,8 @@ int vma_shrink(struct vma_iterator *vmi, struct vm_area_struct *vma,
vma_start_write(vma);
init_vma_prep(&vp, vma);
+ vp.pgoff_unchanged = true;
+ vp.anon_pgoff_unchanged = true;
vma_prepare(&vp);
vma_adjust_trans_huge(vma, vma->vm_start, end, NULL);
diff --git a/mm/vma.h b/mm/vma.h
index 7a683272c0a8..336b4ced82c9 100644
--- a/mm/vma.h
+++ b/mm/vma.h
@@ -28,6 +28,8 @@ struct vma_prepare {
struct vm_area_struct *remove2;
bool skip_vma_uprobe :1;
+ bool pgoff_unchanged :1;
+ bool anon_pgoff_unchanged :1;
};
struct unlink_vma_file_batch {
diff --git a/tools/testing/vma/include/stubs.h b/tools/testing/vma/include/stubs.h
index e4acc6f1fe7b..f0a69393c02a 100644
--- a/tools/testing/vma/include/stubs.h
+++ b/tools/testing/vma/include/stubs.h
@@ -267,6 +267,10 @@ static inline void mapping_rmap_tree_remove(struct vm_area_struct *vma,
{
}
+static inline void mapping_rmap_tree_update_inplace(struct vm_area_struct *vma)
+{
+}
+
static inline void flush_dcache_mmap_unlock(struct address_space *mapping)
{
}
@@ -281,6 +285,10 @@ static inline void anon_rmap_tree_remove(struct anon_vma_chain *avc,
{
}
+static inline void anon_rmap_tree_update_inplace(struct anon_vma_chain *avc)
+{
+}
+
static inline void uprobe_mmap(struct vm_area_struct *vma)
{
}
---
base-commit: e8d0f6a1b2a447d02845984fa6288787543cb03c
change-id: 20260925-speed-up-inplace-rmap-808fbb3848f5
Best regards,
--
Lorenzo Stoakes (ARM) <ljs@kernel.org>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] mm/vma: don't remove VMA from rmap if pgoff unchanged
2026-09-25 18:32 [PATCH] mm/vma: don't remove VMA from rmap if pgoff unchanged Lorenzo Stoakes (ARM)
@ 2026-09-25 18:59 ` Rik van Riel
2026-09-28 15:10 ` Lorenzo Stoakes (ARM)
2026-09-28 15:37 ` Pedro Falcato
2 siblings, 0 replies; 7+ messages in thread
From: Rik van Riel @ 2026-09-25 18:59 UTC (permalink / raw)
To: Lorenzo Stoakes (ARM),
Andrew Morton, David Hildenbrand, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Harry Yoo, Jann Horn, Lance Yang, Pedro Falcato
Cc: linux-mm, linux-kernel, Pan Deng
On Fri, 2026-09-25 at 19:32 +0100, Lorenzo Stoakes (ARM) wrote:
>
> 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.
>
> These changes ultimately result in less rmap lock contention.
>
> Reported-by: Pan Deng <pan.deng@intel.com>
> Closes:
> https://lore.kernel.org/linux-mm/20260924054301.2330822-1-pan.deng@intel.com/
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
Nice optimization.
Reviewed-by: Rik van Riel <riel@surriel.com>
--
All Rights Reversed.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mm/vma: don't remove VMA from rmap if pgoff unchanged
2026-09-25 18:32 [PATCH] mm/vma: don't remove VMA from rmap if pgoff unchanged Lorenzo Stoakes (ARM)
2026-09-25 18:59 ` Rik van Riel
@ 2026-09-28 15:10 ` Lorenzo Stoakes (ARM)
2026-09-28 15:37 ` Pedro Falcato
2 siblings, 0 replies; 7+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-09-28 15:10 UTC (permalink / raw)
To: Andrew Morton, David Hildenbrand, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Rik van Riel, Harry Yoo, Jann Horn, Lance Yang, Pedro Falcato
Cc: linux-mm, linux-kernel, Pan Deng
On Fri, Sep 25, 2026 at 07:32:20PM +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->anon_pgoff_unchanged and vp->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.
>
> It's also necessary to keep the rb_subtree_last field updated in the
> interval tree so implement anon_rmap_tree_update_inplace() and
> mapping_rmap_tree_update_inplace() to do that.
>
> This is done in vma_complete(), after the VMA's range has been updated, so
> in the interim the field may be invalid. However, given the locks described
> above, this cannot be observed until after the state is corrected.
>
> The anonymous rmap is keyed on anon_vma_chains not VMAs, so in those
> instances anon_rmap_tree_update_vma_inplace() iterates over
> vma->anon_vma_chain, invoking anon_rmap_tree_update_inplace() on each one.
>
> For the anon rmap case, with CONFIG_DEBUG_VM_RB set, avc->cached_vma_last
> is also updated in anon_rmap_tree_update_inplace().
>
> 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.
>
> These changes ultimately result in less rmap lock contention.
>
> Reported-by: Pan Deng <pan.deng@intel.com>
> Closes: https://lore.kernel.org/linux-mm/20260924054301.2330822-1-pan.deng@intel.com/
Andrew - Can you switch these 2 tags to:
Suggested-by: Pan Deng <pan.deng@intel.com>
Please, thanks! :)
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
> ---
> Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> ---
> include/linux/mm.h | 3 +++
> mm/interval_tree.c | 33 ++++++++++++++++++++++++++++++++
> mm/vma.c | 40 ++++++++++++++++++++++++++++++++++-----
> mm/vma.h | 2 ++
> tools/testing/vma/include/stubs.h | 8 ++++++++
> 5 files changed, 81 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 6e71eaa4af3f..94c2eb055716 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -4357,6 +4357,8 @@ void mapping_rmap_tree_insert_after(struct vm_area_struct *vma,
> struct address_space *mapping);
> void mapping_rmap_tree_remove(struct vm_area_struct *vma,
> struct address_space *mapping);
> +void mapping_rmap_tree_update_inplace(struct vm_area_struct *vma);
> +
> struct vm_area_struct *
> mapping_rmap_tree_iter_first(struct address_space *mapping,
> pgoff_t pgoff_start, pgoff_t pgoff_last);
> @@ -4374,6 +4376,7 @@ void anon_rmap_tree_insert(struct anon_vma_chain *avc,
> struct anon_vma *anon_vma);
> void anon_rmap_tree_remove(struct anon_vma_chain *avc,
> struct anon_vma *anon_vma);
> +void anon_rmap_tree_update_inplace(struct anon_vma_chain *avc);
> struct anon_vma_chain *
> anon_rmap_tree_iter_first(struct anon_vma *anon_vma,
> pgoff_t pgoff_start, pgoff_t pgoff_last);
> diff --git a/mm/interval_tree.c b/mm/interval_tree.c
> index 7bbbf15cfbf0..eafde5d12ef5 100644
> --- a/mm/interval_tree.c
> +++ b/mm/interval_tree.c
> @@ -64,6 +64,21 @@ void mapping_rmap_tree_remove(struct vm_area_struct *vma,
> __mapping_rmap_tree_remove(vma, &mapping->i_mmap);
> }
>
> +/**
> + * mapping_rmap_tree_update_inplace() - Update file rmap tree to reflect an
> + * in-place change in a VMA's size.
> + * @vma: The VMA whose size has changed.
> + *
> + * The file rmap lock must be held.
> + *
> + * Invalid to do so if @vma->vm_pgoff has changed.
> + */
> +void mapping_rmap_tree_update_inplace(struct vm_area_struct *vma)
> +{
> + /* Propagate all the way up the tree. */
> + __mapping_rmap_tree_augment.propagate(&vma->shared.rb, NULL);
> +}
> +
> struct vm_area_struct *
> mapping_rmap_tree_iter_first(struct address_space *mapping,
> pgoff_t pgoff_start, pgoff_t pgoff_last)
> @@ -111,6 +126,24 @@ void anon_rmap_tree_remove(struct anon_vma_chain *avc,
> __anon_rmap_tree_remove(avc, &anon_vma->rb_root);
> }
>
> +/**
> + * anon_rmap_tree_update_inplace() - Update anon rmap tree to reflect an
> + * in-place change in the size of @avc's VMA.
> + * @avc: The anon_vma_chain whose VMA's size has changed.
> + *
> + * The anon rmap root lock must be held.
> + *
> + * Invalid to do so if the VMA's anonymous pgoff has changed.
> + */
> +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. */
> + __anon_rmap_tree_augment.propagate(&avc->rb, NULL);
> +}
> +
> struct anon_vma_chain *
> anon_rmap_tree_iter_first(struct anon_vma *anon_vma,
> pgoff_t pgoff_start, pgoff_t pgoff_last)
> diff --git a/mm/vma.c b/mm/vma.c
> index 077e23694143..8b333ec0c958 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->pgoff_unchanged = true;
> + if (vma_start_anon_pgoff(vma) == vmg_start_anon_pgoff(vmg))
> + vp->anon_pgoff_unchanged = true;
> }
>
> /*
> @@ -331,6 +338,15 @@ anon_rmap_tree_post_update_vma(struct vm_area_struct *vma)
> anon_rmap_tree_insert(avc, avc->anon_vma);
> }
>
> +static void
> +anon_rmap_tree_update_vma_inplace(struct vm_area_struct *vma)
> +{
> + struct anon_vma_chain *avc;
> +
> + list_for_each_entry(avc, &vma->anon_vma_chain, same_vma)
> + anon_rmap_tree_update_inplace(avc);
> +}
> +
> /*
> * vma_prepare() - Helper function for handling locking VMAs prior to altering
> * @vp: The initialized vma_prepare struct
> @@ -359,14 +375,16 @@ static void vma_prepare(struct vma_prepare *vp)
>
> if (vp->anon_vma) {
> anon_vma_lock_write(vp->anon_vma);
> - anon_rmap_tree_pre_update_vma(vp->vma);
> + if (!vp->anon_pgoff_unchanged)
> + anon_rmap_tree_pre_update_vma(vp->vma);
> if (vp->adj_next)
> anon_rmap_tree_pre_update_vma(vp->adj_next);
> }
>
> if (vp->file) {
> flush_dcache_mmap_lock(vp->mapping);
> - mapping_rmap_tree_remove(vp->vma, vp->mapping);
> + if (!vp->pgoff_unchanged)
> + mapping_rmap_tree_remove(vp->vma, vp->mapping);
> if (vp->adj_next)
> mapping_rmap_tree_remove(vp->adj_next, vp->mapping);
> }
> @@ -387,7 +405,11 @@ static void vma_complete(struct vma_prepare *vp, struct vma_iterator *vmi,
> if (vp->file) {
> if (vp->adj_next)
> mapping_rmap_tree_insert(vp->adj_next, vp->mapping);
> - mapping_rmap_tree_insert(vp->vma, vp->mapping);
> + /* Need only propagate the change inplace. */
> + if (vp->pgoff_unchanged)
> + mapping_rmap_tree_update_inplace(vp->vma);
> + else
> + mapping_rmap_tree_insert(vp->vma, vp->mapping);
> flush_dcache_mmap_unlock(vp->mapping);
> }
>
> @@ -406,7 +428,11 @@ static void vma_complete(struct vma_prepare *vp, struct vma_iterator *vmi,
> }
>
> if (vp->anon_vma) {
> - anon_rmap_tree_post_update_vma(vp->vma);
> + /* Need only propagate the change inplace. */
> + if (vp->anon_pgoff_unchanged)
> + anon_rmap_tree_update_vma_inplace(vp->vma);
> + else
> + anon_rmap_tree_post_update_vma(vp->vma);
> if (vp->adj_next)
> anon_rmap_tree_post_update_vma(vp->adj_next);
> anon_vma_unlock_write(vp->anon_vma);
> @@ -593,6 +619,8 @@ __split_vma(struct vma_iterator *vmi, struct vm_area_struct *vma,
>
> init_vma_prep(&vp, vma);
> vp.insert = new;
> + vp.pgoff_unchanged = !new_below;
> + vp.anon_pgoff_unchanged = !new_below;
> vma_prepare(&vp);
>
> /*
> @@ -1346,6 +1374,8 @@ int vma_shrink(struct vma_iterator *vmi, struct vm_area_struct *vma,
> vma_start_write(vma);
>
> init_vma_prep(&vp, vma);
> + vp.pgoff_unchanged = true;
> + vp.anon_pgoff_unchanged = true;
> vma_prepare(&vp);
> vma_adjust_trans_huge(vma, vma->vm_start, end, NULL);
>
> diff --git a/mm/vma.h b/mm/vma.h
> index 7a683272c0a8..336b4ced82c9 100644
> --- a/mm/vma.h
> +++ b/mm/vma.h
> @@ -28,6 +28,8 @@ struct vma_prepare {
> struct vm_area_struct *remove2;
>
> bool skip_vma_uprobe :1;
> + bool pgoff_unchanged :1;
> + bool anon_pgoff_unchanged :1;
> };
>
> struct unlink_vma_file_batch {
> diff --git a/tools/testing/vma/include/stubs.h b/tools/testing/vma/include/stubs.h
> index e4acc6f1fe7b..f0a69393c02a 100644
> --- a/tools/testing/vma/include/stubs.h
> +++ b/tools/testing/vma/include/stubs.h
> @@ -267,6 +267,10 @@ static inline void mapping_rmap_tree_remove(struct vm_area_struct *vma,
> {
> }
>
> +static inline void mapping_rmap_tree_update_inplace(struct vm_area_struct *vma)
> +{
> +}
> +
> static inline void flush_dcache_mmap_unlock(struct address_space *mapping)
> {
> }
> @@ -281,6 +285,10 @@ static inline void anon_rmap_tree_remove(struct anon_vma_chain *avc,
> {
> }
>
> +static inline void anon_rmap_tree_update_inplace(struct anon_vma_chain *avc)
> +{
> +}
> +
> static inline void uprobe_mmap(struct vm_area_struct *vma)
> {
> }
>
> ---
> base-commit: e8d0f6a1b2a447d02845984fa6288787543cb03c
> change-id: 20260925-speed-up-inplace-rmap-808fbb3848f5
>
> Best regards,
> --
> Lorenzo Stoakes (ARM) <ljs@kernel.org>
>
--
Cheers, Lorenzo
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] mm/vma: don't remove VMA from rmap if pgoff unchanged
2026-09-25 18:32 [PATCH] mm/vma: don't remove VMA from rmap if pgoff unchanged Lorenzo Stoakes (ARM)
2026-09-25 18:59 ` Rik van Riel
2026-09-28 15:10 ` Lorenzo Stoakes (ARM)
@ 2026-09-28 15:37 ` Pedro Falcato
2026-09-28 17:01 ` Lorenzo Stoakes (ARM)
2 siblings, 1 reply; 7+ messages in thread
From: Pedro Falcato @ 2026-09-28 15:37 UTC (permalink / raw)
To: Lorenzo Stoakes (ARM)
Cc: Andrew Morton, David Hildenbrand, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Rik van Riel, Harry Yoo, Jann Horn, Lance Yang, linux-mm,
linux-kernel, Pan Deng
On Fri, Sep 25, 2026 at 07:32:20PM +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->anon_pgoff_unchanged and vp->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.
>
> It's also necessary to keep the rb_subtree_last field updated in the
> interval tree so implement anon_rmap_tree_update_inplace() and
> mapping_rmap_tree_update_inplace() to do that.
>
> This is done in vma_complete(), after the VMA's range has been updated, so
> in the interim the field may be invalid. However, given the locks described
> above, this cannot be observed until after the state is corrected.
>
> The anonymous rmap is keyed on anon_vma_chains not VMAs, so in those
> instances anon_rmap_tree_update_vma_inplace() iterates over
> vma->anon_vma_chain, invoking anon_rmap_tree_update_inplace() on each one.
>
> For the anon rmap case, with CONFIG_DEBUG_VM_RB set, avc->cached_vma_last
> is also updated in anon_rmap_tree_update_inplace().
>
> 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.
>
> These changes ultimately result in less rmap lock contention.
I think this asks for numbers?
>
> Reported-by: Pan Deng <pan.deng@intel.com>
> Closes: https://lore.kernel.org/linux-mm/20260924054301.2330822-1-pan.deng@intel.com/
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
> ---
> Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> ---
> include/linux/mm.h | 3 +++
> mm/interval_tree.c | 33 ++++++++++++++++++++++++++++++++
> mm/vma.c | 40 ++++++++++++++++++++++++++++++++++-----
> mm/vma.h | 2 ++
> tools/testing/vma/include/stubs.h | 8 ++++++++
> 5 files changed, 81 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 6e71eaa4af3f..94c2eb055716 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -4357,6 +4357,8 @@ void mapping_rmap_tree_insert_after(struct vm_area_struct *vma,
> struct address_space *mapping);
> void mapping_rmap_tree_remove(struct vm_area_struct *vma,
> struct address_space *mapping);
> +void mapping_rmap_tree_update_inplace(struct vm_area_struct *vma);
> +
> struct vm_area_struct *
> mapping_rmap_tree_iter_first(struct address_space *mapping,
> pgoff_t pgoff_start, pgoff_t pgoff_last);
> @@ -4374,6 +4376,7 @@ void anon_rmap_tree_insert(struct anon_vma_chain *avc,
> struct anon_vma *anon_vma);
> void anon_rmap_tree_remove(struct anon_vma_chain *avc,
> struct anon_vma *anon_vma);
> +void anon_rmap_tree_update_inplace(struct anon_vma_chain *avc);
> struct anon_vma_chain *
> anon_rmap_tree_iter_first(struct anon_vma *anon_vma,
> pgoff_t pgoff_start, pgoff_t pgoff_last);
> diff --git a/mm/interval_tree.c b/mm/interval_tree.c
> index 7bbbf15cfbf0..eafde5d12ef5 100644
> --- a/mm/interval_tree.c
> +++ b/mm/interval_tree.c
> @@ -64,6 +64,21 @@ void mapping_rmap_tree_remove(struct vm_area_struct *vma,
> __mapping_rmap_tree_remove(vma, &mapping->i_mmap);
> }
>
> +/**
> + * mapping_rmap_tree_update_inplace() - Update file rmap tree to reflect an
> + * in-place change in a VMA's size.
> + * @vma: The VMA whose size has changed.
> + *
> + * The file rmap lock must be held.
> + *
> + * Invalid to do so if @vma->vm_pgoff has changed.
> + */
> +void mapping_rmap_tree_update_inplace(struct vm_area_struct *vma)
> +{
> + /* Propagate all the way up the tree. */
> + __mapping_rmap_tree_augment.propagate(&vma->shared.rb, NULL);
> +}
> +
> struct vm_area_struct *
> mapping_rmap_tree_iter_first(struct address_space *mapping,
> pgoff_t pgoff_start, pgoff_t pgoff_last)
> @@ -111,6 +126,24 @@ void anon_rmap_tree_remove(struct anon_vma_chain *avc,
> __anon_rmap_tree_remove(avc, &anon_vma->rb_root);
> }
>
> +/**
> + * anon_rmap_tree_update_inplace() - Update anon rmap tree to reflect an
> + * in-place change in the size of @avc's VMA.
> + * @avc: The anon_vma_chain whose VMA's size has changed.
> + *
> + * The anon rmap root lock must be held.
> + *
> + * Invalid to do so if the VMA's anonymous pgoff has changed.
> + */
> +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. */
> + __anon_rmap_tree_augment.propagate(&avc->rb, NULL);
> +}
> +
> struct anon_vma_chain *
> anon_rmap_tree_iter_first(struct anon_vma *anon_vma,
> pgoff_t pgoff_start, pgoff_t pgoff_last)
> diff --git a/mm/vma.c b/mm/vma.c
> index 077e23694143..8b333ec0c958 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->pgoff_unchanged = true;
file_pgoff_unchanged perhaps? since we're distinguishing.
In any case, I would prefer if we moved this logic to mm/interval_tree.c, or
any rmap-related header. I don't love opencoding rmap tree assumptions this
deep in vma.c code. WDYT?
Rest obviously looks great to me :)
> + if (vma_start_anon_pgoff(vma) == vmg_start_anon_pgoff(vmg))
> + vp->anon_pgoff_unchanged = true;
> }
>
> /*
> @@ -331,6 +338,15 @@ anon_rmap_tree_post_update_vma(struct vm_area_struct *vma)
> anon_rmap_tree_insert(avc, avc->anon_vma);
> }
>
> +static void
> +anon_rmap_tree_update_vma_inplace(struct vm_area_struct *vma)
> +{
> + struct anon_vma_chain *avc;
> +
> + list_for_each_entry(avc, &vma->anon_vma_chain, same_vma)
> + anon_rmap_tree_update_inplace(avc);
> +}
> +
> /*
> * vma_prepare() - Helper function for handling locking VMAs prior to altering
> * @vp: The initialized vma_prepare struct
> @@ -359,14 +375,16 @@ static void vma_prepare(struct vma_prepare *vp)
>
> if (vp->anon_vma) {
> anon_vma_lock_write(vp->anon_vma);
> - anon_rmap_tree_pre_update_vma(vp->vma);
> + if (!vp->anon_pgoff_unchanged)
> + anon_rmap_tree_pre_update_vma(vp->vma);
> if (vp->adj_next)
> anon_rmap_tree_pre_update_vma(vp->adj_next);
> }
>
> if (vp->file) {
> flush_dcache_mmap_lock(vp->mapping);
> - mapping_rmap_tree_remove(vp->vma, vp->mapping);
> + if (!vp->pgoff_unchanged)
> + mapping_rmap_tree_remove(vp->vma, vp->mapping);
> if (vp->adj_next)
> mapping_rmap_tree_remove(vp->adj_next, vp->mapping);
> }
> @@ -387,7 +405,11 @@ static void vma_complete(struct vma_prepare *vp, struct vma_iterator *vmi,
> if (vp->file) {
> if (vp->adj_next)
> mapping_rmap_tree_insert(vp->adj_next, vp->mapping);
> - mapping_rmap_tree_insert(vp->vma, vp->mapping);
> + /* Need only propagate the change inplace. */
> + if (vp->pgoff_unchanged)
> + mapping_rmap_tree_update_inplace(vp->vma);
> + else
> + mapping_rmap_tree_insert(vp->vma, vp->mapping);
And perhaps similar for this, hiding update vs re-insert in interval tree
code (via a helper) sounds cleaner.
--
Pedro
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] mm/vma: don't remove VMA from rmap if pgoff unchanged
2026-09-28 15:37 ` Pedro Falcato
@ 2026-09-28 17:01 ` Lorenzo Stoakes (ARM)
2026-09-29 13:45 ` Deng, Pan
0 siblings, 1 reply; 7+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-09-28 17:01 UTC (permalink / raw)
To: Pedro Falcato
Cc: Andrew Morton, David Hildenbrand, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Rik van Riel, Harry Yoo, Jann Horn, Lance Yang, linux-mm,
linux-kernel, Pan Deng
On Mon, Sep 28, 2026 at 04:37:52PM +0100, Pedro Falcato wrote:
> On Fri, Sep 25, 2026 at 07:32:20PM +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->anon_pgoff_unchanged and vp->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.
> >
> > It's also necessary to keep the rb_subtree_last field updated in the
> > interval tree so implement anon_rmap_tree_update_inplace() and
> > mapping_rmap_tree_update_inplace() to do that.
> >
> > This is done in vma_complete(), after the VMA's range has been updated, so
> > in the interim the field may be invalid. However, given the locks described
> > above, this cannot be observed until after the state is corrected.
> >
> > The anonymous rmap is keyed on anon_vma_chains not VMAs, so in those
> > instances anon_rmap_tree_update_vma_inplace() iterates over
> > vma->anon_vma_chain, invoking anon_rmap_tree_update_inplace() on each one.
> >
> > For the anon rmap case, with CONFIG_DEBUG_VM_RB set, avc->cached_vma_last
> > is also updated in anon_rmap_tree_update_inplace().
> >
> > 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.
> >
> > These changes ultimately result in less rmap lock contention.
>
> I think this asks for numbers?
Well I don't have any :)
It logically reduces the contention, and that can only be a good thing.
Pan had some numbers, I've asked him to re-run against this one.
>
> >
> > Reported-by: Pan Deng <pan.deng@intel.com>
> > Closes: https://lore.kernel.org/linux-mm/20260924054301.2330822-1-pan.deng@intel.com/
> > Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
> > ---
> > Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> > ---
> > include/linux/mm.h | 3 +++
> > mm/interval_tree.c | 33 ++++++++++++++++++++++++++++++++
> > mm/vma.c | 40 ++++++++++++++++++++++++++++++++++-----
> > mm/vma.h | 2 ++
> > tools/testing/vma/include/stubs.h | 8 ++++++++
> > 5 files changed, 81 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index 6e71eaa4af3f..94c2eb055716 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -4357,6 +4357,8 @@ void mapping_rmap_tree_insert_after(struct vm_area_struct *vma,
> > struct address_space *mapping);
> > void mapping_rmap_tree_remove(struct vm_area_struct *vma,
> > struct address_space *mapping);
> > +void mapping_rmap_tree_update_inplace(struct vm_area_struct *vma);
> > +
> > struct vm_area_struct *
> > mapping_rmap_tree_iter_first(struct address_space *mapping,
> > pgoff_t pgoff_start, pgoff_t pgoff_last);
> > @@ -4374,6 +4376,7 @@ void anon_rmap_tree_insert(struct anon_vma_chain *avc,
> > struct anon_vma *anon_vma);
> > void anon_rmap_tree_remove(struct anon_vma_chain *avc,
> > struct anon_vma *anon_vma);
> > +void anon_rmap_tree_update_inplace(struct anon_vma_chain *avc);
> > struct anon_vma_chain *
> > anon_rmap_tree_iter_first(struct anon_vma *anon_vma,
> > pgoff_t pgoff_start, pgoff_t pgoff_last);
> > diff --git a/mm/interval_tree.c b/mm/interval_tree.c
> > index 7bbbf15cfbf0..eafde5d12ef5 100644
> > --- a/mm/interval_tree.c
> > +++ b/mm/interval_tree.c
> > @@ -64,6 +64,21 @@ void mapping_rmap_tree_remove(struct vm_area_struct *vma,
> > __mapping_rmap_tree_remove(vma, &mapping->i_mmap);
> > }
> >
> > +/**
> > + * mapping_rmap_tree_update_inplace() - Update file rmap tree to reflect an
> > + * in-place change in a VMA's size.
> > + * @vma: The VMA whose size has changed.
> > + *
> > + * The file rmap lock must be held.
> > + *
> > + * Invalid to do so if @vma->vm_pgoff has changed.
> > + */
> > +void mapping_rmap_tree_update_inplace(struct vm_area_struct *vma)
> > +{
> > + /* Propagate all the way up the tree. */
> > + __mapping_rmap_tree_augment.propagate(&vma->shared.rb, NULL);
> > +}
> > +
> > struct vm_area_struct *
> > mapping_rmap_tree_iter_first(struct address_space *mapping,
> > pgoff_t pgoff_start, pgoff_t pgoff_last)
> > @@ -111,6 +126,24 @@ void anon_rmap_tree_remove(struct anon_vma_chain *avc,
> > __anon_rmap_tree_remove(avc, &anon_vma->rb_root);
> > }
> >
> > +/**
> > + * anon_rmap_tree_update_inplace() - Update anon rmap tree to reflect an
> > + * in-place change in the size of @avc's VMA.
> > + * @avc: The anon_vma_chain whose VMA's size has changed.
> > + *
> > + * The anon rmap root lock must be held.
> > + *
> > + * Invalid to do so if the VMA's anonymous pgoff has changed.
> > + */
> > +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. */
> > + __anon_rmap_tree_augment.propagate(&avc->rb, NULL);
> > +}
> > +
> > struct anon_vma_chain *
> > anon_rmap_tree_iter_first(struct anon_vma *anon_vma,
> > pgoff_t pgoff_start, pgoff_t pgoff_last)
> > diff --git a/mm/vma.c b/mm/vma.c
> > index 077e23694143..8b333ec0c958 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->pgoff_unchanged = true;
>
> file_pgoff_unchanged perhaps? since we're distinguishing.
Yeah makes sense.
>
> In any case, I would prefer if we moved this logic to mm/interval_tree.c, or
> any rmap-related header. I don't love opencoding rmap tree assumptions this
> deep in vma.c code. WDYT?
Sure that's fair, will put it over there!
>
>
> Rest obviously looks great to me :)
Thanks :)
>
> > + if (vma_start_anon_pgoff(vma) == vmg_start_anon_pgoff(vmg))
> > + vp->anon_pgoff_unchanged = true;
> > }
> >
> > /*
> > @@ -331,6 +338,15 @@ anon_rmap_tree_post_update_vma(struct vm_area_struct *vma)
> > anon_rmap_tree_insert(avc, avc->anon_vma);
> > }
> >
> > +static void
> > +anon_rmap_tree_update_vma_inplace(struct vm_area_struct *vma)
> > +{
> > + struct anon_vma_chain *avc;
> > +
> > + list_for_each_entry(avc, &vma->anon_vma_chain, same_vma)
> > + anon_rmap_tree_update_inplace(avc);
> > +}
> > +
> > /*
> > * vma_prepare() - Helper function for handling locking VMAs prior to altering
> > * @vp: The initialized vma_prepare struct
> > @@ -359,14 +375,16 @@ static void vma_prepare(struct vma_prepare *vp)
> >
> > if (vp->anon_vma) {
> > anon_vma_lock_write(vp->anon_vma);
> > - anon_rmap_tree_pre_update_vma(vp->vma);
> > + if (!vp->anon_pgoff_unchanged)
> > + anon_rmap_tree_pre_update_vma(vp->vma);
> > if (vp->adj_next)
> > anon_rmap_tree_pre_update_vma(vp->adj_next);
> > }
> >
> > if (vp->file) {
> > flush_dcache_mmap_lock(vp->mapping);
> > - mapping_rmap_tree_remove(vp->vma, vp->mapping);
> > + if (!vp->pgoff_unchanged)
> > + mapping_rmap_tree_remove(vp->vma, vp->mapping);
> > if (vp->adj_next)
> > mapping_rmap_tree_remove(vp->adj_next, vp->mapping);
> > }
> > @@ -387,7 +405,11 @@ static void vma_complete(struct vma_prepare *vp, struct vma_iterator *vmi,
> > if (vp->file) {
> > if (vp->adj_next)
> > mapping_rmap_tree_insert(vp->adj_next, vp->mapping);
> > - mapping_rmap_tree_insert(vp->vma, vp->mapping);
> > + /* Need only propagate the change inplace. */
> > + if (vp->pgoff_unchanged)
> > + mapping_rmap_tree_update_inplace(vp->vma);
> > + else
> > + mapping_rmap_tree_insert(vp->vma, vp->mapping);
>
> And perhaps similar for this, hiding update vs re-insert in interval tree
> code (via a helper) sounds cleaner.
OK will see how that looks!
>
> --
> Pedro
--
Cheers, Lorenzo
^ permalink raw reply [flat|nested] 7+ messages in thread* RE: [PATCH] mm/vma: don't remove VMA from rmap if pgoff unchanged
2026-09-28 17:01 ` Lorenzo Stoakes (ARM)
@ 2026-09-29 13:45 ` Deng, Pan
2026-09-29 14:08 ` Lorenzo Stoakes (ARM)
0 siblings, 1 reply; 7+ messages in thread
From: Deng, Pan @ 2026-09-29 13:45 UTC (permalink / raw)
To: Lorenzo Stoakes (ARM), Pedro Falcato
Cc: Andrew Morton, David Hildenbrand, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Rik van Riel, Harry Yoo, Jann Horn, Lance Yang, linux-mm,
linux-kernel
> > > The anonymous rmap is keyed on anon_vma_chains not VMAs, so in those
> > > instances anon_rmap_tree_update_vma_inplace() iterates over
> > > vma->anon_vma_chain, invoking anon_rmap_tree_update_inplace() on
> each one.
> > >
> > > For the anon rmap case, with CONFIG_DEBUG_VM_RB set, avc-
> >cached_vma_last
> > > is also updated in anon_rmap_tree_update_inplace().
> > >
> > > 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.
> > >
> > > These changes ultimately result in less rmap lock contention.
> >
> > I think this asks for numbers?
>
> Well I don't have any :)
>
> It logically reduces the contention, and that can only be a good thing.
>
> Pan had some numbers, I've asked him to re-run against this one.
>
Yes, UnixBench/execl case, measured on my 2-socket, 192 core / 384
Thread x86-64 system, on v7.3-rc4 with and without this patch, built
from an identical kernel .config.
Configuration, re-applied after each boot:
- cpufreq governor "performance"
- uncore frequency pinned to max
Run rule: 10 runs per kernel, 30s cool-down in between, cmd:
$ ./Run execl -c 384
Result: 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
The speedup is +15.9%.
In addition, I also profiled lock contention data ~10s in the
middle of one iteration, cmd:
$ sudo perf lock contention -ab -l -S vma_prepare -E 8
The dominant lock is i_mmap_rwsem
Result: avg wait on that 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
avg wait is ~11.1% reduction.
Note: under the i_mmap_rwsem write lock, this case only exercises the
split path, for both the file and the anon rmap, while merge and shrink
are not reached when the lock is held.
Best Regards
Pan
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] mm/vma: don't remove VMA from rmap if pgoff unchanged
2026-09-29 13:45 ` Deng, Pan
@ 2026-09-29 14:08 ` Lorenzo Stoakes (ARM)
0 siblings, 0 replies; 7+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-09-29 14:08 UTC (permalink / raw)
To: Deng, Pan
Cc: Pedro Falcato, Andrew Morton, David Hildenbrand, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Rik van Riel, Harry Yoo, Jann Horn, Lance Yang, linux-mm,
linux-kernel, Barry Song
+cc Barry for anon question below
On Tue, Sep 29, 2026 at 01:45:48PM +0000, Deng, Pan wrote:
> > > > The anonymous rmap is keyed on anon_vma_chains not VMAs, so in those
> > > > instances anon_rmap_tree_update_vma_inplace() iterates over
> > > > vma->anon_vma_chain, invoking anon_rmap_tree_update_inplace() on
> > each one.
> > > >
> > > > For the anon rmap case, with CONFIG_DEBUG_VM_RB set, avc-
> > >cached_vma_last
> > > > is also updated in anon_rmap_tree_update_inplace().
> > > >
> > > > 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.
> > > >
> > > > These changes ultimately result in less rmap lock contention.
> > >
> > > I think this asks for numbers?
> >
> > Well I don't have any :)
> >
> > It logically reduces the contention, and that can only be a good thing.
> >
> > Pan had some numbers, I've asked him to re-run against this one.
> >
>
> Yes, UnixBench/execl case, measured on my 2-socket, 192 core / 384
> Thread x86-64 system, on v7.3-rc4 with and without this patch, built
> from an identical kernel .config.
>
> Configuration, re-applied after each boot:
> - cpufreq governor "performance"
> - uncore frequency pinned to max
>
> Run rule: 10 runs per kernel, 30s cool-down in between, cmd:
> $ ./Run execl -c 384
>
> Result: 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
>
> The speedup is +15.9%.
>
> In addition, I also profiled lock contention data ~10s in the
> middle of one iteration, cmd:
> $ sudo perf lock contention -ab -l -S vma_prepare -E 8
>
> The dominant lock is i_mmap_rwsem
>
> Result: avg wait on that 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
>
> avg wait is ~11.1% reduction.
>
> Note: under the i_mmap_rwsem write lock, this case only exercises the
> split path, for both the file and the anon rmap, while merge and shrink
> are not reached when the lock is held.
Thanks Peng! Much appreciated.
I'll fold this into the commit message on respin, should get that out today
(some trivial renaming, etc. functionality will be identical).
Be interesting to look at anon also, there could be impact for android
specifically given zygote.
Suren - do you have any bandwidth for checking whether this patch impacts anon
rmap lock contention?
Barry - I know you've looked at this in the past, are you able to assess whether
this has impact for your workloads?
>
> Best Regards
> Pan
>
--
Cheers, Lorenzo
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-09-29 14:08 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-25 18:32 [PATCH] mm/vma: don't remove VMA from rmap if pgoff unchanged Lorenzo Stoakes (ARM)
2026-09-25 18:59 ` Rik van Riel
2026-09-28 15:10 ` Lorenzo Stoakes (ARM)
2026-09-28 15:37 ` Pedro Falcato
2026-09-28 17:01 ` Lorenzo Stoakes (ARM)
2026-09-29 13:45 ` Deng, Pan
2026-09-29 14:08 ` Lorenzo Stoakes (ARM)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®