Received-SPF: pass (google.com: domain of linux-kernel+bounces-147330-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Precedence: bulk
MIME-Version: 1.0
References: <CALNs47tEZqL201jsExfF1j7m+yW37YRAws-NTF6hwsxohSKoQA@mail.gmail.com>
 <20240416095333.1108884-1-aliceryhl@google.com>
In-Reply-To: <20240416095333.1108884-1-aliceryhl@google.com>
From: Trevor Gross <tmgross@umich.edu>
Date: Tue, 16 Apr 2024 13:47:56 -0400
Message-ID: <CALNs47vUXA9z8+bM4TOU-Q_NY3wbux3OA4nBVxegAWykzTd2zQ@mail.gmail.com>
Subject: Re: [PATCH v5 4/4] rust: add abstraction for `struct page`
To: Alice Ryhl <aliceryhl@google.com>
Cc: a.hindborg@samsung.com, akpm@linux-foundation.org, alex.gaynor@gmail.com, 
	arnd@arndb.de, arve@android.com, benno.lossin@proton.me, 
	bjorn3_gh@protonmail.com, boqun.feng@gmail.com, brauner@kernel.org, 
	cmllamas@google.com, gary@garyguo.net, gregkh@linuxfoundation.org, 
	joel@joelfernandes.org, keescook@chromium.org, linux-kernel@vger.kernel.org, 
	linux-mm@kvack.org, maco@android.com, ojeda@kernel.org, 
	rust-for-linux@vger.kernel.org, surenb@google.com, tkjos@android.com, 
	viro@zeniv.linux.org.uk, wedsonaf@gmail.com, willy@infradead.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Tue, Apr 16, 2024 at 5:53=E2=80=AFAM Alice Ryhl <aliceryhl@google.com> w=
rote:
>
> >> +/// 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 =3D 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 =3D bindings::GFP_KERNEL;
> >> +    /// `GFP_ZERO` returns a zeroed page on success.
> >> +    pub const __GFP_ZERO: gfp_t =3D bindings::__GFP_ZERO;
> >> +    /// `GFP_HIGHMEM` indicates that the allocated memory may be loca=
ted in high memory.
> >> +    pub const __GFP_HIGHMEM: gfp_t =3D 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-wedso=
naf@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