From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id A32D5D6ED01 for ; Thu, 21 Nov 2024 10:45:08 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 28B866B0083; Thu, 21 Nov 2024 05:45:08 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 214486B0088; Thu, 21 Nov 2024 05:45:08 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 08E4F6B0089; Thu, 21 Nov 2024 05:45:08 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id D8DA26B0083 for ; Thu, 21 Nov 2024 05:45:07 -0500 (EST) Received: from smtpin25.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 89A7A120B01 for ; Thu, 21 Nov 2024 10:45:07 +0000 (UTC) X-FDA: 82809767838.25.6F4A76E Received: from mail-wr1-f51.google.com (mail-wr1-f51.google.com [209.85.221.51]) by imf09.hostedemail.com (Postfix) with ESMTP id 4A7C514000A for ; Thu, 21 Nov 2024 10:44:29 +0000 (UTC) Authentication-Results: imf09.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=pPMLfjGK; spf=pass (imf09.hostedemail.com: domain of aliceryhl@google.com designates 209.85.221.51 as permitted sender) smtp.mailfrom=aliceryhl@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1732185701; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=xK4Cfg9DlwufnPePVmxDaoaAvTYsonxko5dLjDtr1k8=; b=WaN/mtJCIN/2ZV6x+F/+H45BPpBjHRgg5SpFXrWzUPmsNTKdfr7E+Fb7lMtrdhnQKDfaMv sjVt84rziiB7GcrVmCLK6b2Eh2txlSLcJV710+8CyZmkyLqZUNWWMCeHmHuITgYpJsQaTq LRRHHl+gqEqfRTwNy3gWatqJed7PkyE= ARC-Authentication-Results: i=1; imf09.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=pPMLfjGK; spf=pass (imf09.hostedemail.com: domain of aliceryhl@google.com designates 209.85.221.51 as permitted sender) smtp.mailfrom=aliceryhl@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1732185701; a=rsa-sha256; cv=none; b=OJ6HOpQ3AHFuXWWP0Pd7OwwBEpVf4psE2m/wSmMLGNHh5TIXaUk3ts3w8VhGAlUPdIErN9 9D1eKp62unypZYm4oDuLlg+5/sYoFWbLSe3N+unBX7K8kqZj2z7L8PRVRBaRZagv/5GMvx NfWFNlrlmH23b9E2vrEEWriiEdYZMA0= Received: by mail-wr1-f51.google.com with SMTP id ffacd0b85a97d-3824a8a5c56so492333f8f.3 for ; Thu, 21 Nov 2024 02:45:05 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1732185904; x=1732790704; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=xK4Cfg9DlwufnPePVmxDaoaAvTYsonxko5dLjDtr1k8=; b=pPMLfjGK+2+x9uLORksYm2QeRCAvt+R/d1PpJOXRkdryWsb/zanvY4IOMyFdPmf8oC zTVKStHquMt0aIsgfdohhUA3Izrkt/VmNM2NoM5qpi+tINByZYzYJ+uWJgM7iVw+P+LG ZvKjIbWhnshwJs0+zMLan5WOLxC7l+1fo2DQ2XY3WkJz/k3xJqFFCBMAByqBEE6Y8UAs EDcxCJ60kZhILrpaZry0WPjtxlZ2LBZOAZJv39ZTjW85t3MIYe7xf11kFFuJjx0J2xtl N/No7QQfJgDzdNTJ6+4yQd3qCKmV2DnuyOZDFd11cyftIdrdrU/BkimSlmy8eQUEqlFh 9O5g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1732185904; x=1732790704; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=xK4Cfg9DlwufnPePVmxDaoaAvTYsonxko5dLjDtr1k8=; b=whVwZrtUtJ6nQhQ3f+ebi5RhGt7jpKb3t9WKpUjmwJBDWLz1DVXtgWbwJymmX73tBn 0uGCHt1G6GsQEyDWbv1hWNnG7V2DLg0ue2DMu3EHNdxOYideK5N1tCGBevmOjtps7R+0 vAmZtZVq3GGFs6xbL7lauHPfRM/Rr7El4Hk4NZXP8yDis+gOh7ycvcJBn8VItfTVXK+H rNOKYSAE8TgFNZaJqXd80qZW0Q55JyqmVcjy3pQt50Dx9mqiyQqSCkvQqpPnbEbZvCoM kZ5trRyzNQOjJiA5BATb1yB3Vrz3b7EhBzZVnQzjcOCNh3L64LI48cCU3ZqlRWtBLTGl Nv5Q== X-Forwarded-Encrypted: i=1; AJvYcCUP3cN+RW2LSbPWkXN/lag3PfIOj0C81VYxVHhU3+e9ba1pU/a7WKHGnjyl0bRGK0y0o28lmNTC4Q==@kvack.org X-Gm-Message-State: AOJu0YyMo90IDFRa+lETu53vTJkxD3sMfqBEYbht5NTt7hL44Udfvd7R eU+KBEijtnCC5YjX17gHYQfUgSpyNclVcHdvVtIE8qNP9xrWZY2n8GzVEvh/M1oDx67DsROlE8f ZUQfYRo3SJNmtnqRWRGFI/fqiMl+tW4rxPLXW X-Gm-Gg: ASbGncvXGg8NTxwwy9SrnFnwUK/jHFx8NXJ4ZnNMEZBbssfBY6jffPEzJsFnPpF09OU bZnuI7HULtvRvEM2jOj2pQOf2YZa5rTrbWek3pyHO+WvVaIvYuqpZ+JcGVlAqPg== X-Google-Smtp-Source: AGHT+IFNLFXfFTPNcf0Jy6eL0FhPoWe5bssVRl6BmWr16Rlaom5gN2QQBv1a2lad6azAjMDYL8YEC8lFmavr2ot49Tw= X-Received: by 2002:a5d:5f86:0:b0:382:5393:43cf with SMTP id ffacd0b85a97d-38254af6394mr4794445f8f.13.1732185904176; Thu, 21 Nov 2024 02:45:04 -0800 (PST) MIME-Version: 1.0 References: <20241120-vma-v8-0-eb31425da66b@google.com> <20241120-vma-v8-4-eb31425da66b@google.com> <40be19b0-4c32-4554-a01f-649c12f889da@lucifer.local> In-Reply-To: <40be19b0-4c32-4554-a01f-649c12f889da@lucifer.local> From: Alice Ryhl Date: Thu, 21 Nov 2024 11:44:52 +0100 Message-ID: Subject: Re: [PATCH v8 4/7] mm: rust: add lock_vma_under_rcu To: Lorenzo Stoakes Cc: Miguel Ojeda , Matthew Wilcox , Vlastimil Babka , John Hubbard , "Liam R. Howlett" , Andrew Morton , Greg Kroah-Hartman , Arnd Bergmann , Christian Brauner , Alex Gaynor , Boqun Feng , Gary Guo , =?UTF-8?Q?Bj=C3=B6rn_Roy_Baron?= , Benno Lossin , linux-kernel@vger.kernel.org, linux-mm@kvack.org, rust-for-linux@vger.kernel.org, Andreas Hindborg Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 4A7C514000A X-Stat-Signature: 85kr1639eo9b7wdko71kg9go6cy5wnmf X-Rspam-User: X-HE-Tag: 1732185869-937541 X-HE-Meta: U2FsdGVkX1+bLrtY9QRAs13hJvgo2iX6ZfxjAo0G0EQ5ZNEAL4iRiAH3vnyvWi+iZbAxBEInMQWilOCLhAdnOj2bIJr5e07PIUkxZ6k268lESETleeKInNOnSTBbIxrzfm3XwHQJkiCXB8buqqVAFHQWvYV5dY92Fbff0d1YnClth0HuDRsdzKSKofWoDC4mWKMFVYpeO2VWoAQajsOxy+5WnHoCgUZ+tNSPmOv5eQYCxUso9xMHs6uNpUeSQ2+s6Z01GLJkLoKygu2+V6D2uzet4kc3k+/OUM7UZtcktzFqi6egCteeGiC0ujDxtxOSURLoHu9BXVfdlSnUC64EMfGRrCx5ZDluAlZ1v7NIb9W97B+i63LXjK4g6qGWyAHpdcZsFvjD1ik246n0ZiETNqpXmkyBLeIRiduPEEe8pq3vDM5yB24m9v7eeai/QrSQKgkUtnTLgGp7xl1tMhoXHNLDrGW39kaCNYWmM76Uy3PuRA/Id5tgZqa0nyi5WSgjwFUXuw6nHvwvI2iB4oFVRPtdBeAVlPWhP7blpxPcWDeAXhsBwJ9XJ/0/whIgYJX2vno0uw4YVOQmzwX3z4CBqOsXi/7HMnUA2j2XF1nkCjqEU0DuTydeyJlg/hO9qWsT5SYKCKspMPCqLTqyQkY/kidqKDFVWyTacA/Q5KF4ZjqZbAUdtmRZY30PzuhUi4qXV/mR0g0rLB4nrG+kfH8hB3R8LBS7WNjwvDURl0/4tmZeJ5YiCRcSEwIhCty7ImsFYgXebCV+hQ8caISGgjEbO4drwqkOOXDnEKxpKMBzrE1taNYzopOuhB/3/Dbci3XleqQxvhBGnQame5yATYI/RUFIOOUwxHzt5qPP4tKRKGpDsLjHtt62+5loIX98wo2syKpvB5ajoR2Mgxi+5Id5g/LSx/v7CxNu4AHXM9YjohyqG2jUGNJnfdlSgoIPzLgj2WRA8uRleSPavULHNfC YEkPjmue HQfkbJ4oLgDuat2Ra/LKFkWbeAjL7PhFvjtacRbh2SJ/HLYljq7erlEF20oEvUmpw3KZb+OmjfmCFDD1xmQxy5d2OfeV+bhfPqXtv6MUc0l+uZ0o5/5yfH3wXEYdjVbA1cCmhSAGveNEPHSx9YyAmNJ63BmSM4k5TmGf2v1udQ2j3MbZ5EsNDMPmvabC6gl0WngQJHdYtkLGgyD7xE9I01IHQea757ndChfVZgHN/kHkMGVqogM5HSg6OgkynUPvE/E+XmidZdiBaDv+dyh7u0KX6Uyf7SwPbfVDxNW5ro+7TJqLLca0n99q8T3KWy40qrIbKK/vGNt6wx8dHtFiVCqyuWOqU85QReY/nz2vIJi0xVXg= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000001, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Wed, Nov 20, 2024 at 8:29=E2=80=AFPM Lorenzo Stoakes wrote: > > On Wed, Nov 20, 2024 at 02:49:58PM +0000, Alice Ryhl wrote: > > All of Rust Binder's existing calls to `vm_insert_page` could be > > optimized to first attempt to use `lock_vma_under_rcu`. This patch > > provides an abstraction to enable that. > > I think there should be a blurb about what the VMA locks are, how they av= oid > contention on the mmap read lock etc. before talking about a use case (th= ough > it's useful to mention the motivating reason!) > > > > > Signed-off-by: Alice Ryhl > > --- > > rust/helpers/mm.c | 5 +++++ > > rust/kernel/mm.rs | 56 +++++++++++++++++++++++++++++++++++++++++++++++= ++++++++ > > 2 files changed, 61 insertions(+) > > > > diff --git a/rust/helpers/mm.c b/rust/helpers/mm.c > > index 7b72eb065a3e..81b510c96fd2 100644 > > --- a/rust/helpers/mm.c > > +++ b/rust/helpers/mm.c > > @@ -43,3 +43,8 @@ struct vm_area_struct *rust_helper_vma_lookup(struct = mm_struct *mm, > > { > > return vma_lookup(mm, addr); > > } > > + > > +void rust_helper_vma_end_read(struct vm_area_struct *vma) > > +{ > > + vma_end_read(vma); > > +} > > diff --git a/rust/kernel/mm.rs b/rust/kernel/mm.rs > > index ace8e7d57afe..a15acb546f68 100644 > > --- a/rust/kernel/mm.rs > > +++ b/rust/kernel/mm.rs > > @@ -13,6 +13,7 @@ > > use core::{ops::Deref, ptr::NonNull}; > > > > pub mod virt; > > +use virt::VmAreaRef; > > > > /// A wrapper for the kernel's `struct mm_struct`. > > /// > > @@ -170,6 +171,32 @@ pub unsafe fn from_raw<'a>(ptr: *const bindings::m= m_struct) -> &'a MmWithUser { > > unsafe { &*ptr.cast() } > > } > > > > + /// Try to lock the vma read lock under rcu. > > This reads oddly, I'd say 'try to acquire the VMA read lock'. It's not re= ally > necessary to mention RCU here I'd say, as while lock_vma_under_rcu() acqu= ires > the RCU lock in order to try to get the VMA read lock, it releases it aft= erwards > and you hold the VMA read luck until you are done with it and don't need = to hold > an RCU lock. > > A reader might otherwise be confused and think an RCU read lock is requir= ed to > be held throughout too which isn't the case (this is maybe a critique of = the > name of the function too, sorry Suren :P). > > > + /// If this operation fails, the vma may still exist. In that case= , you should take the mmap > > + /// read lock and try to use `vma_lookup` instead. > > This also reads oddly, you're more likely (assuming you are not arbitrari= ly > trying to acquire a lock on an address likely to be unmapped soon) to hav= e > failed due to lock contention. > > So I'd say 'this is an optimistic try lock operation, so it may fail, in = which > case you should fall back to taking the mmap read lock'. > > I'm not sure it's necessary to reference vma_lookup() either, especially = as in > future versions of this code we might want to use a VMA iterator instead. Thanks for the doc suggestions, they sound great. > > + /// > > + /// When per-vma locks are disabled, this always returns `None`. > > + #[inline] > > + pub fn lock_vma_under_rcu(&self, vma_addr: usize) -> Option> { > > Ah I love having lock guards available... Something I miss from C++ :>) I've heard that C is starting to get lock guards recently! > > + #[cfg(CONFIG_PER_VMA_LOCK)] > > Ah interesting, so we have an abstraction for kernel config operations! Yeah, it's basically an #ifdef, but the block must still parse even if the config is disabled. Alice