From: Carlos Llamas <cmllamas@google.com>
To: "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
"Arve Hjønnevåg" <arve@android.com>,
"Todd Kjos" <tkjos@android.com>,
"Martijn Coenen" <maco@android.com>,
"Joel Fernandes" <joel@joelfernandes.org>,
"Christian Brauner" <brauner@kernel.org>,
"Carlos Llamas" <cmllamas@google.com>,
"Suren Baghdasaryan" <surenb@google.com>
Cc: linux-kernel@vger.kernel.org, kernel-team@android.com
Subject: [PATCH 19/21] binder: perform page allocation outside of locks
Date: Thu, 2 Nov 2023 18:59:20 +0000 [thread overview]
Message-ID: <20231102185934.773885-20-cmllamas@google.com> (raw)
In-Reply-To: <20231102185934.773885-1-cmllamas@google.com>
Split out the insertion of pages to be done outside of the alloc->mutex
in a separate binder_get_pages_range() routine. Since this is no longer
serialized with other requests we need to make sure we look at the full
range of pages for this buffer, including those shared with neighboring
buffers. The insertion of pages into the vma is still serialized with
the mmap write lock.
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>
---
drivers/android/binder_alloc.c | 85 ++++++++++++++++++++++------------
1 file changed, 56 insertions(+), 29 deletions(-)
diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index 7e0af1786b20..e739be7f2dd4 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -194,6 +194,9 @@ static void binder_free_page_range(struct binder_alloc *alloc,
index = (page_addr - alloc->buffer) / PAGE_SIZE;
page = &alloc->pages[index];
+ if (!page->page_ptr)
+ continue;
+
trace_binder_free_lru_start(alloc, index);
ret = list_lru_add(&binder_alloc_lru, &page->lru);
@@ -214,6 +217,9 @@ static int binder_get_user_page_remote(struct binder_alloc *alloc,
return -ESRCH;
mmap_write_lock(alloc->mm);
+ if (lru_page->page_ptr)
+ goto out;
+
if (!alloc->vma) {
pr_err("%d: %s failed, no vma\n", alloc->pid, __func__);
ret = -ESRCH;
@@ -236,32 +242,64 @@ static int binder_get_user_page_remote(struct binder_alloc *alloc,
goto out;
}
- lru_page->page_ptr = page;
+ /* mark page insertion complete and safe to acquire */
+ smp_store_release(&lru_page->page_ptr, page);
out:
mmap_write_unlock(alloc->mm);
mmput_async(alloc->mm);
return ret;
}
-static int binder_allocate_page_range(struct binder_alloc *alloc,
- unsigned long start, unsigned long end)
+/* The range of pages should include those shared with other buffers */
+static int binder_get_page_range(struct binder_alloc *alloc,
+ unsigned long start, unsigned long end)
{
struct binder_lru_page *page;
unsigned long page_addr;
binder_alloc_debug(BINDER_DEBUG_BUFFER_ALLOC,
- "%d: allocate pages %lx-%lx\n",
+ "%d: get pages %lx-%lx\n",
alloc->pid, start, end);
- if (end <= start)
- return 0;
+ for (page_addr = start; page_addr < end; page_addr += PAGE_SIZE) {
+ unsigned long index;
+ int ret;
+
+ index = (page_addr - alloc->buffer) / PAGE_SIZE;
+ page = &alloc->pages[index];
+
+ /* check if page insertion is marked complete by release */
+ if (smp_load_acquire(&page->page_ptr))
+ continue;
+
+ trace_binder_alloc_page_start(alloc, index);
+
+ ret = binder_get_user_page_remote(alloc, page, page_addr);
+ if (ret)
+ return ret;
+
+ trace_binder_alloc_page_end(alloc, index);
+ }
+
+ return 0;
+}
+
+/* The range of pages should exclude those shared with other buffers */
+static void binder_allocate_page_range(struct binder_alloc *alloc,
+ unsigned long start, unsigned long end)
+{
+ struct binder_lru_page *page;
+ unsigned long page_addr;
+
+ binder_alloc_debug(BINDER_DEBUG_BUFFER_ALLOC,
+ "%d: allocate pages %lx-%lx\n",
+ alloc->pid, start, end);
trace_binder_update_page_range(alloc, true, start, end);
for (page_addr = start; page_addr < end; page_addr += PAGE_SIZE) {
unsigned long index;
bool on_lru;
- int ret;
index = (page_addr - alloc->buffer) / PAGE_SIZE;
page = &alloc->pages[index];
@@ -276,21 +314,9 @@ static int binder_allocate_page_range(struct binder_alloc *alloc,
continue;
}
- trace_binder_alloc_page_start(alloc, index);
-
- ret = binder_get_user_page_remote(alloc, page, page_addr);
- if (ret) {
- binder_free_page_range(alloc, start, page_addr);
- return ret;
- }
-
if (index + 1 > alloc->pages_high)
alloc->pages_high = index + 1;
-
- trace_binder_alloc_page_end(alloc, index);
}
-
- return 0;
}
static inline void binder_alloc_set_vma(struct binder_alloc *alloc,
@@ -410,7 +436,6 @@ static struct binder_buffer *binder_alloc_new_buf_locked(
unsigned long has_page_addr;
unsigned long end_page_addr;
size_t buffer_size;
- int ret;
if (is_async &&
alloc->free_async_space < size + sizeof(struct binder_buffer)) {
@@ -453,18 +478,14 @@ static struct binder_buffer *binder_alloc_new_buf_locked(
"%d: binder_alloc_buf size %zd got buffer %pK size %zd\n",
alloc->pid, size, buffer, buffer_size);
- has_page_addr = (buffer->user_data + buffer_size) & PAGE_MASK;
WARN_ON(n && buffer_size != size);
+
+ has_page_addr = (buffer->user_data + buffer_size) & PAGE_MASK;
end_page_addr = PAGE_ALIGN(buffer->user_data + size);
if (end_page_addr > has_page_addr)
end_page_addr = has_page_addr;
- ret = binder_allocate_page_range(alloc, PAGE_ALIGN(buffer->user_data),
- end_page_addr);
- if (ret) {
- buffer = ERR_PTR(ret);
- goto out;
- }
-
+ binder_allocate_page_range(alloc, PAGE_ALIGN(buffer->user_data),
+ end_page_addr);
if (buffer_size != size) {
new_buffer->user_data = buffer->user_data + size;
list_add(&new_buffer->entry, &buffer->entry);
@@ -491,7 +512,6 @@ static struct binder_buffer *binder_alloc_new_buf_locked(
buffer->oneway_spam_suspect = true;
}
-out:
/* discard possibly unused new_buffer */
kfree(new_buffer);
return buffer;
@@ -520,6 +540,7 @@ struct binder_buffer *binder_alloc_new_buf(struct binder_alloc *alloc,
{
struct binder_buffer *buffer, *next;
size_t size;
+ int ret;
/* Check binder_alloc is fully initialized */
if (!binder_alloc_get_vma(alloc)) {
@@ -564,6 +585,12 @@ struct binder_buffer *binder_alloc_new_buf(struct binder_alloc *alloc,
buffer->extra_buffers_size = extra_buffers_size;
buffer->pid = current->tgid;
+ ret = binder_get_page_range(alloc, buffer->user_data & PAGE_MASK,
+ PAGE_ALIGN(buffer->user_data + size));
+ if (ret) {
+ binder_alloc_free_buf(alloc, buffer);
+ buffer = ERR_PTR(ret);
+ }
out:
return buffer;
}
--
2.42.0.869.gea05f2083d-goog
next prev parent reply other threads:[~2023-11-02 19:02 UTC|newest]
Thread overview: 57+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-02 18:59 [PATCH 00/21] binder: convert alloc->mutex to spinlock Carlos Llamas
2023-11-02 18:59 ` [PATCH 01/21] binder: use EPOLLERR from eventpoll.h Carlos Llamas
2023-11-07 9:07 ` Alice Ryhl
2023-11-02 18:59 ` [PATCH 02/21] binder: fix use-after-free in shinker's callback Carlos Llamas
2023-11-02 19:20 ` Liam R. Howlett
2023-11-02 20:09 ` Carlos Llamas
2023-11-02 20:27 ` Liam R. Howlett
2023-11-07 9:07 ` Alice Ryhl
2023-11-02 18:59 ` [PATCH 03/21] binder: fix race between mmput() and do_exit() Carlos Llamas
2023-11-07 9:07 ` Alice Ryhl
2023-11-02 18:59 ` [PATCH 04/21] binder: fix async space check for 0-sized buffers Carlos Llamas
2023-11-07 9:08 ` Alice Ryhl
2023-11-02 18:59 ` [PATCH 05/21] binder: fix trivial typo of binder_free_buf_locked() Carlos Llamas
2023-11-07 9:08 ` Alice Ryhl
2023-12-01 6:52 ` Carlos Llamas
2023-11-02 18:59 ` [PATCH 06/21] binder: fix comment on binder_alloc_new_buf() return value Carlos Llamas
2023-11-07 9:08 ` Alice Ryhl
2023-11-02 18:59 ` [PATCH 07/21] binder: remove extern from function prototypes Carlos Llamas
2023-11-07 9:08 ` Alice Ryhl
2023-11-02 18:59 ` [PATCH 08/21] binder: keep vma addresses type as unsigned long Carlos Llamas
2023-11-07 9:08 ` Alice Ryhl
2023-12-01 7:01 ` Carlos Llamas
2023-11-02 18:59 ` [PATCH 09/21] binder: split up binder_update_page_range() Carlos Llamas
2023-11-07 9:08 ` Alice Ryhl
2023-12-01 7:03 ` Carlos Llamas
2023-11-02 18:59 ` [PATCH 10/21] binder: do unlocked work in binder_alloc_new_buf() Carlos Llamas
2023-11-07 9:08 ` Alice Ryhl
2023-12-01 7:10 ` Carlos Llamas
2023-11-02 18:59 ` [PATCH 11/21] binder: remove pid param " Carlos Llamas
2023-11-07 9:08 ` Alice Ryhl
2023-11-02 18:59 ` [PATCH 12/21] binder: separate the no-space debugging logic Carlos Llamas
2023-11-07 9:08 ` Alice Ryhl
2023-11-02 18:59 ` [PATCH 13/21] binder: relocate low space calculation Carlos Llamas
2023-11-07 9:08 ` Alice Ryhl
2023-12-01 7:12 ` Carlos Llamas
2023-11-02 18:59 ` [PATCH 14/21] binder: do not add pages to LRU in release path Carlos Llamas
2023-11-07 9:08 ` Alice Ryhl
2023-12-01 7:15 ` Carlos Llamas
2023-11-02 18:59 ` [PATCH 15/21] binder: relocate binder_alloc_clear_buf() Carlos Llamas
2023-11-07 9:08 ` Alice Ryhl
2023-11-02 18:59 ` [PATCH 16/21] binder: refactor page range allocation Carlos Llamas
2023-11-07 9:08 ` Alice Ryhl
2023-12-01 7:19 ` Carlos Llamas
2023-11-02 18:59 ` [PATCH 17/21] binder: malloc new_buffer outside of locks Carlos Llamas
2023-11-07 9:08 ` Alice Ryhl
2023-12-01 7:20 ` Carlos Llamas
2023-11-02 18:59 ` [PATCH 18/21] binder: initialize lru pages in mmap callback Carlos Llamas
2023-11-07 9:08 ` Alice Ryhl
2023-11-02 18:59 ` Carlos Llamas [this message]
2023-11-07 9:08 ` [PATCH 19/21] binder: perform page allocation outside of locks Alice Ryhl
2023-12-01 7:39 ` Carlos Llamas
2023-11-02 18:59 ` [PATCH 20/21] binder: reverse locking order in shrinker callback Carlos Llamas
2023-11-07 9:08 ` Alice Ryhl
2023-12-01 7:42 ` Carlos Llamas
2023-11-02 18:59 ` [PATCH 21/21] binder: switch alloc->mutex to spinlock_t Carlos Llamas
2023-11-07 9:08 ` Alice Ryhl
2023-12-01 7:46 ` Carlos Llamas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231102185934.773885-20-cmllamas@google.com \
--to=cmllamas@google.com \
--cc=arve@android.com \
--cc=brauner@kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=joel@joelfernandes.org \
--cc=kernel-team@android.com \
--cc=linux-kernel@vger.kernel.org \
--cc=maco@android.com \
--cc=surenb@google.com \
--cc=tkjos@android.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®