Re: [PATCH v5 9/9] binder: use per-vma lock in page reclaiming

Suren Baghdasaryan
2024-11-26T18:46:15Z

On Tue, Nov 26, 2024 at 10:45 AM Suren Baghdasaryan <surenb@google.com> wrote:
>
> On Tue, Nov 26, 2024 at 10:40 AM Carlos Llamas <cmllamas@google.com> wrote:
> >
> > Use per-vma locking in the shrinker's callback when reclaiming pages,
> > similar to the page installation logic. This minimizes contention with
> > unrelated vmas improving performance. The mmap_sem is still acquired if
> > the per-vma lock cannot be obtained.
> >
> > Cc: Suren Baghdasaryan <surenb@google.com>
> > Suggested-by: Liam R. Howlett <Liam.Howlett@oracle.com>
> > Reviewed-by: Suren Baghdasaryan <surenb@google.com>
> > Signed-off-by: Carlos Llamas <cmllamas@google.com>
> > ---
> > drivers/android/binder_alloc.c | 29 ++++++++++++++++++++++-------
> > 1 file changed, 22 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
> > index 339db88c1522..8c10c1a6f459 100644
> > --- a/drivers/android/binder_alloc.c
> > +++ b/drivers/android/binder_alloc.c
> > @@ -1128,19 +1128,28 @@ enum lru_status binder_alloc_free_page(struct list_head *item,
> > struct mm_struct *mm = alloc->mm;
> > struct vm_area_struct *vma;
> > unsigned long page_addr;
> > + int mm_locked = 0;
> > size_t index;
> >
> > if (!mmget_not_zero(mm))
> > goto err_mmget;
> > - if (!mmap_read_trylock(mm))
> > - goto err_mmap_read_lock_failed;
> > - if (!mutex_trylock(&alloc->mutex))
> > - goto err_get_alloc_mutex_failed;
> >
> > index = page->index;
> > page_addr = alloc->vm_start + index * PAGE_SIZE;
> >
> > - vma = vma_lookup(mm, page_addr);
> > + /* attempt per-vma lock first */
> > + vma = lock_vma_under_rcu(mm, page_addr);
> > + if (!vma) {
> > + /* fall back to mmap_lock */
> > + if (!mmap_read_trylock(mm))
> > + goto err_mmap_read_lock_failed;
> > + mm_locked = 1;
> > + vma = vma_lookup(mm, page_addr);
> > + }
> > +
> > + if (!mutex_trylock(&alloc->mutex))
> > + goto err_get_alloc_mutex_failed;
> > +
> > /* ensure the vma corresponds to the binder mapping */
>
> You did add a clarifying comment I asked for in
> https://lore.kernel.org/all/CAJuCfpESdY4L_sSwiCYVCX+5y1WOuAjLNPw35pEGzTSyoHFYPA@mail.gmail.com/

s/did/did not

>
> > if (vma && !binder_alloc_is_mapped(alloc))
> > goto err_invalid_vma;
> > @@ -1163,7 +1172,10 @@ enum lru_status binder_alloc_free_page(struct list_head *item,
> > }
> >
> > mutex_unlock(&alloc->mutex);
> > - mmap_read_unlock(mm);
> > + if (mm_locked)
> > + mmap_read_unlock(mm);
> > + else
> > + vma_end_read(vma);
> > mmput_async(mm);
> > __free_page(page);
> >
> > @@ -1172,7 +1184,10 @@ enum lru_status binder_alloc_free_page(struct list_head *item,
> > err_invalid_vma:
> > mutex_unlock(&alloc->mutex);
> > err_get_alloc_mutex_failed:
> > - mmap_read_unlock(mm);
> > + if (mm_locked)
> > + mmap_read_unlock(mm);
> > + else
> > + vma_end_read(vma);
> > err_mmap_read_lock_failed:
> > mmput_async(mm);
> > err_mmget:
> > --
> > 2.47.0.338.g60cca15819-goog
> >