From nobody Sun Nov 24 05:45:32 2024 Received: from mail-pj1-f73.google.com (mail-pj1-f73.google.com [209.85.216.73]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4B9A81822E5 for ; Thu, 7 Nov 2024 04:02:55 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.73 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730952176; cv=none; b=vC+XZcNE3fYvsLLPfMVGbzqiqxqtaOfEXCKMLxBF15MYVFypxlIEyuyTdrrD8/sCD6SuVEh7XkavkyulextZvMSnssgKZHSZFTbHGqe5Oaj88Ag2YSH4QmjnctCZ8F1pqTZ/MDPGmYDm3UEPuAEDMMFwwEkhMU0rmyy10CUUWhQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730952176; c=relaxed/simple; bh=88wq84S5d5ZZylLA/CwWmKpip9hM35aPvuKn6ree2C4=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=rB5n68b51TIZSAgDQcscJTki8W5Un1Lr/TAEvWvZGvXtpiHnRLX+iCrr/o3jE9tDGIlUXg3BgkZcEvyKZnVgaIUVShDnRrIN90PgYqO1zv9TZ7vM0D5urCUjaWyzmV8SqWY5mHmUDEUEO+O70trBI3uVXwutwDyIeorjMTZGcVI= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--cmllamas.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=KOQc1kNn; arc=none smtp.client-ip=209.85.216.73 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--cmllamas.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="KOQc1kNn" Received: by mail-pj1-f73.google.com with SMTP id 98e67ed59e1d1-2e3be80e9f3so1381106a91.1 for ; Wed, 06 Nov 2024 20:02:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1730952174; x=1731556974; darn=vger.kernel.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=pWKTVNEJbjLhQRSA7l18YBajLooEVjsYG+y5xx907h4=; b=KOQc1kNn+dbdMmE6kqLTnjmH2rm8C4pPCIxg21CWux2k3rP6n5xc/TRdVaq4CCT6jy KrLRP5JaAydrHgtXz+KZAjZpAhAVy7/uTy2NYWAVscs403M4+9RmDTYX4cvOhlwU+08b KmpQohtJYjN0H3jLUH5KyHOKrufWX5FKdCBc/8yNh2fbDgetxb8Ur/EUh6P+lvSLmA+w BRrdi4jT3K5iwxPBa5kvY33y9LfDIYI1c1aScaz5DTCeJ2R5eljFuu/tix6K16LhyJhx 2gLmS9JYUHYoh/XDMg47LiMu1hDJrW7pAa62Vd8BEyB3SgWmEqjQ0Co3sghHa5jJGUO/ gUYA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1730952174; x=1731556974; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=pWKTVNEJbjLhQRSA7l18YBajLooEVjsYG+y5xx907h4=; b=LM4dr3Tokvc3dLoX/UnnTZF1O36PQAyQHoMk/ZU/bWWWHpa0H0RU6qP5DrErrGqrGu opgXdx285L2OXAbo6RBdrTWVfY4SED09c3nZh4B1zkbQXw1BYFv6UUefqeoHlklPI3cf xSxaw7RTe3k9dm4QDk4kvw6bXcvIMfgU7gv+uCMWe/pagilDgd+eIKJvkZHH8OExuapP 5qaoW3oKXHNNyvz3zYr+GIeculzEXRZVzBxivHyc39lcevHGePLiZ+67P42zQCwbEnOo MREDqCP1Mj1uUItPvCO8t/w2tyzyx2A/T96EfeBCk5XBXJDxhnZf+4AbpxTKlEppqv7G Tyeg== X-Gm-Message-State: AOJu0YyMyWQGxADVSe7f9DKPUPdGkUybrbOvIdTC0LsCpGFSxbfa/PWR wk8Y8F5WV3cnt0Z0CPD6RjAU1z+csFuTakQcGpbBMC3mV+BnWCq3/6+FOXmpm5bGmCG/ZKIrWxw 8ebDbN0VypA== X-Google-Smtp-Source: AGHT+IEzPCrIrGL9Dx4sFlXSN0sFKe0seRw7LeujrW3v443fa/h3ldKBBYNEwjvRvoQxuL792or67ngOprAnow== X-Received: from xllamas.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5070]) (user=cmllamas job=sendgmr) by 2002:a17:90b:110c:b0:2e3:112:22cd with SMTP id 98e67ed59e1d1-2e9a4bc0d0bmr36705a91.0.1730952174545; Wed, 06 Nov 2024 20:02:54 -0800 (PST) Date: Thu, 7 Nov 2024 04:02:23 +0000 In-Reply-To: <20241107040239.2847143-1-cmllamas@google.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20241107040239.2847143-1-cmllamas@google.com> X-Mailer: git-send-email 2.47.0.199.ga7371fff76-goog Message-ID: <20241107040239.2847143-2-cmllamas@google.com> Subject: [PATCH v2 1/8] Revert "binder: switch alloc->mutex to spinlock_t" From: Carlos Llamas To: Greg Kroah-Hartman , "=?UTF-8?q?Arve=20Hj=C3=B8nnev=C3=A5g?=" , Todd Kjos , Martijn Coenen , Joel Fernandes , Christian Brauner , Carlos Llamas , Suren Baghdasaryan Cc: linux-kernel@vger.kernel.org, kernel-team@android.com, Mukesh Ojha Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" This reverts commit 7710e2cca32e7f3958480e8bd44f50e29d0c2509. In preparation for concurrent page installations, restore the original alloc->mutex which will serialize zap_page_range_single() against page installations in subsequent patches (instead of the mmap_sem). Cc: Mukesh Ojha [cmllamas: fix trivial conflict due to 2c10a20f5e84a] Signed-off-by: Carlos Llamas Reviewed-by: Suren Baghdasaryan --- drivers/android/binder_alloc.c | 46 +++++++++++++++++----------------- drivers/android/binder_alloc.h | 10 ++++---- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c index b3acbc4174fb..7241bf4a3ff2 100644 --- a/drivers/android/binder_alloc.c +++ b/drivers/android/binder_alloc.c @@ -169,9 +169,9 @@ struct binder_buffer *binder_alloc_prepare_to_free(stru= ct binder_alloc *alloc, { struct binder_buffer *buffer; =20 - spin_lock(&alloc->lock); + mutex_lock(&alloc->mutex); buffer =3D binder_alloc_prepare_to_free_locked(alloc, user_ptr); - spin_unlock(&alloc->lock); + mutex_unlock(&alloc->mutex); return buffer; } =20 @@ -597,10 +597,10 @@ struct binder_buffer *binder_alloc_new_buf(struct bin= der_alloc *alloc, if (!next) return ERR_PTR(-ENOMEM); =20 - spin_lock(&alloc->lock); + mutex_lock(&alloc->mutex); buffer =3D binder_alloc_new_buf_locked(alloc, next, size, is_async); if (IS_ERR(buffer)) { - spin_unlock(&alloc->lock); + mutex_unlock(&alloc->mutex); goto out; } =20 @@ -608,7 +608,7 @@ struct binder_buffer *binder_alloc_new_buf(struct binde= r_alloc *alloc, buffer->offsets_size =3D offsets_size; buffer->extra_buffers_size =3D extra_buffers_size; buffer->pid =3D current->tgid; - spin_unlock(&alloc->lock); + mutex_unlock(&alloc->mutex); =20 ret =3D binder_install_buffer_pages(alloc, buffer, size); if (ret) { @@ -785,17 +785,17 @@ void binder_alloc_free_buf(struct binder_alloc *alloc, * We could eliminate the call to binder_alloc_clear_buf() * from binder_alloc_deferred_release() by moving this to * binder_free_buf_locked(). However, that could - * increase contention for the alloc->lock if clear_on_free - * is used frequently for large buffers. This lock is not + * increase contention for the alloc mutex if clear_on_free + * is used frequently for large buffers. The mutex is not * needed for correctness here. */ if (buffer->clear_on_free) { binder_alloc_clear_buf(alloc, buffer); buffer->clear_on_free =3D false; } - spin_lock(&alloc->lock); + mutex_lock(&alloc->mutex); binder_free_buf_locked(alloc, buffer); - spin_unlock(&alloc->lock); + mutex_unlock(&alloc->mutex); } =20 /** @@ -893,7 +893,7 @@ void binder_alloc_deferred_release(struct binder_alloc = *alloc) struct binder_buffer *buffer; =20 buffers =3D 0; - spin_lock(&alloc->lock); + mutex_lock(&alloc->mutex); BUG_ON(alloc->vma); =20 while ((n =3D rb_first(&alloc->allocated_buffers))) { @@ -940,7 +940,7 @@ void binder_alloc_deferred_release(struct binder_alloc = *alloc) page_count++; } } - spin_unlock(&alloc->lock); + mutex_unlock(&alloc->mutex); kvfree(alloc->pages); if (alloc->mm) mmdrop(alloc->mm); @@ -964,7 +964,7 @@ void binder_alloc_print_allocated(struct seq_file *m, struct binder_buffer *buffer; struct rb_node *n; =20 - spin_lock(&alloc->lock); + mutex_lock(&alloc->mutex); for (n =3D rb_first(&alloc->allocated_buffers); n; n =3D rb_next(n)) { buffer =3D rb_entry(n, struct binder_buffer, rb_node); seq_printf(m, " buffer %d: %lx size %zd:%zd:%zd %s\n", @@ -974,7 +974,7 @@ void binder_alloc_print_allocated(struct seq_file *m, buffer->extra_buffers_size, buffer->transaction ? "active" : "delivered"); } - spin_unlock(&alloc->lock); + mutex_unlock(&alloc->mutex); } =20 /** @@ -991,7 +991,7 @@ void binder_alloc_print_pages(struct seq_file *m, int lru =3D 0; int free =3D 0; =20 - spin_lock(&alloc->lock); + mutex_lock(&alloc->mutex); /* * Make sure the binder_alloc is fully initialized, otherwise we might * read inconsistent state. @@ -1007,7 +1007,7 @@ void binder_alloc_print_pages(struct seq_file *m, lru++; } } - spin_unlock(&alloc->lock); + mutex_unlock(&alloc->mutex); seq_printf(m, " pages: %d:%d:%d\n", active, lru, free); seq_printf(m, " pages high watermark: %zu\n", alloc->pages_high); } @@ -1023,10 +1023,10 @@ int binder_alloc_get_allocated_count(struct binder_= alloc *alloc) struct rb_node *n; int count =3D 0; =20 - spin_lock(&alloc->lock); + mutex_lock(&alloc->mutex); for (n =3D rb_first(&alloc->allocated_buffers); n !=3D NULL; n =3D rb_nex= t(n)) count++; - spin_unlock(&alloc->lock); + mutex_unlock(&alloc->mutex); return count; } =20 @@ -1071,8 +1071,8 @@ enum lru_status binder_alloc_free_page(struct list_he= ad *item, goto err_mmget; if (!mmap_read_trylock(mm)) goto err_mmap_read_lock_failed; - if (!spin_trylock(&alloc->lock)) - goto err_get_alloc_lock_failed; + if (!mutex_trylock(&alloc->mutex)) + goto err_get_alloc_mutex_failed; if (!page->page_ptr) goto err_page_already_freed; =20 @@ -1091,7 +1091,7 @@ enum lru_status binder_alloc_free_page(struct list_he= ad *item, trace_binder_unmap_kernel_end(alloc, index); =20 list_lru_isolate(lru, item); - spin_unlock(&alloc->lock); + mutex_unlock(&alloc->mutex); spin_unlock(lock); =20 if (vma) { @@ -1111,8 +1111,8 @@ enum lru_status binder_alloc_free_page(struct list_he= ad *item, =20 err_invalid_vma: err_page_already_freed: - spin_unlock(&alloc->lock); -err_get_alloc_lock_failed: + mutex_unlock(&alloc->mutex); +err_get_alloc_mutex_failed: mmap_read_unlock(mm); err_mmap_read_lock_failed: mmput_async(mm); @@ -1147,7 +1147,7 @@ void binder_alloc_init(struct binder_alloc *alloc) alloc->pid =3D current->group_leader->pid; alloc->mm =3D current->mm; mmgrab(alloc->mm); - spin_lock_init(&alloc->lock); + mutex_init(&alloc->mutex); INIT_LIST_HEAD(&alloc->buffers); } =20 diff --git a/drivers/android/binder_alloc.h b/drivers/android/binder_alloc.h index 70387234477e..a5181916942e 100644 --- a/drivers/android/binder_alloc.h +++ b/drivers/android/binder_alloc.h @@ -9,7 +9,7 @@ #include #include #include -#include +#include #include #include #include @@ -72,7 +72,7 @@ struct binder_lru_page { =20 /** * struct binder_alloc - per-binder proc state for binder allocator - * @lock: protects binder_alloc fields + * @mutex: protects binder_alloc fields * @vma: vm_area_struct passed to mmap_handler * (invariant after mmap) * @mm: copy of task->mm (invariant after open) @@ -96,7 +96,7 @@ struct binder_lru_page { * struct binder_buffer objects used to track the user buffers */ struct binder_alloc { - spinlock_t lock; + struct mutex mutex; struct vm_area_struct *vma; struct mm_struct *mm; unsigned long buffer; @@ -153,9 +153,9 @@ binder_alloc_get_free_async_space(struct binder_alloc *= alloc) { size_t free_async_space; =20 - spin_lock(&alloc->lock); + mutex_lock(&alloc->mutex); free_async_space =3D alloc->free_async_space; - spin_unlock(&alloc->lock); + mutex_unlock(&alloc->mutex); return free_async_space; } =20 --=20 2.47.0.199.ga7371fff76-goog From nobody Sun Nov 24 05:45:32 2024 Received: from mail-yw1-f202.google.com (mail-yw1-f202.google.com [209.85.128.202]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id EB07F18F2DA for ; Thu, 7 Nov 2024 04:02:57 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.202 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730952179; cv=none; b=JFHcsBN3B2UMqZy82AsI2mWcZ+pvzqZ5oJZImK/lqqG7nQhYfXUFQdj7ex0Xx19fabxP9IK6U1gjAhkaYJUC2btAqEa3xai/LGu6XkVfYIBgX41nM9tBSmR+Bqt/kIJQA3W8rjk5AHoR+fJnP4Al6Th1N2io+oBDmo5+M5Hwfi8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730952179; c=relaxed/simple; bh=b3NB17IoZxtPjgmyAnkktSR8S47PnlT/gM+D1o9Rwz8=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=g1NLRAqa37kDXfqW+grw+o57kkOUs7AaXOJf6BCpdOvmVGSKitaGjDR6d9o0IEDk9ijgo3un8npDAmEOG1Y1BmXVa6VgcNARf0+DC/m2tbTffNS0oKwWkowUGRIIjQ9tz0Fr1Tm0ewXdGR4l6S/ESlhMhV7tPoy9KuyJdkdiLAU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--cmllamas.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=euvg0qe/; arc=none smtp.client-ip=209.85.128.202 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--cmllamas.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="euvg0qe/" Received: by mail-yw1-f202.google.com with SMTP id 00721157ae682-6ea8d6fb2ffso10439487b3.2 for ; Wed, 06 Nov 2024 20:02:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1730952177; x=1731556977; darn=vger.kernel.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=PoA41PKnjl7Bcop6YK1T7osmkskExFu/kuDntua0J3Y=; b=euvg0qe/n8Me0CcJi5Y9oRewWVik3bJtOsxqE6kUDnV7sS/LlIVTCfApqJl3EOpjg0 NXN3ZMtHTiACM9jzB5TKhwZiS4ASXNik7ufUUvT9LkATzz4DO7rjO84AcbKxL3uXKvHA SOhtI9rXrtFEuBg4oJ/u9G8d+a9+5CmOweELzC/mQTufpAs1+1PIekfdZQQF+J+5QA0u /H97qwhdPcznkiTkX5u/TG9T7VTrZ0yw6qMf86/FGvYEocB2s+S6UuLVO4EGr9zeE88z c0/iptxWkCfVxNsW0sGF4YUi90tieOPaNgHZ5V7c9kJwdUcra4ffEGKCkUD0KTxPU0CJ 50PA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1730952177; x=1731556977; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=PoA41PKnjl7Bcop6YK1T7osmkskExFu/kuDntua0J3Y=; b=TNGjSxp3mkw0EaEjiqlRhuay1CMxnLYojpHk/5gstdBVCT8C1IXT1nHNv0Y/oq+iQW 7dDZjBY5kuxcyse5s4MNs2q6YZhjUWiofCK9Q93HOTZj1NuRxoCkahAPPizjjiT6G3MQ 7GirDK9jnLovX4FrWX0PzBI+TgWm17ByfOijcznysVSLnrY9USxYjRseTYfA+CKpnVLU CfPW5crmvFzI4mY4uNbJPzvRblGeYP72RabXQ9GOnlVq+mCGqEK2vVD0X9eWiEMfJL8Q 0KAdJXXcRMEx56FUgDpeVedCACxJVPqdcbuGN1HsVBQTOCd6GceC6O7MWznb8oAURRT8 t4GQ== X-Gm-Message-State: AOJu0Yy+NiBbqM2mWybH/9++JxaCvoxMfjTAbth8BuGEPJlT1QS4CiE6 elhmcXIrJYerfj/bFIYgYGAH6C/5nodyQ7+dL70S1mEsYZgwn21WMqhO3MubpX/zktp2Lm5PnSe YvnPh6xdX5A== X-Google-Smtp-Source: AGHT+IHtv5RChbrnmtW5n+BrsQUYznKzLonVoC9N7KFiCDxROpnWYtAtAQ0ePbakaHJNWDyAvGo11Rp43/XFMg== X-Received: from xllamas.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5070]) (user=cmllamas job=sendgmr) by 2002:a05:690c:308a:b0:6e9:f188:8638 with SMTP id 00721157ae682-6e9f1888858mr14589497b3.7.1730952176904; Wed, 06 Nov 2024 20:02:56 -0800 (PST) Date: Thu, 7 Nov 2024 04:02:24 +0000 In-Reply-To: <20241107040239.2847143-1-cmllamas@google.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20241107040239.2847143-1-cmllamas@google.com> X-Mailer: git-send-email 2.47.0.199.ga7371fff76-goog Message-ID: <20241107040239.2847143-3-cmllamas@google.com> Subject: [PATCH v2 2/8] binder: concurrent page installation From: Carlos Llamas To: Greg Kroah-Hartman , "=?UTF-8?q?Arve=20Hj=C3=B8nnev=C3=A5g?=" , Todd Kjos , Martijn Coenen , Joel Fernandes , Christian Brauner , Carlos Llamas , Suren Baghdasaryan Cc: linux-kernel@vger.kernel.org, kernel-team@android.com, David Hildenbrand , Barry Song , "Liam R. Howlett" Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" Allow multiple callers to install pages simultaneously by downgrading the mmap_sem to non-exclusive mode. Races to the same PTE are handled using get_user_pages_remote() to retrieve the already installed page. This method significantly reduces contention in the mmap semaphore. To ensure safety, vma_lookup() is used (instead of alloc->vma) to avoid operating on an isolated VMA. In addition, zap_page_range_single() is called under the alloc->mutex to avoid racing with the shrinker. Many thanks to Barry Song who posted a similar approach [1]. Link: https://lore.kernel.org/all/20240902225009.34576-1-21cnbao@gmail.com/= [1] Cc: David Hildenbrand Cc: Barry Song Cc: Suren Baghdasaryan Cc: Liam R. Howlett Signed-off-by: Carlos Llamas Reviewed-by: Suren Baghdasaryan --- drivers/android/binder_alloc.c | 64 +++++++++++++++++++++------------- 1 file changed, 40 insertions(+), 24 deletions(-) diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c index 7241bf4a3ff2..acdc05552603 100644 --- a/drivers/android/binder_alloc.c +++ b/drivers/android/binder_alloc.c @@ -221,26 +221,14 @@ static int binder_install_single_page(struct binder_a= lloc *alloc, struct binder_lru_page *lru_page, unsigned long addr) { + struct vm_area_struct *vma; struct page *page; - int ret =3D 0; + long npages; + int ret; =20 if (!mmget_not_zero(alloc->mm)) return -ESRCH; =20 - /* - * Protected with mmap_sem in write mode as multiple tasks - * might race to install the same page. - */ - mmap_write_lock(alloc->mm); - if (binder_get_installed_page(lru_page)) - goto out; - - if (!alloc->vma) { - pr_err("%d: %s failed, no vma\n", alloc->pid, __func__); - ret =3D -ESRCH; - goto out; - } - page =3D alloc_page(GFP_KERNEL | __GFP_HIGHMEM | __GFP_ZERO); if (!page) { pr_err("%d: failed to allocate page\n", alloc->pid); @@ -248,19 +236,47 @@ static int binder_install_single_page(struct binder_a= lloc *alloc, goto out; } =20 - ret =3D vm_insert_page(alloc->vma, addr, page); - if (ret) { - pr_err("%d: %s failed to insert page at offset %lx with %d\n", - alloc->pid, __func__, addr - alloc->buffer, ret); + mmap_read_lock(alloc->mm); + vma =3D vma_lookup(alloc->mm, addr); + if (!vma || vma !=3D alloc->vma) { + mmap_read_unlock(alloc->mm); __free_page(page); - ret =3D -ENOMEM; + pr_err("%d: %s failed, no vma\n", alloc->pid, __func__); + ret =3D -ESRCH; goto out; } =20 - /* Mark page installation complete and safe to use */ - binder_set_installed_page(lru_page, page); + ret =3D vm_insert_page(vma, addr, page); + switch (ret) { + case -EBUSY: + /* + * EBUSY is ok. Someone installed the pte first but the + * lru_page->page_ptr has not been updated yet. Discard + * our page and look up the one already installed. + */ + ret =3D 0; + __free_page(page); + npages =3D get_user_pages_remote(alloc->mm, addr, 1, 0, &page, NULL); + if (npages <=3D 0) { + pr_err("%d: failed to find page at offset %lx\n", + alloc->pid, addr - alloc->buffer); + ret =3D -ESRCH; + break; + } + fallthrough; + case 0: + /* Mark page installation complete and safe to use */ + binder_set_installed_page(lru_page, page); + break; + default: + __free_page(page); + pr_err("%d: %s failed to insert page at offset %lx with %d\n", + alloc->pid, __func__, addr - alloc->buffer, ret); + ret =3D -ENOMEM; + break; + } + mmap_read_unlock(alloc->mm); out: - mmap_write_unlock(alloc->mm); mmput_async(alloc->mm); return ret; } @@ -1091,7 +1107,6 @@ enum lru_status binder_alloc_free_page(struct list_he= ad *item, trace_binder_unmap_kernel_end(alloc, index); =20 list_lru_isolate(lru, item); - mutex_unlock(&alloc->mutex); spin_unlock(lock); =20 if (vma) { @@ -1102,6 +1117,7 @@ enum lru_status binder_alloc_free_page(struct list_he= ad *item, trace_binder_unmap_user_end(alloc, index); } =20 + mutex_unlock(&alloc->mutex); mmap_read_unlock(mm); mmput_async(mm); __free_page(page_to_free); --=20 2.47.0.199.ga7371fff76-goog From nobody Sun Nov 24 05:45:32 2024 Received: from mail-yb1-f201.google.com (mail-yb1-f201.google.com [209.85.219.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 462A6191461 for ; Thu, 7 Nov 2024 04:03:00 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.219.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730952181; cv=none; b=Ipr9ISyU2ttxc6QdqFJ0bmEYQyeCXhwbZcPdyqEbqg8F1GltQ56bm+akv7If6Xoq3bw6ezegDvt4E4k07BkhNAEPFiWbqPTw2m3XBsvIUOeqeHDsCy78zQOpzePOgwh++nA2KHFePEgSRERWhUU8Hrne2yvNycRjc4RAO92QNx4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730952181; c=relaxed/simple; bh=ACA94rHA3OF/Q/ELh+C5/rtsQZN4ryYvpCJhXM/RVto=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=h5LJIq+hBBvLH1tcFiy2Vb7jyXodq3in3SMAYaelUQBR37yl7XQ/BUc0qEp/QmA0v43TLiHFoHMoWpSqPjfHHR+Vj4fmK3bcnaqapf1qdjyHLo08Te90Ok7m0w4nw9AxiTjXYrk4KFid9Oy8UaRc25XkwwfzF+zIMBw8a4DB9iA= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--cmllamas.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=whF32zkY; arc=none smtp.client-ip=209.85.219.201 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--cmllamas.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="whF32zkY" Received: by mail-yb1-f201.google.com with SMTP id 3f1490d57ef6-e30b8fd4ca1so955356276.3 for ; Wed, 06 Nov 2024 20:03:00 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1730952179; x=1731556979; darn=vger.kernel.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=ErYo73+gAWhwTYciX1LO4komCfmpQSriXqDoh6eSQFU=; b=whF32zkYpaC48RRYNGrI6K76W+lp7NIDtxP5R7WC0YPisuHNEWuq48ZHRE7KhvpsGx tVwdCBGw8TnODYawYXtrZvCxZ30XQWiTOMAYrGNmpuSxunk97ZnO4Uuul7ixGcKeHaAG 2JQcu7TF0hrsgJQSExj3aF5NiZJxOoNJuNnOdAtOfgYmyA13Rab+qANKZC49N+FkwZJG snyvWTwDKWYxr7M4Ie0ZPHgTIAKjp1QUgBV9kAZmIhz4w8CwPzDHTDmbfy4DOVDv4swP 2Z28TEr4kJPS7pi35FN28TAFBwVWDNoUCgvEQ9AM7x276+VrbedxbwgKm9rBJTl6PUSB IiFg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1730952179; x=1731556979; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=ErYo73+gAWhwTYciX1LO4komCfmpQSriXqDoh6eSQFU=; b=i08dJcswuJAEfur2+nRQ00j/o6PvVp0I/DxPfw3xxXjE2IRqr34n72lEeaDe6sTAqD QdDcacDuDCahYanf0X2QBXh6cF8cJnH9puzTO6B5k0p/e5loG3N0e9B1oWckFlGDHZLH UOSGhVwYFIEPFxu+yb7C4dE8J01f2I6Q76uZZXmXQfqmHy4/6UNl2KwUtgmjyg166vCp A/4XYUycBCaObMzMjY8M2ghNehypP0b1/3PsuBfftl/7F+wyxE6B8mtn16wcVqj+eYTc AZCLClag/1spM3vQgeMcSdw9jUuOYrnYYKs2CzBIpZlG3SWOp9uzCSoPbIC1/u1qvRwG xAbw== X-Gm-Message-State: AOJu0YzUbYt7g6b7R2SMk+ObkClg6czxj4xcasn5oFqjkjrMViGMX/nX /P2VLybucpd7baCYR46Agc/jBqpAdXFDY1LOwn9ray4EhamEMH5eltuZP2NabxS4Bls9uQV58o8 viqwmoOfbCQ== X-Google-Smtp-Source: AGHT+IFQsyOzE0bv1vZ+uFApEsJWUvfChQs+d4izWRMPA8b2quSBJ1ZlzmGu+B70umq1qYq4VeFOZVK5HgafzQ== X-Received: from xllamas.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5070]) (user=cmllamas job=sendgmr) by 2002:a25:d813:0:b0:e28:ee84:e4d8 with SMTP id 3f1490d57ef6-e336f3e2204mr494276.3.1730952179189; Wed, 06 Nov 2024 20:02:59 -0800 (PST) Date: Thu, 7 Nov 2024 04:02:25 +0000 In-Reply-To: <20241107040239.2847143-1-cmllamas@google.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20241107040239.2847143-1-cmllamas@google.com> X-Mailer: git-send-email 2.47.0.199.ga7371fff76-goog Message-ID: <20241107040239.2847143-4-cmllamas@google.com> Subject: [PATCH v2 3/8] binder: select correct nid for pages in LRU From: Carlos Llamas To: Greg Kroah-Hartman , "=?UTF-8?q?Arve=20Hj=C3=B8nnev=C3=A5g?=" , Todd Kjos , Martijn Coenen , Joel Fernandes , Christian Brauner , Carlos Llamas , Suren Baghdasaryan Cc: linux-kernel@vger.kernel.org, kernel-team@android.com, Nhat Pham , Johannes Weiner Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" The numa node id for binder pages is currently being derived from the lru entry under struct binder_lru_page. However, this object doesn't reflect the node id of the struct page items allocated separately. Instead, select the correct node id from the page itself. This was made possible since commit 0a97c01cd20b ("list_lru: allow explicit memcg and NUMA node selection"). Cc: Nhat Pham Cc: Johannes Weiner Cc: Suren Baghdasaryan Signed-off-by: Carlos Llamas Reviewed-by: Suren Baghdasaryan --- drivers/android/binder_alloc.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c index acdc05552603..7e205b508ce9 100644 --- a/drivers/android/binder_alloc.c +++ b/drivers/android/binder_alloc.c @@ -210,7 +210,10 @@ static void binder_lru_freelist_add(struct binder_allo= c *alloc, =20 trace_binder_free_lru_start(alloc, index); =20 - ret =3D list_lru_add_obj(&binder_freelist, &page->lru); + ret =3D list_lru_add(&binder_freelist, + &page->lru, + page_to_nid(page->page_ptr), + NULL); WARN_ON(!ret); =20 trace_binder_free_lru_end(alloc, index); @@ -333,7 +336,10 @@ static void binder_lru_freelist_del(struct binder_allo= c *alloc, if (page->page_ptr) { trace_binder_alloc_lru_start(alloc, index); =20 - on_lru =3D list_lru_del_obj(&binder_freelist, &page->lru); + on_lru =3D list_lru_del(&binder_freelist, + &page->lru, + page_to_nid(page->page_ptr), + NULL); WARN_ON(!on_lru); =20 trace_binder_alloc_lru_end(alloc, index); @@ -946,8 +952,10 @@ void binder_alloc_deferred_release(struct binder_alloc= *alloc) if (!alloc->pages[i].page_ptr) continue; =20 - on_lru =3D list_lru_del_obj(&binder_freelist, - &alloc->pages[i].lru); + on_lru =3D list_lru_del(&binder_freelist, + &alloc->pages[i].lru, + page_to_nid(alloc->pages[i].page_ptr), + NULL); binder_alloc_debug(BINDER_DEBUG_BUFFER_ALLOC, "%s: %d: page %d %s\n", __func__, alloc->pid, i, --=20 2.47.0.199.ga7371fff76-goog From nobody Sun Nov 24 05:45:32 2024 Received: from mail-yb1-f201.google.com (mail-yb1-f201.google.com [209.85.219.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 2FF591922F3 for ; Thu, 7 Nov 2024 04:03:02 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.219.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730952185; cv=none; b=qZdAcsKrzQzLGRYP9TpD4S1POQkHY8ykdPVVWZs+nQso/kxROkJVBwUuV9kVuy8ffRWTlTKwsDWHrdTyHVtPd2Xt4MRoAks1BlhFGPScue0Q39SMQj99QhJsBNETdB7HV5yIKJJ0a+sB5Q2WRwELBXyVj0Mh5NDpGYWnuzhuQbg= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730952185; c=relaxed/simple; bh=B7rq/yxyFdLbPHYpkdzORHBsQJKHzIxH7aXLYNLzfCw=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=kh8JFrDMQ0fhnIsRoVMcJtYSyupXDAWQqty97Iv+SU3KttBhe2Sd9f5ztBVAFpk8yH0CEKHvlsT3oOvUkPpeQUdfuNRpTKzZcB/Qd8QI43UeAag246Zw9lHzINCVc6zO3VRw7QGXTv24XZ7fX73lE/Ikn2ic0dYiDaHYVgy/UtU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--cmllamas.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=WS7gPesP; arc=none smtp.client-ip=209.85.219.201 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--cmllamas.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="WS7gPesP" Received: by mail-yb1-f201.google.com with SMTP id 3f1490d57ef6-e297a366304so929671276.2 for ; Wed, 06 Nov 2024 20:03:02 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1730952182; x=1731556982; darn=vger.kernel.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=cVzizMcUOU+SF+xbwa6ZnFPtrSG4MBzEqyFqh8SIB2c=; b=WS7gPesPMeJi8lEDYkf56tcKTxFT6JiPW8K9Wo+0lLuDDrfK6h4/hjMuDRfEM5sNdA FNUvqoiWWRjrfAhMtzqLHd/meu7hxysDPolFYaxzJKP71JXUEcXRhTqG2A2rNLGITjXg OBsmumpc4JVHuo8WJMa0U7nNJh6rnLf+BT7HCa59ytOjtjbjr1/xiZj8OijMwgaYQfrG W2kLoJQqRm0tPsvRNmlnrH7NpwY9Mo1otA8YnO+3h8ynmTPBkBV65bNIQoaVm8KlRTM8 0U8y4dGIGtVpWnUGrNmaXax3JCobDJYY6BL1rFnZRnKN/nBGrPwHMHax2CHfrVjruDUF JByQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1730952182; x=1731556982; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=cVzizMcUOU+SF+xbwa6ZnFPtrSG4MBzEqyFqh8SIB2c=; b=Z8gZVsGXGMWbpVASuleHd1oycZ91pd980YmBrjCqLZcT75Uk0CxBiWqxSIAUQCv4v2 PMbwXlqMwCCBBdE8mm1sEpF9Z4YFpFdWhOa7Qm1+k2c+3ltcLN7+ldG+0iZLnD+Q9eD/ aCXOaGrCs4bsamsAePIIbfX5Lwcf+8mmSRI+1Mtp11cJpqwJ81f9Adu3YbMm4S2q3AMs HGSj3isJSeev3vXIK5opQXDMV97vq+fEf/9Mo05MfMmSC56zFMES0QEWhFpQ3ic+QNJ1 WOn1mEK8dm/X6dGF8I1Hul6AE05pRzxADbg+KjcM34o1l2J5ucdPSOfCqdo9RW3rwmYl TCkg== X-Gm-Message-State: AOJu0YxHoBiMcXiX0fjcmhZMGq8ZZ5X4pHJMjdU+G9sqzqUFJhkEiMU7 11zWLysX3W+sdNcS6MfG20ZWgKLKNih9WxHrzuF8ANT6IlCGGceFx8kxfRqSosXDuY5MFKfljkz +sjrQjIM+Jg== X-Google-Smtp-Source: AGHT+IG3CJ9CtNOB+6OaaejO8KFghtUsaykavIIWjcjaIcx6hKdPTdzYFU3IgGs/Euco3yNd+MalGtjmLJvr3A== X-Received: from xllamas.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5070]) (user=cmllamas job=sendgmr) by 2002:a25:b11f:0:b0:e33:3af2:4631 with SMTP id 3f1490d57ef6-e333af24708mr15165276.4.1730952182084; Wed, 06 Nov 2024 20:03:02 -0800 (PST) Date: Thu, 7 Nov 2024 04:02:26 +0000 In-Reply-To: <20241107040239.2847143-1-cmllamas@google.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20241107040239.2847143-1-cmllamas@google.com> X-Mailer: git-send-email 2.47.0.199.ga7371fff76-goog Message-ID: <20241107040239.2847143-5-cmllamas@google.com> Subject: [PATCH v2 4/8] binder: remove struct binder_lru_page From: Carlos Llamas To: Greg Kroah-Hartman , "=?UTF-8?q?Arve=20Hj=C3=B8nnev=C3=A5g?=" , Todd Kjos , Martijn Coenen , Joel Fernandes , Christian Brauner , Carlos Llamas , Suren Baghdasaryan Cc: linux-kernel@vger.kernel.org, kernel-team@android.com, Matthew Wilcox , "Liam R. Howlett" Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" Remove the redundant struct binder_lru_page concept. Instead, let's use available struct page->lru and page->private members directly to achieve the same functionality. This reduces the maximum memory allocated for alloc->pages from 32768 down to 8192 bytes (aarch64). Savings are per binder instance. Cc: Matthew Wilcox Cc: Liam R. Howlett Signed-off-by: Carlos Llamas Reviewed-by: Suren Baghdasaryan --- drivers/android/binder_alloc.c | 102 +++++++++++++----------- drivers/android/binder_alloc.h | 16 +--- drivers/android/binder_alloc_selftest.c | 14 ++-- 3 files changed, 63 insertions(+), 69 deletions(-) diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c index 7e205b508ce9..46789fa530a1 100644 --- a/drivers/android/binder_alloc.c +++ b/drivers/android/binder_alloc.c @@ -176,25 +176,26 @@ struct binder_buffer *binder_alloc_prepare_to_free(st= ruct binder_alloc *alloc, } =20 static inline void -binder_set_installed_page(struct binder_lru_page *lru_page, +binder_set_installed_page(struct binder_alloc *alloc, + unsigned long index, struct page *page) { /* Pairs with acquire in binder_get_installed_page() */ - smp_store_release(&lru_page->page_ptr, page); + smp_store_release(&alloc->pages[index], page); } =20 static inline struct page * -binder_get_installed_page(struct binder_lru_page *lru_page) +binder_get_installed_page(struct binder_alloc *alloc, unsigned long index) { /* Pairs with release in binder_set_installed_page() */ - return smp_load_acquire(&lru_page->page_ptr); + return smp_load_acquire(&alloc->pages[index]); } =20 static void binder_lru_freelist_add(struct binder_alloc *alloc, unsigned long start, unsigned long end) { - struct binder_lru_page *page; unsigned long page_addr; + struct page *page; =20 trace_binder_update_page_range(alloc, false, start, end); =20 @@ -203,16 +204,15 @@ static void binder_lru_freelist_add(struct binder_all= oc *alloc, int ret; =20 index =3D (page_addr - alloc->buffer) / PAGE_SIZE; - page =3D &alloc->pages[index]; - - if (!binder_get_installed_page(page)) + page =3D binder_get_installed_page(alloc, index); + if (!page) continue; =20 trace_binder_free_lru_start(alloc, index); =20 ret =3D list_lru_add(&binder_freelist, &page->lru, - page_to_nid(page->page_ptr), + page_to_nid(page), NULL); WARN_ON(!ret); =20 @@ -220,8 +220,25 @@ static void binder_lru_freelist_add(struct binder_allo= c *alloc, } } =20 +static struct page *binder_page_alloc(struct binder_alloc *alloc, + unsigned long index, + unsigned long addr) +{ + struct page *page; + + page =3D alloc_page(GFP_KERNEL | __GFP_HIGHMEM | __GFP_ZERO); + if (!page) + return NULL; + + page->private =3D (unsigned long)alloc; + INIT_LIST_HEAD(&page->lru); + page->index =3D index; + + return page; +} + static int binder_install_single_page(struct binder_alloc *alloc, - struct binder_lru_page *lru_page, + unsigned long index, unsigned long addr) { struct vm_area_struct *vma; @@ -232,9 +249,8 @@ static int binder_install_single_page(struct binder_all= oc *alloc, if (!mmget_not_zero(alloc->mm)) return -ESRCH; =20 - page =3D alloc_page(GFP_KERNEL | __GFP_HIGHMEM | __GFP_ZERO); + page =3D binder_page_alloc(alloc, index, addr); if (!page) { - pr_err("%d: failed to allocate page\n", alloc->pid); ret =3D -ENOMEM; goto out; } @@ -254,7 +270,7 @@ static int binder_install_single_page(struct binder_all= oc *alloc, case -EBUSY: /* * EBUSY is ok. Someone installed the pte first but the - * lru_page->page_ptr has not been updated yet. Discard + * alloc->pages[index] has not been updated yet. Discard * our page and look up the one already installed. */ ret =3D 0; @@ -269,7 +285,7 @@ static int binder_install_single_page(struct binder_all= oc *alloc, fallthrough; case 0: /* Mark page installation complete and safe to use */ - binder_set_installed_page(lru_page, page); + binder_set_installed_page(alloc, index, page); break; default: __free_page(page); @@ -288,7 +304,6 @@ static int binder_install_buffer_pages(struct binder_al= loc *alloc, struct binder_buffer *buffer, size_t size) { - struct binder_lru_page *page; unsigned long start, final; unsigned long page_addr; =20 @@ -300,14 +315,12 @@ static int binder_install_buffer_pages(struct binder_= alloc *alloc, int ret; =20 index =3D (page_addr - alloc->buffer) / PAGE_SIZE; - page =3D &alloc->pages[index]; - - if (binder_get_installed_page(page)) + if (binder_get_installed_page(alloc, index)) continue; =20 trace_binder_alloc_page_start(alloc, index); =20 - ret =3D binder_install_single_page(alloc, page, page_addr); + ret =3D binder_install_single_page(alloc, index, page_addr); if (ret) return ret; =20 @@ -321,8 +334,8 @@ static int binder_install_buffer_pages(struct binder_al= loc *alloc, static void binder_lru_freelist_del(struct binder_alloc *alloc, unsigned long start, unsigned long end) { - struct binder_lru_page *page; unsigned long page_addr; + struct page *page; =20 trace_binder_update_page_range(alloc, true, start, end); =20 @@ -331,14 +344,14 @@ static void binder_lru_freelist_del(struct binder_all= oc *alloc, bool on_lru; =20 index =3D (page_addr - alloc->buffer) / PAGE_SIZE; - page =3D &alloc->pages[index]; + page =3D binder_get_installed_page(alloc, index); =20 - if (page->page_ptr) { + if (page) { trace_binder_alloc_lru_start(alloc, index); =20 on_lru =3D list_lru_del(&binder_freelist, &page->lru, - page_to_nid(page->page_ptr), + page_to_nid(page), NULL); WARN_ON(!on_lru); =20 @@ -759,11 +772,10 @@ static struct page *binder_alloc_get_page(struct bind= er_alloc *alloc, (buffer->user_data - alloc->buffer); pgoff_t pgoff =3D buffer_space_offset & ~PAGE_MASK; size_t index =3D buffer_space_offset >> PAGE_SHIFT; - struct binder_lru_page *lru_page; =20 - lru_page =3D &alloc->pages[index]; *pgoffp =3D pgoff; - return lru_page->page_ptr; + + return binder_get_installed_page(alloc, index); } =20 /** @@ -838,7 +850,7 @@ int binder_alloc_mmap_handler(struct binder_alloc *allo= c, { struct binder_buffer *buffer; const char *failure_string; - int ret, i; + int ret; =20 if (unlikely(vma->vm_mm !=3D alloc->mm)) { ret =3D -EINVAL; @@ -861,17 +873,12 @@ int binder_alloc_mmap_handler(struct binder_alloc *al= loc, alloc->pages =3D kvcalloc(alloc->buffer_size / PAGE_SIZE, sizeof(alloc->pages[0]), GFP_KERNEL); - if (alloc->pages =3D=3D NULL) { + if (!alloc->pages) { ret =3D -ENOMEM; failure_string =3D "alloc page array"; goto err_alloc_pages_failed; } =20 - for (i =3D 0; i < alloc->buffer_size / PAGE_SIZE; i++) { - alloc->pages[i].alloc =3D alloc; - INIT_LIST_HEAD(&alloc->pages[i].lru); - } - buffer =3D kzalloc(sizeof(*buffer), GFP_KERNEL); if (!buffer) { ret =3D -ENOMEM; @@ -947,20 +954,22 @@ void binder_alloc_deferred_release(struct binder_allo= c *alloc) int i; =20 for (i =3D 0; i < alloc->buffer_size / PAGE_SIZE; i++) { + struct page *page; bool on_lru; =20 - if (!alloc->pages[i].page_ptr) + page =3D binder_get_installed_page(alloc, i); + if (!page) continue; =20 on_lru =3D list_lru_del(&binder_freelist, - &alloc->pages[i].lru, - page_to_nid(alloc->pages[i].page_ptr), + &page->lru, + page_to_nid(page), NULL); binder_alloc_debug(BINDER_DEBUG_BUFFER_ALLOC, "%s: %d: page %d %s\n", __func__, alloc->pid, i, on_lru ? "on lru" : "active"); - __free_page(alloc->pages[i].page_ptr); + __free_page(page); page_count++; } } @@ -1009,7 +1018,7 @@ void binder_alloc_print_allocated(struct seq_file *m, void binder_alloc_print_pages(struct seq_file *m, struct binder_alloc *alloc) { - struct binder_lru_page *page; + struct page *page; int i; int active =3D 0; int lru =3D 0; @@ -1022,8 +1031,8 @@ void binder_alloc_print_pages(struct seq_file *m, */ if (binder_alloc_get_vma(alloc) !=3D NULL) { for (i =3D 0; i < alloc->buffer_size / PAGE_SIZE; i++) { - page =3D &alloc->pages[i]; - if (!page->page_ptr) + page =3D binder_get_installed_page(alloc, i); + if (!page) free++; else if (list_empty(&page->lru)) active++; @@ -1083,8 +1092,8 @@ enum lru_status binder_alloc_free_page(struct list_he= ad *item, void *cb_arg) __must_hold(lock) { - struct binder_lru_page *page =3D container_of(item, typeof(*page), lru); - struct binder_alloc *alloc =3D page->alloc; + struct page *page =3D container_of(item, typeof(*page), lru); + struct binder_alloc *alloc =3D (struct binder_alloc *)page->private; struct mm_struct *mm =3D alloc->mm; struct vm_area_struct *vma; struct page *page_to_free; @@ -1097,10 +1106,8 @@ enum lru_status binder_alloc_free_page(struct list_h= ead *item, goto err_mmap_read_lock_failed; if (!mutex_trylock(&alloc->mutex)) goto err_get_alloc_mutex_failed; - if (!page->page_ptr) - goto err_page_already_freed; =20 - index =3D page - alloc->pages; + index =3D page->index; page_addr =3D alloc->buffer + index * PAGE_SIZE; =20 vma =3D vma_lookup(mm, page_addr); @@ -1109,8 +1116,8 @@ enum lru_status binder_alloc_free_page(struct list_he= ad *item, =20 trace_binder_unmap_kernel_start(alloc, index); =20 - page_to_free =3D page->page_ptr; - page->page_ptr =3D NULL; + page_to_free =3D page; + binder_set_installed_page(alloc, index, NULL); =20 trace_binder_unmap_kernel_end(alloc, index); =20 @@ -1134,7 +1141,6 @@ enum lru_status binder_alloc_free_page(struct list_he= ad *item, return LRU_REMOVED_RETRY; =20 err_invalid_vma: -err_page_already_freed: mutex_unlock(&alloc->mutex); err_get_alloc_mutex_failed: mmap_read_unlock(mm); diff --git a/drivers/android/binder_alloc.h b/drivers/android/binder_alloc.h index a5181916942e..5c2473e95494 100644 --- a/drivers/android/binder_alloc.h +++ b/drivers/android/binder_alloc.h @@ -58,18 +58,6 @@ struct binder_buffer { int pid; }; =20 -/** - * struct binder_lru_page - page object used for binder shrinker - * @page_ptr: pointer to physical page in mmap'd space - * @lru: entry in binder_freelist - * @alloc: binder_alloc for a proc - */ -struct binder_lru_page { - struct list_head lru; - struct page *page_ptr; - struct binder_alloc *alloc; -}; - /** * struct binder_alloc - per-binder proc state for binder allocator * @mutex: protects binder_alloc fields @@ -83,7 +71,7 @@ struct binder_lru_page { * @allocated_buffers: rb tree of allocated buffers sorted by address * @free_async_space: VA space available for async buffers. This is * initialized at mmap time to 1/2 the full VA space - * @pages: array of binder_lru_page + * @pages: array of struct page * * @buffer_size: size of address space specified via mmap * @pid: pid for associated binder_proc (invariant after in= it) * @pages_high: high watermark of offset in @pages @@ -104,7 +92,7 @@ struct binder_alloc { struct rb_root free_buffers; struct rb_root allocated_buffers; size_t free_async_space; - struct binder_lru_page *pages; + struct page **pages; size_t buffer_size; int pid; size_t pages_high; diff --git a/drivers/android/binder_alloc_selftest.c b/drivers/android/bind= er_alloc_selftest.c index 81442fe20a69..c6941b9abad9 100644 --- a/drivers/android/binder_alloc_selftest.c +++ b/drivers/android/binder_alloc_selftest.c @@ -105,10 +105,10 @@ static bool check_buffer_pages_allocated(struct binde= r_alloc *alloc, page_addr =3D buffer->user_data; for (; page_addr < end; page_addr +=3D PAGE_SIZE) { page_index =3D (page_addr - alloc->buffer) / PAGE_SIZE; - if (!alloc->pages[page_index].page_ptr || - !list_empty(&alloc->pages[page_index].lru)) { + if (!alloc->pages[page_index] || + !list_empty(&alloc->pages[page_index]->lru)) { pr_err("expect alloc but is %s at page index %d\n", - alloc->pages[page_index].page_ptr ? + alloc->pages[page_index] ? "lru" : "free", page_index); return false; } @@ -148,10 +148,10 @@ static void binder_selftest_free_buf(struct binder_al= loc *alloc, * if binder shrinker ran during binder_alloc_free_buf * calls above. */ - if (list_empty(&alloc->pages[i].lru)) { + if (list_empty(&alloc->pages[i]->lru)) { pr_err_size_seq(sizes, seq); pr_err("expect lru but is %s at page index %d\n", - alloc->pages[i].page_ptr ? "alloc" : "free", i); + alloc->pages[i] ? "alloc" : "free", i); binder_selftest_failures++; } } @@ -168,9 +168,9 @@ static void binder_selftest_free_page(struct binder_all= oc *alloc) } =20 for (i =3D 0; i < (alloc->buffer_size / PAGE_SIZE); i++) { - if (alloc->pages[i].page_ptr) { + if (alloc->pages[i]) { pr_err("expect free but is %s at page index %d\n", - list_empty(&alloc->pages[i].lru) ? + list_empty(&alloc->pages[i]->lru) ? "alloc" : "lru", i); binder_selftest_failures++; } --=20 2.47.0.199.ga7371fff76-goog From nobody Sun Nov 24 05:45:32 2024 Received: from mail-pg1-f202.google.com (mail-pg1-f202.google.com [209.85.215.202]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6D530191461 for ; Thu, 7 Nov 2024 04:03:05 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.215.202 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730952187; cv=none; b=FiRdMCsuBZPcL5nH0Xc+/9Cmw8zyJvbYnGnhf9OGET+YhKx6uo1KEtUDnAuCS9O6KxlDv2i4S4M3/mSkVu0IOjChiWzwtbfV0FAvYKFWow04Eg500MqK23fV3joFBg3TmTFtgdCF8I7pPjeg/YrT3SB4ecgUhoqIaLp0W9zyZV4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730952187; c=relaxed/simple; bh=JrXBqOo2JhoPajzziRHF9ByUvQg4sbbU4dHzwS1P/dU=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=tkVERrXVTi7qoIn68iaKPuWO61Sv+vUU+nBlQR3oVwai6qZ/5ijyVODeKoqcMqLCA3J4mkMmRQDQhkbEXgfwag2bOZIc3JUmEeTatWM7Q0zzojDpgDfB+0NwGYv/2FoJPXP0lnWPXpPjBp43Pkod1pwXK1LYyiQRL8fWymFBdPM= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--cmllamas.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=L6LeaMeF; arc=none smtp.client-ip=209.85.215.202 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--cmllamas.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="L6LeaMeF" Received: by mail-pg1-f202.google.com with SMTP id 41be03b00d2f7-7ed98536f95so421877a12.1 for ; Wed, 06 Nov 2024 20:03:05 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1730952185; x=1731556985; darn=vger.kernel.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=VR6K66HQm7qElaALR5w1qrjpqMA/hWE59p9IBcioHF4=; b=L6LeaMeFj4vnZRDkCW/wrFJ8/I4mdFYxxwyf8BmIxBDmox7/ANQJUHMO0GnSOXtQp5 4yHR+QkLsxg+NNFEnrQkPpja3hXCVne84koHrhcCo/nsl4tt9HowDnieiFxV+hjjl5LM vIKVhFXUfZLBYvjQLC8k14TZjLfeANn6cR0uMUzBSpCVeh1RxlB6b1LULVqp/wMBa6Xj 2IytdmGeFrREpAajm3xxAk9+C1etK3Hwd7q3klkbZUDE4Sy/BZyl1+YI1dwqcnidctri +Gj1IcvAre+KzZwfRe6wwqLK5hPlTwGqgHEjPkXzhrMqPQJQQqvE8IGe5ZGV8zz/GtBX GJDQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1730952185; x=1731556985; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=VR6K66HQm7qElaALR5w1qrjpqMA/hWE59p9IBcioHF4=; b=pqpb3olGx98hgj+n5Lxzc6ZtPp2sogLtyVK84oe+JYy3Vw3tbQX1cPa+8PyoIQrgjw ZExifZwlkc2mVMvgWZ54lIM0vClxFpjs7VU8nFQqyVfvJybqcgqmRoa74jg2b/Jh5F/N MNGTH3sFgd0QcUjCl2R8E9i+jNMKvLLOQ/+Q+81SGND1pbm7acP/pauqhiJf7ZPg19Jm +rGwrQ+M1Y/1rgMlh2r9eYCpfP95MOu4OOjGE5B4MiVp/5YQT1/DN//nU0u4sYJ+2iUo gviYC0AZv59kZCYMSsnK8IpBMjK6c/agECsgcvhUJoiMnarIx0oyg6y8Ii9aV3PbUBsh fAXw== X-Gm-Message-State: AOJu0Yz/8AbBAWWsjhzsHoSWRIzoXH0z7/xPDO/jLjTQKT9uLs6D5PA2 GKagvoTnmtB2Ngwqg0FS+AEWUckqsa1Z5vhhL0pVt0oqdEM7c2Y0ywKA64vhXAVM4OiczSZywz6 FQr3s2Y+rWw== X-Google-Smtp-Source: AGHT+IGzTSkxUrm1MUGUYs5zsF8w+IstRURotU6hnUiYOUqUTJ7+iZFLzQYNfXpOOIrqTc04S4LmfsYOhni34Q== X-Received: from xllamas.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5070]) (user=cmllamas job=sendgmr) by 2002:a63:f50c:0:b0:7ea:f1c2:8a9 with SMTP id 41be03b00d2f7-7f41c42f222mr5737a12.0.1730952184702; Wed, 06 Nov 2024 20:03:04 -0800 (PST) Date: Thu, 7 Nov 2024 04:02:27 +0000 In-Reply-To: <20241107040239.2847143-1-cmllamas@google.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20241107040239.2847143-1-cmllamas@google.com> X-Mailer: git-send-email 2.47.0.199.ga7371fff76-goog Message-ID: <20241107040239.2847143-6-cmllamas@google.com> Subject: [PATCH v2 5/8] binder: use alloc->mapped to save the vma state From: Carlos Llamas To: Greg Kroah-Hartman , "=?UTF-8?q?Arve=20Hj=C3=B8nnev=C3=A5g?=" , Todd Kjos , Martijn Coenen , Joel Fernandes , Christian Brauner , Carlos Llamas , Suren Baghdasaryan Cc: linux-kernel@vger.kernel.org, kernel-team@android.com, Minchan Kim , "Liam R. Howlett" , Matthew Wilcox Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" It is unsafe to use alloc->vma outside of the mmap_sem. Instead, add a new boolean alloc->mapped to save the vma state (mapped or unmmaped) and use this to validate several paths. This change is in preparation for removing the alloc->vma pointer in a subsequent patch. Cc: Minchan Kim Cc: Liam R. Howlett Cc: Matthew Wilcox Cc: Suren Baghdasaryan Signed-off-by: Carlos Llamas Reviewed-by: Suren Baghdasaryan --- drivers/android/binder_alloc.c | 29 ++++++++++++++++++------- drivers/android/binder_alloc.h | 2 ++ drivers/android/binder_alloc_selftest.c | 2 +- 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c index 46789fa530a1..7239b92ef20a 100644 --- a/drivers/android/binder_alloc.c +++ b/drivers/android/binder_alloc.c @@ -220,6 +220,19 @@ static void binder_lru_freelist_add(struct binder_allo= c *alloc, } } =20 +static inline +void binder_alloc_set_mapped(struct binder_alloc *alloc, bool state) +{ + /* pairs with smp_load_acquire in binder_alloc_is_mapped() */ + smp_store_release(&alloc->mapped, state); +} + +static inline bool binder_alloc_is_mapped(struct binder_alloc *alloc) +{ + /* pairs with smp_store_release in binder_alloc_set_mapped() */ + return smp_load_acquire(&alloc->mapped); +} + static struct page *binder_page_alloc(struct binder_alloc *alloc, unsigned long index, unsigned long addr) @@ -257,7 +270,7 @@ static int binder_install_single_page(struct binder_all= oc *alloc, =20 mmap_read_lock(alloc->mm); vma =3D vma_lookup(alloc->mm, addr); - if (!vma || vma !=3D alloc->vma) { + if (!vma || !binder_alloc_is_mapped(alloc)) { mmap_read_unlock(alloc->mm); __free_page(page); pr_err("%d: %s failed, no vma\n", alloc->pid, __func__); @@ -611,7 +624,7 @@ struct binder_buffer *binder_alloc_new_buf(struct binde= r_alloc *alloc, int ret; =20 /* Check binder_alloc is fully initialized */ - if (!binder_alloc_get_vma(alloc)) { + if (!binder_alloc_is_mapped(alloc)) { binder_alloc_debug(BINDER_DEBUG_USER_ERROR, "%d: binder_alloc_buf, no vma\n", alloc->pid); @@ -893,7 +906,7 @@ int binder_alloc_mmap_handler(struct binder_alloc *allo= c, alloc->free_async_space =3D alloc->buffer_size / 2; =20 /* Signal binder_alloc is fully initialized */ - binder_alloc_set_vma(alloc, vma); + binder_alloc_set_mapped(alloc, true); =20 return 0; =20 @@ -923,7 +936,7 @@ void binder_alloc_deferred_release(struct binder_alloc = *alloc) =20 buffers =3D 0; mutex_lock(&alloc->mutex); - BUG_ON(alloc->vma); + BUG_ON(alloc->mapped); =20 while ((n =3D rb_first(&alloc->allocated_buffers))) { buffer =3D rb_entry(n, struct binder_buffer, rb_node); @@ -1029,7 +1042,7 @@ void binder_alloc_print_pages(struct seq_file *m, * Make sure the binder_alloc is fully initialized, otherwise we might * read inconsistent state. */ - if (binder_alloc_get_vma(alloc) !=3D NULL) { + if (binder_alloc_is_mapped(alloc)) { for (i =3D 0; i < alloc->buffer_size / PAGE_SIZE; i++) { page =3D binder_get_installed_page(alloc, i); if (!page) @@ -1069,12 +1082,12 @@ int binder_alloc_get_allocated_count(struct binder_= alloc *alloc) * @alloc: binder_alloc for this proc * * Called from binder_vma_close() when releasing address space. - * Clears alloc->vma to prevent new incoming transactions from + * Clears alloc->mapped to prevent new incoming transactions from * allocating more buffers. */ void binder_alloc_vma_close(struct binder_alloc *alloc) { - binder_alloc_set_vma(alloc, NULL); + binder_alloc_set_mapped(alloc, false); } =20 /** @@ -1111,7 +1124,7 @@ enum lru_status binder_alloc_free_page(struct list_he= ad *item, page_addr =3D alloc->buffer + index * PAGE_SIZE; =20 vma =3D vma_lookup(mm, page_addr); - if (vma && vma !=3D binder_alloc_get_vma(alloc)) + if (vma && !binder_alloc_is_mapped(alloc)) goto err_invalid_vma; =20 trace_binder_unmap_kernel_start(alloc, index); diff --git a/drivers/android/binder_alloc.h b/drivers/android/binder_alloc.h index 5c2473e95494..89f6fa25c670 100644 --- a/drivers/android/binder_alloc.h +++ b/drivers/android/binder_alloc.h @@ -75,6 +75,7 @@ struct binder_buffer { * @buffer_size: size of address space specified via mmap * @pid: pid for associated binder_proc (invariant after in= it) * @pages_high: high watermark of offset in @pages + * @mapped: whether the vm area is mapped * @oneway_spam_detected: %true if oneway spam detection fired, clear that * flag once the async buffer has returned to a healthy state * @@ -96,6 +97,7 @@ struct binder_alloc { size_t buffer_size; int pid; size_t pages_high; + bool mapped; bool oneway_spam_detected; }; =20 diff --git a/drivers/android/binder_alloc_selftest.c b/drivers/android/bind= er_alloc_selftest.c index c6941b9abad9..2dda82d0d5e8 100644 --- a/drivers/android/binder_alloc_selftest.c +++ b/drivers/android/binder_alloc_selftest.c @@ -291,7 +291,7 @@ void binder_selftest_alloc(struct binder_alloc *alloc) if (!binder_selftest_run) return; mutex_lock(&binder_selftest_lock); - if (!binder_selftest_run || !alloc->vma) + if (!binder_selftest_run || !alloc->mapped) goto done; pr_info("STARTED\n"); binder_selftest_alloc_offset(alloc, end_offset, 0); --=20 2.47.0.199.ga7371fff76-goog From nobody Sun Nov 24 05:45:32 2024 Received: from mail-pj1-f74.google.com (mail-pj1-f74.google.com [209.85.216.74]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 0739E192B90 for ; Thu, 7 Nov 2024 04:03:07 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.74 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730952189; cv=none; b=ngUR76YDaFBJ1tyDomu0PHdDveOcqWQ7CfkiPiLx4HWcV6c7zJf7262xfYT7wM19GoBsvYWcuerqyAn7gA+UvsQOsOJvqMFA3Kql662mxMikZrQdaC3BcWiuOHDQdoMs8yjTBlYExV0+11gRvbRbbjVLUc2UmYTdwBHXl+NpTMU= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730952189; c=relaxed/simple; bh=noVMPsxz7gnOXfT074HrLmzedzvtN+Mhx45a8w5ZR2U=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=NKfhp1+ckMEymoTPFwFH8pIaLOybBp1dopick6FIDxf7VNr/Tqbytyq5SlIOXtyPHoU1aNhQM49AaZqqPW5w/lcmA7G0yPIe4ry4VnqdYsDDpUBFQ/ftAKmQ6p4JORtjYGxDMI+OeQiVznTd0wpQL36thzyWeasGTF+UuD5vIi0= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--cmllamas.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=Ol9i1I2F; arc=none smtp.client-ip=209.85.216.74 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--cmllamas.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="Ol9i1I2F" Received: by mail-pj1-f74.google.com with SMTP id 98e67ed59e1d1-2e2d17b0e86so583204a91.3 for ; Wed, 06 Nov 2024 20:03:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1730952187; x=1731556987; darn=vger.kernel.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=OGsgJ8yqUfMcfrrc0Lq3bkxwEd6Y6CbCViBngZVv6No=; b=Ol9i1I2FNSygaO/eo+ni0bDvdFW3jN8T+XxWpG6WJ9VKSJNiB2/tDPRvIeYJmSPqQv 7CCQGq7zCnq1i19Ui09rveJtpMyNH5LzilA5T2FoJUoYCHDakXVqN7R9+4+BOdq1YJ9p +9fLBJbpQsDlWr5HPZ9ggx1xAN3lGysHicmfY6bafPW9VxTs3t3Y33Ttum5++tLlI99h gqRszD6aGsbEL1iEyBs4IsKmymjqnnzeb/SpULNY5mo3wdcm7bxkkY+tN25KEqRlbHO6 HtVWu105S0W8HpVVWtFQ+wr62+GyM1RSKAl4lWWFw2LANFtGWR7aUI69vK5mnjPvwl6S b7xQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1730952187; x=1731556987; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=OGsgJ8yqUfMcfrrc0Lq3bkxwEd6Y6CbCViBngZVv6No=; b=Fu+H+r5PPEx1w6eUD5DBfFzRx1Z3YgTQDChrYNGR43L4/9uJvMKJYunkTp0UipkAwP WBKZ4OutHEiwjSM129n8Jl1KRZD6+3ge0sHZ8W11nctmpWhvGWlNY8DtrVLaiKlaURDd O1JosUqwojKnXiUfg8s2dNT4JaGD0o0ACt+EGZnRudnIYEKKBd4z3+UEQmv2/f/KSj63 WTHet2lErrRdVrFPVvHOtEzL3me4PMe3TBPvOOH9OdCS8r0WkZ7vD8dVFJfqvflm/3nt JTLA5EdZGaytZPrhz5FhjyKas7L6bGqBVJ8A3p1w4p+Ai5uSkFB9HWgzUDw/OhJFiu26 Bcww== X-Gm-Message-State: AOJu0Yx++s0gXbmlv6TXg8ZCRdjQ4RCvmSdCocyIjnq2Lr04b83PKRgh NXCIEjEKsnG5BLVG0Z6OjPenUzMKEBdzsInyJIvjZzUScw9wRw/XVw+92bLBQ+bju3k4EDk4bqB VOS+D2pqdwQ== X-Google-Smtp-Source: AGHT+IEHzYzkFFvY5RxacsZqR/q0S6nuas6iXnNgQiyipYj7bpf9h6nCNhyvxpHxdYsNcHxkK1ASq3ry6j7Vyw== X-Received: from xllamas.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5070]) (user=cmllamas job=sendgmr) by 2002:a17:90a:708a:b0:2e2:c183:8b1c with SMTP id 98e67ed59e1d1-2e9a7695607mr4190a91.7.1730952187278; Wed, 06 Nov 2024 20:03:07 -0800 (PST) Date: Thu, 7 Nov 2024 04:02:28 +0000 In-Reply-To: <20241107040239.2847143-1-cmllamas@google.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20241107040239.2847143-1-cmllamas@google.com> X-Mailer: git-send-email 2.47.0.199.ga7371fff76-goog Message-ID: <20241107040239.2847143-7-cmllamas@google.com> Subject: [PATCH v2 6/8] binder: remove cached alloc->vma pointer From: Carlos Llamas To: Greg Kroah-Hartman , "=?UTF-8?q?Arve=20Hj=C3=B8nnev=C3=A5g?=" , Todd Kjos , Martijn Coenen , Joel Fernandes , Christian Brauner , Carlos Llamas , Suren Baghdasaryan Cc: linux-kernel@vger.kernel.org, kernel-team@android.com, Minchan Kim , "Liam R. Howlett" , Matthew Wilcox Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" All usage of the alloc->vma pointer has been removed from binder. Instead, paths has been refactored to use either vma_lookup() or the alloc->mapped state. Using the alloc->vma was unsafe and required the mmap_sem in exclusive mode, which caused several performance and security issues in the past. Cc: Minchan Kim Cc: Liam R. Howlett Cc: Matthew Wilcox Cc: Suren Baghdasaryan Signed-off-by: Carlos Llamas Reviewed-by: Suren Baghdasaryan --- drivers/android/binder_alloc.c | 14 -------------- drivers/android/binder_alloc.h | 3 --- 2 files changed, 17 deletions(-) diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c index 7239b92ef20a..13b476cc5b62 100644 --- a/drivers/android/binder_alloc.c +++ b/drivers/android/binder_alloc.c @@ -377,20 +377,6 @@ static void binder_lru_freelist_del(struct binder_allo= c *alloc, } } =20 -static inline void binder_alloc_set_vma(struct binder_alloc *alloc, - struct vm_area_struct *vma) -{ - /* pairs with smp_load_acquire in binder_alloc_get_vma() */ - smp_store_release(&alloc->vma, vma); -} - -static inline struct vm_area_struct *binder_alloc_get_vma( - struct binder_alloc *alloc) -{ - /* pairs with smp_store_release in binder_alloc_set_vma() */ - return smp_load_acquire(&alloc->vma); -} - static void debug_no_space_locked(struct binder_alloc *alloc) { size_t largest_alloc_size =3D 0; diff --git a/drivers/android/binder_alloc.h b/drivers/android/binder_alloc.h index 89f6fa25c670..b9e2e9dc90b3 100644 --- a/drivers/android/binder_alloc.h +++ b/drivers/android/binder_alloc.h @@ -61,8 +61,6 @@ struct binder_buffer { /** * struct binder_alloc - per-binder proc state for binder allocator * @mutex: protects binder_alloc fields - * @vma: vm_area_struct passed to mmap_handler - * (invariant after mmap) * @mm: copy of task->mm (invariant after open) * @buffer: base of per-proc address space mapped via mmap * @buffers: list of all buffers for this proc @@ -86,7 +84,6 @@ struct binder_buffer { */ struct binder_alloc { struct mutex mutex; - struct vm_area_struct *vma; struct mm_struct *mm; unsigned long buffer; struct list_head buffers; --=20 2.47.0.199.ga7371fff76-goog From nobody Sun Nov 24 05:45:32 2024 Received: from mail-pg1-f201.google.com (mail-pg1-f201.google.com [209.85.215.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4CAEA1922F3 for ; Thu, 7 Nov 2024 04:03:11 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.215.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730952193; cv=none; b=An8Q1c26KO5qflHxHft1t3DS6bFudNHjOVkdvWYx6ob1emdCXUDPLvc/MlTExjHUIoGkkQeTVpL2bIJSekNKqCZi37pgdhzin5T5G87QNsGJznk/xR/gmRa+2SkImNZcYFhqAXgziU9/WjVvWO2IwNk86qPu6SRPdHRQ0MXO17Q= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730952193; c=relaxed/simple; bh=TqeovCy0WNRNW18CEBvgwgaGqzhMEkEJIrEa1q9kYb4=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=QEHokGXepQ2nHb1StgEP6JC2BGVutlNARj3fb6M856IQYk2f+Kbjkof/EersAs7uGQ6z+nRC1cBNiG/sKyS4EKci7j5mVuXjJkpaVUIu5rtm+pkI909UHVkaklU43o2pwVU/n7DWgNf8WO6afuLqEM4fylE7D8zHB3CDA/OT7BI= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--cmllamas.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=M8zefXp8; arc=none smtp.client-ip=209.85.215.201 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--cmllamas.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="M8zefXp8" Received: by mail-pg1-f201.google.com with SMTP id 41be03b00d2f7-7edd6662154so417847a12.0 for ; Wed, 06 Nov 2024 20:03:11 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1730952190; x=1731556990; darn=vger.kernel.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=ycccrtPYjiEiPgNw4dfS3DRPV4HB2iHJR7ZI2MWA6bo=; b=M8zefXp8hlq/A425IbduWQsup7/6pQuQR1UiHYh8mrUZGCylW8zsTEqve1RJyiQuXk 5DfPovzFXFDEUL9ZLTCouohRyDK5ARBeDk7voR5SSgubyoRPvU4CJ601Hd6nIVAQZRLZ ob6d/EsN2zfuJGzybKMj1r38uASWfzOPszJQL3iWMN0QcddjQcjVALm8ptJOKNZ4cBG4 gJk4M8RJKXg8H4rewnzyqNX1mPPCtxlDFCY9RIdLPs/LkT6UEpqneHEFFWmh5Ytj/g35 nIZTADsnad7rFP8gAczs8jYOf3C3ULSTHGk3MMgZgRaCEGrLyRpYR4SunUs3n7+jyxac cdwA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1730952190; x=1731556990; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=ycccrtPYjiEiPgNw4dfS3DRPV4HB2iHJR7ZI2MWA6bo=; b=IvRXmQU1p54uUPhUUW0kb0+BDatshXfbIH2mXcHN1ISfFnhyApQOlDmCrb1XP2n7Pi 4XTb6SJ/W8AzaEWN//bwVaE9lT7oO0sgDjzttnB4bAKy9wqWrGIV5AdeGDLnVn1NYZJC T/Sxp+RCp/491V3gbJ4rpmSqPbVF6NM4JzDoqOKzZs5Q/Io2xOEZrS175freCX7Mcs/5 jn0o+jynWvV9m0xIxcZMPJ3EoxRKGxunbJ2v4gTZ917mD5jPOYyx+5uyRHaVf+hvOOVT IFDj15d7qP0XAj1WfqRibF1gKqsoc9Gu3D9PMWoPggW0jtOtPWsEjxTa09dCcMhexaU7 LP8A== X-Gm-Message-State: AOJu0Yz6mEwLwtp98ClxVwgvKby8CQFxU6/C6Y8QCh67CyxhJspWRAo3 AzeVj0ZMrppwfWksv23AUChbThZ791uuBJlW6Vh7HHa8jsnsSwvWlhekGDGw74sdGNm23ZHR5bP jI1/m6Cr2xw== X-Google-Smtp-Source: AGHT+IFKxxXQUmEcIgtrFClvjCJa+aGLMqel+uetWst7u04uf8x0xNCi/8C09042KRlL+LTaLtw+EpG3yIsbCg== X-Received: from xllamas.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5070]) (user=cmllamas job=sendgmr) by 2002:a05:6a02:189:b0:7ea:3fa9:9fce with SMTP id 41be03b00d2f7-7f42385745emr1424a12.7.1730952190205; Wed, 06 Nov 2024 20:03:10 -0800 (PST) Date: Thu, 7 Nov 2024 04:02:29 +0000 In-Reply-To: <20241107040239.2847143-1-cmllamas@google.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20241107040239.2847143-1-cmllamas@google.com> X-Mailer: git-send-email 2.47.0.199.ga7371fff76-goog Message-ID: <20241107040239.2847143-8-cmllamas@google.com> Subject: [PATCH v2 7/8] binder: rename alloc->buffer to vm_start From: Carlos Llamas To: Greg Kroah-Hartman , "=?UTF-8?q?Arve=20Hj=C3=B8nnev=C3=A5g?=" , Todd Kjos , Martijn Coenen , Joel Fernandes , Christian Brauner , Carlos Llamas , Suren Baghdasaryan Cc: linux-kernel@vger.kernel.org, kernel-team@android.com Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" The alloc->buffer field in struct binder_alloc stores the starting address of the mapped vma, rename this field to alloc->vm_start to better reflect its purpose. It also avoids confusion with the binder buffer concept, e.g. transaction->buffer. No functional changes in this patch. Signed-off-by: Carlos Llamas Reviewed-by: Suren Baghdasaryan --- drivers/android/binder.c | 2 +- drivers/android/binder_alloc.c | 28 ++++++++++++------------- drivers/android/binder_alloc.h | 4 ++-- drivers/android/binder_alloc_selftest.c | 2 +- drivers/android/binder_trace.h | 2 +- 5 files changed, 19 insertions(+), 19 deletions(-) diff --git a/drivers/android/binder.c b/drivers/android/binder.c index 978740537a1a..57265cabec43 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -6350,7 +6350,7 @@ static void print_binder_transaction_ilocked(struct s= eq_file *m, seq_printf(m, " node %d", buffer->target_node->debug_id); seq_printf(m, " size %zd:%zd offset %lx\n", buffer->data_size, buffer->offsets_size, - proc->alloc.buffer - buffer->user_data); + proc->alloc.vm_start - buffer->user_data); } =20 static void print_binder_work_ilocked(struct seq_file *m, diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c index 13b476cc5b62..814435a2601a 100644 --- a/drivers/android/binder_alloc.c +++ b/drivers/android/binder_alloc.c @@ -61,7 +61,7 @@ static size_t binder_alloc_buffer_size(struct binder_allo= c *alloc, struct binder_buffer *buffer) { if (list_is_last(&buffer->entry, &alloc->buffers)) - return alloc->buffer + alloc->buffer_size - buffer->user_data; + return alloc->vm_start + alloc->buffer_size - buffer->user_data; return binder_buffer_next(buffer)->user_data - buffer->user_data; } =20 @@ -203,7 +203,7 @@ static void binder_lru_freelist_add(struct binder_alloc= *alloc, size_t index; int ret; =20 - index =3D (page_addr - alloc->buffer) / PAGE_SIZE; + index =3D (page_addr - alloc->vm_start) / PAGE_SIZE; page =3D binder_get_installed_page(alloc, index); if (!page) continue; @@ -291,7 +291,7 @@ static int binder_install_single_page(struct binder_all= oc *alloc, npages =3D get_user_pages_remote(alloc->mm, addr, 1, 0, &page, NULL); if (npages <=3D 0) { pr_err("%d: failed to find page at offset %lx\n", - alloc->pid, addr - alloc->buffer); + alloc->pid, addr - alloc->vm_start); ret =3D -ESRCH; break; } @@ -303,7 +303,7 @@ static int binder_install_single_page(struct binder_all= oc *alloc, default: __free_page(page); pr_err("%d: %s failed to insert page at offset %lx with %d\n", - alloc->pid, __func__, addr - alloc->buffer, ret); + alloc->pid, __func__, addr - alloc->vm_start, ret); ret =3D -ENOMEM; break; } @@ -327,7 +327,7 @@ static int binder_install_buffer_pages(struct binder_al= loc *alloc, unsigned long index; int ret; =20 - index =3D (page_addr - alloc->buffer) / PAGE_SIZE; + index =3D (page_addr - alloc->vm_start) / PAGE_SIZE; if (binder_get_installed_page(alloc, index)) continue; =20 @@ -356,7 +356,7 @@ static void binder_lru_freelist_del(struct binder_alloc= *alloc, unsigned long index; bool on_lru; =20 - index =3D (page_addr - alloc->buffer) / PAGE_SIZE; + index =3D (page_addr - alloc->vm_start) / PAGE_SIZE; page =3D binder_get_installed_page(alloc, index); =20 if (page) { @@ -708,8 +708,8 @@ static void binder_free_buf_locked(struct binder_alloc = *alloc, BUG_ON(buffer->free); BUG_ON(size > buffer_size); BUG_ON(buffer->transaction !=3D NULL); - BUG_ON(buffer->user_data < alloc->buffer); - BUG_ON(buffer->user_data > alloc->buffer + alloc->buffer_size); + BUG_ON(buffer->user_data < alloc->vm_start); + BUG_ON(buffer->user_data > alloc->vm_start + alloc->buffer_size); =20 if (buffer->async_transaction) { alloc->free_async_space +=3D buffer_size; @@ -768,7 +768,7 @@ static struct page *binder_alloc_get_page(struct binder= _alloc *alloc, pgoff_t *pgoffp) { binder_size_t buffer_space_offset =3D buffer_offset + - (buffer->user_data - alloc->buffer); + (buffer->user_data - alloc->vm_start); pgoff_t pgoff =3D buffer_space_offset & ~PAGE_MASK; size_t index =3D buffer_space_offset >> PAGE_SHIFT; =20 @@ -867,7 +867,7 @@ int binder_alloc_mmap_handler(struct binder_alloc *allo= c, SZ_4M); mutex_unlock(&binder_alloc_mmap_lock); =20 - alloc->buffer =3D vma->vm_start; + alloc->vm_start =3D vma->vm_start; =20 alloc->pages =3D kvcalloc(alloc->buffer_size / PAGE_SIZE, sizeof(alloc->pages[0]), @@ -885,7 +885,7 @@ int binder_alloc_mmap_handler(struct binder_alloc *allo= c, goto err_alloc_buf_struct_failed; } =20 - buffer->user_data =3D alloc->buffer; + buffer->user_data =3D alloc->vm_start; list_add(&buffer->entry, &alloc->buffers); buffer->free =3D 1; binder_insert_free_buffer(alloc, buffer); @@ -900,7 +900,7 @@ int binder_alloc_mmap_handler(struct binder_alloc *allo= c, kvfree(alloc->pages); alloc->pages =3D NULL; err_alloc_pages_failed: - alloc->buffer =3D 0; + alloc->vm_start =3D 0; mutex_lock(&binder_alloc_mmap_lock); alloc->buffer_size =3D 0; err_already_mapped: @@ -1001,7 +1001,7 @@ void binder_alloc_print_allocated(struct seq_file *m, buffer =3D rb_entry(n, struct binder_buffer, rb_node); seq_printf(m, " buffer %d: %lx size %zd:%zd:%zd %s\n", buffer->debug_id, - buffer->user_data - alloc->buffer, + buffer->user_data - alloc->vm_start, buffer->data_size, buffer->offsets_size, buffer->extra_buffers_size, buffer->transaction ? "active" : "delivered"); @@ -1107,7 +1107,7 @@ enum lru_status binder_alloc_free_page(struct list_he= ad *item, goto err_get_alloc_mutex_failed; =20 index =3D page->index; - page_addr =3D alloc->buffer + index * PAGE_SIZE; + page_addr =3D alloc->vm_start + index * PAGE_SIZE; =20 vma =3D vma_lookup(mm, page_addr); if (vma && !binder_alloc_is_mapped(alloc)) diff --git a/drivers/android/binder_alloc.h b/drivers/android/binder_alloc.h index b9e2e9dc90b3..05a01d980f61 100644 --- a/drivers/android/binder_alloc.h +++ b/drivers/android/binder_alloc.h @@ -62,7 +62,7 @@ struct binder_buffer { * struct binder_alloc - per-binder proc state for binder allocator * @mutex: protects binder_alloc fields * @mm: copy of task->mm (invariant after open) - * @buffer: base of per-proc address space mapped via mmap + * @vm_start: base of per-proc address space mapped via mmap * @buffers: list of all buffers for this proc * @free_buffers: rb tree of buffers available for allocation * sorted by size @@ -85,7 +85,7 @@ struct binder_buffer { struct binder_alloc { struct mutex mutex; struct mm_struct *mm; - unsigned long buffer; + unsigned long vm_start; struct list_head buffers; struct rb_root free_buffers; struct rb_root allocated_buffers; diff --git a/drivers/android/binder_alloc_selftest.c b/drivers/android/bind= er_alloc_selftest.c index 2dda82d0d5e8..d2d086d2c037 100644 --- a/drivers/android/binder_alloc_selftest.c +++ b/drivers/android/binder_alloc_selftest.c @@ -104,7 +104,7 @@ static bool check_buffer_pages_allocated(struct binder_= alloc *alloc, end =3D PAGE_ALIGN(buffer->user_data + size); page_addr =3D buffer->user_data; for (; page_addr < end; page_addr +=3D PAGE_SIZE) { - page_index =3D (page_addr - alloc->buffer) / PAGE_SIZE; + page_index =3D (page_addr - alloc->vm_start) / PAGE_SIZE; if (!alloc->pages[page_index] || !list_empty(&alloc->pages[page_index]->lru)) { pr_err("expect alloc but is %s at page index %d\n", diff --git a/drivers/android/binder_trace.h b/drivers/android/binder_trace.h index fe38c6fc65d0..16de1b9e72f7 100644 --- a/drivers/android/binder_trace.h +++ b/drivers/android/binder_trace.h @@ -328,7 +328,7 @@ TRACE_EVENT(binder_update_page_range, TP_fast_assign( __entry->proc =3D alloc->pid; __entry->allocate =3D allocate; - __entry->offset =3D start - alloc->buffer; + __entry->offset =3D start - alloc->vm_start; __entry->size =3D end - start; ), TP_printk("proc=3D%d allocate=3D%d offset=3D%zu size=3D%zu", --=20 2.47.0.199.ga7371fff76-goog From nobody Sun Nov 24 05:45:32 2024 Received: from mail-pf1-f201.google.com (mail-pf1-f201.google.com [209.85.210.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5CCD8194137 for ; Thu, 7 Nov 2024 04:03:13 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730952194; cv=none; b=FYSPi9knCJ8k/PESSebYIJF363C/ImeQbBA+gRttIYFEpBFAS7A763dJYwp5MljXf+BwgZwJgKnyVYA1B0GUXF7nI306dyqUvSWwpOwBDQ7whFXI3b6maxnL6ytzA3I32AfBG/a6KjjOmswCcg8/Kec3GcTulQikDRhvh4Jg1Ig= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730952194; c=relaxed/simple; bh=QeLO5CweM43xvKPgGpxsla8P+CWhmaCF9BF4DW0pZVA=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=ApMbFrsjDgFsa4tLd+Q2fKHixXIto+sX98Suvqe1WFsT9ah6KyS5/mJmSgL1ufXf4Er8Whq/PrkgtVO2MLJhjE/6GVzOsDSERgWwVfnMAN7rqBtSCpMde8tZgDkuoy482EBmMs4BLEMkl8Yw6GEKTQA0Xl0GgxOiPRYuqazlgLw= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--cmllamas.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=mzG3QnqQ; arc=none smtp.client-ip=209.85.210.201 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--cmllamas.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="mzG3QnqQ" Received: by mail-pf1-f201.google.com with SMTP id d2e1a72fcca58-71e55c9d23cso469988b3a.0 for ; Wed, 06 Nov 2024 20:03:13 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1730952193; x=1731556993; darn=vger.kernel.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=1I8PhWwXfD9Dckv+8oqyYv+UjXvs0aSVowyzRmrk8To=; b=mzG3QnqQLsP1zyMXDVnweiFUgXr63Cw0EphfQ4ERe5EoNFnj0cGDBM6pT9WAddZAHy K6MeEZMHNQVXHf3xlEQomeE65OiRq8crZgrgyGB5WpnfiUWLJroFT0kDK+pYFSh0Ibr2 4sC/lvSve15Dz/YZ3iK2L/IVX6iT151eZdKthTsGr5axa3VwoCwLi+Ovo7jjkecmyLs7 Vp1ZLt+PnxZXzlD/S6sH44Q8peN1CCSWdd6y4Y96Lvziwhz4Rf+LN56SMtklWbMM+CbK qgBXbGh/EMbtN8vCNNcC0G1un+Xthv5aEiquu3ucA/SLZr/c17a7PaAlqE1MclzGSyJm 7Jsw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1730952193; x=1731556993; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=1I8PhWwXfD9Dckv+8oqyYv+UjXvs0aSVowyzRmrk8To=; b=AZ58n25Rh4KXKBzTOMzVRg1us8GXrhl/YIOUmK5ZyY+pzv/+EKGOzaVKZIjVnOfkrW yGe7Mtro71AB8WCzxbWsD6NVrvmzvzxr3/McmLm8DDVhTMTdId5bhoTYuF9ddbnQ67Y8 82yJ6/M3UbXTuzCUwT69+xqbeRtjjFMquGAFlbs+lf/DGlOKuc8w0C4UCEOrGSNKCnsJ TTaxAXKGe/AaTmOxEwxAZQthz6ZGfjy/hjO/X+4Imvz12x8EkzwpkYXJtk5FZmmPNPdq eaPBLMOQG8StVNPhIOggMY9s1M50Ftl3e+hdf/bTPFO8DWvujot4YVSv7s61Wr5mo/nB jlBg== X-Gm-Message-State: AOJu0Yypw76uAyBkFepmVG+B8mbXlA0FzY049ewnehdZe0NAlQVuu8Ca YxBUTQBQoKhxxH7b8j/V0v8WRQrXnPKn+mgxchW9F0l48ntJcOcMDDB5j4zpAk6RcdpPTMA9/Nb /UaOtbHe/Yg== X-Google-Smtp-Source: AGHT+IH428ZLhClaHyRkD1K0hT9OQaRA/HomA+uiHXNnVIBDOhkf7PCI9vy4dOcT6I5phcYEihJHZuONtC/kuQ== X-Received: from xllamas.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5070]) (user=cmllamas job=sendgmr) by 2002:a05:6a00:73b1:b0:71e:5e28:306a with SMTP id d2e1a72fcca58-72403e2e188mr79647b3a.0.1730952192713; Wed, 06 Nov 2024 20:03:12 -0800 (PST) Date: Thu, 7 Nov 2024 04:02:30 +0000 In-Reply-To: <20241107040239.2847143-1-cmllamas@google.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20241107040239.2847143-1-cmllamas@google.com> X-Mailer: git-send-email 2.47.0.199.ga7371fff76-goog Message-ID: <20241107040239.2847143-9-cmllamas@google.com> Subject: [PATCH v2 8/8] binder: use per-vma lock in page installation From: Carlos Llamas To: Greg Kroah-Hartman , "=?UTF-8?q?Arve=20Hj=C3=B8nnev=C3=A5g?=" , Todd Kjos , Martijn Coenen , Joel Fernandes , Christian Brauner , Carlos Llamas , Suren Baghdasaryan Cc: linux-kernel@vger.kernel.org, kernel-team@android.com, Nhat Pham , Johannes Weiner , Barry Song , Hillf Danton , Lorenzo Stoakes Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" Use per-vma locking for concurrent page installations, this minimizes contention with unrelated vmas improving performance. The mmap_lock is still acquired when needed though, e.g. before get_user_pages_remote(). Many thanks to Barry Song who posted a similar approach [1]. Link: https://lore.kernel.org/all/20240902225009.34576-1-21cnbao@gmail.com/= [1] Cc: Nhat Pham Cc: Johannes Weiner Cc: Barry Song Cc: Suren Baghdasaryan Cc: Hillf Danton Cc: Lorenzo Stoakes Signed-off-by: Carlos Llamas Reviewed-by: Suren Baghdasaryan --- drivers/android/binder_alloc.c | 85 +++++++++++++++++++++++----------- 1 file changed, 57 insertions(+), 28 deletions(-) diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c index 814435a2601a..debfa541e01b 100644 --- a/drivers/android/binder_alloc.c +++ b/drivers/android/binder_alloc.c @@ -233,6 +233,56 @@ static inline bool binder_alloc_is_mapped(struct binde= r_alloc *alloc) return smp_load_acquire(&alloc->mapped); } =20 +static struct page *binder_page_lookup(struct mm_struct *mm, + unsigned long addr) +{ + struct page *page; + long ret; + + if (!mmget_not_zero(mm)) + return NULL; + + mmap_read_lock(mm); + ret =3D get_user_pages_remote(mm, addr, 1, 0, &page, NULL); + mmap_read_unlock(mm); + mmput_async(mm); + + return ret > 0 ? page : NULL; +} + +static int binder_page_insert(struct binder_alloc *alloc, + unsigned long addr, + struct page *page) +{ + struct mm_struct *mm =3D alloc->mm; + struct vm_area_struct *vma; + int ret =3D -ESRCH; + + if (!mmget_not_zero(mm)) + return -ESRCH; + + /* attempt per-vma lock first */ + vma =3D lock_vma_under_rcu(mm, addr); + if (!vma) + goto lock_mmap; + + if (binder_alloc_is_mapped(alloc)) + ret =3D vm_insert_page(vma, addr, page); + vma_end_read(vma); + goto done; + +lock_mmap: + /* fall back to mmap_lock */ + mmap_read_lock(mm); + vma =3D vma_lookup(mm, addr); + if (vma && binder_alloc_is_mapped(alloc)) + ret =3D vm_insert_page(vma, addr, page); + mmap_read_unlock(mm); +done: + mmput_async(mm); + return ret; +} + static struct page *binder_page_alloc(struct binder_alloc *alloc, unsigned long index, unsigned long addr) @@ -254,31 +304,14 @@ static int binder_install_single_page(struct binder_a= lloc *alloc, unsigned long index, unsigned long addr) { - struct vm_area_struct *vma; struct page *page; - long npages; int ret; =20 - if (!mmget_not_zero(alloc->mm)) - return -ESRCH; - page =3D binder_page_alloc(alloc, index, addr); - if (!page) { - ret =3D -ENOMEM; - goto out; - } - - mmap_read_lock(alloc->mm); - vma =3D vma_lookup(alloc->mm, addr); - if (!vma || !binder_alloc_is_mapped(alloc)) { - mmap_read_unlock(alloc->mm); - __free_page(page); - pr_err("%d: %s failed, no vma\n", alloc->pid, __func__); - ret =3D -ESRCH; - goto out; - } + if (!page) + return -ENOMEM; =20 - ret =3D vm_insert_page(vma, addr, page); + ret =3D binder_page_insert(alloc, addr, page); switch (ret) { case -EBUSY: /* @@ -288,12 +321,11 @@ static int binder_install_single_page(struct binder_a= lloc *alloc, */ ret =3D 0; __free_page(page); - npages =3D get_user_pages_remote(alloc->mm, addr, 1, 0, &page, NULL); - if (npages <=3D 0) { + page =3D binder_page_lookup(alloc->mm, addr); + if (!page) { pr_err("%d: failed to find page at offset %lx\n", alloc->pid, addr - alloc->vm_start); - ret =3D -ESRCH; - break; + return -ESRCH; } fallthrough; case 0: @@ -304,12 +336,9 @@ static int binder_install_single_page(struct binder_al= loc *alloc, __free_page(page); pr_err("%d: %s failed to insert page at offset %lx with %d\n", alloc->pid, __func__, addr - alloc->vm_start, ret); - ret =3D -ENOMEM; break; } - mmap_read_unlock(alloc->mm); -out: - mmput_async(alloc->mm); + return ret; } =20 --=20 2.47.0.199.ga7371fff76-goog