Re: [RFC PATCH 0/1] binder: switch alloc->mutex back to spinlock

[Alice Ryhl <aliceryhl@google.com>]

On Wed, Aug 05, 2026 at 11:27:51PM +0800, Bo Zhang wrote:
> Hi,
> 
> This patch switches the binder_alloc mutex back to a spinlock to reduce
> transaction latency.
> 
> Background:
> 
> Commit 7710e2cca32e ("binder: switch alloc->mutex to spinlock_t")
> originally converted the mutex to a spinlock for performance. It was
> later reverted by commit 8b52c7261e04 in preparation for commit
> d1716b4b78fb ("binder: concurrent page installation"), which states:
> 
>   "zap_page_range_single() is called under the alloc->mutex to avoid
>    racing with the shrinker."
> 
> Analysis:
> 
> After inspection, holding the lock across zap_vma_range() in the shrinker
> path is unnecessary. The page installation side does NOT hold alloc->mutex,
> so the mutex provides no mutual exclusion between install and zap. The 
> actual synchronization is guaranteed at the PTE lock level:
> 
>   1. vm_insert_page() acquires the PTE lock to set the PTE entry.
>   2. zap_vma_range() acquires the PTE lock to clear the PTE entry.
>   3. On race conditions, the install path calls binder_page_lookup
>      which uses get_user_pages_remote() to atomically pin the page
>      under PTE lock, preventing use-after-free regardless of zap timing.
> 
> The alloc lock only needs to protect buffer metadata (rb-trees, pages
> array, LRU list operations), all of which are non-sleeping and complete
> before zap_vma_range() is called.
> 
> By moving spin_unlock() before zap_vma_range() in the shrinker path, we
> can safely convert back to a spinlock.
> 
> Performance (binderThroughputTest, Qualcomm SM8850, 2 workers, 10 runs):
> 
>                     mutex               spinlock
>   throughput:    27k-59k iter/s       79k-84k iter/s  (~80% improvement)
>   average:       0.031-0.068ms        0.022-0.023ms   (~45% reduction)
>   P99:           0.088-0.148ms        0.050-0.062ms   (~55% reduction)
>   variance:      high (2x spread)     low (stable)
> 
> The spinlock eliminates priority inversion where low-priority tasks
> holding the mutex sleep, blocking high-priority binder transactions.
> 
> Looking forward to feedback on this analysis.

This will not work due to the following race condition between T1 and T2:

T1 binder_alloc_free_page() removes the page from alloc->pages
T2 binder_alloc_new_buf_locked() runs
T2 binder_install_single_page() gets -EBUSY
T2 binder_page_lookup() looks up the page in the vma
T1 binder_alloc_free_page() calls zap_vma_range()

In this scenario, T2 looks up a page that it expects to be "pinned" and
not removable by the shrinker, but the shrinker zaps it anyway.

Today this cannot happen because binder_alloc_new_buf_locked() runs
under the mutex, which ensures that zap_vma_range() finishes removing
the page before binder_install_single_page() runs.

Alice