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 54F11C02196 for ; Tue, 4 Feb 2025 15:06:04 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DF31C6B0088; Tue, 4 Feb 2025 10:06:03 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id DA2DE6B008A; Tue, 4 Feb 2025 10:06:03 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C6B216B0089; Tue, 4 Feb 2025 10:06:03 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 4CFF4280001 for ; Tue, 4 Feb 2025 10:06:03 -0500 (EST) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 62026C09E3 for ; Tue, 4 Feb 2025 15:06:02 +0000 (UTC) X-FDA: 83082587364.04.28B515F Received: from mail-wm1-f44.google.com (mail-wm1-f44.google.com [209.85.128.44]) by imf13.hostedemail.com (Postfix) with ESMTP id 6A68A20026 for ; Tue, 4 Feb 2025 15:05:56 +0000 (UTC) Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=ODb3iwwm; spf=pass (imf13.hostedemail.com: domain of aliceryhl@google.com designates 209.85.128.44 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=1738681556; 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=6uoHeoAH7U5NCxnZILdDHxMpNV/SysSvNvz/L9W1pYA=; b=cXW60ZbltnON1cgQZcNAqQs7pbYJ7pGknsGk4QWLXnIDHQ0XQOIQOuM3/7QI9OKRZBPFZv Pebb0P7kB66f+mXn/Bp7g4DG5rR6+Giedr2X3ivGIJa/4fVLoMCDX5SdryVI9q6kn9Kbdk IERNUxC5NWvezJ9xMU/LRdIgOW5jPt4= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1738681556; a=rsa-sha256; cv=none; b=hNRTQr55xrjMZjrZqPM8+JI7So9mjlP3C2pnNO4KiOpCbiy6+/2CLj/wTj3hepmE3mnvwt qsqlaBXNwK32i+hZwQEQGc1b7/TUl7Q7lGxzMJ6tNMDhQEMtgQgCFIsfEz0X1WvFLTcy0X CUU1Hx1yZ9l785s8mvFy0BtSq4EPZ5c= ARC-Authentication-Results: i=1; imf13.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=ODb3iwwm; spf=pass (imf13.hostedemail.com: domain of aliceryhl@google.com designates 209.85.128.44 as permitted sender) smtp.mailfrom=aliceryhl@google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-wm1-f44.google.com with SMTP id 5b1f17b1804b1-438a3216fc2so56994245e9.1 for ; Tue, 04 Feb 2025 07:05:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1738681554; x=1739286354; 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=6uoHeoAH7U5NCxnZILdDHxMpNV/SysSvNvz/L9W1pYA=; b=ODb3iwwmTBwtWI+XyAKwWX5cxCF4I7ut0A+N00oMnPJy2s/0rtWngHxUyMXV2FKzOi GFabmG3cZSFA/u8Iu2fozK5q+duxHETwP4XYEQhwIeU07XlstsjR6EnM7DQZot6osCRY H0uQqJkEn2conXSjyI+v5W0BPcsYuTZlxYGhSGHdqdmmT9kYcymMsxbzwVjUU6ScxF2Q PDDHTe28uifqqE+fZyz2tA9Z88PWaRF3lgEqQ9gJGFiJqKCNxigW8diFMbR7sEP+541g naTMX1dCVdtgePIarCU8R5wxQc//dMRgrfrGIy5VhX6VlTD8JuxcOHeLA+NX60/Sihr6 NfBw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738681554; x=1739286354; 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=6uoHeoAH7U5NCxnZILdDHxMpNV/SysSvNvz/L9W1pYA=; b=MUAWSf6jdf3ZVdYOsYbWdqWowbgZt5KzdvV89zgAKk35QFQDxpws316SuBTc2yHckg hClo9vjy0ELOhj8TFFCdrVxAbVZX5ApmtdiO5u+oYP5ZXa8AD6jOVmcYq4Qe4DgCmclw +UCp0g62n/QwBO2nTjhhiZs3Nl479OAQnjpMxCLCwbEB0WwzH+HR4n7qr7dISs+euMSZ UEieZSqXSQY1necPFarDMDVfa74PQcxywdT7FeloJ/jfiv4JvUOYthbWEPfQGclSnSb/ +wYrgDhHRxLnyo8F3toQUBpzagMgok864acUuqdVy2ndLrYGk1/fD75lg3W8Lt0VhwkH 0gTA== X-Forwarded-Encrypted: i=1; AJvYcCWwAxM6xD4kF/rJU2upt05cfAgtXDa8GGzq9JYaRlwEIagdyVOIF/Poh6PJW+FApRxQ97rgmXnmgg==@kvack.org X-Gm-Message-State: AOJu0YxWrSMUhjA6q1L52R+MEpNiqW6+RFeuLNqsQ0dd1kue7ew7QgpF K/IKAcocqqogf2ITNwQAMjiqDmegOX+f47664lhvsnvogm/zAhvn1Ut17GEVxqUcY0fIGR9qiJB vsFatjUfJ2HGnLVjfJUb45iqRUbg6No/wYZoA X-Gm-Gg: ASbGncvnLg8ewyEAZo0WTVQCnuMwjnpV11wiY8Mvu/j+vYfoZjltbO4dkt1X3wbz3Pa He55qJyGklBB67z0gzp5bl9bql4zIbd97FgsaDfodx7l6p/FHfPmR9OlvOuJFNb62eZMWtkAif6 bhn9Dx7/MFuiI3VenkplNJmkW87w== X-Google-Smtp-Source: AGHT+IFCNjH/V9P/Lg+ufbLepR71wOHfKTGoMwPDfuParFiZ8STiGxTr/Z3uh2hphaB4DIojQ500u5f0LwM9KxR09+Q= X-Received: by 2002:a05:600c:1c1e:b0:432:7c08:d0ff with SMTP id 5b1f17b1804b1-438dc40b8e6mr210901225e9.23.1738681553649; Tue, 04 Feb 2025 07:05:53 -0800 (PST) MIME-Version: 1.0 References: <20250203-vma-v13-0-2b998268a396@google.com> <20250203-vma-v13-2-2b998268a396@google.com> In-Reply-To: From: Alice Ryhl Date: Tue, 4 Feb 2025 16:05:41 +0100 X-Gm-Features: AWEUYZkL94U6cCe6csQSLh-rnC1NVTtoJ_Iv8CKpRx6GMPpdxyIltoAvX5ckcwA 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: 6A68A20026 X-Stat-Signature: hn7tpsmjhmtryi5tneydjsp8rw8e1mun X-Rspam-User: X-Rspamd-Server: rspam12 X-HE-Tag: 1738681556-210084 X-HE-Meta: U2FsdGVkX1/qLQFi2dI0btqaMI8UVTIXVaL0UC2oHiffnEKtK/F5qC7fKLphyeJttLV41pNKkdcQp4hGuY/os2OFo2bh+kvf7POPDuKettXDKnIhaNOe0KQdGLX2qQDhrlKfHyDDK8YrPb/mEXTMdcLJJ+wNjaQl2uVVD6XzNTzAlNeeLX9CdKDF0MY7TXL/Ayj0IZGkW2G43Hx1JiLscWF8LWousYVVOjVxCtpa94s51eWk1BRUvpxKeIDpFh/8Ky4A3RBydIEpZbI+y0tpYzHHfbRZueL0qTj8LFziAcil1MbMAZx/Nvb1dFUeIUUnifQl4JlKkgnCUCehDWRf4zZ5nEGpT3ehuVf6wrG+GQ31zVITHKZcf0ZQRCQKe1CprSxPpGmgQuchiWupLpnMUI52cyKbY3v/snoOlKZZUXILBXCj3ijYysSOfxME+ic1mNXaVGnhV+cc8eEGX18Osp6XIlUJpqWQIprnjNqHQp2ok04ht7s6oLvDClraalNG7yaU5JRJs+y1SlxPvOJKG78s84kfNBttMHXb8cZrsql08cDp+lwcQteeJBdE+Tihmi2TVCdt9I2VFxoYbdUT/F8t9yMqj1SjGpeFMfU7oVJfaJQBYVma3kRyX5v1EKrJujVipI5iD8UIZIZWLyKeL0wfoLgKdB4bWe6bPsS0yyOuo9ee/p+CHgGbIqlZsouHWtyU2mRqeaZ5XUoVS7qlVufJcEn1sDtC/gje8eP82QLzjTN/bK9G0qoSUhuX2zCjPQNG07FJoINwq9elCksqhftNKHyxstyNeEAUNxI5RWqy5ThG6JoEiY1HqM0k32w51DyGrLgpsl6N/pdZijzicfjidukN062Y7PVZiqK0B8C2iiMZS8bduopQTJpb70jLfQqIKWNI+rgIQcm9DrOvVUsRXmgfWDBzt72C90SeswMFZJuVq7SEYifRHIbdaGFCQp/i+J/jX5kkfuKBoTN DkLDnWAJ zFEt41jZSHcHUouwVqBFAp3nY3Y91ZFvOcMZXFUWrDvA8AapSpJQF8aOD4AXwifThuz18MQ3XG8q4sLggs7kQNb2jQNe+jpiX1Rnu4fdl5E7b8+RTCQZ6J1KlQdLq7ccEpqldm8MltegN6CdKbeiXRpUhii28nr8OlOOyTnZ88ubjWKnyVNIp5vZ64CbB9q1Fw6X89MwsYVo1QZc+H8tlpMKXcMCZ7mEi42SyRe3g8JKPnwHu9Aw0oBTTFv7Bgmdyk7/r4hW5vSzGu0QE3DXS0Bxc2xfghxwaEG0IifAxeLhTsZAzCKHOXIdRvJc+1g/C2EkX14yNuCx8mVZvIHn3WB4ju21D73OjsKDmUfy+1BoK4zU= X-Bogosity: Unsure, tests=bogofilter, spamicity=0.499943, 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 Tue, Feb 4, 2025 at 3:55=E2=80=AFPM Liam R. Howlett wrote: > > * Alice Ryhl [250204 07:45]: > > On Mon, Feb 3, 2025 at 4:44=E2=80=AFPM Liam R. Howlett wrote: > > > > ... > > > > > > > > > +impl<'a> MmapReadGuard<'a> { > > > > + /// Look up a vma at the given address. > > > > + #[inline] > > > > + pub fn vma_lookup(&self, vma_addr: usize) -> Option<&virt::VmA= reaRef> { > > > > + // SAFETY: We hold a reference to the mm, so the pointer m= ust be valid. Any value is okay > > > > + // for `vma_addr`. > > > > > > Is this true? In C we hold a reference to the mm and the vma can sti= ll > > > go away. We get safety from the locking on the C side. > > > > Notice that this function is in the `impl MmapReadGuard` block. This > > implies that you *must* hold the mmap read guard to call this > > function. > > > > The safety comment should probably be updated to mention that we hold t= he guard. > > Thanks. > > Does it imply you must hold the lock or does it enforce that the lock is > held? Not sure what distinction you're asking about. To obtain a MmapReadGuard object, you need to call the `mmap_read_lock` function, which locks the mmap lock and returns an MmapReadGuard. When the MmapReadGuard object goes out of scope, its destructor runs which unlocks the lock. You can only call this method given a reference to a MmapReadGuard, so you can only call it after calling mmap_read_lock and before its destructor runs. > > > > + 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 th= e pointer is valid. Furthermore, > > > > + // the returned area will borrow from this read lock g= uard, so it can only be used > > > > + // while the mmap read lock is still held. > > > > > > So We have complicated the locking of the vmas with rcu 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 must hold the > > mmap read lock *or* the vma read lock while you have a VmAreaRef > > reference. This particular method achieves that requirement by holding > > the mmap read lock. But there is also a Rust lock_vma_under_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 that 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? Is the mmap read lock not enough to read from the vma? > > > > + // SAFETY: The caller ensures that the invariants are sati= sfied for the duration of 'a. > > > > + unsafe { &*vma.cast() } > > > > + } > > > > + > > > > + /// Returns a raw pointer to this area. > > > > + #[inline] > > > > + pub fn as_ptr(&self) -> *mut bindings::vm_area_struct { > > > > + self.vma.get() > > > > + } > > > > + > > > > + /// Access the underlying `mm_struct`. > > > > + #[inline] > > > > + pub fn mm(&self) -> &MmWithUser { > > > > + // SAFETY: By the type invariants, this `vm_area_struct` i= s valid and we hold the mmap/vma > > > > + // read lock or stronger. This implies that the underlying= mm has a non-zero value of > > > > + // `mm_users`. > > > > > > Again, I'm not sure this statement still holds as it once did? > > > > If it's possible to hold the mmap/vma read lock for a vma that is part > > of an mm whose mm_users count has dropped to zero, then it is > > incorrect for us to have this method. > > The non-zero statement is fine. After re-reading it, I think this is > accurate. Okay, thanks! > Thanks for taking the time to explain things. Of course! Alice