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 5BF3DC54E58 for ; Thu, 21 Mar 2024 13:43:08 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E2D986B0082; Thu, 21 Mar 2024 09:43:07 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id DDDA06B0083; Thu, 21 Mar 2024 09:43:07 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id CCC856B0088; Thu, 21 Mar 2024 09:43:07 -0400 (EDT) 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 BDEA46B0082 for ; Thu, 21 Mar 2024 09:43:07 -0400 (EDT) Received: from smtpin16.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 6DD401C1711 for ; Thu, 21 Mar 2024 13:43:07 +0000 (UTC) X-FDA: 81921162414.16.E258AE3 Received: from mail-vs1-f48.google.com (mail-vs1-f48.google.com [209.85.217.48]) by imf08.hostedemail.com (Postfix) with ESMTP id C1B73160016 for ; Thu, 21 Mar 2024 13:43:04 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=bawmZqAr; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf08.hostedemail.com: domain of aliceryhl@google.com designates 209.85.217.48 as permitted sender) smtp.mailfrom=aliceryhl@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1711028584; 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=2vO+ESJOepqx5jEo9fnNt4JlqMEXC9syS7LVjRc4fMA=; b=DHn1iQ80/KaqOBxKv8upx6fHO2AodFWq++A9gN6x06CscPWp9zRYDgvgnXHn5S/d5mxlWA fU2u777WeC7H0JtPCrk48ApLjZApDUF33scszATK/71qtObhSMrWrJC+gqwMQ1C16PoRgp xaVTg2S3GeQJEYDOMQi3awFT/A5yBE0= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=bawmZqAr; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf08.hostedemail.com: domain of aliceryhl@google.com designates 209.85.217.48 as permitted sender) smtp.mailfrom=aliceryhl@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1711028584; a=rsa-sha256; cv=none; b=ngeoa0qQdUd9sBX+Ycf3TW8pO3AfDZ7QeYuGY0J8M6QzS67yWrX2ulLLIIL5Kv8xit+FoF jjG0kjuaCEmhZ2lyHF2oF3x4vCOLKkp43u0mOJtV6lN1Sh6RPQ63JK7slIsRcLOGGTcne8 css8U10SfgQYcQDq631I+WSk5W3pODQ= Received: by mail-vs1-f48.google.com with SMTP id ada2fe7eead31-4767347552bso640746137.0 for ; Thu, 21 Mar 2024 06:43:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1711028584; x=1711633384; 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=2vO+ESJOepqx5jEo9fnNt4JlqMEXC9syS7LVjRc4fMA=; b=bawmZqArpJKrec8HMdXpkrZdybQV8w4IS0/81Ii12zWyMVghsXaZ6sfMbVMhfcam2B WJ7QQ3+KAQBIeyPiVUzVJKVRgEhZBhyGGpRW9Ld20zLb5Vu440b6frdjSpxfBN1+gnvk gaPMKJ4RRMlhwRzICM4g3M+GwTSe0fZ1EhXTGDmGCfWOawhAGYJmfaRUBgTl4KOv4zYt l29wzcGxilh+9VZgQ9NCG4SKzcCexxEFvCaRKCk7wqClvuF/2cAzOHVuOdAxTdSw9ic9 Z8dfKKVMDdJaaOgLujF8Gboro+OrtdyXLSvNEJw5uBst2D8j9I/C8hgHgaqAmnNCQH6I fabQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1711028584; x=1711633384; 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=2vO+ESJOepqx5jEo9fnNt4JlqMEXC9syS7LVjRc4fMA=; b=E3sRqFAs407m1gebl8p1XvEpLJperQwcsecwV95PEaR7HPT1AoqtTHCuvfXJxYYMFG IjcZGGpFjYz+igI1gWN4VSN3fEQPGbfmzpXFoXlC1iK5j5LbFVqADKvlpbU0oYkCIa+a xpvt9eeIjAnvBX+1z+Q4j/jFSvlsaMUDEbVsuvduO0PRg/MI8907/9JO/NABY3ZSWKFJ hwU7EWNqtcQJGLgNSmGOxh7UOGRko/NCXl7ooIixZzLXkojNMugUntGTAzygBkemX/U6 OI3Bh0OgYY+bRQ3EGz4UhiwocwfVVQww5Qv9e/HxFsDBo9cwS0NgK1DayJbsalYg3hhR ngTw== X-Forwarded-Encrypted: i=1; AJvYcCWAeJ3hlU77oxbslvvucpConFirFc+jrcknzQ3Kx/fIMPV0WeLVxqLQ5A1+93tBsEoAb1UenWb+jTKqMVfU9NsI9N0= X-Gm-Message-State: AOJu0YwGrPrKkpTWAcsIj1fa+3V67Ur6Sk68y7wVknSN8hZmnNptKSrb UwvXatsDR4SLOFZdzC6P8qFMBebSQ3P0e828OsLLDIurNmllpLtsCO8wpYIET/RaigcN/Cr02Rm 4m2tpnx1WfIAru6sadWu3GIEjVnK+vy7xETOz X-Google-Smtp-Source: AGHT+IFpF8BS4GfselaMHG+IryrYPtK57pw+o2yCIAHSrUdVDoy4xFzEjFVwLzlJP0+H8lVOFbL+cmq9d4teTS+XZZU= X-Received: by 2002:a05:6102:3588:b0:476:c74a:1544 with SMTP id h8-20020a056102358800b00476c74a1544mr934771vsu.6.1711028583745; Thu, 21 Mar 2024 06:43:03 -0700 (PDT) MIME-Version: 1.0 References: <20240320084630.2727355-1-aliceryhl@google.com> <9ed03148-5ceb-40f2-9c2d-31e2b8918888@proton.me> In-Reply-To: <9ed03148-5ceb-40f2-9c2d-31e2b8918888@proton.me> From: Alice Ryhl Date: Thu, 21 Mar 2024 14:42:52 +0100 Message-ID: Subject: Re: [PATCH v3 4/4] rust: add abstraction for `struct page` To: Benno Lossin Cc: a.hindborg@samsung.com, akpm@linux-foundation.org, alex.gaynor@gmail.com, arnd@arndb.de, arve@android.com, bjorn3_gh@protonmail.com, boqun.feng@gmail.com, brauner@kernel.org, cmllamas@google.com, gary@garyguo.net, gregkh@linuxfoundation.org, joel@joelfernandes.org, keescook@chromium.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, maco@android.com, ojeda@kernel.org, rust-for-linux@vger.kernel.org, surenb@google.com, tkjos@android.com, viro@zeniv.linux.org.uk, wedsonaf@gmail.com, willy@infradead.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: C1B73160016 X-Stat-Signature: 8dbyuinw1pjpqzeqzqmrmcyxo1prgnr3 X-Rspam-User: X-HE-Tag: 1711028584-335021 X-HE-Meta: U2FsdGVkX1+HaAUxveSP4F4hVnVuRJJrNS33t//ug8qqiL0M1ujE6t5ymutJXU4oHilH/Uo7XhwX38XBEAks0KSznG7FfujcX6ZVNU7yZE1KzqUwzCz9v31bboz5lQTCtFahHy1tBKrGiKcPa0ebsO/oDYFRpYRLlt5DBqTThq7UFJYW7jO2hoVHePmr7uklfKPtZMTKX8pQyvh4StQCiiTUGCHUO/6NNK/N5rNn5eJ4SO/zPaLXKkiV5r9rss5EO4SRhXHMwviUvUZGtPVTjue9JxX4Kz9VWfFUytRuVBWsOpbMwYZRPP4pTLEbgaM8MSLSEyf1rOOm/aeWeMyCaTPfHRYlRoBVnZEB2t9u1aLJs1HgPMt5mxUitw4ln9Q7kA1YRpHCAXm3RXG3qggcaFX2Ag5EWQ1kkK7P4FndAY6U1H6DPf7KmiGj70wSX+X2aCSZOUJ+Db3j3cTm79f+9hX9aFItsuYHILsbQt0Z+wjNciI2ZnndWTEqsMDRInelf6T6vM78u8Qb9zvdQaWx8D9B/xOMIxGIQPr2zU9IaLSxNG9HnLwwYqC87rXx/9Zt9VIm11653QouD8aeIT3ldlgfJGXkS8dj03kOEa3sgko26giUSLAZE9j5Yph07LsETU8c6Lc77LSPVzisw+5m9Lo4+8zF9VTKa4gJc55Zhv6YyHxLvIxV9/d+EUbbirzv6AzbZLliOCXlTMpe8AJGT9yoewniBM6hBiUS0DU1voyw7jR7ZrXDerNIZu3bSapnFklDVp/abk0XhH+jKrWcCxl2A6OhI4Mqoe+rVyl4g3NrdON9YGh0caNG3Mldfw8vuy2ZmCqy2ILlCodDOaEZLKhf+2ssquv2dqlhz2t4tyPpf0KrZgwXOZeuD9cLearyahFtJ2/g6ednCTK4SUMasoph57jKcDaWmw+KPO8VDomh9acfsEMdGSNs8J/vXSDhJw8Yoz1jI/KM1hC1gJs vN+rYVG3 FuXqDy9Koqdmfri/REkBJWNP4ooNmhs4+0GE5Y04QznjfG++DoZj+cqzYJ3LefnI9gK6d X-Bogosity: Ham, tests=bogofilter, spamicity=0.000059, 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 Thu, Mar 21, 2024 at 2:16=E2=80=AFPM Benno Lossin wrote: > > On 3/20/24 09:46, Alice Ryhl wrote: > >> On 3/11/24 11:47, Alice Ryhl wrote: > >>> +/// A pointer to a page that owns the page allocation. > >>> +/// > >>> +/// # Invariants > >>> +/// > >>> +/// The pointer points at a page, and has ownership over the page. > >> > >> Why not "`page` is valid"? > >> Do you mean by ownership of the page that `page` has ownership of the > >> allocation, or does that entail any other property/privilege? > > > > I can add "at a valid page". > > I don't think that helps, what you need as an invariant is that the > pointer is valid. To me "points at a page" implies that the pointer is valid. I mean, if it was dangling, it would not point at a page? But I can reword to something else if you have a preferred phrasing. > >>> + /// Runs a piece of code with this page mapped to an address. > >>> + /// > >>> + /// The page is unmapped when this call returns. > >>> + /// > >>> + /// It is up to the caller to use the provided raw pointer corre= ctly. > >> > >> This says nothing about what 'correctly' means. What I gathered from t= he > >> implementation is that the supplied pointer is valid for the execution > >> of `f` for `PAGE_SIZE` bytes. > >> What other things are you allowed to rely upon? > >> > >> Is it really OK for this function to be called from multiple threads? > >> Could that not result in the same page being mapped multiple times? If > >> that is fine, what about potential data races when two threads write t= o > >> the pointer given to `f`? > >> > >>> + pub fn with_page_mapped(&self, f: impl FnOnce(*mut u8) -> T) = -> T { > > > > I will say: > > > > /// It is up to the caller to use the provided raw pointer correctly. > > /// The pointer is valid for `PAGE_SIZE` bytes and for the duration in > > /// which the closure is called. Depending on the gfp flags and kernel > > /// configuration, the pointer may only be mapped on the current thread= , > > /// and in those cases, dereferencing it on other threads is UB. Other > > /// than that, the usual rules for dereferencing a raw pointer apply. > > /// (E.g., don't cause data races, the memory may be uninitialized, and > > /// so on.) > > I would simplify and drop "depending on the gfp flags and kernel..." and > just say that the pointer is only valid on the current thread. Sure, that works for me. > Also would it make sense to make the pointer type *mut [u8; PAGE_SIZE]? I think it's a trade-off. That makes the code more error-prone, since `pointer::add` now doesn't move by a number of bytes, but a number of pages. > > It's okay to map it multiple times from different threads. > > Do you still need to take care of data races? > So would it be fine to execute this code on two threads in parallel? > > static PAGE: Page =3D ...; // assume we have a page accessible by bo= th threads > > PAGE.with_page_mapped(|ptr| { > loop { > unsafe { ptr.write(0) }; > pr_info!("{}", unsafe { ptr.read() }); > } > }); Like I said, the usual pointer rules apply. Two threads can access it in parallel as long as one of the following are satisfied: * Both accesses are reads. * Both accesses are atomic. * They access disjoint byte ranges. Other than the fact that it uses a thread-local mapping on machines that can't address all of their memory at the same time, it's completely normal memory. It's literally just a PAGE_SIZE-aligned allocation of PAGE_SIZE bytes. > If this is not allowed, I don't really like the API. As a raw version it > would be fine, but I think we should have a safer version (eg by taking > `&mut self`). I don't understand what you mean. It is the *most* raw API that `Page` has. I can make them private if you want me to. The API cannot take `&mut self` because I need to be able to unsafely perform concurrent writes to disjoint byte ranges. Alice