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 3A2F1C02180 for ; Wed, 15 Jan 2025 11:03:52 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id BE1396B0088; Wed, 15 Jan 2025 06:03:51 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id B8F9B6B0089; Wed, 15 Jan 2025 06:03:51 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A2FAC6B008A; Wed, 15 Jan 2025 06:03:51 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 81F5C6B0088 for ; Wed, 15 Jan 2025 06:03:51 -0500 (EST) Received: from smtpin29.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id DE1B01611C3 for ; Wed, 15 Jan 2025 11:03:50 +0000 (UTC) X-FDA: 83009401020.29.F2FEBAF Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf03.hostedemail.com (Postfix) with ESMTP id 35E1220011 for ; Wed, 15 Jan 2025 11:03:49 +0000 (UTC) Authentication-Results: imf03.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=bj9C7rCH; spf=pass (imf03.hostedemail.com: domain of a.hindborg@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=a.hindborg@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1736939029; 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=kh5ZBqlW/rzxiq84zmwJmnCd5YVkEkowrY2cylSgMmc=; b=TnFmd7oa8ZCnxXBjEb+44fYrE3BR7N/sk6I1gyUxlCJhRLnLz+VVGdoOSynK1nPz9ku9UQ ZxuwStAX8/Bby1LPLCyEibzqGB9ea21OTT42fEJgCevRjwCYNohBtfdIjj73mSwpf1205l 6GWXq7cdF+8VjeCl8DhYZQPS0grWIT4= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1736939029; a=rsa-sha256; cv=none; b=XnsB6Lk7hwkOGVxTocRCKpU+RpP7qMUcfqUr7QqA7E997yclOZG473Yb+dGcMVEJbGtpTp oDNtHc5B73O8pGxy/BEMHOd9lEsMhEdzykMobI54zEplGLU9xqKyohDxm4qsLhEuo1L9Ot JCaZtqw5/hTqM3XmOKgGip6o10brr6I= ARC-Authentication-Results: i=1; imf03.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=bj9C7rCH; spf=pass (imf03.hostedemail.com: domain of a.hindborg@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=a.hindborg@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 5C8545C56C8; Wed, 15 Jan 2025 11:03:07 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 08CC9C4CEE2; Wed, 15 Jan 2025 11:03:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1736939027; bh=2mzV9xftHa8ZCv5+6EBT2YCEpW+sOtYrUsyHCKAgMmo=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=bj9C7rCHFl7bq3lQ/xaW5rt9NKgWlFNFsAwRoUKPsBCRiusHcjlU/9zF5D1HuLkt7 rKJe85U+AO3EJwMj/P69Wl68hNqXbN33EsYbzS3wAOMv8LFU76sbqT/NSqXChiA9lx +GXxhzipCwDL5DOLbQ34SdHZNjZ/vbK7vCOp86SZjxQJeJfUHDVH2UkxCWfr38mKJj 7ClbOqXvYw0b6u5bTHMXcpFAQYU1vQwXWWLTLDYnDlaU++2+WeRJEFzn9AAw2cOiOc jQ2WEvcHffEvfhF6HlqrqzEcB3FSs4dUgdF2v6jCvvZ3FIc1WueRzAl9/WrmI7Ssnd gSWvtkW6rOUhQ== From: Andreas Hindborg To: "Alice Ryhl" Cc: "Miguel Ojeda" , "Matthew Wilcox" , "Lorenzo Stoakes" , "Vlastimil Babka" , "John Hubbard" , "Liam R. Howlett" , "Andrew Morton" , "Greg Kroah-Hartman" , "Arnd Bergmann" , "Christian Brauner" , "Jann Horn" , "Suren Baghdasaryan" , "Alex Gaynor" , "Boqun Feng" , "Gary Guo" , =?utf-8?Q?Bj=C3=B6rn?= Roy Baron , "Benno Lossin" , "Trevor Gross" , , , Subject: Re: [PATCH v11 6/8] mm: rust: add VmAreaNew for f_ops->mmap() In-Reply-To: (Alice Ryhl's message of "Mon, 13 Jan 2025 11:17:06 +0100") References: <20241211-vma-v11-0-466640428fc3@google.com> <20241211-vma-v11-6-466640428fc3@google.com> <87o71bagpf.fsf@kernel.org> <87a5c0bdr0.fsf@kernel.org> User-Agent: mu4e 1.12.7; emacs 29.4 Date: Wed, 15 Jan 2025 10:57:49 +0100 Message-ID: <87o708e6w2.fsf@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 35E1220011 X-Stat-Signature: rk3fb63xpyw7odxnxe6nadqrqmwkk984 X-Rspam-User: X-HE-Tag: 1736939029-292926 X-HE-Meta: U2FsdGVkX1+MyhjwbAuNxtY5I/nEEWrYSwjew5OJhgB2ytqNgem9lyzWhwaB8FrN7BqOeORnFhJP39HwS3u2sc/UrTiKGpi7gZwX8PUNC3PryVNVfzxVTsdctRVPkNK5jq1P0WHwULb91ao+isVYD+Ht/8oo4FalbkijcOqjFhDT0b59cF68soS/Y8K/NcDRpRHX14ZxX+8ZEMtOYNb+1rVgRqj2UKpH+/6OnaX67LN3ZdfUXjLEekcmoagxHjvu+GFhfCWwIzcpkLM4sBViqiKIa9O0nWKNcZclsYW8ICf8KbOFCvh6P0M/oexY6wMq9enTRnsIIyANd4R4BCjdWZ2qAO2auvQ2gZ3QxAQToXHEBB3U6EJ0TnsrF4gOFDEJ5wLyR9KSDV4USGZeRrb3Dbz1vq/+VtpIAJD7zL60XJCWw6Pk6zCwt6K23sKEkD1IY33jTFaZkr3T1VUFC32Iu0GktPiNbhCUQ8D5yU52/HY1ZWWBq7DFjpnUQwMKgJ65Tq5LuKSezf5VE+reGx2W5OpUI32550WMne4STBh88+li4kRoJFYZXcGbH5tAWrFhxK3pnUgQfGJ44DAnkaSJBBZJZp2m9rmf+zsZ4EDPcfx7B3HClYAE5KIoxrm+2DNj84jps15H1GCzwhetwrfGm32tEgjEbxaKJH6bI00VHYEYSxvNIAdYsP+S+hpTjRm54wbmo9+iR6i4i8wmKIctOrlHnaGdDTMaMaatvWh6Nh7YXQcR1uplX+KmvkrioygGsnT7UHXK50Q3HHo4vcT4O913t4n02MmyrurruieeVPdRUbEFXFVvPR05O4/A1RzuweHDhNVA68MQUIR9rXrm3JP73dq1hcES0F9jD3b+9m5qCpFSTPX1zW8/BYxGPmY+KXRsod3bppND+0jLYfhJogStYWhOF0drDDZ3tX7JyAVQyAowkA5rb8wURQkzj+xRh2IDevG9f07XmHn4hRF sqrPThuW EgnkrpWxjpPLQrsCmooWzAOQm4ZtkAyZ4KmDnROZmgoy2gPddv04yN0FrkpP/Z1dkp2eSrjY+sXs6A4YwupuCA4oxe7rO4/9ujOx7PS4Ye5BIFfewafERPESFq+BpWgT1IZYLTBTAGk+iUsE70bffNjrziu96TX02QkquG04Iwh5eQ+uFrpaRu3JBcJt0mXo3qd6lI9XNd6hotX/epis7w6mdl50eOZnuQmMX/5RlFsELj/xpjXDbV6QHUA8m0+PPmqVNybIqTqKPC8RSLe8eBp4Wh+nAjX/NJQcIsdnYb+XWwdZGtpGp59JVgb84a+XnNA9LHu0z3Wdh+LOPFlZE6pRH3U6o4Yq4Wjh5AZ4KpcNszzDTs6mPxgqPofTMGwScvxJJrUW1eVugzN0+nTQYuH/vnI9MfvnGXgb1 X-Bogosity: Ham, tests=bogofilter, spamicity=0.196880, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: "Alice Ryhl" writes: > On Thu, Jan 9, 2025 at 9:19=E2=80=AFAM Andreas Hindborg wrote: >> >> "Alice Ryhl" writes: >> >> > On Mon, Dec 16, 2024 at 3:51=E2=80=AFPM Andreas Hindborg wrote: >> >> >> >> "Alice Ryhl" writes: >> >> >> >> > This type will be used when setting up a new vma in an f_ops->mmap() >> >> > hook. Using a separate type from VmAreaRef allows us to have a sepa= rate >> >> > set of operations that you are only able to use during the mmap() h= ook. >> >> > For example, the VM_MIXEDMAP flag must not be changed after the ini= tial >> >> > setup that happens during the f_ops->mmap() hook. >> >> > >> >> > To avoid setting invalid flag values, the methods for clearing >> >> > VM_MAYWRITE and similar involve a check of VM_WRITE, and return an = error >> >> > if VM_WRITE is set. Trying to use `try_clear_maywrite` without chec= king >> >> > the return value results in a compilation error because the `Result` >> >> > type is marked #[must_use]. >> >> > >> >> > For now, there's only a method for VM_MIXEDMAP and not VM_PFNMAP. W= hen >> >> > we add a VM_PFNMAP method, we will need some way to prevent you from >> >> > setting both VM_MIXEDMAP and VM_PFNMAP on the same vma. >> >> > >> >> > Acked-by: Lorenzo Stoakes (for mm bits) >> >> > Reviewed-by: Jann Horn >> >> > Signed-off-by: Alice Ryhl >> >> > --- >> >> > rust/kernel/mm/virt.rs | 181 +++++++++++++++++++++++++++++++++++++= +++++++++++- >> >> > 1 file changed, 180 insertions(+), 1 deletion(-) >> >> > >> >> > diff --git a/rust/kernel/mm/virt.rs b/rust/kernel/mm/virt.rs >> >> > index 3a23854e14f4..6d9ba56d4f95 100644 >> >> > --- a/rust/kernel/mm/virt.rs >> >> > +++ b/rust/kernel/mm/virt.rs >> >> > @@ -6,7 +6,7 @@ >> >> > >> >> > use crate::{ >> >> > bindings, >> >> > - error::{to_result, Result}, >> >> > + error::{code::EINVAL, to_result, Result}, >> >> > mm::MmWithUser, >> >> > page::Page, >> >> > types::Opaque, >> >> > @@ -171,6 +171,185 @@ pub fn vm_insert_page(&self, address: usize, = page: &Page) -> Result { >> >> > } >> >> > } >> >> > >> >> > +/// A builder for setting up a vma in an `f_ops->mmap()` hook. >> >> >> >> Reading this line, I would expect to be able to chain update methods = as >> >> in `Builder::new().prop_a().prop_b().build()`. Could/should this type >> >> accommodate a proper builder pattern? Or is "builder" not the right w= ord >> >> to use here? >> > >> > You cannot create values of this type yourself. Only the C >> > infrastructure can do so. >> > >> > What would you call it if not "builder"? >> >> It looks more like a newtype with a bunch of setters and getters. It >> also does not have a method to instantiate (`build()` or similar). So >> how about newtype? > > I don't think newtype is helpful. Ultimately, the f_ops->mmap() hook > is a *constructor* for a VMA, and the VmAreaNew type represents a VMA > whose constructor is currently running. The "method to instantiate" is > called "return". > > fn mmap(new_vma: &VmAreaNew) -> Result { > // VMAs for this driver must not be mapped as executable > new_vma.try_clear_may_exec()?; > > // we are done constructing the vma, so return > Ok(()) > } > > Alice Right. Let's update the docs for the `mmap` hook then: + /// Handle for mmap. + fn mmap( + _device: ::Borrowed<'_>, + _file: &File, + _vma: &VmAreaNew, + ) -> Result { + kernel::build_error!(VTABLE_DEFAULT_ERROR) + } + ``` Handle for mmap. This function is invoked when a user space process invokes the `mmap` system call on `_file`. The function is a callback that is part of the VMA initializer. The kernel will do initial setup of the VMA before calling this function. The function can then interact with the VMA initialization by calling methods of `_vma`. If the function does not return an error, the kernel will complete intialization of the VMA according to the properties of `_vma`. ``` But I still do not think "builder" is the right term. `VmAreaNew` is more like a configuration object to pass initialization properties of the VMA back to the kernel. How about "configuration object"? Best regards, Andreas Hindborg