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

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

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/

> 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
>