[PATCH v2 1/2] rust: alloc: specify the minimum alignment of each allocator

Alice Ryhl posted 2 patches 1 month, 3 weeks ago
[PATCH v2 1/2] rust: alloc: specify the minimum alignment of each allocator
Posted by Alice Ryhl 1 month, 3 weeks ago
The kernel's allocators sometimes provide a higher alignment than the
end-user requested, so add a new constant on the Allocator trait to let
the allocator specify what its minimum guaranteed alignment is.

This allows the ForeignOwnable trait to provide a more accurate value of
FOREIGN_ALIGN when using a pointer type such as Box, which will be
useful with certain collections such as XArray that store its own data
in the low bits of pointers.

Reviewed-by: Benno Lossin <lossin@kernel.org>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 rust/kernel/alloc.rs           | 8 ++++++++
 rust/kernel/alloc/allocator.rs | 8 ++++++++
 2 files changed, 16 insertions(+)

diff --git a/rust/kernel/alloc.rs b/rust/kernel/alloc.rs
index a2c49e5494d334bfde67328464dafcdb31052947..907301334d8c111eb26719ad89700a289e03f037 100644
--- a/rust/kernel/alloc.rs
+++ b/rust/kernel/alloc.rs
@@ -137,6 +137,14 @@ pub mod flags {
 /// - Implementers must ensure that all trait functions abide by the guarantees documented in the
 ///   `# Guarantees` sections.
 pub unsafe trait Allocator {
+    /// The minimum alignment satisfied by all allocations from this allocator.
+    ///
+    /// # Guarantees
+    ///
+    /// Any pointer allocated by this allocator is guaranteed to be aligned to `MIN_ALIGN` even if
+    /// the requested layout has a smaller alignment.
+    const MIN_ALIGN: usize;
+
     /// Allocate memory based on `layout` and `flags`.
     ///
     /// On success, returns a buffer represented as `NonNull<[u8]>` that satisfies the layout
diff --git a/rust/kernel/alloc/allocator.rs b/rust/kernel/alloc/allocator.rs
index aa2dfa9dca4c309e5a9eafc7da6a8a9bd7b54b11..25fc9f9ae3b4e471a08d77130b374bd1397f7384 100644
--- a/rust/kernel/alloc/allocator.rs
+++ b/rust/kernel/alloc/allocator.rs
@@ -17,6 +17,8 @@
 use crate::bindings;
 use crate::pr_warn;
 
+const ARCH_KMALLOC_MINALIGN: usize = bindings::ARCH_KMALLOC_MINALIGN as usize;
+
 /// The contiguous kernel allocator.
 ///
 /// `Kmalloc` is typically used for physically contiguous allocations up to page size, but also
@@ -128,6 +130,8 @@ unsafe fn call(
 // - passing a pointer to a valid memory allocation is OK,
 // - `realloc` satisfies the guarantees, since `ReallocFunc::call` has the same.
 unsafe impl Allocator for Kmalloc {
+    const MIN_ALIGN: usize = ARCH_KMALLOC_MINALIGN;
+
     #[inline]
     unsafe fn realloc(
         ptr: Option<NonNull<u8>>,
@@ -145,6 +149,8 @@ unsafe fn realloc(
 // - passing a pointer to a valid memory allocation is OK,
 // - `realloc` satisfies the guarantees, since `ReallocFunc::call` has the same.
 unsafe impl Allocator for Vmalloc {
+    const MIN_ALIGN: usize = kernel::page::PAGE_SIZE;
+
     #[inline]
     unsafe fn realloc(
         ptr: Option<NonNull<u8>>,
@@ -169,6 +175,8 @@ unsafe fn realloc(
 // - passing a pointer to a valid memory allocation is OK,
 // - `realloc` satisfies the guarantees, since `ReallocFunc::call` has the same.
 unsafe impl Allocator for KVmalloc {
+    const MIN_ALIGN: usize = ARCH_KMALLOC_MINALIGN;
+
     #[inline]
     unsafe fn realloc(
         ptr: Option<NonNull<u8>>,

-- 
2.50.1.703.g449372360f-goog
Re: [PATCH v2 1/2] rust: alloc: specify the minimum alignment of each allocator
Posted by Danilo Krummrich 1 month, 3 weeks ago
On Mon Aug 11, 2025 at 2:31 PM CEST, Alice Ryhl wrote:
> diff --git a/rust/kernel/alloc/allocator.rs b/rust/kernel/alloc/allocator.rs
> index aa2dfa9dca4c309e5a9eafc7da6a8a9bd7b54b11..25fc9f9ae3b4e471a08d77130b374bd1397f7384 100644
> --- a/rust/kernel/alloc/allocator.rs
> +++ b/rust/kernel/alloc/allocator.rs
> @@ -17,6 +17,8 @@
>  use crate::bindings;
>  use crate::pr_warn;
>  
> +const ARCH_KMALLOC_MINALIGN: usize = bindings::ARCH_KMALLOC_MINALIGN as usize;

I think this needs the following diff:

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 84d60635e8a9..4ad9add117ea 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -84,6 +84,7 @@

 /* `bindgen` gets confused at certain things. */
 const size_t RUST_CONST_HELPER_ARCH_SLAB_MINALIGN = ARCH_SLAB_MINALIGN;
+const size_t RUST_CONST_HELPER_ARCH_KMALLOC_MINALIGN = ARCH_KMALLOC_MINALIGN;
 const size_t RUST_CONST_HELPER_PAGE_SIZE = PAGE_SIZE;
 const gfp_t RUST_CONST_HELPER_GFP_ATOMIC = GFP_ATOMIC;
 const gfp_t RUST_CONST_HELPER_GFP_KERNEL = GFP_KERNEL;
diff --git a/rust/kernel/alloc/allocator.rs b/rust/kernel/alloc/allocator.rs
index 25fc9f9ae3b4..5003907f0240 100644
--- a/rust/kernel/alloc/allocator.rs
+++ b/rust/kernel/alloc/allocator.rs
@@ -17,7 +17,7 @@
 use crate::bindings;
 use crate::pr_warn;

-const ARCH_KMALLOC_MINALIGN: usize = bindings::ARCH_KMALLOC_MINALIGN as usize;
+const ARCH_KMALLOC_MINALIGN: usize = bindings::ARCH_KMALLOC_MINALIGN;

 /// The contiguous kernel allocator.
 ///


No need to resend I can fix it up when applying the patch.
Re: [PATCH v2 1/2] rust: alloc: specify the minimum alignment of each allocator
Posted by Alice Ryhl 1 month, 3 weeks ago
On Tue, Aug 12, 2025 at 07:52:35PM +0200, Danilo Krummrich wrote:
> On Mon Aug 11, 2025 at 2:31 PM CEST, Alice Ryhl wrote:
> > diff --git a/rust/kernel/alloc/allocator.rs b/rust/kernel/alloc/allocator.rs
> > index aa2dfa9dca4c309e5a9eafc7da6a8a9bd7b54b11..25fc9f9ae3b4e471a08d77130b374bd1397f7384 100644
> > --- a/rust/kernel/alloc/allocator.rs
> > +++ b/rust/kernel/alloc/allocator.rs
> > @@ -17,6 +17,8 @@
> >  use crate::bindings;
> >  use crate::pr_warn;
> >  
> > +const ARCH_KMALLOC_MINALIGN: usize = bindings::ARCH_KMALLOC_MINALIGN as usize;
> 
> I think this needs the following diff:
> 
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index 84d60635e8a9..4ad9add117ea 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -84,6 +84,7 @@
> 
>  /* `bindgen` gets confused at certain things. */
>  const size_t RUST_CONST_HELPER_ARCH_SLAB_MINALIGN = ARCH_SLAB_MINALIGN;
> +const size_t RUST_CONST_HELPER_ARCH_KMALLOC_MINALIGN = ARCH_KMALLOC_MINALIGN;
>  const size_t RUST_CONST_HELPER_PAGE_SIZE = PAGE_SIZE;
>  const gfp_t RUST_CONST_HELPER_GFP_ATOMIC = GFP_ATOMIC;
>  const gfp_t RUST_CONST_HELPER_GFP_KERNEL = GFP_KERNEL;
> diff --git a/rust/kernel/alloc/allocator.rs b/rust/kernel/alloc/allocator.rs
> index 25fc9f9ae3b4..5003907f0240 100644
> --- a/rust/kernel/alloc/allocator.rs
> +++ b/rust/kernel/alloc/allocator.rs
> @@ -17,7 +17,7 @@
>  use crate::bindings;
>  use crate::pr_warn;
> 
> -const ARCH_KMALLOC_MINALIGN: usize = bindings::ARCH_KMALLOC_MINALIGN as usize;
> +const ARCH_KMALLOC_MINALIGN: usize = bindings::ARCH_KMALLOC_MINALIGN;
> 
>  /// The contiguous kernel allocator.
>  ///
> 
> 
> No need to resend I can fix it up when applying the patch.

Hmm. Maybe that depends on the configuration? The constant was generated
for me. Either way, happy with the suggested change.

Alice
Re: [PATCH v2 1/2] rust: alloc: specify the minimum alignment of each allocator
Posted by Danilo Krummrich 1 month, 3 weeks ago
On Wed Aug 13, 2025 at 9:52 AM CEST, Alice Ryhl wrote:
> On Tue, Aug 12, 2025 at 07:52:35PM +0200, Danilo Krummrich wrote:
>> On Mon Aug 11, 2025 at 2:31 PM CEST, Alice Ryhl wrote:
>> > diff --git a/rust/kernel/alloc/allocator.rs b/rust/kernel/alloc/allocator.rs
>> > index aa2dfa9dca4c309e5a9eafc7da6a8a9bd7b54b11..25fc9f9ae3b4e471a08d77130b374bd1397f7384 100644
>> > --- a/rust/kernel/alloc/allocator.rs
>> > +++ b/rust/kernel/alloc/allocator.rs
>> > @@ -17,6 +17,8 @@
>> >  use crate::bindings;
>> >  use crate::pr_warn;
>> >  
>> > +const ARCH_KMALLOC_MINALIGN: usize = bindings::ARCH_KMALLOC_MINALIGN as usize;
>> 
>> I think this needs the following diff:
>> 
>> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
>> index 84d60635e8a9..4ad9add117ea 100644
>> --- a/rust/bindings/bindings_helper.h
>> +++ b/rust/bindings/bindings_helper.h
>> @@ -84,6 +84,7 @@
>> 
>>  /* `bindgen` gets confused at certain things. */
>>  const size_t RUST_CONST_HELPER_ARCH_SLAB_MINALIGN = ARCH_SLAB_MINALIGN;
>> +const size_t RUST_CONST_HELPER_ARCH_KMALLOC_MINALIGN = ARCH_KMALLOC_MINALIGN;
>>  const size_t RUST_CONST_HELPER_PAGE_SIZE = PAGE_SIZE;
>>  const gfp_t RUST_CONST_HELPER_GFP_ATOMIC = GFP_ATOMIC;
>>  const gfp_t RUST_CONST_HELPER_GFP_KERNEL = GFP_KERNEL;
>> diff --git a/rust/kernel/alloc/allocator.rs b/rust/kernel/alloc/allocator.rs
>> index 25fc9f9ae3b4..5003907f0240 100644
>> --- a/rust/kernel/alloc/allocator.rs
>> +++ b/rust/kernel/alloc/allocator.rs
>> @@ -17,7 +17,7 @@
>>  use crate::bindings;
>>  use crate::pr_warn;
>> 
>> -const ARCH_KMALLOC_MINALIGN: usize = bindings::ARCH_KMALLOC_MINALIGN as usize;
>> +const ARCH_KMALLOC_MINALIGN: usize = bindings::ARCH_KMALLOC_MINALIGN;
>> 
>>  /// The contiguous kernel allocator.
>>  ///
>> 
>> 
>> No need to resend I can fix it up when applying the patch.
>
> Hmm. Maybe that depends on the configuration? The constant was generated
> for me. Either way, happy with the suggested change.

That is a bit odd, I'd like to understand this before merging.

All of the definitions in the kernel are defines that shouldn't be picked up by
bindgen. Are you sure you do not have additional local changes?
Re: [PATCH v2 1/2] rust: alloc: specify the minimum alignment of each allocator
Posted by Alice Ryhl 1 month, 3 weeks ago
On Wed, Aug 13, 2025 at 11:14 AM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Wed Aug 13, 2025 at 9:52 AM CEST, Alice Ryhl wrote:
> > On Tue, Aug 12, 2025 at 07:52:35PM +0200, Danilo Krummrich wrote:
> >> On Mon Aug 11, 2025 at 2:31 PM CEST, Alice Ryhl wrote:
> >> > diff --git a/rust/kernel/alloc/allocator.rs b/rust/kernel/alloc/allocator.rs
> >> > index aa2dfa9dca4c309e5a9eafc7da6a8a9bd7b54b11..25fc9f9ae3b4e471a08d77130b374bd1397f7384 100644
> >> > --- a/rust/kernel/alloc/allocator.rs
> >> > +++ b/rust/kernel/alloc/allocator.rs
> >> > @@ -17,6 +17,8 @@
> >> >  use crate::bindings;
> >> >  use crate::pr_warn;
> >> >
> >> > +const ARCH_KMALLOC_MINALIGN: usize = bindings::ARCH_KMALLOC_MINALIGN as usize;
> >>
> >> I think this needs the following diff:
> >>
> >> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> >> index 84d60635e8a9..4ad9add117ea 100644
> >> --- a/rust/bindings/bindings_helper.h
> >> +++ b/rust/bindings/bindings_helper.h
> >> @@ -84,6 +84,7 @@
> >>
> >>  /* `bindgen` gets confused at certain things. */
> >>  const size_t RUST_CONST_HELPER_ARCH_SLAB_MINALIGN = ARCH_SLAB_MINALIGN;
> >> +const size_t RUST_CONST_HELPER_ARCH_KMALLOC_MINALIGN = ARCH_KMALLOC_MINALIGN;
> >>  const size_t RUST_CONST_HELPER_PAGE_SIZE = PAGE_SIZE;
> >>  const gfp_t RUST_CONST_HELPER_GFP_ATOMIC = GFP_ATOMIC;
> >>  const gfp_t RUST_CONST_HELPER_GFP_KERNEL = GFP_KERNEL;
> >> diff --git a/rust/kernel/alloc/allocator.rs b/rust/kernel/alloc/allocator.rs
> >> index 25fc9f9ae3b4..5003907f0240 100644
> >> --- a/rust/kernel/alloc/allocator.rs
> >> +++ b/rust/kernel/alloc/allocator.rs
> >> @@ -17,7 +17,7 @@
> >>  use crate::bindings;
> >>  use crate::pr_warn;
> >>
> >> -const ARCH_KMALLOC_MINALIGN: usize = bindings::ARCH_KMALLOC_MINALIGN as usize;
> >> +const ARCH_KMALLOC_MINALIGN: usize = bindings::ARCH_KMALLOC_MINALIGN;
> >>
> >>  /// The contiguous kernel allocator.
> >>  ///
> >>
> >>
> >> No need to resend I can fix it up when applying the patch.
> >
> > Hmm. Maybe that depends on the configuration? The constant was generated
> > for me. Either way, happy with the suggested change.
>
> That is a bit odd, I'd like to understand this before merging.
>
> All of the definitions in the kernel are defines that shouldn't be picked up by
> bindgen.

It is possible for bindgen to pick up a #define in some cases. The
main case where bindgen fails is when the macro is defined in terms of
a function-like macro. This is why we see so many failures with _IO*
macros.

>  Are you sure you do not have additional local changes?

Yes.

Alice
Re: [PATCH v2 1/2] rust: alloc: specify the minimum alignment of each allocator
Posted by Danilo Krummrich 1 month, 3 weeks ago
On Wed Aug 13, 2025 at 11:32 AM CEST, Alice Ryhl wrote:
> On Wed, Aug 13, 2025 at 11:14 AM Danilo Krummrich <dakr@kernel.org> wrote:
>>
>> On Wed Aug 13, 2025 at 9:52 AM CEST, Alice Ryhl wrote:
>> > On Tue, Aug 12, 2025 at 07:52:35PM +0200, Danilo Krummrich wrote:
>> >> On Mon Aug 11, 2025 at 2:31 PM CEST, Alice Ryhl wrote:
>> >> > diff --git a/rust/kernel/alloc/allocator.rs b/rust/kernel/alloc/allocator.rs
>> >> > index aa2dfa9dca4c309e5a9eafc7da6a8a9bd7b54b11..25fc9f9ae3b4e471a08d77130b374bd1397f7384 100644
>> >> > --- a/rust/kernel/alloc/allocator.rs
>> >> > +++ b/rust/kernel/alloc/allocator.rs
>> >> > @@ -17,6 +17,8 @@
>> >> >  use crate::bindings;
>> >> >  use crate::pr_warn;
>> >> >
>> >> > +const ARCH_KMALLOC_MINALIGN: usize = bindings::ARCH_KMALLOC_MINALIGN as usize;
>> >>
>> >> I think this needs the following diff:
>> >>
>> >> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
>> >> index 84d60635e8a9..4ad9add117ea 100644
>> >> --- a/rust/bindings/bindings_helper.h
>> >> +++ b/rust/bindings/bindings_helper.h
>> >> @@ -84,6 +84,7 @@
>> >>
>> >>  /* `bindgen` gets confused at certain things. */
>> >>  const size_t RUST_CONST_HELPER_ARCH_SLAB_MINALIGN = ARCH_SLAB_MINALIGN;
>> >> +const size_t RUST_CONST_HELPER_ARCH_KMALLOC_MINALIGN = ARCH_KMALLOC_MINALIGN;
>> >>  const size_t RUST_CONST_HELPER_PAGE_SIZE = PAGE_SIZE;
>> >>  const gfp_t RUST_CONST_HELPER_GFP_ATOMIC = GFP_ATOMIC;
>> >>  const gfp_t RUST_CONST_HELPER_GFP_KERNEL = GFP_KERNEL;
>> >> diff --git a/rust/kernel/alloc/allocator.rs b/rust/kernel/alloc/allocator.rs
>> >> index 25fc9f9ae3b4..5003907f0240 100644
>> >> --- a/rust/kernel/alloc/allocator.rs
>> >> +++ b/rust/kernel/alloc/allocator.rs
>> >> @@ -17,7 +17,7 @@
>> >>  use crate::bindings;
>> >>  use crate::pr_warn;
>> >>
>> >> -const ARCH_KMALLOC_MINALIGN: usize = bindings::ARCH_KMALLOC_MINALIGN as usize;
>> >> +const ARCH_KMALLOC_MINALIGN: usize = bindings::ARCH_KMALLOC_MINALIGN;
>> >>
>> >>  /// The contiguous kernel allocator.
>> >>  ///
>> >>
>> >>
>> >> No need to resend I can fix it up when applying the patch.
>> >
>> > Hmm. Maybe that depends on the configuration? The constant was generated
>> > for me. Either way, happy with the suggested change.
>>
>> That is a bit odd, I'd like to understand this before merging.
>>
>> All of the definitions in the kernel are defines that shouldn't be picked up by
>> bindgen.
>
> It is possible for bindgen to pick up a #define in some cases. The
> main case where bindgen fails is when the macro is defined in terms of
> a function-like macro. This is why we see so many failures with _IO*
> macros.

I think I see it now, ARCH_KMALLOC_MINALIGN seems to be either a literal or
__alignof__(unsigned long long), either directly or indirecty through
ARCH_DMA_MINALIGN. bindgen doesn't like the __alignof__() extension.

So, I assume you were on arm64? :)
Re: [PATCH v2 1/2] rust: alloc: specify the minimum alignment of each allocator
Posted by Alice Ryhl 1 month, 3 weeks ago
On Wed, Aug 13, 2025 at 12:19 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Wed Aug 13, 2025 at 11:32 AM CEST, Alice Ryhl wrote:
> > On Wed, Aug 13, 2025 at 11:14 AM Danilo Krummrich <dakr@kernel.org> wrote:
> >>
> >> On Wed Aug 13, 2025 at 9:52 AM CEST, Alice Ryhl wrote:
> >> > On Tue, Aug 12, 2025 at 07:52:35PM +0200, Danilo Krummrich wrote:
> >> >> On Mon Aug 11, 2025 at 2:31 PM CEST, Alice Ryhl wrote:
> >> >> > diff --git a/rust/kernel/alloc/allocator.rs b/rust/kernel/alloc/allocator.rs
> >> >> > index aa2dfa9dca4c309e5a9eafc7da6a8a9bd7b54b11..25fc9f9ae3b4e471a08d77130b374bd1397f7384 100644
> >> >> > --- a/rust/kernel/alloc/allocator.rs
> >> >> > +++ b/rust/kernel/alloc/allocator.rs
> >> >> > @@ -17,6 +17,8 @@
> >> >> >  use crate::bindings;
> >> >> >  use crate::pr_warn;
> >> >> >
> >> >> > +const ARCH_KMALLOC_MINALIGN: usize = bindings::ARCH_KMALLOC_MINALIGN as usize;
> >> >>
> >> >> I think this needs the following diff:
> >> >>
> >> >> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> >> >> index 84d60635e8a9..4ad9add117ea 100644
> >> >> --- a/rust/bindings/bindings_helper.h
> >> >> +++ b/rust/bindings/bindings_helper.h
> >> >> @@ -84,6 +84,7 @@
> >> >>
> >> >>  /* `bindgen` gets confused at certain things. */
> >> >>  const size_t RUST_CONST_HELPER_ARCH_SLAB_MINALIGN = ARCH_SLAB_MINALIGN;
> >> >> +const size_t RUST_CONST_HELPER_ARCH_KMALLOC_MINALIGN = ARCH_KMALLOC_MINALIGN;
> >> >
> >> > Hmm. Maybe that depends on the configuration? The constant was generated
> >> > for me. Either way, happy with the suggested change.
> >>
> >> That is a bit odd, I'd like to understand this before merging.
> >>
> >> All of the definitions in the kernel are defines that shouldn't be picked up by
> >> bindgen.
> >
> > It is possible for bindgen to pick up a #define in some cases. The
> > main case where bindgen fails is when the macro is defined in terms of
> > a function-like macro. This is why we see so many failures with _IO*
> > macros.
>
> I think I see it now, ARCH_KMALLOC_MINALIGN seems to be either a literal or
> __alignof__(unsigned long long), either directly or indirecty through
> ARCH_DMA_MINALIGN. bindgen doesn't like the __alignof__() extension.

That sounds right. So yeah, please go ahead and pick it up with the bindgen fix.

> So, I assume you were on arm64? :)

I did in fact build for arm64, yes.

Alice