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

[Dave Hansen]

On 6/12/26 09:41, Suren Baghdasaryan wrote:
>> I think the key to distinguishing between:
>>
>>         vma==NULL because there's no VMA
>> and
>>         vma==NULL because of a trylock failure
>>
>> is binder_alloc_is_mapped(). It won't return false until vm_ops->close()
>> finishes. vm_ops->close() shouldn't be able to happen while
>> lock_vma_under_rcu() is held. So if you've got a non-NULL VMA, you've
>> also got a stable is binder_alloc_is_mapped().
> By "stable binder_alloc_is_mapped()" do you mean it would always be
> true?

By stable, I meant that it can't change.

	vma = lock_vma_under_rcu()
	mapped = binder_alloc_is_mapped();
	<window>
	vma_end_read(vma);

During <window> it can't go from true=>false or false=>true.

false=>true never happens from what I can tell. It's just plain
impossible given the current code.

true=>false is locked out because when lock_vma_under_rcu() is held.

> Asking because in your patch you removed this condition:
> 
> -         if (vma && !binder_alloc_is_mapped(alloc))
> -                  goto err_invalid_vma;
> 
> So, previously if we found the VMA but binder_alloc_is_mapped()==false
> we would bail out and now we don't. Are you reasoning that this
> combination is impossible?

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.

That's why I did:

-        if (vma) {
+        if (mapped) {

The question isn't whether a VMA was found. The question is whether the
binder VMA is still mapped at page_addr. *That* is best inferred from
binder_alloc_is_mapped(), not the VMA lookup.

At least that's what I decided after staring at it for far too long.