Received-SPF: pass (google.com: domain of linux-kernel+bounces-125556-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Precedence: bulk
MIME-Version: 1.0
References: <20240328013603.206764-1-wedsonaf@gmail.com> <ZgcHfG5Hdhv39SLU@boqun-archlinux>
In-Reply-To: <ZgcHfG5Hdhv39SLU@boqun-archlinux>
From: Wedson Almeida Filho <wedsonaf@gmail.com>
Date: Fri, 29 Mar 2024 21:54:25 -0300
Message-ID: <CANeycqoSe5mTuxUh+M3LbLeXTVOU0zzJhxwqFeww0xHXBJg_Sg@mail.gmail.com>
Subject: Re: [PATCH v3 00/10] Allocation APIs
To: Boqun Feng <boqun.feng@gmail.com>
Cc: rust-for-linux@vger.kernel.org, Miguel Ojeda <ojeda@kernel.org>, 
	Alex Gaynor <alex.gaynor@gmail.com>, Gary Guo <gary@garyguo.net>, 
	=?UTF-8?Q?Bj=C3=B6rn_Roy_Baron?= <bjorn3_gh@protonmail.com>, 
	Benno Lossin <benno.lossin@proton.me>, Andreas Hindborg <a.hindborg@samsung.com>, 
	Alice Ryhl <aliceryhl@google.com>, linux-kernel@vger.kernel.org, 
	Wedson Almeida Filho <walmeida@microsoft.com>
Content-Type: text/plain; charset="UTF-8"

On Fri, 29 Mar 2024 at 15:25, Boqun Feng <boqun.feng@gmail.com> wrote:
>
> On Wed, Mar 27, 2024 at 10:35:53PM -0300, Wedson Almeida Filho wrote:
> > From: Wedson Almeida Filho <walmeida@microsoft.com>
> >
> > Revamp how we use the `alloc` crate.
> >
> > We currently have a fork of the crate with changes to `Vec`; other
> > changes have been upstreamed (to the Rust project). This series removes
> > the fork and exposes all the functionality as extension traits.
> >
> > Additionally, it also introduces allocation flag parameters to all
> > functions that may result in allocations (e.g., `Box::new`, `Arc::new`,
> > `Vec::push`, etc.) without the `try_` prefix -- the names are available
> > because we build `alloc` with `no_global_oom_handling`.
> >
> > Lastly, the series also removes our reliance on the `allocator_api`
> > unstable feature.
> >
> > Long term, we still want to make such functionality available in
> > upstream Rust, but this allows us to make progress now and reduces our
> > maintainance burden.
> >
> > In summary:
> > 1. Removes `alloc` fork
> > 2. Removes use of `allocator_api` unstable feature
> > 3. Introduces flags (e.g., GFP_KERNEL, GFP_ATOMIC) when allocating
> >
> > ---
> >
>
> FWIW, I've put this into rust-dev:
>
>         https://github.com/Rust-for-Linux/linux rust-dev
>
> a few adjustments are needed to work with other commits that have been
> queued in rust-dev, so I add an commit on the top for everyone's
> reference. (Besides this commit, I also removed all updates to our own
> alloc in Miguel's 1.77.0 compiler version bump patch)
>
> Regards,
> Boqun
>
> ---------------------------------------------------->8
> diff --git a/rust/kernel/file.rs b/rust/kernel/file.rs
> index 8902f490ccc8..a5930f0f2bc5 100644
> --- a/rust/kernel/file.rs
> +++ b/rust/kernel/file.rs
> @@ -6,13 +6,15 @@
>  //! [`include/linux/file.h`](srctree/include/linux/file.h)
>
>  use crate::{
> +    alloc::AllocError,

I would do `alloc::{flags::*, AlocError},` here and add not add
`prelude::*` below.

>      bindings,
>      cred::Credential,
>      error::{code::*, Error, Result},
> +    prelude::*,
>      types::{ARef, AlwaysRefCounted, NotThreadSafe, Opaque},
>  };
>  use alloc::boxed::Box;
> -use core::{alloc::AllocError, mem, ptr};
> +use core::{mem, ptr};
>
>  /// Flags associated with a [`File`].
>  pub mod flags {
> @@ -348,10 +350,13 @@ impl DeferredFdCloser {
>      pub fn new() -> Result<Self, AllocError> {
>          Ok(Self {
>              // INVARIANT: The `file` pointer is null, so the type invariant does not apply.
> -            inner: Box::try_new(DeferredFdCloserInner {
> -                twork: mem::MaybeUninit::uninit(),
> -                file: core::ptr::null_mut(),
> -            })?,
> +            inner: Box::new(
> +                DeferredFdCloserInner {
> +                    twork: mem::MaybeUninit::uninit(),
> +                    file: core::ptr::null_mut(),
> +                },
> +                GFP_KERNEL,
> +            )?,
>          })
>      }
>
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index c4a5e175b574..13a2166c4f41 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -302,7 +302,7 @@ pub fn ptr_eq(this: &Self, other: &Self) -> bool {
>      /// ```
>      /// use kernel::sync::{Arc, UniqueArc};
>      ///
> -    /// let arc = Arc::try_new(42)?;
> +    /// let arc = Arc::new(42, GFP_KERNEL)?;
>      /// let unique_arc = arc.into_unique_or_drop();
>      ///
>      /// // The above conversion should succeed since refcount of `arc` is 1.
> @@ -316,7 +316,7 @@ pub fn ptr_eq(this: &Self, other: &Self) -> bool {
>      /// ```
>      /// use kernel::sync::{Arc, UniqueArc};
>      ///
> -    /// let arc = Arc::try_new(42)?;
> +    /// let arc = Arc::new(42, GFP_KERNEL)?;
>      /// let another = arc.clone();
>      ///
>      /// let unique_arc = arc.into_unique_or_drop();