linux-kernel - Re: [PATCH v5 4/4] rust: add abstraction for `struct page`

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Tue, 16 Apr 2024 13:47:56 -0400
From: Trevor Gross <tmgross@...ch.edu>
To: Alice Ryhl <aliceryhl@...gle.com>
Cc: a.hindborg@...sung.com, akpm@...ux-foundation.org, alex.gaynor@...il.com, 
	arnd@...db.de, arve@...roid.com, benno.lossin@...ton.me, 
	bjorn3_gh@...tonmail.com, boqun.feng@...il.com, brauner@...nel.org, 
	cmllamas@...gle.com, gary@...yguo.net, gregkh@...uxfoundation.org, 
	joel@...lfernandes.org, keescook@...omium.org, linux-kernel@...r.kernel.org, 
	linux-mm@...ck.org, maco@...roid.com, ojeda@...nel.org, 
	rust-for-linux@...r.kernel.org, surenb@...gle.com, tkjos@...roid.com, 
	viro@...iv.linux.org.uk, wedsonaf@...il.com, willy@...radead.org
Subject: Re: [PATCH v5 4/4] rust: add abstraction for `struct page`

On Tue, Apr 16, 2024 at 5:53 AM Alice Ryhl <aliceryhl@...gle.com> wrote:
>
> >> +/// Flags for the "get free page" function that underlies all memory allocations.
> >> +pub mod flags {
> >> +    /// gfp flags.
> >
> > Uppercase acronym, maybe with a description:
> >
> >     GFP (Get Free Page) flags.
> >
> >> +    #[allow(non_camel_case_types)]
> >> +    pub type gfp_t = bindings::gfp_t;
> >
> > Why not GfpFlags, do we do this elsewhere?
> >
> >> +    /// `GFP_KERNEL` is typical for kernel-internal allocations. The caller requires `ZONE_NORMAL`
> >> +    /// or a lower zone for direct access but can direct reclaim.
> >> +    pub const GFP_KERNEL: gfp_t = bindings::GFP_KERNEL;
> >> +    /// `GFP_ZERO` returns a zeroed page on success.
> >> +    pub const __GFP_ZERO: gfp_t = bindings::__GFP_ZERO;
> >> +    /// `GFP_HIGHMEM` indicates that the allocated memory may be located in high memory.
> >> +    pub const __GFP_HIGHMEM: gfp_t = bindings::__GFP_HIGHMEM;
> >
> > It feels a bit weird to have dunder constants on the rust side that
> > aren't also `#[doc(hidden)]` or just nonpublic. Makes me think they
> > are an implementation detail or not really meant to be used - could
> > you update the docs if this is the case?
>
> All of this is going away in the next version because it will be based
> on [1], which defines the gfp flags type for us.
>
> [1]: https://lore.kernel.org/rust-for-linux/20240328013603.206764-1-wedsonaf@gmail.com/

Great, thanks for the link.

> > Could you add an example of how to use this correctly?
>
> This is a private function, you're not supposed to use it directly.
> Anyone who is modifying this file directly can look at the existing
> users for examples.

Ah you're right, missed this bit.

Thanks for the followup.

Trevor