Re: [PATCH v3 00/10] Allocation APIs

From: Danilo Krummrich
Date: Thu Apr 25 2024 - 11:36:45 EST

Next message: Peter Yin: "[PATCH v2 0/2] hwmon: (pmbus) Add support for Infineon XDP710"
Previous message: Conor Dooley: "Re: [PATCH v1] iio: adc: PAC1934: fix accessing out of bounds array index"
Next in thread: Benno Lossin: "Re: [PATCH v3 00/10] Allocation APIs"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

(adding folks from [1])

On Tue, Apr 23, 2024 at 05:43:08PM +0200, Danilo Krummrich wrote:
> Hi all,
>
> On 3/28/24 02:35, Wedson Almeida Filho wrote:
> > From: Wedson Almeida Filho <walmeida@xxxxxxxxxxxxx>
> >
> > 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
>
> With that series, how do we implement alternative allocators, such as
> (k)vmalloc or DMA coherent?
>
> For instance, I recently sketched up some firmware bindings we want to
> use in Nova providing
>
> fn copy<A: core::alloc::Allocator>(&self, alloc: A) -> Result<Vec<u8, A>>
> [1]
>
> making use of Vec::try_with_capacity_in(). How would I implement
> something similar now?

I want to follow up on this topic after also bringing it up in yesterday's
weekly Rust call.

In the call a few ideas were discussed, e.g. whether we could just re-enable the
allocator_api feature and try getting it stabilized.

With the introduction of alloc::Flags (gfp_t abstraction) allocator_api might
not be a viable choice anymore.

I think it would work for (k)vmalloc, where we could pass the page flags through
const generics for instance.

But I don't see how it could work with kmem_cache, where we can't just create a
new allocator instance when we want to change the page flags, but need to
support allocations with different page flags on the same allocator (same
kmem_cache) instance.

So, I think we have to create our own allocator trait / API.

Any other thoughts on that?

- Danilo

[1] https://lore.kernel.org/rust-for-linux/20240408094738.00005e59.zhiw@xxxxxxxxxx/

