Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) client-ip=23.128.96.38;
Date:   Mon,  4 Dec 2023 11:57:11 +0000
In-Reply-To: <20231201172212.1813387-20-cmllamas@google.com>
Mime-Version: 1.0
References: <20231201172212.1813387-20-cmllamas@google.com>
Message-ID: <20231204115712.41114-1-aliceryhl@google.com>
Subject: Re: [PATCH v2 19/28] binder: perform page installation outside of locks
From:   Alice Ryhl <aliceryhl@google.com>
To:     cmllamas@google.com
Cc:     arve@android.com, brauner@kernel.org, gregkh@linuxfoundation.org,
        joel@joelfernandes.org, kernel-team@android.com,
        linux-kernel@vger.kernel.org, maco@android.com, surenb@google.com,
        tkjos@android.com, Alice Ryhl <aliceryhl@google.com>
Content-Type: text/plain; charset="utf-8"
Precedence: bulk

> Split out the insertion of pages to be outside of the alloc->mutex in a
> separate binder_install_buffer_pages() routine. Since this is no longer
> serialized, we must look at the full range of pages used by the buffers.
> The installation is protected with mmap_sem in write mode since multiple
> tasks might race to install the same page.
> 
> Besides avoiding unnecessary nested locking this helps in preparation of
> switching the alloc->mutex into a spinlock_t in subsequent patches.
> 
> Signed-off-by: Carlos Llamas <cmllamas@google.com>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

> +static inline struct page *
> +binder_get_installed_page(struct binder_lru_page *lru_page)
> +{
> +	/* Pairs with release in binder_set_installed_page() */
> +	return smp_load_acquire(&lru_page->page_ptr);
> +}

Technically some of the uses of this method do not actually need to be
atomic (because they never race with `binder_set_installed_page`), but I
don't mind using it anyway.

Alice