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 9740CC02198 for ; Wed, 5 Feb 2025 19:26:56 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 126F4280001; Wed, 5 Feb 2025 14:26:56 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 086436B0093; Wed, 5 Feb 2025 14:26:56 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E1B23280001; Wed, 5 Feb 2025 14:26:55 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id C160A6B0089 for ; Wed, 5 Feb 2025 14:26:55 -0500 (EST) Received: from smtpin16.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 6D46CA081E for ; Wed, 5 Feb 2025 19:26:55 +0000 (UTC) X-FDA: 83086873590.16.8E911AB Received: from mail-pj1-f49.google.com (mail-pj1-f49.google.com [209.85.216.49]) by imf14.hostedemail.com (Postfix) with ESMTP id 909D710000E for ; Wed, 5 Feb 2025 19:26:53 +0000 (UTC) Authentication-Results: imf14.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=qSDJSx91; spf=pass (imf14.hostedemail.com: domain of aliceryhl@google.com designates 209.85.216.49 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=1738783613; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to: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=MmbnpngJ3DL1qyen+QtnZGx2BFNC9m60wSqAfP+/agE=; b=irSA6JGZ6QnsvqaNXGRPBhvGhS2pzB/OJJWbdYnIkXrgGB2LU1sF3Vuo1IV90Grv621HYH VYGNkXPYeU0PmbkozcHIqSeO54DKiEDrHWmkeBQ27xbTw6FmrZxaJiPWURnR7L558NatlU h3kZABwhVjbluNZ8SubHmBBuPyDUD+w= ARC-Authentication-Results: i=1; imf14.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=qSDJSx91; spf=pass (imf14.hostedemail.com: domain of aliceryhl@google.com designates 209.85.216.49 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=1738783613; a=rsa-sha256; cv=none; b=LkfhSatqw3ohgIhESsghM25AXL9I61JVuq/jZhW/OjqO1v7Vb9pZabu21b9TT2Medx+eLZ uA2geX0oG1r6bYlJIT98SbabBxn+GrcpQq5ze21JqK4nwvuYwoWRCY5+ddoFBeK1sDeQMp tnLa/CgBGK6Vp4zavk4+sUlczufm0FE= Received: by mail-pj1-f49.google.com with SMTP id 98e67ed59e1d1-2f42992f608so97809a91.0 for ; Wed, 05 Feb 2025 11:26:53 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1738783612; x=1739388412; darn=kvack.org; h=content-transfer-encoding:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=MmbnpngJ3DL1qyen+QtnZGx2BFNC9m60wSqAfP+/agE=; b=qSDJSx910NKVcIdtKDuAOfzhiwCCOgVmpk9o8fSMrRLayTwUx7hL+i80oxli0E7lRO c1uJoscfSN6x+EwHWf4gXtso8XcTP/LGKeYnVUdElImJnMZ+Z1VHj7sdK+aSFA5QiNSY JSpZiZ+4ArCtJRvlFdjv0VvpZyFGujCU3gNjFkNDeaF/dHrRKx4df2nLZxsj5dcoQ1jn yRT7ACykhDLGewqsIw5gHykZzAQt0UsyyRX5GhYyIWCxDkzE5aHQBJCyrmBUzHZzrCD/ d1xA367ss4Xd1WiZdCm5TRHrj9muam+Eq+jpHBGHeaS3WdeQ6ZdbEfHQWUcClEolqeft kGaQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738783612; x=1739388412; h=content-transfer-encoding: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=MmbnpngJ3DL1qyen+QtnZGx2BFNC9m60wSqAfP+/agE=; b=gV8ifJRmDabohV5aZkiTAaBgK6Toq2MEyodmrBxQlcgt6u3vwoFOIytnvimK7Cxszs ibvgaJ29ydacbIxUXljkCc2wQt/nl3q6FRMBzDQcGBPfvXRz0kzqDlFmcinV1PFu4yXp IYSi8B5khAOd1v8nDE3q9O6JbI5/A1ybro9j5ZvSCpnAreJivZqk7krtFzEWbkygjnz7 Vow6GPip1MVqGFXC40rSlaz/R1F7cTIcCrWjChNxmfWC716ro3XuVyNJgb0IVCJJBHcu qYTHlvhKInnthNWt8nbMZI/xks9yaTwMoihuTen+V1Vzat2feknNudKlcc9Bmc++Zr2k dmBw== X-Forwarded-Encrypted: i=1; AJvYcCWgSTxcg+w5NFCoT8dDZ7KZJbCdOo2Ob+f7dXi+C/QU2Lvg1FijoHIiZC2sD96yMTIvci2HGvda7g==@kvack.org X-Gm-Message-State: AOJu0YzO5h9FjWMLHDd6x9mqOQCZg3kneO1TFn+4GUbsbX+O1/ABkFbn 5Fyxc6YmAcrnPE6rAnwiofzqS3cN2foJY9doUX71exRF3Gvq5JfFdKbRcv4oWh+6JhT8BKLh3vA rWDJdBxg0b66Abo9jVSMBNH7q3ALHnDCFdjkA X-Gm-Gg: ASbGnculNNHUhmzlH7JEJdnbn2/7z/ubKoHvfkldSSak+GmEKv+gRmsC7vv4jOvFK06 pYvF09yWf3B7hXyzIyPE04xwVDwLDSTYchmeLH0PHlQbyEG7UOLtI9H9naPvu6OBs79rfEvIzWQ == X-Google-Smtp-Source: AGHT+IHzVkEKXP6lO5RnqJmb9qi2tv+pxW8vj1o+pqT6K3FSm02zP+0yw9hTQ2smxMLPtNWgNGDZjFBTp2DOwaCNi30= X-Received: by 2002:a17:90b:3a4f:b0:2ee:8e75:4aeb with SMTP id 98e67ed59e1d1-2f9e07891d7mr6950893a91.17.1738783612153; Wed, 05 Feb 2025 11:26:52 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: Alice Ryhl Date: Wed, 5 Feb 2025 20:26:37 +0100 X-Gm-Features: AWEUYZnnZJe07H7TIydA2x3nzJT0dH0fV9QfxfWxxhay7a-Ui4HLr0prchezvEo Message-ID: Subject: Re: [PATCH v13 2/8] mm: rust: add vm_area_struct methods that require read access To: "Liam R. Howlett" , Alice Ryhl , Miguel Ojeda , Matthew Wilcox , Lorenzo Stoakes , Vlastimil Babka , John Hubbard , Andrew Morton , Greg Kroah-Hartman , Arnd Bergmann , Jann Horn , Suren Baghdasaryan , Alex Gaynor , Boqun Feng , Gary Guo , =?UTF-8?Q?Bj=C3=B6rn_Roy_Baron?= , Benno Lossin , Andreas Hindborg , Trevor Gross , linux-kernel@vger.kernel.org, linux-mm@kvack.org, rust-for-linux@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 909D710000E X-Stat-Signature: bd3szw19ctxny4upoktku4om5sxnpg4a X-Rspamd-Server: rspam08 X-Rspam-User: X-HE-Tag: 1738783613-159371 X-HE-Meta: U2FsdGVkX1+9q4DU9EHMHA1A2TBwBauBeELbXaqX0xHvzVe2MmCMck/uh4a9qelxG5WYi8q5QtT3B93xkl0SuuS+Z21m1vxLUrCD50T9ZzHNz9lrLSQGVgkGzDlmlwfaLVeH6UcmHvQuH0u3cj+119q0JekKmQa21ijiuqgS8qcuWfM+LzjSUsAzAlnG/7sPW2xEvVFJ+mW3P8uRMgPXuWDTDicEZ1qNGm+H+FP2/RZ+UGpPkm2fLs0mbrPt69IFQR77KmKy0FHa8wRkjwp4TM7OXUL+nycl4Y0Oz71ZE5ZgTp6++upt5rjetoD4BvcILtVrZ1xOzACnLNhCDOPnM5rOMWeXbNp7blv8RQ3jrWKEmxfTKpYf416zikKoJ/pxm2c17cQdmM3fwcEwGIxGmeghRVqvlwJrfRZHXrVX8nLy3phPrXd/hn+hjJKBkuauD6tXUiF90IMac4a+wqxmeayj3HhiVIOvx0x9ztLtmQUuMBGB1A2u5sVailqIUsKwKvrMk9OrSBcycPWwsFoKawhB7Q3B7+VqqnQey3PPMawYiQTeimDU4pBJEBNn9vdMTEPPlFjRcFWg21WvEIgzRCIuK0k66QgUwnugMtkmvrw4DoglwZoADflJf93J1yzXy8x3nb2PtBFDns26+gRSZEfjykK/RK85ov69Ia31j0h7SPwKGZD2D5O0ZczfsAr8GN70Qy9K0yrSGQS9RrEEjarJ1ATSLQKUZzRqSNv2nCDAb0T6Lm19//RPGYDcw1yGZyCBUKc19k/SHdZLQLHJwc1hgSR578SAbGiImHzsgdG0WIzxVyruXNMoQ+i205deg9q8g9HFaSk7Mf8rQpmvpoAr6rcSEBdYFRGSR7b0RVv/uNYtdOnnQGquUxTqAzebKqdDVkJDRcpCI3ae5WAAUKqkNaldLJhy9j3MnSFro+24kXk40GTUL0jm2ChXb8cNsjzQL+W8nerT/SNIf0F vaOe5Am8 SlyaStu2/BrJ69OZVTzp96iXJUqDN52lhURR9MnGc8/9nmYyrm8q+3Fy3l0qGdleb5ks0NX6hvMpQ8XLa59iUel/foy0pvZimtpIaXVsU/QQcgBs6dTMGGiEfGNeLv6r8tNA6hPf8HrM+m2uojt30zg1oimLLcjaD5XzO8v7y5Y8k/WGAU0ENBw0viRVasMz5taszdnkTRvBfs3+xZI72K4ydKnvicUsxJTFFck+UU0xm5RP0FM2rEZ2RCkNic3Z2oKe5vMBlg8sjgVzZ8OOqcAa2tzpt7rJD+blv3bViz+E3jzyL1+qJOO42LuF/KGhIMdq0+Q6EdYucunjmuMJ79LO8IEzzF4YwK53AFFLlETOIoGs= X-Bogosity: Unsure, tests=bogofilter, spamicity=0.500000, 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, Feb 5, 2025 at 8:22=E2=80=AFPM Liam R. Howlett wrote: > > * Alice Ryhl [250205 14:13]: > > On Wed, Feb 5, 2025 at 8:10=E2=80=AFPM Liam R. Howlett wrote: > > > > > > * Alice Ryhl [250205 10:24]: > > > > On Wed, Feb 5, 2025 at 3:38=E2=80=AFPM Liam R. Howlett wrote: > > > > > > > > > > * Alice Ryhl [250205 07:10]: > > > > > > On Tue, Feb 4, 2025 at 4:46=E2=80=AFPM Liam R. Howlett wrote: > > > > > > > > > > > > + let vma =3D unsafe { bindings::vma_lookup(= self.mm.as_raw(), vma_addr) }; > > > > > > > > > > > > + > > > > > > > > > > > > + if vma.is_null() { > > > > > > > > > > > > + None > > > > > > > > > > > > + } else { > > > > > > > > > > > > + // SAFETY: We just checked that a vma = was found, so the pointer is valid. Furthermore, > > > > > > > > > > > > + // the returned area will borrow from = this read lock guard, so it can only be used > > > > > > > > > > > > + // while the mmap read lock is still h= eld. > > > > > > > > > > > > > > > > > > > > > > So We have complicated the locking of the vmas with r= cu and per-vma > > > > > > > > > > > locking recently. We are now able to look up and use= a vma under the > > > > > > > > > > > rcu read lock. Does this translate to rust model? > > > > > > > > > > > > > > > > > > > > > > I believe this is true in recent version of binder as= well? > > > > > > > > > > > > > > > > > > > > Yes. The safety requirements of VmAreaRef is that you m= ust hold the > > > > > > > > > > mmap read lock *or* the vma read lock while you have a = VmAreaRef > > > > > > > > > > reference. This particular method achieves that require= ment by holding > > > > > > > > > > the mmap read lock. But there is also a Rust lock_vma_u= nder_rcu(), see > > > > > > > > > > patch 4 for that. > > > > > > > > > > > > > > > > > > Right, okay. Thanks. You can get the reference by only = holding the rcu > > > > > > > > > read lock, but you should hold the vma lock to ensure tha= t the vma > > > > > > > > > itself (and not just the pointer) is safe to use. > > > > > > > > > > > > > > > > Hmm... To modify the vma, you must hold the mmap *and* vma = write lock, > > > > > > > > so holding the mmap read lock prevents mutations? > > > > > > > > > > > > > > Sorry, I think I confused things with my answer. Your code i= s fine. > > > > > > > The phrasing of the "only be used while the mmap read lock is= still > > > > > > > held" made me wonder about further clarification on the locki= ng here > > > > > > > (because the locking is confusing). > > > > > > > > > > > > > > Yes, mmap read lock means there are no writers that can modif= y the vma. > > > > > > > Essentially, you are using the lock to ensure the entire vma = space isn't > > > > > > > changed during your operation - which is heavier than just lo= cking one > > > > > > > vma. > > > > > > > > > > > > I could extend the safety comment like this: > > > > > > > > > > > > SAFETY: We just checked that a vma was found, so the pointer is= valid. > > > > > > Furthermore, the returned area will borrow from this read lock = guard, > > > > > > so it can only be used while the mmap read lock is still held. = This > > > > > > ensures that there are no writers because writers must hold bot= h the > > > > > > mmap and vma write lock. > > > > > > > > > > How about just changing the last part to: > > > > > > > > > > Furthermore, the returned vma is still under the protection of th= e read > > > > > lock guard and can be used while the mmap read lock is still held= . > > > > > > > > Well, the important part here is that you can't do this: > > > > > > > > let guard =3D mm.mmap_read_lock(); > > > > let vma =3D guard.vma_lookup(...)?; > > > > drop(guard); > > > > vma.foo(); > > > > > > > > since that would use the vma after the lock has been released. The > > > > reason that the above is prevented is because `vma` borrows from > > > > `guard`, so you can only use `vma` while `guard` is still valid. > > > > > > > > > > But it implies that this isn't valid: > > > > > > let guard =3D mm.mmap_read_lock(); > > > let vma =3D guard.vma_lookup(...)?; > > > > > > vma_lock(vma); > > > > > > drop(guard); > > > vma.foo(); > > > > > > vma_unlock(vma); > > > > > > See mm/userfaultfd.c:uffd_lock_vma(), which falls back to mmap read l= ock > > > to do this if rcu lock + lock_vma_under_rcu() is unable to lock the v= ma. > > > > This patchset does not have the functionality for doing that, but it's > > definitely possible to add. > > > > I don't think that's necessary right now. It's just that I read that > comment and it seemed to imply something that isn't strictly true with > the "only valid" part. I think? Is rust doing something that makes it > true? The code would look like: let guard =3D mm.mmap_read_lock(); let vma =3D guard.vma_lookup(...)?; let vma_guard =3D vma.vma_lock(); drop(guard); // this is mmap_read_unlock() vma_guard.foo(); drop(vma_guard); // this is vma_unlock() So you couldn't use vma after drop(guard), but vma_guard would be usable. (And of course the drop calls also happen automatically at the end of the scope if you do not drop it explicitly.) Alice