From: Gary Guo <gary@garyguo.net>
To: Hanqing Zhao <ouckarlzhao@gmail.com>
Cc: rust-for-linux <rust-for-linux@vger.kernel.org>
Subject: Re: [KernelAllocator] Usage of GlobalAlloc
Date: Sat, 22 May 2021 20:17:58 +0100 [thread overview]
Message-ID: <20210522201758.00006b1e@garyguo.net> (raw)
In-Reply-To: <CAKmvQ+UpHb0tBbYo9pR-YJa=SC3bd2T8pE_R8tLkOqKCsK9kXA@mail.gmail.com>
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
next prev parent reply other threads:[~2021-05-22 19:18 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-21 17:42 [KernelAllocator] Usage of GlobalAlloc Hanqing Zhao
2021-05-22 8:22 ` Gary Guo
[not found] ` <CAKmvQ+UpHb0tBbYo9pR-YJa=SC3bd2T8pE_R8tLkOqKCsK9kXA@mail.gmail.com>
2021-05-22 19:17 ` Gary Guo [this message]
2021-05-22 20:30 ` Hanqing Zhao
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210522201758.00006b1e@garyguo.net \
--to=gary@garyguo.net \
--cc=ouckarlzhao@gmail.com \
--cc=rust-for-linux@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).