[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87o71bagpf.fsf@kernel.org>
Date: Mon, 16 Dec 2024 14:41:00 +0100
From: Andreas Hindborg <a.hindborg@...nel.org>
To: "Alice Ryhl" <aliceryhl@...gle.com>
Cc: "Miguel Ojeda" <ojeda@...nel.org>, "Matthew Wilcox"
<willy@...radead.org>, "Lorenzo Stoakes" <lorenzo.stoakes@...cle.com>,
"Vlastimil Babka" <vbabka@...e.cz>, "John Hubbard"
<jhubbard@...dia.com>, "Liam R. Howlett" <Liam.Howlett@...cle.com>,
"Andrew Morton" <akpm@...ux-foundation.org>, "Greg Kroah-Hartman"
<gregkh@...uxfoundation.org>, "Arnd Bergmann" <arnd@...db.de>,
"Christian Brauner" <brauner@...nel.org>, "Jann Horn"
<jannh@...gle.com>, "Suren Baghdasaryan" <surenb@...gle.com>, "Alex
Gaynor" <alex.gaynor@...il.com>, "Boqun Feng" <boqun.feng@...il.com>,
"Gary Guo" <gary@...yguo.net>, Björn Roy Baron
<bjorn3_gh@...tonmail.com>, "Benno Lossin" <benno.lossin@...ton.me>,
"Trevor Gross" <tmgross@...ch.edu>, <linux-kernel@...r.kernel.org>,
<linux-mm@...ck.org>, <rust-for-linux@...r.kernel.org>
Subject: Re: [PATCH v11 6/8] mm: rust: add VmAreaNew for f_ops->mmap()
"Alice Ryhl" <aliceryhl@...gle.com> 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 separate
> set of operations that you are only able to use during the mmap() hook.
> For example, the VM_MIXEDMAP flag must not be changed after the initial
> 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 checking
> 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. When
> 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 <lorenzo.stoakes@...cle.com> (for mm bits)
> Reviewed-by: Jann Horn <jannh@...gle.com>
> Signed-off-by: Alice Ryhl <aliceryhl@...gle.com>
> ---
> 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 word
to use here?
Best regards,
Andreas Hindborg
Powered by blists - more mailing lists