Re: [PATCH v2 2/5] binder: Make shrinker rely solely on per-VMA lock

[Dave Hansen <dave.hansen@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 728 bytes --]

On 6/12/26 10:44, Suren Baghdasaryan wrote:
>> It's not impossible, but I do think it is irrelevant. Or at least that
>> the *VMA* is irrelevant in this case. binder_alloc_is_mapped()==false
>> means that the binder VMA is gone. It's not in the maple tree, and it's
>> not coming back. If a VMA is found, it's an impostor.
> Right, but before your change we were bailing out early. With your
> change we would be generating the traces and freeing the page. I think
> that's a functional change. Was that your intention?

Yeah, it was intentional.

I think the existing behavior is buggy. It also complicates the goal of
removing the mmap lock fallback. I've broken that behavior change out
into a separate patch. (attached here)

[-- Attachment #2: binder-impostor-fix.patch --]
[-- Type: text/x-patch, Size: 2462 bytes --]


tl;dr: Stop relying on VMA lookups to determine when to reclaim
pages. Instead, use binder-internal metadata.

== Background ==

Each 'struct binder_alloc' has one and only one place where it is
recorded as having been mapped. It can be munmap()'d. But after that,
binder_alloc_mmap_handler() will return errors for it being "already
mapped". So, binder mmap()s are a one-shot thing.

But, the original mmap() location is special even after munmap(). It
is still recorded in alloc->vm_start and never cleared out.
binder_alloc_free_page() continues to look up VMAs at that address.

== Problem ==

That leads to some suboptimal behavior. The moment an "impostor" VMA
is created at the old binder address, the shrinker function will
always hit the:

	if (vma && !binder_alloc_is_mapped(alloc))

case and LRU_SKIP all pages.

== Solution ==

Stop using the VMA to drive zapping decisions. Instead, use
binder_alloc_is_mapped().

== Discussion ==

Here's some pseudocode for how this behavior could be triggered:

	addr = mmap(..., len, binder_fd);
	// pages can be reclaimed
	munmap(addr, len);
	// pages can still be reclaimed
	mmap(addr, len, MAP_ANONYMOUS|MAP_PRIVATE, -1, ...);
	// Pages can no longer be reclaimed

There are plenty of ways the code could be restructured now
that it is less dependent on VMAs. But I've left that for future
patches.

---

 b/drivers/android/binder_alloc.c |   10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff -puN drivers/android/binder_alloc.c~binder-impostor-fix drivers/android/binder_alloc.c
--- a/drivers/android/binder_alloc.c~binder-impostor-fix	2026-06-12 10:46:06.704707233 -0700
+++ b/drivers/android/binder_alloc.c	2026-06-12 11:34:15.304460520 -0700
@@ -1164,14 +1164,6 @@ enum lru_status binder_alloc_free_page(s
 	if (!mutex_trylock(&alloc->mutex))
 		goto err_get_alloc_mutex_failed;
 
-	/*
-	 * Since a binder_alloc can only be mapped once, we ensure
-	 * the vma corresponds to this mapping by checking whether
-	 * the binder_alloc is still mapped.
-	 */
-	if (vma && !binder_alloc_is_mapped(alloc))
-		goto err_invalid_vma;
-
 	trace_binder_unmap_kernel_start(alloc, index);
 
 	page_to_free = alloc->pages[index];
@@ -1182,7 +1174,7 @@ enum lru_status binder_alloc_free_page(s
 	list_lru_isolate(lru, item);
 	spin_unlock(&lru->lock);
 
-	if (vma) {
+	if (binder_alloc_is_mapped(alloc)) {
 		trace_binder_unmap_user_start(alloc, index);
 
 		zap_vma_range(vma, page_addr, PAGE_SIZE);
_