mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
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 v2 23/28] binder: document the final page calculation
Date: Fri,  1 Dec 2023 17:21:52 +0000	[thread overview]
Message-ID: <20231201172212.1813387-24-cmllamas@google.com> (raw)
In-Reply-To: <20231201172212.1813387-1-cmllamas@google.com>

The code to determine the page range for binder_lru_freelist_del() is
quite obscure. It leverages the buffer_size calculated before doing an
oversized buffer split. This is used to figure out if the last page is
being shared with another active buffer. If so, the page gets trimmed
out of the range as it has been previously removed from the freelist.

This would be equivalent to getting the start page of the next in-use
buffer explicitly. However, the code for this is much larger as we can
see in binder_free_buf_locked() routine. Instead, lets settle on
documenting the tricky step and using better names for now.

I believe an ideal solution would be to count the binder_page->users to
determine when a page should be added or removed from the freelist.
However, this is a much bigger change than what I'm willing to risk at
this time.

Signed-off-by: Carlos Llamas <cmllamas@google.com>
---
 drivers/android/binder_alloc.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index 052a8c3b0ce1..edd9714ec9f5 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -446,8 +446,8 @@ static struct binder_buffer *binder_alloc_new_buf_locked(
 	struct rb_node *n = alloc->free_buffers.rb_node;
 	struct rb_node *best_fit = NULL;
 	struct binder_buffer *buffer;
-	unsigned long has_page_addr;
-	unsigned long end_page_addr;
+	unsigned long next_used_page;
+	unsigned long curr_last_page;
 	size_t buffer_size;
 
 	if (is_async && alloc->free_async_space < size) {
@@ -500,12 +500,16 @@ 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;
-	end_page_addr = PAGE_ALIGN(buffer->user_data + size);
-	if (end_page_addr > has_page_addr)
-		end_page_addr = has_page_addr;
+	/*
+	 * Now we remove the pages from the freelist. A clever calculation
+	 * with buffer_size determines if the last page is shared with an
+	 * adjacent in-use buffer. In such case, the page has been already
+	 * removed from the freelist so we trim our range short.
+	 */
+	next_used_page = (buffer->user_data + buffer_size) & PAGE_MASK;
+	curr_last_page = PAGE_ALIGN(buffer->user_data + size);
 	binder_lru_freelist_del(alloc, PAGE_ALIGN(buffer->user_data),
-				end_page_addr);
+				min(next_used_page, curr_last_page));
 
 	rb_erase(&buffer->rb_node, &alloc->free_buffers);
 	buffer->free = 0;
-- 
2.43.0.rc2.451.g8631bc7472-goog


  parent reply	other threads:[~2023-12-01 17:24 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-01 17:21 [PATCH v2 00/28] binder: convert alloc->mutex to spinlock Carlos Llamas
2023-12-01 17:21 ` [PATCH v2 01/28] binder: use EPOLLERR from eventpoll.h Carlos Llamas
2023-12-01 17:21 ` [PATCH v2 02/28] binder: fix use-after-free in shinker's callback Carlos Llamas
2023-12-01 17:21 ` [PATCH v2 03/28] binder: fix race between mmput() and do_exit() Carlos Llamas
2024-01-18 19:29   ` Carlos Llamas
2024-01-19  5:48     ` Greg Kroah-Hartman
2024-01-19 17:06       ` Carlos Llamas
2024-01-19 17:37         ` Carlos Llamas
2024-01-20  6:37           ` Greg Kroah-Hartman
2024-01-22 18:05             ` Carlos Llamas
2023-12-01 17:21 ` [PATCH v2 04/28] binder: fix async space check for 0-sized buffers Carlos Llamas
2024-01-18 19:32   ` Carlos Llamas
2024-01-19  5:48     ` Greg Kroah-Hartman
2024-01-19 17:11       ` Carlos Llamas
2024-01-22 15:05         ` Greg Kroah-Hartman
2023-12-01 17:21 ` [PATCH v2 05/28] binder: fix unused alloc->free_async_space Carlos Llamas
2023-12-04 11:56   ` Alice Ryhl
2024-01-18 19:33   ` Carlos Llamas
2024-01-19  5:49     ` Greg Kroah-Hartman
2024-01-19 17:27       ` Carlos Llamas
2024-01-22 15:04         ` Greg Kroah-Hartman
2024-01-22 15:05           ` Greg Kroah-Hartman
2024-01-22 18:08             ` Carlos Llamas
2024-01-22 18:35               ` Greg Kroah-Hartman
2023-12-01 17:21 ` [PATCH v2 06/28] binder: fix trivial typo of binder_free_buf_locked() Carlos Llamas
2023-12-01 17:21 ` [PATCH v2 07/28] binder: fix comment on binder_alloc_new_buf() return value Carlos Llamas
2023-12-01 17:21 ` [PATCH v2 08/28] binder: remove extern from function prototypes Carlos Llamas
2023-12-01 17:21 ` [PATCH v2 09/28] binder: keep vma addresses type as unsigned long Carlos Llamas
2023-12-04 11:56   ` Alice Ryhl
2023-12-01 17:21 ` [PATCH v2 10/28] binder: split up binder_update_page_range() Carlos Llamas
2023-12-01 17:21 ` [PATCH v2 11/28] binder: do unlocked work in binder_alloc_new_buf() Carlos Llamas
2023-12-04 11:57   ` Alice Ryhl
2023-12-04 14:22     ` Carlos Llamas
2023-12-04 14:23       ` Alice Ryhl
2023-12-01 17:21 ` [PATCH v2 12/28] binder: remove pid param " Carlos Llamas
2023-12-01 17:21 ` [PATCH v2 13/28] binder: separate the no-space debugging logic Carlos Llamas
2023-12-01 17:21 ` [PATCH v2 14/28] binder: relocate low space calculation Carlos Llamas
2023-12-01 17:21 ` [PATCH v2 15/28] binder: relocate binder_alloc_clear_buf() Carlos Llamas
2023-12-01 17:21 ` [PATCH v2 16/28] binder: refactor page range allocation Carlos Llamas
2023-12-01 17:21 ` [PATCH v2 17/28] binder: malloc new_buffer outside of locks Carlos Llamas
2023-12-04 11:57   ` Alice Ryhl
2023-12-01 17:21 ` [PATCH v2 18/28] binder: initialize lru pages in mmap callback Carlos Llamas
2023-12-01 17:21 ` [PATCH v2 19/28] binder: perform page installation outside of locks Carlos Llamas
2023-12-04 11:57   ` Alice Ryhl
2023-12-01 17:21 ` [PATCH v2 20/28] binder: remove redundant debug log Carlos Llamas
2023-12-04 11:57   ` Alice Ryhl
2023-12-01 17:21 ` [PATCH v2 21/28] binder: make oversized buffer code more readable Carlos Llamas
2023-12-04 11:57   ` Alice Ryhl
2023-12-01 17:21 ` [PATCH v2 22/28] binder: rename lru shrinker utilities Carlos Llamas
2023-12-04 11:57   ` Alice Ryhl
2023-12-01 17:21 ` Carlos Llamas [this message]
2023-12-04 11:57   ` [PATCH v2 23/28] binder: document the final page calculation Alice Ryhl
2023-12-04 14:39     ` Carlos Llamas
2023-12-04 14:43       ` Alice Ryhl
2023-12-04 14:53         ` Carlos Llamas
2023-12-01 17:21 ` [PATCH v2 24/28] binder: collapse print_binder_buffer() into caller Carlos Llamas
2023-12-04 11:57   ` Alice Ryhl
2023-12-01 17:21 ` [PATCH v2 25/28] binder: refactor binder_delete_free_buffer() Carlos Llamas
2023-12-04 11:57   ` Alice Ryhl
2023-12-01 17:21 ` [PATCH v2 26/28] binder: avoid user addresses in debug logs Carlos Llamas
2023-12-04 11:57   ` Alice Ryhl
2023-12-01 17:21 ` [PATCH v2 27/28] binder: reverse locking order in shrinker callback Carlos Llamas
2023-12-04 11:57   ` Alice Ryhl
2023-12-04 14:45     ` Carlos Llamas
2023-12-04 14:47       ` Alice Ryhl
2023-12-04 15:01         ` Carlos Llamas
2023-12-01 17:21 ` [PATCH v2 28/28] binder: switch alloc->mutex to spinlock_t 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=20231201172212.1813387-24-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®