All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lance Yang <lance.yang@linux.dev>
To: ljs@kernel.org
Cc: akpm@linux-foundation.org, david@kernel.org, liam@infradead.org,
	vbabka@kernel.org, rppt@kernel.org, surenb@google.com,
	mhocko@suse.com, riel@surriel.com, harry@kernel.org,
	jannh@google.com, lance.yang@linux.dev, pfalcato@suse.de,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	pan.deng@intel.com
Subject: Re: [PATCH v2] mm/vma: don't remove VMA from rmap if pgoff unchanged
Date: Thu,  1 Oct 2026 23:45:11 +0800	[thread overview]
Message-ID: <20261001154511.23931-1-lance.yang@linux.dev> (raw)
In-Reply-To: <20260930-speed-up-inplace-rmap-v2-1-ac1aa19708aa@kernel.org>


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@intel.com>
>Reviewed-by: Rik van Riel <riel@surriel.com>
>Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
>---

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@linux.dev>

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

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

Cheers, Lance


  reply	other threads:[~2026-10-01 15:45 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-30 17:53 [PATCH v2] mm/vma: don't remove VMA from rmap if pgoff unchanged Lorenzo Stoakes (ARM)
2026-10-01 15:45 ` Lance Yang [this message]
2026-10-02 14:05   ` Lorenzo Stoakes (ARM)
2026-10-02 14:15     ` Lance Yang
2026-10-02 14:24 ` Pedro Falcato
2026-10-02 14:27   ` Lorenzo Stoakes (ARM)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261001154511.23931-1-lance.yang@linux.dev \
    --to=lance.yang@linux.dev \
    --cc=akpm@linux-foundation.org \
    --cc=david@kernel.org \
    --cc=harry@kernel.org \
    --cc=jannh@google.com \
    --cc=liam@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=ljs@kernel.org \
    --cc=mhocko@suse.com \
    --cc=pan.deng@intel.com \
    --cc=pfalcato@suse.de \
    --cc=riel@surriel.com \
    --cc=rppt@kernel.org \
    --cc=surenb@google.com \
    --cc=vbabka@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.