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 5BB84C4345F for ; Thu, 18 Apr 2024 18:53:29 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DF13D6B0099; Thu, 18 Apr 2024 14:53:28 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id DA0FB6B009B; Thu, 18 Apr 2024 14:53:28 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C68EA6B009C; Thu, 18 Apr 2024 14:53:28 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id A92506B0099 for ; Thu, 18 Apr 2024 14:53:28 -0400 (EDT) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 9BF63C13EF for ; Thu, 18 Apr 2024 18:53:27 +0000 (UTC) X-FDA: 82023550854.22.5D3C8A9 Received: from mail-qv1-f44.google.com (mail-qv1-f44.google.com [209.85.219.44]) by imf08.hostedemail.com (Postfix) with ESMTP id 8BD3F160023 for ; Thu, 18 Apr 2024 18:53:25 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=MCiEeLJZ; spf=pass (imf08.hostedemail.com: domain of boqun.feng@gmail.com designates 209.85.219.44 as permitted sender) smtp.mailfrom=boqun.feng@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1713466405; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=9r7Cqzwpa0N6ahjvlQwho0MrL0UhOcI3CGtbp3521uQ=; b=gYuPy/P5g4pldv43Tf5JskeUsd5rVbnCxHteabf9RFQ/HEIxFjTrn6CXsxEK9Eqe1A9+8e QW1NoYv+XX7mrHRaXd4oOC+TUWssrs3T2WksxSKG5kxEZGoh+ElYYKmLh5L884SqMcYWYJ G2mMdJoo5VEeACrLrwKP6TpX+1oeyiY= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=MCiEeLJZ; spf=pass (imf08.hostedemail.com: domain of boqun.feng@gmail.com designates 209.85.219.44 as permitted sender) smtp.mailfrom=boqun.feng@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1713466405; a=rsa-sha256; cv=none; b=35pCuXBTaqtsAtgUMHQSE5VXz66FqY0BtGr/gtEscIW+p5xEySC/M72+PPsl6Aj7lBgU03 siqxvyZYqylYtn6UdB4xhcCJ6E+xBuUrPJX7nYwRknrSxaKmIYE9/8Kq1aM6XkiNfzp3sC UzAte49UiFK8ER0LnIyhbTCdWabXMoI= Received: by mail-qv1-f44.google.com with SMTP id 6a1803df08f44-69b137d09e3so7960716d6.1 for ; Thu, 18 Apr 2024 11:53:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1713466403; x=1714071203; darn=kvack.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:feedback-id:from:to:cc:subject:date :message-id:reply-to; bh=9r7Cqzwpa0N6ahjvlQwho0MrL0UhOcI3CGtbp3521uQ=; b=MCiEeLJZZGi9UA4AwtZT9XZr6uQv0mkZWJlse+hWmig7QuYjvpkwz21AnvPFA8jCie A+SiI59Bpe3Tq35A0pTIT9eqINspI7ckCKdYEJeMG/NUL/ueCixNRxT24aMi/6SFoTnN eDxqtku1LgFGtR10VKHyVB3RJD492OboDSdJmmB7f+9n05LEzBcAXoocg35EUgDSsfsh DkYu80K2I2GHnCIDrDl0BDR4eO4EkKXB6wHqxL7O0qcZIP1H13K9cJpPerE12cXvD99a IakrA6pTI6eg9a1goqeQ/kvXlKPn4q+3GUtMzDzl7fl8rkaZGNFz3+ZUUdszCldyCC8m 2s7w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713466403; x=1714071203; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:feedback-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=9r7Cqzwpa0N6ahjvlQwho0MrL0UhOcI3CGtbp3521uQ=; b=KaIf8YkdR5YG/qHPYl21qn7UHmVuXLO6yo+iovfwxsKDbUXWcbEvEGJB1M2pP0RUKN Mab55Mu9XA89E0nzYWy8S95YY+3f8QMJZRebi1hLz4L8mqL6C84C4KCfDkesIsJk8rrH 8eD3h1uQGnzLeGVEugKJcH5hkl017j/U55RMRZSKRDjTb6TKdTkhTgdFvoL1pR0ojvKd aV26uCyvgf1nNlwQxkLi7ctEqNIM5GwRIge/0le3V1IdlrxvxhhzPB5o0MoozwdriXWK JcExA3OM6xbGY7YLe90TQaATSQfqmOFCed+FwxvOI8STV5gUtiZB8BzWf2py8hCojMIC HAMg== X-Forwarded-Encrypted: i=1; AJvYcCWg+dPnMZwDnfiP7iupApm2AJuM3D9DvTKlBj11tsKr1pImMYcMozrHIXJAGcwLMZId4obISj4TR+rpVPvYyOSjnhM= X-Gm-Message-State: AOJu0YwDTjaarSnptJmR6q07rpX0g+75w9gvQqpLH9fBaP3yOEnixIAa SQXlcnAFJOBDoTBOwfAApDV9uMOVD/a/i+dFsYQd33VkX31PbSHb X-Google-Smtp-Source: AGHT+IGYj8Z9DdNusXwRbFDJgf6QW2DcyYlg/XNuCin6S3U7WarnpdkqqVX9H+XSmkhJvov6jUOCIA== X-Received: by 2002:a0c:c252:0:b0:69b:1e3b:61d8 with SMTP id w18-20020a0cc252000000b0069b1e3b61d8mr3630668qvh.29.1713466403444; Thu, 18 Apr 2024 11:53:23 -0700 (PDT) Received: from fauth1-smtp.messagingengine.com (fauth1-smtp.messagingengine.com. [103.168.172.200]) by smtp.gmail.com with ESMTPSA id g11-20020a0caacb000000b006994062299dsm886050qvb.33.2024.04.18.11.53.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 18 Apr 2024 11:53:22 -0700 (PDT) Received: from compute6.internal (compute6.nyi.internal [10.202.2.47]) by mailfauth.nyi.internal (Postfix) with ESMTP id DC63D1200032; Thu, 18 Apr 2024 14:53:21 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute6.internal (MEProxy); Thu, 18 Apr 2024 14:53:21 -0400 X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvledrudektddguddvlecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpeffhffvvefukfhfgggtuggjsehttdertddttddvnecuhfhrohhmpeeuohhq uhhnucfhvghnghcuoegsohhquhhnrdhfvghnghesghhmrghilhdrtghomheqnecuggftrf grthhtvghrnhephedugfduffffteeutddvheeuveelvdfhleelieevtdeguefhgeeuveei udffiedvnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomh epsghoqhhunhdomhgvshhmthhprghuthhhphgvrhhsohhnrghlihhthidqieelvdeghedt ieegqddujeejkeehheehvddqsghoqhhunhdrfhgvnhhgpeepghhmrghilhdrtghomhesfh higihmvgdrnhgrmhgv X-ME-Proxy: Feedback-ID: iad51458e:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 18 Apr 2024 14:53:21 -0400 (EDT) Date: Thu, 18 Apr 2024 11:52:56 -0700 From: Boqun Feng To: Alice Ryhl Cc: Miguel Ojeda , Matthew Wilcox , Al Viro , Andrew Morton , Kees Cook , Alex Gaynor , Wedson Almeida Filho , Gary Guo , =?iso-8859-1?Q?Bj=F6rn?= Roy Baron , Benno Lossin , Andreas Hindborg , Greg Kroah-Hartman , Arve =?iso-8859-1?B?SGr4bm5lduVn?= , Todd Kjos , Martijn Coenen , Joel Fernandes , Carlos Llamas , Suren Baghdasaryan , Arnd Bergmann , Trevor Gross , linux-mm@kvack.org, linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org, Christian Brauner Subject: Re: [PATCH v6 4/4] rust: add abstraction for `struct page` Message-ID: References: <20240418-alice-mm-v6-0-cb8f3e5d688f@google.com> <20240418-alice-mm-v6-4-cb8f3e5d688f@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240418-alice-mm-v6-4-cb8f3e5d688f@google.com> X-Stat-Signature: tuxx6k3da7assycwm3hr9wj3gqkujwj8 X-Rspamd-Queue-Id: 8BD3F160023 X-Rspamd-Server: rspam10 X-Rspam-User: X-HE-Tag: 1713466405-888286 X-HE-Meta: U2FsdGVkX18QA1J8By9wqLvsLrVNCjS3vRLgbB67O86AuO22S5TNi7tqnP1cl6qAyMdLyjywD4DEsd19Nw2uFqnimUyvuNrfghVOyPNdSRFL7ZxXr0FVYwL7vnKn699Ox4wB+Bw9kNmVuSr0hYXHnkF+xGjzoH+nr5yMAt8vl0xkloCCPobd+G2zdo+dNufc/ZsWRb+hWIbrQ0NNoieN4ovf6yYNGPm8JpSSDi7gRmxCRs48pcoKZEo/Tz85tv2nC31SzdIIrXnGgzQHeTG3gBe8wdyaDl8XWTYaVaohq3oclFBqU8Lu3UQ9ZewUFK3Z05Z9ab39opHavRkoZidYVLEh7WApYdXYn2QpBXZKWcRux/DuszBAekZvCDlnFeQ5+WWW2prI4LhzzkiWn/FEpPHRKWKSaLoDTsbww5DEdkhqgcrp0K8iDdGCAsNyPCvkQB8MaTsqZ7dtGkV6+IG2Xzuqpgz24tIJQtL3pQKiF5MJCmrKm872ZY+JKgrzBq4/nyHAUCVuCTJTx1qKs3zpOYNdJ2az4m5daFV8CNWn66hjULuL9TvOw00Bx6IqOeyHYpGsffZUHQoN4min/zy90TjkckxP5nU6jd//CMxlSnkXunTjBQQZwu+4fIOgtTrB0nnPeuZplWFceW34WVend1QAT0mI3qtStGjUzt8Enc+N7XFguu8+8lZeJqP5uq3vDiCSvp+KryXHCgND8WMhcPktg5mY62O++Ufnq10MsTrf/KoR1X25movJH7JMKY+78yICC5PAMkH1wDAA2uAxfFmOcU9u2YhgzsVa8ZHKHG/7Oq3DanDD+sRXkE2ZxZC8hzn25fM5nHuhSTQ7nHGHTIhqOqV5Vu2EGFN0qJlJtYVb7zRzNjkMgja6fSCSE0EQc4Ji+eIzr2fdo/Utznc5slnaroJ8jbicMKYMTva3W3jol2ET1n0b7o0nQv4H0SPID8QX5jLEST5YJXe1+7A +f37nsQT yIsD8il9mO8j08L9jLoT+i3RA2ChsarMZVA1EpO0T9iBf/erOV3geaYh0KREQZFBvdf8sRukhhlZ2JJSc5FWClCXiVyZQTTZRec3Y/UomU56IJQop+PnwobKIgqx79/OxfnDeQ8Ps1VWhtejrthbxBdYHs+tMCPE1N9kbZ4CJq8JyKt0= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, 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, Apr 18, 2024 at 08:59:20AM +0000, Alice Ryhl wrote: > Adds a new struct called `Page` that wraps a pointer to `struct page`. > This struct is assumed to hold ownership over the page, so that Rust > code can allocate and manage pages directly. > > The page type has various methods for reading and writing into the page. > These methods will temporarily map the page to allow the operation. All > of these methods use a helper that takes an offset and length, performs > bounds checks, and returns a pointer to the given offset in the page. > > This patch only adds support for pages of order zero, as that is all > Rust Binder needs. However, it is written to make it easy to add support > for higher-order pages in the future. To do that, you would add a const > generic parameter to `Page` that specifies the order. Most of the > methods do not need to be adjusted, as the logic for dealing with > mapping multiple pages at once can be isolated to just the > `with_pointer_into_page` method. > Thank you for doing this, and breaking the chicken-and-egg problem chain ;-) For sure, the whole package of page API would need more time to design, implement and review, but this patch looks good enough to me. > Rust Binder needs to manage pages directly as that is how transactions > are delivered: Each process has an mmap'd region for incoming > transactions. When an incoming transaction arrives, the Binder driver > will choose a region in the mmap, allocate and map the relevant pages > manually, and copy the incoming transaction directly into the page. This > architecture allows the driver to copy transactions directly from the > address space of one process to another, without an intermediate copy > to a kernel buffer. > > This code is based on Wedson's page abstractions from the old rust > branch, but it has been modified by Alice by removing the incomplete > support for higher-order pages, by introducing the `with_*` helpers > to consolidate the bounds checking logic into a single place, and > various other changes. > > Co-developed-by: Wedson Almeida Filho > Signed-off-by: Wedson Almeida Filho > Reviewed-by: Andreas Hindborg > Reviewed-by: Trevor Gross > Reviewed-by: Benno Lossin > Signed-off-by: Alice Ryhl Reviewed-by: Boqun Feng Something I want to bring up for discussion below: [...] > + /// Runs a piece of code with a raw pointer to a slice of this page, with bounds checking. > + /// > + /// If `f` is called, then it will be called with a pointer that points at `off` bytes into the > + /// page, and the pointer will be valid for at least `len` bytes. The pointer is only valid on > + /// this task, as this method uses a local mapping. > + /// > + /// If `off` and `len` refers to a region outside of this page, then this method returns > + /// `EINVAL` and does not call `f`. > + /// > + /// # Using the raw pointer > + /// > + /// It is up to the caller to use the provided raw pointer correctly. The pointer is valid for > + /// `len` bytes and for the duration in which the closure is called. The pointer might only be > + /// mapped on the current thread, and when that is the case, dereferencing it on other threads > + /// is UB. Other than that, the usual rules for dereferencing a raw pointer apply: don't cause > + /// data races, the memory may be uninitialized, and so on. > + /// > + /// If multiple threads map the same page at the same time, then they may reference with > + /// different addresses. However, even if the addresses are different, the underlying memory is > + /// still the same for these purposes (e.g., it's still a data race if they both write to the > + /// same underlying byte at the same time). > + fn with_pointer_into_page( > + &self, > + off: usize, > + len: usize, > + f: impl FnOnce(*mut u8) -> Result, I wonder whether the way to go here is making this function signature: fn with_slice_in_page ( &self, off: usize, len: usize, f: iml FnOnce(&UnsafeCell<[u8]>) -> Result ) -> Result , because in this way, it makes a bit more clear that what memory that `f` can access, in other words, the users are less likely to use the pointer in a wrong way. But that depends on whether `&UnsafeCell<[u8]>` is the correct abstraction and the ecosystem around it: for example, I feel like these two functions: fn len(slice: &UnsafeCell<[u8]>) -> usize fn as_ptr(slice: &UnsafeCell<[u8]>) -> *mut u8 should be trivially safe, but I might be wrong. Again this is just for future discussion. Regards, Boqun > + ) -> Result { > + let bounds_ok = off <= PAGE_SIZE && len <= PAGE_SIZE && (off + len) <= PAGE_SIZE; > + > + if bounds_ok { > + self.with_page_mapped(move |page_addr| { > + // SAFETY: The `off` integer is at most `PAGE_SIZE`, so this pointer offset will > + // result in a pointer that is in bounds or one off the end of the page. > + f(unsafe { page_addr.add(off) }) > + }) > + } else { > + Err(EINVAL) > + } > + } > + [...] > > -- > 2.44.0.683.g7961c838ac-goog >