Re: [KernelAllocator] Usage of GlobalAlloc

[Gary Guo <gary@garyguo.net>]

On Fri, 21 May 2021 13:42:40 -0400
Hanqing Zhao <ouckarlzhao@gmail.com> wrote:

> While implementing an alloc crate, I intend to pass custom parameters
> and extra parameters such as
> memory flags (GFP_KERNEL, GFP_ATOMIC, etc) through the `Layout`
> parameter.

This sounds very wrong. `Layout` should only contain size and align,
but not other stuff. `Layout` is part of libcore, and it is a lang item
(aka part of the Rust language, tightly coupled to the compiler), so
you shouldn't change that.

> However, the current implementation of GlobalAlloc
> (https://github.com/Rust-for-Linux/linux/blob/rust/rust/kernel/allocator.rs#L13)
> is not actually used,
> because the kernel allocation directly resorts to `__rust_alloc`
> (
> https://github.com/Rust-for-Linux/linux/blob/rust/rust/kernel/allocator.rs\#L34),
> `__rust_dealloc`, etc.

This is a complicated. Rust will call `__rust_alloc` for all
allocations that use the global allocator. Rust will also generate
`__rust_alloc` automatically if the target is a binary, `.a` or `.so`,
but it wouldn't generate it for `.o`.

So we currently manually implement these functions. Perhaps we should
redirect `__rust_alloc` to `<KernelAllocator as GlobalAlloc>::alloc`,
but since `alloc` also just calls `bindings::krealloc` it does not
matter much.

> While implementing an alloc crate, I intend to pass custom parameters
> and extra parameters such as
> memory flags (GFP_KERNEL, GFP_ATOMIC, etc) through the `Layout`
> parameter.

Back to start. Why do you need to want to specify these through
`Layout`? As for the flags, we've discussed about this topic ~1 month
ago in the meeting
(https://lore.kernel.org/rust-for-linux/alpine.DEB.2.11.2104241558400.11174@titan.ldpreload.com/)
weeks ago in the meeting, and we've concluded that one should
invoke kmalloc themselves and use unsafe APIs to construct any
collections if they want sepcial flags (e.g. GFP_DMA).

A quick grep shows that in drivers/ there are 30k GFP_'s, and 27k of
them are GFP_KERNEL, 3k being GFP_ATOMIC, just 1k for the rest. If
possible we want to automatically detect the context and use
GFP_ATOMIC in interrupt contexts (see notes), but if it's not
possible probably we only need an `core::alloc::Alloc` that does
GFP_ATOMIC and do not need to have the allocator support arbitrary
flags.

- Gary Guo