Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Alice Ryhl <aliceryhl@google.com>
To: Suren Baghdasaryan <surenb@google.com>
Cc: akpm@linux-foundation.org, dave.hansen@linux.intel.com,
	 Liam.Howlett@oracle.com, ljs@kernel.org, david@redhat.com,
	 willy@infradead.org, shakeel.butt@linux.dev, vbabka@kernel.org,
	 jannh@google.com, arve@android.com, cmllamas@google.com,
	christian@brauner.io,  tkjos@android.com, dsahern@kernel.org,
	davem@davemloft.net,  gregkh@linuxfoundation.org,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	 netdev@vger.kernel.org
Subject: Re: [PATCH v3 2/5] binder: Make shrinker rely solely on per-VMA lock
Date: Mon, 3 Aug 2026 09:48:19 +0000	[thread overview]
Message-ID: <anBj40CVAhcGMnQ4@google.com> (raw)
In-Reply-To: <20260802215459.2769283-3-surenb@google.com>

On Sun, Aug 02, 2026 at 02:54:56PM -0700, Suren Baghdasaryan wrote:
> From: Dave Hansen <dave.hansen@linux.intel.com>
> 
> tl;dr: lock_vma_under_rcu() is already a trylock. No need to do both
> it and mmap_read_trylock().
> 
> Long Version:
> 
> == Background ==
> 
> Historically, binder used an mmap_read_trylock() in its shrinker code.
> This ensures that reclaim is not blocked on an mmap_lock. Commit
> 95bc2d4a9020 ("binder: use per-vma lock in page reclaiming") added
> support for the per-VMA lock, but left mmap_read_trylock() as a
> fallback.
> 
> This was presumably because the per-VMA locking can fail for several
> reasons and most (all?) lock_vma_under_rcu() callers have a fallback
> to mmap_read_trylock().
> 
> == Problem ==
> 
> The fallback is not worth the complexity here. lock_vma_under_rcu() is
> essentially already a non-blocking trylock. The main reason it fails
> is also the reason mmap_read_trylock() fails: something is holding
> mmap_write_lock().
> 
> The only remedy for a collision with mmap_write_lock() is to wait,
> which this code can not do. So the "fallback" after
> lock_vma_under_rcu() failure is not really a fallback: it is really
> likely to just be retrying in vain. That retry in an of itself isn't
> horrible. But it adds complexity.
> 
> == Solution ==
> 
> Now that per-VMA locks are universally available, lock_vma_under_rcu()
> will not persistently fail. Rely on it alone and simplify the code.
> 
> Full disclosure: I originally tried to do this with
> lock_vma_under_rcu_wait(), but it did not fit well with the mmap_lock
> trylock semantics. Claude caught this in a review and suggested the
> approach in this path. It seemed sane to me. So, Suggesed-by: Claude,
> I guess.
> 
> Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
> Signed-off-by: Suren Baghdasaryan <surenb@google.com>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>


  reply	other threads:[~2026-08-03 13:27 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-02 21:54 [PATCH v3 0/5] mm: Unconditional per-VMA locks and cleanups Suren Baghdasaryan
2026-08-02 21:54 ` [PATCH v3 1/5] mm: Make per-VMA locks available universally Suren Baghdasaryan
2026-08-03 10:49   ` Lorenzo Stoakes (ARM)
2026-08-03 14:01   ` Vlastimil Babka (SUSE)
2026-08-03 17:45     ` Suren Baghdasaryan
2026-08-03 15:24   ` Suren Baghdasaryan
2026-08-03 16:08     ` Lorenzo Stoakes (ARM)
2026-08-03 17:41       ` Suren Baghdasaryan
2026-08-03 17:45         ` Suren Baghdasaryan
2026-08-03 21:12       ` Jann Horn
2026-08-03 19:33   ` Jann Horn
2026-08-03 19:43     ` Suren Baghdasaryan
2026-08-02 21:54 ` [PATCH v3 2/5] binder: Make shrinker rely solely on per-VMA lock Suren Baghdasaryan
2026-08-03  9:48   ` Alice Ryhl [this message]
2026-08-03 10:50     ` Lorenzo Stoakes (ARM)
2026-08-03 11:11       ` Lorenzo Stoakes (ARM)
2026-08-03 11:33         ` Lorenzo Stoakes (ARM)
2026-08-03 18:02       ` Suren Baghdasaryan
2026-08-03 11:10   ` Lorenzo Stoakes (ARM)
2026-08-03 18:31     ` Suren Baghdasaryan
2026-08-02 21:54 ` [PATCH v3 3/5] mm: Add RCU-based VMA lookup helper that waits for writers Suren Baghdasaryan
2026-08-03 11:28   ` Lorenzo Stoakes (ARM)
2026-08-03 19:01     ` Suren Baghdasaryan
2026-08-03 14:55   ` Vlastimil Babka (SUSE)
2026-08-03 15:00     ` Lorenzo Stoakes (ARM)
2026-08-03 16:24       ` Vlastimil Babka (SUSE)
2026-08-03 16:43         ` Lorenzo Stoakes (ARM)
2026-08-03 19:13           ` Suren Baghdasaryan
2026-08-02 21:54 ` [PATCH v3 4/5] binder: Remove mmap_lock fallback Suren Baghdasaryan
2026-08-03 10:34   ` Alice Ryhl
2026-08-03 19:14     ` Suren Baghdasaryan
2026-08-03 11:33   ` Lorenzo Stoakes (ARM)
2026-08-03 19:16     ` Suren Baghdasaryan
2026-08-02 21:54 ` [PATCH v3 5/5] tcp: Remove mmap_lock fallback path Suren Baghdasaryan
2026-08-03  2:11 ` [PATCH v3 0/5] mm: Unconditional per-VMA locks and cleanups Barry Song
2026-08-03 17:51   ` Suren Baghdasaryan

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=anBj40CVAhcGMnQ4@google.com \
    --to=aliceryhl@google.com \
    --cc=Liam.Howlett@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=arve@android.com \
    --cc=christian@brauner.io \
    --cc=cmllamas@google.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=davem@davemloft.net \
    --cc=david@redhat.com \
    --cc=dsahern@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jannh@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=ljs@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=shakeel.butt@linux.dev \
    --cc=surenb@google.com \
    --cc=tkjos@android.com \
    --cc=vbabka@kernel.org \
    --cc=willy@infradead.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