[KernelAllocator] Usage of GlobalAlloc

[KernelAllocator] Usage of GlobalAlloc

[Hanqing Zhao]

Hi all,

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.

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.

In this way, it hinders me to use the `layout` parameter.
Is this intended or avoidable?

Best,
Hanqing Zhao
Georgia Tech

Re: [KernelAllocator] Usage of GlobalAlloc

[Gary Guo]

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

Re: [KernelAllocator] Usage of GlobalAlloc

[Gary Guo]

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))
```

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

Re: [KernelAllocator] Usage of GlobalAlloc

[Hanqing Zhao]

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