| Message ID | 20240812182355.11641-1-dakr@kernel.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | Generic `Allocator` support for Rust | expand |
Hi Danilo, On Mon, Aug 12, 2024 at 08:22:46PM +0200, Danilo Krummrich wrote: > Hi, > > This patch series adds generic kernel allocator support for Rust, which so far > is limited to `kmalloc` allocations. > > In order to abstain from (re-)adding unstable Rust features to the kernel, this > patch series does not extend the `Allocator` trait from Rust's `alloc` crate, > nor does it extend the `BoxExt` and `VecExt` extensions. > > Instead, this series introduces a kernel specific `Allocator` trait, which is > implemented by the `Kmalloc`, `Vmalloc` and `KVmalloc` allocators, also > implemented in the context of this series. > > As a consequence we need our own kernel `Box<T, A>` and `Vec<T, A>` types. > Additionally, this series adds the following type aliases: > > ``` > pub type KBox<T> = Box<T, Kmalloc>; > pub type VBox<T> = Box<T, Vmalloc>; > pub type KVBox<T> = Box<T, KVmalloc>; > > > pub type KVec<T> = Vec<T, Kmalloc>; > pub type VVec<T> = Vec<T, Vmalloc>; > pub type KVVec<T> = Vec<T, KVmalloc>; > ``` > > With that, we can start using the kernel `Box` and `Vec` types throughout the > tree and remove the now obolete extensions `BoxExt` and `VecExt`. > > For a final cleanup, this series removes the last minor dependencies to Rust's > `alloc` crate and removes it from the entire kernel build. > > The series ensures not to break the `rusttest` make target by implementing the > `allocator_test` module providing a stub implementation for all kernel > `Allocator`s. > > This patch series passes all KUnit tests, including the ones added by this > series. Additionally, the tests were run with `kmemleak` and `KASAN` enabled, > without any issues. > > This series is based on [1], which hit -mm/mm-unstable, and is also available > in [2]. > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/log/?h=mm/krealloc > [2] https://git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/log/?h=rust/mm > > Changes in v5: > - (safety) comment / documentation fixes suggested by Alice, Benno and Gary > - remove `Unique<T>` and implement `Send` and `Sync` for `Box` and `Vec` > - use `KMALLOC_MAX_SIZE` for `KVmalloc` test and add a `Kmalloc` test that > expects to fail for `KMALLOC_MAX_SIZE` > - create use constants `KREALLOC`, `VREALLOC` and `KVREALLOC` for > `ReallocFuncs` > - drop `Box::drop_contents` for now, will add it again, once I actually rebase > on the original patch that introduces it I'm trying to put your series on rust-dev, but I hit a few conflicts due to the conflict with `Box::drop_contents`, which has been in rust-dev for a while. And the conflict is not that trivial for me to resolve. So just a head-up, that's a requirement for me to put it on rust-dev for more tests from my end ;-) Regards, Boqun > - improve usage of `size_of_val` in `Box` > - move `InPlaceInit` and `ForeignOwnable` impls into kbox.rs > - fix missing `Box` conversions in rnull.rs > - reworked `Cmalloc` to keep track of the size of memory allocations itself > - remove `GlobalAlloc` together with the `alloc` crate to avoid a linker error > - remove `alloc` from scripts/generate_rust_analyzer.py > [...]
On 8/14/24 9:32 PM, Boqun Feng wrote: > Hi Danilo, > > On Mon, Aug 12, 2024 at 08:22:46PM +0200, Danilo Krummrich wrote: >> Hi, >> >> This patch series adds generic kernel allocator support for Rust, which so far >> is limited to `kmalloc` allocations. >> >> In order to abstain from (re-)adding unstable Rust features to the kernel, this >> patch series does not extend the `Allocator` trait from Rust's `alloc` crate, >> nor does it extend the `BoxExt` and `VecExt` extensions. >> >> Instead, this series introduces a kernel specific `Allocator` trait, which is >> implemented by the `Kmalloc`, `Vmalloc` and `KVmalloc` allocators, also >> implemented in the context of this series. >> >> As a consequence we need our own kernel `Box<T, A>` and `Vec<T, A>` types. >> Additionally, this series adds the following type aliases: >> >> ``` >> pub type KBox<T> = Box<T, Kmalloc>; >> pub type VBox<T> = Box<T, Vmalloc>; >> pub type KVBox<T> = Box<T, KVmalloc>; >> >> >> pub type KVec<T> = Vec<T, Kmalloc>; >> pub type VVec<T> = Vec<T, Vmalloc>; >> pub type KVVec<T> = Vec<T, KVmalloc>; >> ``` >> >> With that, we can start using the kernel `Box` and `Vec` types throughout the >> tree and remove the now obolete extensions `BoxExt` and `VecExt`. >> >> For a final cleanup, this series removes the last minor dependencies to Rust's >> `alloc` crate and removes it from the entire kernel build. >> >> The series ensures not to break the `rusttest` make target by implementing the >> `allocator_test` module providing a stub implementation for all kernel >> `Allocator`s. >> >> This patch series passes all KUnit tests, including the ones added by this >> series. Additionally, the tests were run with `kmemleak` and `KASAN` enabled, >> without any issues. >> >> This series is based on [1], which hit -mm/mm-unstable, and is also available >> in [2]. >> >> [1] https://git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/log/?h=mm/krealloc >> [2] https://git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/log/?h=rust/mm >> >> Changes in v5: >> - (safety) comment / documentation fixes suggested by Alice, Benno and Gary >> - remove `Unique<T>` and implement `Send` and `Sync` for `Box` and `Vec` >> - use `KMALLOC_MAX_SIZE` for `KVmalloc` test and add a `Kmalloc` test that >> expects to fail for `KMALLOC_MAX_SIZE` >> - create use constants `KREALLOC`, `VREALLOC` and `KVREALLOC` for >> `ReallocFuncs` >> - drop `Box::drop_contents` for now, will add it again, once I actually rebase >> on the original patch that introduces it > > I'm trying to put your series on rust-dev, but I hit a few conflicts due > to the conflict with `Box::drop_contents`, which has been in rust-dev > for a while. And the conflict is not that trivial for me to resolve. > So just a head-up, that's a requirement for me to put it on rust-dev for > more tests from my end ;-) I'll rebase later on and send you a branch. > > Regards, > Boqun > >> - improve usage of `size_of_val` in `Box` >> - move `InPlaceInit` and `ForeignOwnable` impls into kbox.rs >> - fix missing `Box` conversions in rnull.rs >> - reworked `Cmalloc` to keep track of the size of memory allocations itself >> - remove `GlobalAlloc` together with the `alloc` crate to avoid a linker error >> - remove `alloc` from scripts/generate_rust_analyzer.py >> > [...] >
On Wed, Aug 14, 2024 at 12:32:15PM -0700, Boqun Feng wrote: > Hi Danilo, > > I'm trying to put your series on rust-dev, but I hit a few conflicts due > to the conflict with `Box::drop_contents`, which has been in rust-dev > for a while. And the conflict is not that trivial for me to resolve. > So just a head-up, that's a requirement for me to put it on rust-dev for > more tests from my end ;-) I rebased everything and you can fetch them from [1]. I resolved the following conflicts: - for `Box`, implement - `drop_contents` - `manually_drop_contents` [2] - ``move_out` [2] - `BorrowedMut` for `ForeignOwnable` for `Box<T, A>` and `Pin<Box<T, A>>` - `InPlaceWrite` and updated `InPlaceInit` - for `RBTreeNode`, make use of `Box::move_out` to replace the original implementation partially moving out of `Box` @Alice: Please have a look at the changes for `RBTreeNode`. Maybe it's also worth having them in a separate patch. - Danilo [1] https://git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/log/?h=rust-dev/mm [2] https://git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/commit/?h=rust-dev/mm&id=ef80ccca2ccebf3c7bcafdc13d1bfe81341cbe63 [3] https://git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/diff/rust/kernel/rbtree.rs?h=rust-dev/mm&id=c361d66df7fb7760064fbca6bf9d72171c352a73 > > Regards, > Boqun
On Thu, Aug 15, 2024 at 4:52 AM Danilo Krummrich <dakr@kernel.org> wrote: > > On Wed, Aug 14, 2024 at 12:32:15PM -0700, Boqun Feng wrote: > > Hi Danilo, > > > > I'm trying to put your series on rust-dev, but I hit a few conflicts due > > to the conflict with `Box::drop_contents`, which has been in rust-dev > > for a while. And the conflict is not that trivial for me to resolve. > > So just a head-up, that's a requirement for me to put it on rust-dev for > > more tests from my end ;-) > > I rebased everything and you can fetch them from [1]. > > I resolved the following conflicts: > > - for `Box`, implement > - `drop_contents` > - `manually_drop_contents` [2] Not sure I like this name. It sounds like something that runs the destructor, but it does the exact opposite. > - ``move_out` [2] > - `BorrowedMut` for `ForeignOwnable` for `Box<T, A>` and `Pin<Box<T, A>>` > - `InPlaceWrite` and updated `InPlaceInit` > - for `RBTreeNode`, make use of `Box::move_out` to replace the original > implementation partially moving out of `Box` > > @Alice: Please have a look at the changes for `RBTreeNode`. Maybe it's also > worth having them in a separate patch. RBTree changes LGTM. Alice
On Thu, Aug 15, 2024 at 11:20:32AM +0200, Alice Ryhl wrote: > On Thu, Aug 15, 2024 at 4:52 AM Danilo Krummrich <dakr@kernel.org> wrote: > > > > On Wed, Aug 14, 2024 at 12:32:15PM -0700, Boqun Feng wrote: > > > Hi Danilo, > > > > > > I'm trying to put your series on rust-dev, but I hit a few conflicts due > > > to the conflict with `Box::drop_contents`, which has been in rust-dev > > > for a while. And the conflict is not that trivial for me to resolve. > > > So just a head-up, that's a requirement for me to put it on rust-dev for > > > more tests from my end ;-) > > > > I rebased everything and you can fetch them from [1]. > > > > I resolved the following conflicts: > > > > - for `Box`, implement > > - `drop_contents` > > - `manually_drop_contents` [2] > > Not sure I like this name. It sounds like something that runs the > destructor, but it does the exact opposite. I thought it kinda makes sense, since it's analogous to `ManuallyDrop::new`. What about `Box::forget_contents` instead? > > > - ``move_out` [2] > > - `BorrowedMut` for `ForeignOwnable` for `Box<T, A>` and `Pin<Box<T, A>>` > > - `InPlaceWrite` and updated `InPlaceInit` > > - for `RBTreeNode`, make use of `Box::move_out` to replace the original > > implementation partially moving out of `Box` > > > > @Alice: Please have a look at the changes for `RBTreeNode`. Maybe it's also > > worth having them in a separate patch. > > RBTree changes LGTM. > > Alice >
On Thu, Aug 15, 2024 at 2:33 PM Danilo Krummrich <dakr@kernel.org> wrote: > > On Thu, Aug 15, 2024 at 11:20:32AM +0200, Alice Ryhl wrote: > > On Thu, Aug 15, 2024 at 4:52 AM Danilo Krummrich <dakr@kernel.org> wrote: > > > > > > On Wed, Aug 14, 2024 at 12:32:15PM -0700, Boqun Feng wrote: > > > > Hi Danilo, > > > > > > > > I'm trying to put your series on rust-dev, but I hit a few conflicts due > > > > to the conflict with `Box::drop_contents`, which has been in rust-dev > > > > for a while. And the conflict is not that trivial for me to resolve. > > > > So just a head-up, that's a requirement for me to put it on rust-dev for > > > > more tests from my end ;-) > > > > > > I rebased everything and you can fetch them from [1]. > > > > > > I resolved the following conflicts: > > > > > > - for `Box`, implement > > > - `drop_contents` > > > - `manually_drop_contents` [2] > > > > Not sure I like this name. It sounds like something that runs the > > destructor, but it does the exact opposite. > > I thought it kinda makes sense, since it's analogous to `ManuallyDrop::new`. > > What about `Box::forget_contents` instead? One option is `into_manually_drop`. This uses the convention of using the `into_*` prefix for conversions that take ownership of the original value. Alice
On Thu, Aug 15, 2024 at 02:34:50PM +0200, Alice Ryhl wrote: > On Thu, Aug 15, 2024 at 2:33 PM Danilo Krummrich <dakr@kernel.org> wrote: > > > > On Thu, Aug 15, 2024 at 11:20:32AM +0200, Alice Ryhl wrote: > > > On Thu, Aug 15, 2024 at 4:52 AM Danilo Krummrich <dakr@kernel.org> wrote: > > > > > > > > On Wed, Aug 14, 2024 at 12:32:15PM -0700, Boqun Feng wrote: > > > > > Hi Danilo, > > > > > > > > > > I'm trying to put your series on rust-dev, but I hit a few conflicts due > > > > > to the conflict with `Box::drop_contents`, which has been in rust-dev > > > > > for a while. And the conflict is not that trivial for me to resolve. > > > > > So just a head-up, that's a requirement for me to put it on rust-dev for > > > > > more tests from my end ;-) > > > > > > > > I rebased everything and you can fetch them from [1]. > > > > > > > > I resolved the following conflicts: > > > > > > > > - for `Box`, implement > > > > - `drop_contents` > > > > - `manually_drop_contents` [2] > > > > > > Not sure I like this name. It sounds like something that runs the > > > destructor, but it does the exact opposite. > > > > I thought it kinda makes sense, since it's analogous to `ManuallyDrop::new`. > > > > What about `Box::forget_contents` instead? > > One option is `into_manually_drop`. This uses the convention of using > the `into_*` prefix for conversions that take ownership of the > original value. The signature of the current `Box::manually_drop_contents` is the same as for `Box::drop_contents`, namely `fn manually_drop_contents(this: Self) -> Box<MaybeUninit<T>, A>`. `into_manually_drop` seems misleading for for returning a `Box<MaybeUninit<T>, A>`. I still think `forget_contents` hits it quite well. Just as `drop_contents` drops the value, `forget_contents` makes the `Box` forget the value. > > Alice >
On 15.08.24 15:33, Danilo Krummrich wrote: > On Thu, Aug 15, 2024 at 02:34:50PM +0200, Alice Ryhl wrote: >> On Thu, Aug 15, 2024 at 2:33 PM Danilo Krummrich <dakr@kernel.org> wrote: >>> >>> On Thu, Aug 15, 2024 at 11:20:32AM +0200, Alice Ryhl wrote: >>>> On Thu, Aug 15, 2024 at 4:52 AM Danilo Krummrich <dakr@kernel.org> wrote: >>>>> >>>>> On Wed, Aug 14, 2024 at 12:32:15PM -0700, Boqun Feng wrote: >>>>>> Hi Danilo, >>>>>> >>>>>> I'm trying to put your series on rust-dev, but I hit a few conflicts due >>>>>> to the conflict with `Box::drop_contents`, which has been in rust-dev >>>>>> for a while. And the conflict is not that trivial for me to resolve. >>>>>> So just a head-up, that's a requirement for me to put it on rust-dev for >>>>>> more tests from my end ;-) >>>>> >>>>> I rebased everything and you can fetch them from [1]. >>>>> >>>>> I resolved the following conflicts: >>>>> >>>>> - for `Box`, implement >>>>> - `drop_contents` >>>>> - `manually_drop_contents` [2] >>>> >>>> Not sure I like this name. It sounds like something that runs the >>>> destructor, but it does the exact opposite. >>> >>> I thought it kinda makes sense, since it's analogous to `ManuallyDrop::new`. >>> >>> What about `Box::forget_contents` instead? >> >> One option is `into_manually_drop`. This uses the convention of using >> the `into_*` prefix for conversions that take ownership of the >> original value. > > The signature of the current `Box::manually_drop_contents` is the same as for > `Box::drop_contents`, namely > `fn manually_drop_contents(this: Self) -> Box<MaybeUninit<T>, A>`. > > `into_manually_drop` seems misleading for for returning a > `Box<MaybeUninit<T>, A>`. > > I still think `forget_contents` hits it quite well. Just as `drop_contents` > drops the value, `forget_contents` makes the `Box` forget the value. I think `forget_contents` sounds good. Can you please add some more docs to that function though? Like an example and change "Manually drops the contents, but keeps the allocation" to "Forgets the contents (does not run the destructor), but keeps the allocation.". Another thing that I spotted while looking at the patch, `move_out` doesn't need the `transmute_copy`, you should be able to just call `read` on the pointer. --- Cheers, Benno
On Thu, Aug 15, 2024 at 01:39:05PM +0000, Benno Lossin wrote: > On 15.08.24 15:33, Danilo Krummrich wrote: > > On Thu, Aug 15, 2024 at 02:34:50PM +0200, Alice Ryhl wrote: > >> On Thu, Aug 15, 2024 at 2:33 PM Danilo Krummrich <dakr@kernel.org> wrote: > >>> > >>> On Thu, Aug 15, 2024 at 11:20:32AM +0200, Alice Ryhl wrote: > >>>> On Thu, Aug 15, 2024 at 4:52 AM Danilo Krummrich <dakr@kernel.org> wrote: > >>>>> > >>>>> On Wed, Aug 14, 2024 at 12:32:15PM -0700, Boqun Feng wrote: > >>>>>> Hi Danilo, > >>>>>> > >>>>>> I'm trying to put your series on rust-dev, but I hit a few conflicts due > >>>>>> to the conflict with `Box::drop_contents`, which has been in rust-dev > >>>>>> for a while. And the conflict is not that trivial for me to resolve. > >>>>>> So just a head-up, that's a requirement for me to put it on rust-dev for > >>>>>> more tests from my end ;-) > >>>>> > >>>>> I rebased everything and you can fetch them from [1]. > >>>>> > >>>>> I resolved the following conflicts: > >>>>> > >>>>> - for `Box`, implement > >>>>> - `drop_contents` > >>>>> - `manually_drop_contents` [2] > >>>> > >>>> Not sure I like this name. It sounds like something that runs the > >>>> destructor, but it does the exact opposite. > >>> > >>> I thought it kinda makes sense, since it's analogous to `ManuallyDrop::new`. > >>> > >>> What about `Box::forget_contents` instead? > >> > >> One option is `into_manually_drop`. This uses the convention of using > >> the `into_*` prefix for conversions that take ownership of the > >> original value. > > > > The signature of the current `Box::manually_drop_contents` is the same as for > > `Box::drop_contents`, namely > > `fn manually_drop_contents(this: Self) -> Box<MaybeUninit<T>, A>`. > > > > `into_manually_drop` seems misleading for for returning a > > `Box<MaybeUninit<T>, A>`. > > > > I still think `forget_contents` hits it quite well. Just as `drop_contents` > > drops the value, `forget_contents` makes the `Box` forget the value. > > I think `forget_contents` sounds good. Can you please add some more docs > to that function though? Like an example and change "Manually drops the > contents, but keeps the allocation" to "Forgets the contents (does not > run the destructor), but keeps the allocation.". I can't really think of a good real world example other than moving out of a `Box`, can you? Otherwise, maybe it just shouldn't be public? > > Another thing that I spotted while looking at the patch, `move_out` > doesn't need the `transmute_copy`, you should be able to just call > `read` on the pointer. While technically it's the same I thought `transmute_copy` is considered better, since it has less stict safety requirements? For `transmute_copy` we only need to say, that dst has the same type as src, whereas for `read` the pointer must be valid for reads, properly aligned and point to an initialized value. > > --- > Cheers, > Benno >
On 15.08.24 16:09, Danilo Krummrich wrote: > On Thu, Aug 15, 2024 at 01:39:05PM +0000, Benno Lossin wrote: >> On 15.08.24 15:33, Danilo Krummrich wrote: >>> On Thu, Aug 15, 2024 at 02:34:50PM +0200, Alice Ryhl wrote: >>>> On Thu, Aug 15, 2024 at 2:33 PM Danilo Krummrich <dakr@kernel.org> wrote: >>>>> >>>>> On Thu, Aug 15, 2024 at 11:20:32AM +0200, Alice Ryhl wrote: >>>>>> On Thu, Aug 15, 2024 at 4:52 AM Danilo Krummrich <dakr@kernel.org> wrote: >>>>>>> >>>>>>> On Wed, Aug 14, 2024 at 12:32:15PM -0700, Boqun Feng wrote: >>>>>>>> Hi Danilo, >>>>>>>> >>>>>>>> I'm trying to put your series on rust-dev, but I hit a few conflicts due >>>>>>>> to the conflict with `Box::drop_contents`, which has been in rust-dev >>>>>>>> for a while. And the conflict is not that trivial for me to resolve. >>>>>>>> So just a head-up, that's a requirement for me to put it on rust-dev for >>>>>>>> more tests from my end ;-) >>>>>>> >>>>>>> I rebased everything and you can fetch them from [1]. >>>>>>> >>>>>>> I resolved the following conflicts: >>>>>>> >>>>>>> - for `Box`, implement >>>>>>> - `drop_contents` >>>>>>> - `manually_drop_contents` [2] >>>>>> >>>>>> Not sure I like this name. It sounds like something that runs the >>>>>> destructor, but it does the exact opposite. >>>>> >>>>> I thought it kinda makes sense, since it's analogous to `ManuallyDrop::new`. >>>>> >>>>> What about `Box::forget_contents` instead? >>>> >>>> One option is `into_manually_drop`. This uses the convention of using >>>> the `into_*` prefix for conversions that take ownership of the >>>> original value. >>> >>> The signature of the current `Box::manually_drop_contents` is the same as for >>> `Box::drop_contents`, namely >>> `fn manually_drop_contents(this: Self) -> Box<MaybeUninit<T>, A>`. >>> >>> `into_manually_drop` seems misleading for for returning a >>> `Box<MaybeUninit<T>, A>`. >>> >>> I still think `forget_contents` hits it quite well. Just as `drop_contents` >>> drops the value, `forget_contents` makes the `Box` forget the value. >> >> I think `forget_contents` sounds good. Can you please add some more docs >> to that function though? Like an example and change "Manually drops the >> contents, but keeps the allocation" to "Forgets the contents (does not >> run the destructor), but keeps the allocation.". > > I can't really think of a good real world example other than moving out of a > `Box`, can you? Otherwise, maybe it just shouldn't be public? Oh I thought you had a user for that function. In that case making it private makes a lot of sense. Also, `drop_contents` can be implemented with `forget_contents`, but that might be a good follow up as a good-first-issue. >> Another thing that I spotted while looking at the patch, `move_out` >> doesn't need the `transmute_copy`, you should be able to just call >> `read` on the pointer. > > While technically it's the same I thought `transmute_copy` is considered better, > since it has less stict safety requirements? I would avoid `transmute_copy` as much as possible, transmuting signifies to me that you somehow need to change the type and `transmute_copy` means that the compiler isn't even able to figure out that the two types have the same size (they are allowed to have different size [only `Src` larger than `Dst` though], but I most often have used this with generics where it was guaranteed to be the same type, but the compiler failed to notice it.). > For `transmute_copy` we only need to say, that dst has the same type as src, > whereas for `read` the pointer must be valid for reads, properly aligned and > point to an initialized value. You can input a reference, so you get pointer validity for free, though you still need to put that in the SAFETY comment. --- Cheers, Benno
On Thu, Aug 15, 2024 at 04:52:42AM +0200, Danilo Krummrich wrote: > On Wed, Aug 14, 2024 at 12:32:15PM -0700, Boqun Feng wrote: > > Hi Danilo, > > > > I'm trying to put your series on rust-dev, but I hit a few conflicts due > > to the conflict with `Box::drop_contents`, which has been in rust-dev > > for a while. And the conflict is not that trivial for me to resolve. > > So just a head-up, that's a requirement for me to put it on rust-dev for > > more tests from my end ;-) > > I rebased everything and you can fetch them from [1]. > Thanks! I will take a look later today or tomorrow. > I resolved the following conflicts: > > - for `Box`, implement > - `drop_contents` > - `manually_drop_contents` [2] > - ``move_out` [2] Have you considered naming this `into_inner` which is aligned to std `Box`? Regards, Boqun > - `BorrowedMut` for `ForeignOwnable` for `Box<T, A>` and `Pin<Box<T, A>>` > - `InPlaceWrite` and updated `InPlaceInit` > - for `RBTreeNode`, make use of `Box::move_out` to replace the original > implementation partially moving out of `Box` > > @Alice: Please have a look at the changes for `RBTreeNode`. Maybe it's also > worth having them in a separate patch. > > - Danilo > > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/log/?h=rust-dev/mm > [2] https://git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/commit/?h=rust-dev/mm&id=ef80ccca2ccebf3c7bcafdc13d1bfe81341cbe63 > [3] https://git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/diff/rust/kernel/rbtree.rs?h=rust-dev/mm&id=c361d66df7fb7760064fbca6bf9d72171c352a73 > > > > > Regards, > > Boqun
On 8/15/24 7:19 PM, Boqun Feng wrote: > On Thu, Aug 15, 2024 at 04:52:42AM +0200, Danilo Krummrich wrote: >> On Wed, Aug 14, 2024 at 12:32:15PM -0700, Boqun Feng wrote: >>> Hi Danilo, >>> >>> I'm trying to put your series on rust-dev, but I hit a few conflicts due >>> to the conflict with `Box::drop_contents`, which has been in rust-dev >>> for a while. And the conflict is not that trivial for me to resolve. >>> So just a head-up, that's a requirement for me to put it on rust-dev for >>> more tests from my end ;-) >> >> I rebased everything and you can fetch them from [1]. >> > > Thanks! I will take a look later today or tomorrow. I'll also send a v6 soon, which will also be based on rust-dev, so can also take this one then. > >> I resolved the following conflicts: >> >> - for `Box`, implement >> - `drop_contents` >> - `manually_drop_contents` [2] >> - ``move_out` [2] > > Have you considered naming this `into_inner` which is aligned to std > `Box`? Seems viable too. I can rename it. > > Regards, > Boqun > >> - `BorrowedMut` for `ForeignOwnable` for `Box<T, A>` and `Pin<Box<T, A>>` >> - `InPlaceWrite` and updated `InPlaceInit` >> - for `RBTreeNode`, make use of `Box::move_out` to replace the original >> implementation partially moving out of `Box` >> >> @Alice: Please have a look at the changes for `RBTreeNode`. Maybe it's also >> worth having them in a separate patch. >> >> - Danilo >> >> >> [1] https://git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/log/?h=rust-dev/mm >> [2] https://git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/commit/?h=rust-dev/mm&id=ef80ccca2ccebf3c7bcafdc13d1bfe81341cbe63 >> [3] https://git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/diff/rust/kernel/rbtree.rs?h=rust-dev/mm&id=c361d66df7fb7760064fbca6bf9d72171c352a73 >> >>> >>> Regards, >>> Boqun >
Hi, This patch series adds generic kernel allocator support for Rust, which so far is limited to `kmalloc` allocations. In order to abstain from (re-)adding unstable Rust features to the kernel, this patch series does not extend the `Allocator` trait from Rust's `alloc` crate, nor does it extend the `BoxExt` and `VecExt` extensions. Instead, this series introduces a kernel specific `Allocator` trait, which is implemented by the `Kmalloc`, `Vmalloc` and `KVmalloc` allocators, also implemented in the context of this series. As a consequence we need our own kernel `Box<T, A>` and `Vec<T, A>` types. Additionally, this series adds the following type aliases: ``` pub type KBox<T> = Box<T, Kmalloc>; pub type VBox<T> = Box<T, Vmalloc>; pub type KVBox<T> = Box<T, KVmalloc>; pub type KVec<T> = Vec<T, Kmalloc>; pub type VVec<T> = Vec<T, Vmalloc>; pub type KVVec<T> = Vec<T, KVmalloc>; ``` With that, we can start using the kernel `Box` and `Vec` types throughout the tree and remove the now obolete extensions `BoxExt` and `VecExt`. For a final cleanup, this series removes the last minor dependencies to Rust's `alloc` crate and removes it from the entire kernel build. The series ensures not to break the `rusttest` make target by implementing the `allocator_test` module providing a stub implementation for all kernel `Allocator`s. This patch series passes all KUnit tests, including the ones added by this series. Additionally, the tests were run with `kmemleak` and `KASAN` enabled, without any issues. This series is based on [1], which hit -mm/mm-unstable, and is also available in [2]. [1] https://git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/log/?h=mm/krealloc [2] https://git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/log/?h=rust/mm Changes in v5: - (safety) comment / documentation fixes suggested by Alice, Benno and Gary - remove `Unique<T>` and implement `Send` and `Sync` for `Box` and `Vec` - use `KMALLOC_MAX_SIZE` for `KVmalloc` test and add a `Kmalloc` test that expects to fail for `KMALLOC_MAX_SIZE` - create use constants `KREALLOC`, `VREALLOC` and `KVREALLOC` for `ReallocFuncs` - drop `Box::drop_contents` for now, will add it again, once I actually rebase on the original patch that introduces it - improve usage of `size_of_val` in `Box` - move `InPlaceInit` and `ForeignOwnable` impls into kbox.rs - fix missing `Box` conversions in rnull.rs - reworked `Cmalloc` to keep track of the size of memory allocations itself - remove `GlobalAlloc` together with the `alloc` crate to avoid a linker error - remove `alloc` from scripts/generate_rust_analyzer.py Changes in v4: - (safety) comment fixes suggested by Alice and Boqun - remove `Box::from_raw_alloc` and `Box::into_raw_alloc`, we don't need them - in `Box::drop` call `size_of_val` before `drop_in_place` - implement ForeignOwnable for Pin<Box<T>> as suggested by Alice - in `Vec::extend_with`, iterate over `n` instead of `spare.len()` - for `Vmalloc` and `KVmalloc` fail allocation for alignments larger than PAGE_SIZE for now (will add support for larger alignments in a separate series) - implement `Cmalloc` in `allocator_test` and type alias all kernel allocator types to it, such that we can use the kernel's `Box` and `Vec` types in userspace tests (rusttest) - this makes patch "rust: str: test: replace `alloc::format`" rather trivial Changes in v3: - Box: - minor documentation fixes - removed unnecessary imports in doc tests - dropeed `self` argument from some remaining `Box` methods - implement `InPlaceInit` for Box<T, A> rather than specifically for `KBox<T>` - Vec: - minor documentation fixes - removed useless `Vec::allocator` method - in `Vec::extend_with` use `Vec::spare_capacity_mut` instead of raw pointer operations - added a few missing safety comments - pass GFP flags to `Vec::collect` - fixed a rustdoc warning in alloc.rs - fixed the allocator_test module to implement the `Allocator` trait correctly - rebased to rust-next Changes in v2: - preserve `impl GlobalAlloc for Kmalloc` and remove it at the end (Benno) - remove `&self` parameter from all `Allocator` functions (Benno) - various documentation fixes for `Allocator` (Benno) - use `NonNull<u8>` for `Allocator::free` and `Option<NonNull<u8>>` for `Allocator::realloc` (Benno) - fix leak of `IntoIter` in `Vec::collect` (Boqun) - always realloc (try to shrink) in `Vec::collect`, it's up the the `Allocator` to provide a heuristic whether it makes sense to actually shrink - rename `KBox<T, A>` -> `Box<T, A>` and `KVec<T, A>` -> `Vec<T, A>` and provide type aliases `KBox<T>`, `VBox<T>`, `KVBox<T>`, etc. - This allows for much cleaner code and, in combination with removing `&self` parameters from `Allocator`s, gets us rid of the need for `Box::new` and `Box::new_alloc` and all other "_alloc" postfixed functions. - Before: `KBox::new_alloc(foo, Vmalloc)?` - After: `VBox::new(foo)?`, which resolves to `Box::<Foo, Vmalloc>::new(foo)?; Danilo Krummrich (26): rust: alloc: add `Allocator` trait rust: alloc: separate `aligned_size` from `krealloc_aligned` rust: alloc: rename `KernelAllocator` to `Kmalloc` rust: alloc: implement `Allocator` for `Kmalloc` rust: alloc: add module `allocator_test` rust: alloc: implement `Vmalloc` allocator rust: alloc: implement `KVmalloc` allocator rust: alloc: add __GFP_NOWARN to `Flags` rust: alloc: implement kernel `Box` rust: treewide: switch to our kernel `Box` type rust: alloc: remove `BoxExt` extension rust: alloc: add `Box` to prelude rust: alloc: implement kernel `Vec` type rust: alloc: implement `IntoIterator` for `Vec` rust: alloc: implement `collect` for `IntoIter` rust: treewide: switch to the kernel `Vec` type rust: alloc: remove `VecExt` extension rust: alloc: add `Vec` to prelude rust: error: use `core::alloc::LayoutError` rust: error: check for config `test` in `Error::name` rust: alloc: implement `contains` for `Flags` rust: alloc: implement `Cmalloc` in module allocator_test rust: str: test: replace `alloc::format` rust: alloc: update module comment of alloc.rs kbuild: rust: remove the `alloc` crate and `GlobalAlloc` MAINTAINERS: add entry for the Rust `alloc` module MAINTAINERS | 7 + drivers/block/rnull.rs | 4 +- rust/Makefile | 44 +- rust/bindings/bindings_helper.h | 1 + rust/exports.c | 1 - rust/helpers.c | 16 +- rust/kernel/alloc.rs | 121 +++- rust/kernel/alloc/allocator.rs | 156 +++-- rust/kernel/alloc/allocator_test.rs | 182 ++++++ rust/kernel/alloc/box_ext.rs | 56 -- rust/kernel/alloc/kbox.rs | 437 ++++++++++++++ rust/kernel/alloc/kvec.rs | 875 ++++++++++++++++++++++++++++ rust/kernel/alloc/vec_ext.rs | 185 ------ rust/kernel/error.rs | 6 +- rust/kernel/init.rs | 83 +-- rust/kernel/init/__internal.rs | 2 +- rust/kernel/lib.rs | 1 - rust/kernel/prelude.rs | 5 +- rust/kernel/str.rs | 35 +- rust/kernel/sync/arc.rs | 17 +- rust/kernel/sync/condvar.rs | 4 +- rust/kernel/sync/lock/mutex.rs | 2 +- rust/kernel/sync/lock/spinlock.rs | 2 +- rust/kernel/sync/locked_by.rs | 2 +- rust/kernel/types.rs | 25 +- rust/kernel/uaccess.rs | 17 +- rust/kernel/workqueue.rs | 20 +- rust/macros/lib.rs | 12 +- samples/rust/rust_minimal.rs | 4 +- scripts/Makefile.build | 7 +- scripts/generate_rust_analyzer.py | 11 +- 31 files changed, 1853 insertions(+), 487 deletions(-) create mode 100644 rust/kernel/alloc/allocator_test.rs delete mode 100644 rust/kernel/alloc/box_ext.rs create mode 100644 rust/kernel/alloc/kbox.rs create mode 100644 rust/kernel/alloc/kvec.rs delete mode 100644 rust/kernel/alloc/vec_ext.rs base-commit: c0d669a6bbe161a3afa26dea3d8491863e352e59