>
> - Danilo
>
> [1] https://gitlab.freedesktop.org/drm/nova/-/blob/topic/firmware/rust/kernel/firmware.rs?ref_type=heads#L63
>
> >
> > ---
> >
> > Changes in v3:
> > - Rebased on top of the latest `rust-next` branch.
> > - Updated `krealloc_aligned` to use `Flags` instead of `bindings::gfp_t`.
> > - Added __GFP_ZERO to flags, as part of the previous change.
> > - Avoiding temporary stack value in `Box::new_uninit`.
> > - Implement `Box::new` using `Box::new_uninit` (so only one of them actually
> > allocates).
> > - Added examples/tests to `VecExt` methods.
> > - Fixed bug in length in `extend_from_slice`
> > - Link to v2: https://lore.kernel.org/rust-for-linux/20240327023531.187880-1-wedsonaf@xxxxxxxxx/T/#t
> >
> > Changes in v2:
> > - Updated description of `alloc` crate.
> > - Renamed vecext and boxext modules to vec_ext and box_ext.
> > - Added derive directive to `AllocError`.
> > - Updated safety comment in `BoxExt::new`.
> > - Updated `VecExt::push` and `VecExt::extend_from_slice` to use
> > `spare_capacity_mut`
> > - Added directive to not compile `destructure` and `rebuild` when `test` or
> > `testlib` are configured. Otherwise we have a warning because `push` and
> > `extend_from_slice` don't use them anymore.
> > - Updated indentation in `Arc::new_uninit`
> > - Moved the removal of `TryReserveError` convesion to `Error` to patch 7, where
> > usage of `TryReserveError` is actually removed.
> > - Link to v1: https://lore.kernel.org/rust-for-linux/20240325195418.166013-1-wedsonaf@xxxxxxxxx/T/#t
> >
> > Wedson Almeida Filho (10):
> > rust: kernel: move `allocator` module under `alloc`
> > rust: alloc: introduce the `VecExt` trait
> > kbuild: use the upstream `alloc` crate
> > rust: alloc: remove our fork of the `alloc` crate
> > rust: alloc: introduce allocation flags
> > rust: alloc: introduce the `BoxExt` trait
> > rust: alloc: update `VecExt` to take allocation flags
> > rust: sync: update `Arc` and `UniqueArc` to take allocation flags
> > rust: init: update `init` module to take allocation flags
> > rust: kernel: remove usage of `allocator_api` unstable feature
> >
> > rust/Makefile | 16 +-
> > rust/alloc/README.md | 36 -
> > rust/alloc/alloc.rs | 452 ----
> > rust/alloc/boxed.rs | 2463 -----------------
> > rust/alloc/collections/mod.rs | 160 --
> > rust/alloc/lib.rs | 288 --
> > rust/alloc/raw_vec.rs | 611 -----
> > rust/alloc/slice.rs | 890 -------
> > rust/alloc/vec/drain.rs | 255 --
> > rust/alloc/vec/extract_if.rs | 115 -
> > rust/alloc/vec/into_iter.rs | 454 ----
> > rust/alloc/vec/is_zero.rs | 204 --
> > rust/alloc/vec/mod.rs | 3683 --------------------------
> > rust/alloc/vec/partial_eq.rs | 49 -
> > rust/alloc/vec/set_len_on_drop.rs | 35 -
> > rust/alloc/vec/spec_extend.rs | 119 -
> > rust/bindings/bindings_helper.h | 3 +
> > rust/kernel/alloc.rs | 74 +
> > rust/kernel/{ => alloc}/allocator.rs | 17 +-
> > rust/kernel/alloc/box_ext.rs | 59 +
> > rust/kernel/alloc/vec_ext.rs | 176 ++
> > rust/kernel/error.rs | 13 +-
> > rust/kernel/init.rs | 57 +-
> > rust/kernel/lib.rs | 5 +-
> > rust/kernel/prelude.rs | 2 +
> > rust/kernel/str.rs | 6 +-
> > rust/kernel/sync/arc.rs | 50 +-
> > rust/kernel/sync/condvar.rs | 2 +-
> > rust/kernel/sync/lock/mutex.rs | 2 +-
> > rust/kernel/sync/lock/spinlock.rs | 2 +-
> > rust/kernel/types.rs | 4 +-
> > rust/kernel/workqueue.rs | 14 +-
> > samples/rust/rust_minimal.rs | 6 +-
> > samples/rust/rust_print.rs | 4 +-
> > scripts/generate_rust_analyzer.py | 2 +-
> > 35 files changed, 405 insertions(+), 9923 deletions(-)
> > delete mode 100644 rust/alloc/README.md
> > delete mode 100644 rust/alloc/alloc.rs
> > delete mode 100644 rust/alloc/boxed.rs
> > delete mode 100644 rust/alloc/collections/mod.rs
> > delete mode 100644 rust/alloc/lib.rs
> > delete mode 100644 rust/alloc/raw_vec.rs
> > delete mode 100644 rust/alloc/slice.rs
> > delete mode 100644 rust/alloc/vec/drain.rs
> > delete mode 100644 rust/alloc/vec/extract_if.rs
> > delete mode 100644 rust/alloc/vec/into_iter.rs
> > delete mode 100644 rust/alloc/vec/is_zero.rs
> > delete mode 100644 rust/alloc/vec/mod.rs
> > delete mode 100644 rust/alloc/vec/partial_eq.rs
> > delete mode 100644 rust/alloc/vec/set_len_on_drop.rs
> > delete mode 100644 rust/alloc/vec/spec_extend.rs
> > create mode 100644 rust/kernel/alloc.rs
> > rename rust/kernel/{ => alloc}/allocator.rs (86%)
> > create mode 100644 rust/kernel/alloc/box_ext.rs
> > create mode 100644 rust/kernel/alloc/vec_ext.rs
> >
> >
> > base-commit: 768409cff6cc89fe1194da880537a09857b6e4db

Next message: Peter Yin: "[PATCH v2 0/2] hwmon: (pmbus) Add support for Infineon XDP710"
Previous message: Conor Dooley: "Re: [PATCH v1] iio: adc: PAC1934: fix accessing out of bounds array index"
Next in thread: Benno Lossin: "Re: [PATCH v3 00/10] Allocation APIs"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]