Received-SPF: pass (google.com: domain of linux-kernel+bounces-158923-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Thu, 25 Apr 2024 16:09:46 +0000
To: Danilo Krummrich <dakr@redhat.com>, Wedson Almeida Filho <wedsonaf@gmail.com>, Zhi Wang <zhiw@nvidia.com>, rust-for-linux@vger.kernel.org
From: Benno Lossin <benno.lossin@proton.me>
Cc: 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>, ajanulgu@redhat.com, Andy Currid <acurrid@nvidia.com>, Neo Jia <cjia@nvidia.com>, John Hubbard <jhubbard@nvidia.com>
Subject: Re: [PATCH v3 00/10] Allocation APIs
Message-ID: <74cbdaf7-360e-47e3-bda4-4661422a11ae@proton.me>
In-Reply-To: <Zip4dFpt8xhk7UBL@cassiopeiae>
References: <Zip4dFpt8xhk7UBL@cassiopeiae>
Feedback-ID: 71780778:user:proton
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable

On 25.04.24 17:36, Danilo Krummrich wrote:
> (adding folks from [1])
>=20
> 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?
>=20
> I want to follow up on this topic after also bringing it up in yesterday'=
s
> weekly Rust call.
>=20
> In the call a few ideas were discussed, e.g. whether we could just re-ena=
ble the
> allocator_api feature and try getting it stabilized.
>=20
> With the introduction of alloc::Flags (gfp_t abstraction) allocator_api m=
ight
> 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;
        =20
         fn allocate_with_flags(&self, layout: Layout, flags: Self::Flags) =
-> Result<NonNull<[u8]>, AllocError>;

         /* ... */
     }
    =20
     impl AllocatorWithFlags for Global { /* ... */ }
    =20
     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].


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.
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

>=20
> I think it would work for (k)vmalloc, where we could pass the page flags =
through
> const generics for instance.
>=20
> But I don't see how it could work with kmem_cache, where we can't just cr=
eate 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.

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.

--=20
Cheers,
Benno

>=20
> So, I think we have to create our own allocator trait / API.
>=20
> Any other thoughts on that?
>=20
> - Danilo
>=20
> [1] https://lore.kernel.org/rust-for-linux/20240408094738.00005e59.zhiw@n=
vidia.com/
>=20