Messages in this thread | | | Date | Thu, 25 Apr 2024 20:42:41 +0200 | From | Danilo Krummrich <> | Subject | Re: [PATCH v3 00/10] Allocation APIs |
| |
On Thu, Apr 25, 2024 at 04:09:46PM +0000, Benno Lossin wrote: > On 25.04.24 17:36, Danilo Krummrich wrote: > > (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@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 > >> > >> 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. > > Bringing in some more context from the meeting: Gary suggested we create > a custom trait for allocators that can also handle allocation flags: > > pub trait AllocatorWithFlags: Allocator { > type Flags; > > fn allocate_with_flags(&self, layout: Layout, flags: Self::Flags) -> Result<NonNull<[u8]>, AllocError>; > > /* ... */ > } > > impl AllocatorWithFlags for Global { /* ... */ } > > impl<T, A> VecExt<T> for Vec<T, A> where A: AllocatorWithFlags { > /* ... */ > } > > I think that this would work, but we would have to ensure that users are > only allowed to call allocating functions if they are functions that we > control. For example `Vec::try_reserve` [1] would still use the normal > `Allocator` trait that doesn't support our flags. > Gary noted that this could be solved by `klint` [2].
I agree, extending the Allocator trait should work.
Regarding allocating functions we don't control, isn't that the case already? AFAICS, we're currently always falling back to GFP_KERNEL when calling Vec::try_reserve().
But yes, I also think it would be better to enforce being explicit.
Given that, is there any value extending the existing Allocator trait at all?
> > > But we only need to extend the allocator API, if you want to use the std > library types that allocate. If you would also be happy with a custom > newtype wrapper, then we could also do that.
What do you mean with "custom newtype wrapper"?
> I think that we probably want a more general solution (ie `Allocator` > enriched with flags), but we would have to design that before you can > use it. > > > [1]: https://doc.rust-lang.org/alloc/vec/struct.Vec.html#method.try_reserve > [2]: https://github.com/Rust-for-Linux/klint > > > > > 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. > > I think that you can write the `kmem_cache` abstraction without using > the allocator api. You just give the function that allocates a `flags` > argument like in C.
Guess you mean letting the kmem_cache implementation construct the corresponding container? Something like:
KmemCache<T>::alloc_box(flags: alloc::Flags) -> Box<T>
I think that'd make a lot of sense, since the size of an allocation is fixed anyways.
> > The `Allocator` API might make it more *convenient* to use it, because > you don't have to explicitly pass the flags every time (since the flags > are determined by the allocator). But I have also heard that it might be > desirable to always be explicit. > > -- > Cheers, > Benno > > > > > 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@nvidia.com/ > > >
| |