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

[Bo Zhang]

On Sat, Aug 08, 2026 at 01:49:46PM +0000, Alice Ryhl wrote:
> No, this will deadlock if the thread installing the page is SCHED_FIFO.
> 
> Waiting for the shrinker by repeatedly checking whether the shrinker is
> done in a loop does not work.
> 
> As an aside, your solution also adds similar waiting for concurrent
> installers. If the successful installer gets preempted between
> binder_page_insert() and binder_set_installed_page(), then your code
> will also repeatedly retry until said thread gets scheduled in again. So
> there is a similar deadlock if the inserter that failed the race is
> SCHED_FIFO.

Hi Alice,

Thanks for the feedback. You're right, the busy-wait approach doesn't
work for SCHED_FIFO threads.

How about splitting into two locks: a spinlock for the hot path and a
mutex for page install/zap serialization?

  struct binder_alloc {
      spinlock_t lock;            /* protect buffer metadata (hot path) */
      struct mutex install_mutex; /* page install vs shrinker zap */
  };

The spinlock protects buffer allocation/free meta-data, which is the
high-frequency path hit on every binder transaction.

The install_mutex serializes binder_install_single_page() against
zap_vma_range() in the shrinker:

  - Install side holds install_mutex across the entire install sequence
    (vm_insert_page and binder_set_installed_page), so concurrent
    installers are also serialized, no more EBUSY between installers.

  - Shrinker holds install_mutex across pages[index]=NULL and
    zap_vma_range(), which make them atomic to the install side.

  - Since install_mutex serializes all installers, concurrent installers
    can no longer race on the same PTE, so the second one will see
    pages[index] != NULL under the mutex and skip. This eliminates the
    need for binder_page_lookup() (GUP). EBUSY is now only a defensive
    case for an unlikely shrinker race, handled by returning -EAGAIN.
    The retry blocks on mutex_lock() not spinning, which is safe for 
    SCHED_FIFO.

  - binder_lru_freelist_del() (under spinlock) may see a page that the
    shrinker has already isolated from the LRU. In such case list_lru_del
    returns false, It's acceptable that the subsequent install will sync
    with the shrinker via install_mutex. (We should remove WARN_ON(!on_lru))

The sketch may like:

  /* install (cold path) */
  mutex_lock(&alloc->install_mutex);
  if (binder_get_installed_page(alloc, index)) {
      mutex_unlock(&alloc->install_mutex);
      ...
      return 0;
  }
  ret = vm_insert_page(...);
  if (ret == -EBUSY) {
      mutex_unlock(&alloc->install_mutex);
      return -EAGAIN;  /* caller retries */
  }
  binder_set_installed_page(alloc, index, page);
  mutex_unlock(&alloc->install_mutex);

  /* shrinker */
  spin_lock(&alloc->lock);
  list_lru_isolate(lru, item);
  spin_unlock(&alloc->lock);

  mutex_lock(&alloc->install_mutex);
  binder_set_installed_page(alloc, index, NULL);
  zap_vma_range(...);
  mutex_unlock(&alloc->install_mutex);
  binder_free_page(page_to_free);

A key detail is that pages[index]=NULL is moved out of the spinlock
and into the install_mutex critical section. This ensures that from
the install side's perspective, pages[index]=NULL and zap_vma_range()
are atomic, the install side cannot observe pages[index]==NULL while
the old PTE still exists. The spinlock only protects list_lru_isolate()
now, which is sufficient since binder_lru_freelist_del() synchronizes
LRU access via the lru's own internal lock.

The key point is that the original single mutex protects both buffer
metadata and page install/zap together, causing unnecessary contention
on the hot path. With the split, the hot path uses only the spinlock and
never touches install_mutex. The install_mutex is only contended on the
cold path when pages need to be installed or reclaimed, which is rare
during normal operation since pages are already present.

Does this direction look better?

Bo