Received-SPF: pass (google.com: domain of linux-kernel+bounces-120208-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240325195418.166013-1-wedsonaf@gmail.com> <20240325195418.166013-11-wedsonaf@gmail.com>
 <0LNSiT6NSGVXUXFMn80Z2GCSdjCmJfd1idFp6dTgEf4hfDyjNHq59jLZD9U6BcWVCo1QVvRvHb5Pjr7DOUlqnHabGeOAhmLguV0kGcruqS4=@proton.me>
In-Reply-To: <0LNSiT6NSGVXUXFMn80Z2GCSdjCmJfd1idFp6dTgEf4hfDyjNHq59jLZD9U6BcWVCo1QVvRvHb5Pjr7DOUlqnHabGeOAhmLguV0kGcruqS4=@proton.me>
From: Wedson Almeida Filho <wedsonaf@gmail.com>
Date: Tue, 26 Mar 2024 23:13:32 -0300
Message-ID: <CANeycqpCz=3Ab=K0JRFu6v0YvEU66ikK2B8KZbjC+sDd1BoqTA@mail.gmail.com>
Subject: Re: [PATCH 10/10] rust: kernel: remove usage of `allocator_api`
 unstable feature
To: Benno Lossin <benno.lossin@proton.me>
Cc: rust-for-linux@vger.kernel.org, Miguel Ojeda <ojeda@kernel.org>, 
	Alex Gaynor <alex.gaynor@gmail.com>, Boqun Feng <boqun.feng@gmail.com>, 
	Gary Guo <gary@garyguo.net>, =?UTF-8?Q?Bj=C3=B6rn_Roy_Baron?= <bjorn3_gh@protonmail.com>, 
	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 Tue, 26 Mar 2024 at 12:27, Benno Lossin <benno.lossin@proton.me> wrote:
>
> On 25.03.24 20:54, Wedson Almeida Filho wrote:
> > From: Wedson Almeida Filho <walmeida@microsoft.com>
> >
> > With the adoption of `BoxExt` and `VecExt`, we don't need the functions
> > provided by this feature (namely the methods prefixed with `try_` and
> > different allocator per collection instance).
> >
> > We do need `AllocError`, but we define our own as it is a trivial empty
> > struct.
> >
> > Signed-off-by: Wedson Almeida Filho <walmeida@microsoft.com>
> > ---
> >  rust/kernel/alloc.rs        |  3 +++
> >  rust/kernel/alloc/boxext.rs |  3 +--
> >  rust/kernel/alloc/vecext.rs |  4 ++--
> >  rust/kernel/error.rs        | 13 ++-----------
> >  rust/kernel/init.rs         |  3 +--
> >  rust/kernel/lib.rs          |  1 -
> >  rust/kernel/str.rs          |  3 +--
> >  rust/kernel/sync/arc.rs     |  4 ++--
> >  rust/kernel/workqueue.rs    |  3 +--
> >  9 files changed, 13 insertions(+), 24 deletions(-)
> >
> > diff --git a/rust/kernel/alloc.rs b/rust/kernel/alloc.rs
> > index 5712c81b1308..f17f48130f13 100644
> > --- a/rust/kernel/alloc.rs
> > +++ b/rust/kernel/alloc.rs
> > @@ -8,6 +8,9 @@
> >  pub mod boxext;
> >  pub mod vecext;
> >
> > +/// Indicates an allocation error.
> > +pub struct AllocError;
>
> I think it would be sensible to add this now:
>     #[derive(Copy, Clone, PartialEq, Eq, Debug)]

Agreed. Will add in v2.

> > +
> >  /// Flags to be used when allocating memory.
> >  ///
> >  /// They can be combined with the operators `|`, `&`, and `!`.
> > diff --git a/rust/kernel/alloc/boxext.rs b/rust/kernel/alloc/boxext.rs
> > index 26a918df7acf..3cdda0f635a3 100644
> > --- a/rust/kernel/alloc/boxext.rs
> > +++ b/rust/kernel/alloc/boxext.rs
> > @@ -2,9 +2,8 @@
> >
> >  //! Extensions to [`Box`] for fallible allocations.
> >
> > -use super::Flags;
> > +use super::{AllocError, Flags};
> >  use alloc::boxed::Box;
> > -use core::alloc::AllocError;
> >  use core::mem::MaybeUninit;
> >  use core::result::Result;
> >
> > diff --git a/rust/kernel/alloc/vecext.rs b/rust/kernel/alloc/vecext.rs
> > index 1d4d51b45a49..85ca6a6db6de 100644
> > --- a/rust/kernel/alloc/vecext.rs
> > +++ b/rust/kernel/alloc/vecext.rs
> > @@ -2,8 +2,8 @@
> >
> >  //! Extensions to [`Vec`] for fallible allocations.
> >
> > -use super::Flags;
> > -use alloc::{alloc::AllocError, vec::Vec};
> > +use super::{AllocError, Flags};
> > +use alloc::vec::Vec;
> >  use core::{mem::ManuallyDrop, result::Result};
> >
> >  /// Extensions to [`Vec`].
> > diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
> > index 4786d3ee1e92..fc986bc24c6d 100644
> > --- a/rust/kernel/error.rs
> > +++ b/rust/kernel/error.rs
> > @@ -4,12 +4,9 @@
> >  //!
> >  //! C header: [`include/uapi/asm-generic/errno-base.h`](srctree/include/uapi/asm-generic/errno-base.h)
> >
> > -use crate::str::CStr;
> > +use crate::{alloc::AllocError, str::CStr};
> >
> > -use alloc::{
> > -    alloc::{AllocError, LayoutError},
> > -    collections::TryReserveError,
> > -};
> > +use alloc::alloc::LayoutError;
> >
> >  use core::convert::From;
> >  use core::fmt;
> > @@ -192,12 +189,6 @@ fn from(_: Utf8Error) -> Error {
> >      }
> >  }
> >
> > -impl From<TryReserveError> for Error {
> > -    fn from(_: TryReserveError) -> Error {
> > -        code::ENOMEM
> > -    }
> > -}
> > -
>
> Why are you removing this?

