Re: [PATCH v11 3/8] mm: rust: add vm_insert_page

[Andreas Hindborg <a.hindborg@kernel.org>]

"Alice Ryhl" <aliceryhl@google.com> writes:

> On Mon, Dec 16, 2024 at 3:51 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>
>> "Alice Ryhl" <aliceryhl@google.com> writes:
>>
>> > The vm_insert_page method is only usable on vmas with the VM_MIXEDMAP
>> > flag, so we introduce a new type to keep track of such vmas.
>> >
>> > The approach used in this patch assumes that we will not need to encode
>> > many flag combinations in the type. I don't think we need to encode more
>> > than VM_MIXEDMAP and VM_PFNMAP as things are now. However, if that
>> > becomes necessary, using generic parameters in a single type would scale
>> > better as the number of flags increases.
>> >
>> > Acked-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com> (for mm bits)
>> > Signed-off-by: Alice Ryhl <aliceryhl@google.com>
>> > ---
>> >  rust/kernel/mm/virt.rs | 71 +++++++++++++++++++++++++++++++++++++++++++++++++-
>> >  1 file changed, 70 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/rust/kernel/mm/virt.rs b/rust/kernel/mm/virt.rs
>> > index 68c763169cf0..3a23854e14f4 100644
>> > --- a/rust/kernel/mm/virt.rs
>> > +++ b/rust/kernel/mm/virt.rs
>> > @@ -4,7 +4,15 @@
>> >
>> >  //! Virtual memory.
>> >
>> > -use crate::{bindings, mm::MmWithUser, types::Opaque};
>> > +use crate::{
>> > +    bindings,
>> > +    error::{to_result, Result},
>> > +    mm::MmWithUser,
>> > +    page::Page,
>> > +    types::Opaque,
>> > +};
>> > +
>> > +use core::ops::Deref;
>> >
>> >  /// A wrapper for the kernel's `struct vm_area_struct` with read access.
>> >  ///
>> > @@ -100,6 +108,67 @@ pub fn zap_page_range_single(&self, address: usize, size: usize) {
>> >              )
>> >          };
>> >      }
>> > +
>> > +    /// Check whether the `VM_MIXEDMAP` flag is set.
>>
>> Perhaps "Check whether the `VM_MIXEDMAP` flag is set. If so, return
>> `Some`, otherwise `None` ?
>
> How about
>
> If the `VM_MIXEDMAP` flag is set, returns a `VmAreaMixedMap` to this
> VMA, otherwise returns `None`.
>
> This follows the example of slice::as_ascii

Sounds good 👍


Best regards,
Andreas Hindborg