mirror of https://lore.kernel.org/lkml/
 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 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 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®