We don't use `TryReserveError` anymore. I'll move this chunk to the
patch 7 in v2, which is where we remove the use of this error.

>
> --
> Cheers,
> Benno
>
> >  impl From<LayoutError> for Error {
> >      fn from(_: LayoutError) -> Error {
> >          code::ENOMEM
> > diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
> > index 1a8b0464db0c..1f7a6b5892ac 100644
> > --- a/rust/kernel/init.rs
> > +++ b/rust/kernel/init.rs
> > @@ -211,14 +211,13 @@
> >  //! [`pin_init!`]: crate::pin_init!
> >
> >  use crate::{
> > -    alloc::{boxext::BoxExt, Flags},
> > +    alloc::{boxext::BoxExt, AllocError, Flags},
> >      error::{self, Error},
> >      sync::UniqueArc,
> >      types::{Opaque, ScopeGuard},
> >  };
> >  use alloc::boxed::Box;
> >  use core::{
> > -    alloc::AllocError,
> >      cell::UnsafeCell,
> >      convert::Infallible,
> >      marker::PhantomData,
> > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> > index 51f30e55bd00..5c641233e26d 100644
> > --- a/rust/kernel/lib.rs
> > +++ b/rust/kernel/lib.rs
> > @@ -12,7 +12,6 @@
> >  //! do so first instead of bypassing this crate.
> >
> >  #![no_std]
> > -#![feature(allocator_api)]
> >  #![feature(coerce_unsized)]
> >  #![feature(dispatch_from_dyn)]
> >  #![feature(new_uninit)]
> > diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
> > index 34dbc85b5220..5206b18f882b 100644
> > --- a/rust/kernel/str.rs
> > +++ b/rust/kernel/str.rs
> > @@ -2,8 +2,7 @@
> >
> >  //! String representations.
> >
> > -use crate::alloc::{flags::*, vecext::VecExt};
> > -use alloc::alloc::AllocError;
> > +use crate::alloc::{flags::*, vecext::VecExt, AllocError};
> >  use alloc::vec::Vec;
> >  use core::fmt::{self, Write};
> >  use core::ops::{self, Deref, Index};
> > diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> > index af539c5eb4bc..8db25f73b37f 100644
> > --- a/rust/kernel/sync/arc.rs
> > +++ b/rust/kernel/sync/arc.rs
> > @@ -16,7 +16,7 @@
> >  //! [`Arc`]: https://doc.rust-lang.org/std/sync/struct.Arc.html
> >
> >  use crate::{
> > -    alloc::{boxext::BoxExt, Flags},
> > +    alloc::{boxext::BoxExt, AllocError, Flags},
> >      bindings,
> >      error::{self, Error},
> >      init::{self, InPlaceInit, Init, PinInit},
> > @@ -25,7 +25,7 @@
> >  };
> >  use alloc::boxed::Box;
> >  use core::{
> > -    alloc::{AllocError, Layout},
> > +    alloc::Layout,
> >      fmt,
> >      marker::{PhantomData, Unsize},
> >      mem::{ManuallyDrop, MaybeUninit},
> > diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
> > index 09a0f158ff1b..50837e56efc7 100644
> > --- a/rust/kernel/workqueue.rs
> > +++ b/rust/kernel/workqueue.rs
> > @@ -132,9 +132,8 @@
> >  //!
> >  //! C header: [`include/linux/workqueue.h`](srctree/include/linux/workqueue.h)
> >
> > -use crate::alloc::Flags;
> > +use crate::alloc::{AllocError, Flags};
> >  use crate::{bindings, prelude::*, sync::Arc, sync::LockClassKey, types::Opaque};
> > -use alloc::alloc::AllocError;
> >  use alloc::boxed::Box;
> >  use core::marker::PhantomData;
> >  use core::pin::Pin;
> > --
> > 2.34.1
> >