Re: [KernelAllocator] Usage of GlobalAlloc

[Hanqing Zhao <ouckarlzhao@gmail.com>]

On Sat, May 22, 2021 at 3:18 PM Gary Guo <gary@garyguo.net> wrote:
>
> On Sat, 22 May 2021 06:46:41 -0400
> Hanqing Zhao <ouckarlzhao@gmail.com> wrote:
>
> > On Sat, May 22, 2021 at 4:22 AM Gary Guo <gary@garyguo.net> wrote:
> > >
> > > 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.
> > >
> >
> > Yes. I actually modified the rust library for some experimental
> > safety features.
>
> `core` is tightly coupled with the version of rustc, so unless the
> feature you are suggesting will likely ended up in upstream Rust, I
> doubt that we can use the modification.
>
> `Layout` is truly just for layout of the type, so don't put any
> irrelevant bits there. You can experiment on designs of a new allocator
> APIs, but please don't change `Layout`.
>
> > > > 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
> > > GlobalAlloc>`bindings::krealloc` it does not
> > > matter much.
> > >
> >
> > Because I do not use `kmalloc` as a backend, I probably need to find
> > alternative ways to
> > enable the usage of GlobalAlloc. It would be better if I can find a
> > way to pass layout as parameter
> > instead of the size and align parameter for `__rust_alloc`.
>
>
> Just replace the code of `__rust_alloc` with
> `KernelAllocator.alloc(Layout::from_size_align_unchecked(size,
> align))`, or whatever other type that implements `GlobalAlloc`.
>
> > > > 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).
> > >
> >
> > memory flags are one of my concerns. I am also passing some parameters
> > for demonstrating
> > experimental security features.
>
>
> Why are you trying to use `GlobalAlloc` though? Have you looked into
> the `Allocator` API? There's nothing preventing you to add a flag there:
> ```
> struct MyAlloc(u32);
>
> impl Allocator for MyAlloc { /*blah*/ }
>
> Box::try_new_in(blah, MyAlloc(myflag))
> ```
>

Because I want to have a drop-in replacement for benchmarking.
After getting those performance numbers, then I can introduce it as
a new allocator api.

> >
> > My implementation actually does not rely on `kmalloc`, the memory
> > allocation can be
> > managed in Rust independently and does not need to be handled by
> > kernel slab.
> >
> > > 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.
> > >
> >
> > Preventing the sleep-in-atomic-context bugs is also one of my
> > concerns. For the memory allocation occured in rust, we can
> > automatically adjust the flags through some new abstractions like
> > something called `atomic_context` that takes as input a clouse.
> > Although we still need to come up with a way to deal with sleepale
> > APIs.
> >
>
> I had a thought about that recently and I believe that we can probably
> make use of static analysis (or Rust pluggable lints) to handle
> sleepable APIs.
>
> Please use "Reply All" next time. You didn't CC the mailing list.
>
> - Gary
>

Best,
Hanqing Zhao
Georgia Tech