[PATCH v2 0/2] Take ARCH_KMALLOC_MINALIGN into account for build-time XArray check

[PATCH v2 0/2] Take ARCH_KMALLOC_MINALIGN into account for build-time XArray check

[Alice Ryhl]

The Rust bindings for XArray include a build-time check to ensure that
you can only use the XArray with pointers that are 4-byte aligned.
Because of that, there is currently a build failure if you attempt to
create an XArray<KBox<T>> where T is a 1-byte or 2-byte aligned type.
However, this error is incorrect as KBox<_> is guaranteed to be a
pointer that comes from kmalloc, and kmalloc always produces pointers
that are at least 4-byte aligned.

To fix this, we augment the compile-time logic that computes the
alignment of KBox<_> to take the minimum alignment of its allocator into
account.

Intended to land through alloc-next under the RUST [ALLOC] entry.

Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
Changes in v2:
- Reword guarantee on `const MIN_ALIGN`.
- Change formatting of if/else in kbox.rs.
- Rebase on v6.17-rc1.
- Link to v1: https://lore.kernel.org/r/20250715-align-min-allocator-v1-0-3e1b2a5516c0@google.com

---
Alice Ryhl (2):
      rust: alloc: specify the minimum alignment of each allocator
      rust: alloc: take the allocator into account for FOREIGN_ALIGN

 rust/kernel/alloc.rs           |  8 ++++++++
 rust/kernel/alloc/allocator.rs |  8 ++++++++
 rust/kernel/alloc/kbox.rs      | 13 +++++++++----
 rust/kernel/sync/arc.rs        |  6 +++---
 4 files changed, 28 insertions(+), 7 deletions(-)
---
base-commit: 062b3e4a1f880f104a8d4b90b767788786aa7b78
change-id: 20250715-align-min-allocator-b31aee53cbda

Best regards,
-- 
Alice Ryhl <aliceryhl@google.com>

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

[Alice Ryhl]

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

[PATCH v2 2/2] rust: alloc: take the allocator into account for FOREIGN_ALIGN

[Alice Ryhl]

When converting a Box<T> into a void pointer, the allocator might
guarantee a higher alignment than the type itself does, and in that case
it is guaranteed that the void pointer has that higher alignment.

This is quite useful when combined with the XArray, which you can only
create using a ForeignOwnable whose FOREIGN_ALIGN is at least 4. This
means that you can now always use a Box<T> with the XArray no matter the
alignment of T.

Reviewed-by: Benno Lossin <lossin@kernel.org>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 rust/kernel/alloc/kbox.rs | 13 +++++++++----
 rust/kernel/sync/arc.rs   |  6 +++---
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/rust/kernel/alloc/kbox.rs b/rust/kernel/alloc/kbox.rs
index 856d05aa60f13485d8afc98f0b7fe7593867b5a1..eedab0be1eff78a8150524346348ec0759f852e2 100644
--- a/rust/kernel/alloc/kbox.rs
+++ b/rust/kernel/alloc/kbox.rs
@@ -401,12 +401,17 @@ fn try_init<E>(init: impl Init<T, E>, flags: Flags) -> Result<Self, E>
 }
 
 // SAFETY: The pointer returned by `into_foreign` comes from a well aligned
-// pointer to `T`.
+// pointer to `T` allocated by `A`.
 unsafe impl<T: 'static, A> ForeignOwnable for Box<T, A>
 where
     A: Allocator,
 {
-    const FOREIGN_ALIGN: usize = core::mem::align_of::<T>();
+    const FOREIGN_ALIGN: usize = if core::mem::align_of::<T>() < A::MIN_ALIGN {
+        A::MIN_ALIGN
+    } else {
+        core::mem::align_of::<T>()
+    };
+
     type Borrowed<'a> = &'a T;
     type BorrowedMut<'a> = &'a mut T;
 
@@ -435,12 +440,12 @@ unsafe fn borrow_mut<'a>(ptr: *mut c_void) -> &'a mut T {
 }
 
 // SAFETY: The pointer returned by `into_foreign` comes from a well aligned
-// pointer to `T`.
+// pointer to `T` allocated by `A`.
 unsafe impl<T: 'static, A> ForeignOwnable for Pin<Box<T, A>>
 where
     A: Allocator,
 {
-    const FOREIGN_ALIGN: usize = core::mem::align_of::<T>();
+    const FOREIGN_ALIGN: usize = <Box<T, A> as ForeignOwnable>::FOREIGN_ALIGN;
     type Borrowed<'a> = Pin<&'a T>;
     type BorrowedMut<'a> = Pin<&'a mut T>;
 
diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
index 63a66761d0c7d752e09ce7372bc230661b2f7c6d..74121cf935f364c16799b5c31cc88714dfd6b702 100644
--- a/rust/kernel/sync/arc.rs
+++ b/rust/kernel/sync/arc.rs
@@ -373,10 +373,10 @@ pub fn into_unique_or_drop(self) -> Option<Pin<UniqueArc<T>>> {
     }
 }
 
-// SAFETY: The pointer returned by `into_foreign` comes from a well aligned
-// pointer to `ArcInner<T>`.
+// SAFETY: The pointer returned by `into_foreign` was originally allocated as an
+// `KBox<ArcInner<T>>`, so that type is what determines the alignment.
 unsafe impl<T: 'static> ForeignOwnable for Arc<T> {
-    const FOREIGN_ALIGN: usize = core::mem::align_of::<ArcInner<T>>();
+    const FOREIGN_ALIGN: usize = <KBox<ArcInner<T>> as ForeignOwnable>::FOREIGN_ALIGN;
 
     type Borrowed<'a> = ArcBorrow<'a, T>;
     type BorrowedMut<'a> = Self::Borrowed<'a>;

-- 
2.50.1.703.g449372360f-goog

Re: [PATCH v2 0/2] Take ARCH_KMALLOC_MINALIGN into account for build-time XArray check

[Andreas Hindborg]

"Alice Ryhl" <aliceryhl@google.com> writes:

> The Rust bindings for XArray include a build-time check to ensure that
> you can only use the XArray with pointers that are 4-byte aligned.
> Because of that, there is currently a build failure if you attempt to
> create an XArray<KBox<T>> where T is a 1-byte or 2-byte aligned type.
> However, this error is incorrect as KBox<_> is guaranteed to be a
> pointer that comes from kmalloc, and kmalloc always produces pointers
> that are at least 4-byte aligned.
>
> To fix this, we augment the compile-time logic that computes the
> alignment of KBox<_> to take the minimum alignment of its allocator into
> account.
>
> Intended to land through alloc-next under the RUST [ALLOC] entry.
>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>

Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>


Best regards,
Andreas Hindborg

Re: [PATCH v2 0/2] Take ARCH_KMALLOC_MINALIGN into account for build-time XArray check

[Liam R. Howlett]

* Alice Ryhl <aliceryhl@google.com> [250811 08:32]:
> The Rust bindings for XArray include a build-time check to ensure that
> you can only use the XArray with pointers that are 4-byte aligned.
> Because of that, there is currently a build failure if you attempt to
> create an XArray<KBox<T>> where T is a 1-byte or 2-byte aligned type.
> However, this error is incorrect as KBox<_> is guaranteed to be a
> pointer that comes from kmalloc, and kmalloc always produces pointers
> that are at least 4-byte aligned.
> 
> To fix this, we augment the compile-time logic that computes the
> alignment of KBox<_> to take the minimum alignment of its allocator into
> account.
> 
> Intended to land through alloc-next under the RUST [ALLOC] entry.
> 
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>

Acked-by: Liam R. Howlett <Liam.Howlett@oracle.com>

> ---
> Changes in v2:
> - Reword guarantee on `const MIN_ALIGN`.
> - Change formatting of if/else in kbox.rs.
> - Rebase on v6.17-rc1.
> - Link to v1: https://lore.kernel.org/r/20250715-align-min-allocator-v1-0-3e1b2a5516c0@google.com
> 
> ---
> Alice Ryhl (2):
>       rust: alloc: specify the minimum alignment of each allocator
>       rust: alloc: take the allocator into account for FOREIGN_ALIGN
> 
>  rust/kernel/alloc.rs           |  8 ++++++++
>  rust/kernel/alloc/allocator.rs |  8 ++++++++
>  rust/kernel/alloc/kbox.rs      | 13 +++++++++----
>  rust/kernel/sync/arc.rs        |  6 +++---
>  4 files changed, 28 insertions(+), 7 deletions(-)
> ---
> base-commit: 062b3e4a1f880f104a8d4b90b767788786aa7b78
> change-id: 20250715-align-min-allocator-b31aee53cbda
> 
> Best regards,
> -- 
> Alice Ryhl <aliceryhl@google.com>
> 

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

[Danilo Krummrich]

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

[Alice Ryhl]

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

[Danilo Krummrich]

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

[Alice Ryhl]

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

[Danilo Krummrich]

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

[Alice Ryhl]

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

Re: [PATCH v2 0/2] Take ARCH_KMALLOC_MINALIGN into account for build-time XArray check

[Danilo Krummrich]

On Mon Aug 11, 2025 at 2:31 PM CEST, Alice Ryhl wrote:
> Alice Ryhl (2):
>       rust: alloc: specify the minimum alignment of each allocator

    [ Add helper for ARCH_KMALLOC_MINALIGN; remove cast to usize. - Danilo ]

>       rust: alloc: take the allocator into account for FOREIGN_ALIGN

Applied to alloc-next, thanks!

--

@Andrew: Just a heads-up, this has a minor conflict with your tree, which
should also show up in linux-next soon.

diff --cc rust/kernel/alloc.rs
index b39c279236f5,907301334d8c..000000000000
--- a/rust/kernel/alloc.rs
+++ b/rust/kernel/alloc.rs
@@@ -164,7 -137,15 +164,15 @@@ impl NumaNode
  /// - 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`.
 +    /// Allocate memory based on `layout`, `flags` and `nid`.
      ///
      /// On success, returns a buffer represented as `NonNull<[u8]>` that satisfies the layout
      /// constraints (i.e. minimum size and alignment as specified by `layout`).

Re: [PATCH v2 0/2] Take ARCH_KMALLOC_MINALIGN into account for build-time XArray check

[Thorsten Leemhuis]

On 15.08.25 21:12, Danilo Krummrich wrote:
> On Mon Aug 11, 2025 at 2:31 PM CEST, Alice Ryhl wrote:
>> Alice Ryhl (2):
>>       rust: alloc: specify the minimum alignment of each allocator
> 
>     [ Add helper for ARCH_KMALLOC_MINALIGN; remove cast to usize. - Danilo ]
> 
>>       rust: alloc: take the allocator into account for FOREIGN_ALIGN
> 
> Applied to alloc-next, thanks!

I had a build error with my daily linux-next builds for Fedora (using
the Fedora rawhide config) on arm64 (ppc64le and x86_64 worked fine).
From a quick look it seems it might be due to this series, which 
turned up in -next today:

"""
error[E0428]: the name `ARCH_KMALLOC_MINALIGN` is defined multiple times
      --> /builddir/build/BUILD/kernel-6.17.0-build/kernel-next-20250818/linux-6.17.0-0.0.next.20250818.423.vanilla.fc44.aarch64/rust/bindings/bindings_generated.rs:134545:1
       |
9622   | pub const ARCH_KMALLOC_MINALIGN: u32 = 8;
       | ----------------------------------------- previous definition of the value `ARCH_KMALLOC_MINALIGN` here
...
134545 | pub const ARCH_KMALLOC_MINALIGN: usize = 8;
       | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `ARCH_KMALLOC_MINALIGN` redefined here
       |
       = note: `ARCH_KMALLOC_MINALIGN` must be defined only once in the value namespace of this module

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0428`.
make[2]: *** [rust/Makefile:539: rust/bindings.o] Error 1
make[1]: *** [/builddir/build/BUILD/kernel-6.17.0-build/kernel-next-20250818/linux-6.17.0-0.0.next.20250818.423.vanilla.fc44.aarch64/Makefile:1294: prepare] Error 2
make: *** [Makefile:256: __sub-make] Error 2
"""

Full log:
https://download.copr.fedorainfracloud.org/results/@kernel-vanilla/next/fedora-rawhide-aarch64/09439461-next-next-all/builder-live.log.gz

Ciao, Thorsten

Re: [PATCH v2 0/2] Take ARCH_KMALLOC_MINALIGN into account for build-time XArray check

[Miguel Ojeda]

On Mon, Aug 18, 2025 at 1:09 PM Thorsten Leemhuis <linux@leemhuis.info> wrote:
>
> on arm64 (ppc64le and x86_64 worked fine).

Yeah, it happens in arm, arm64, loongarch64 and riscv64 -- we don't
have powerpc support for Rust upstream yet (we had it originally
out-of-tree, but it never got upstreamed).

Cheers,
Miguel

Re: [PATCH v2 0/2] Take ARCH_KMALLOC_MINALIGN into account for build-time XArray check

[Danilo Krummrich]

On Mon Aug 18, 2025 at 1:09 PM CEST, Thorsten Leemhuis wrote:
> error[E0428]: the name `ARCH_KMALLOC_MINALIGN` is defined multiple times
>       --> /builddir/build/BUILD/kernel-6.17.0-build/kernel-next-20250818/linux-6.17.0-0.0.next.20250818.423.vanilla.fc44.aarch64/rust/bindings/bindings_generated.rs:134545:1
>        |
> 9622   | pub const ARCH_KMALLOC_MINALIGN: u32 = 8;
>        | ----------------------------------------- previous definition of the value `ARCH_KMALLOC_MINALIGN` here
> ...
> 134545 | pub const ARCH_KMALLOC_MINALIGN: usize = 8;
>        | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `ARCH_KMALLOC_MINALIGN` redefined here
>        |
>        = note: `ARCH_KMALLOC_MINALIGN` must be defined only once in the value namespace of this module
>
> error: aborting due to 1 previous error

Thanks for reporting, the diff below should fix it, I'll send a patch soon.

diff --git a/rust/bindgen_parameters b/rust/bindgen_parameters
index 0f96af8b9a7f..02b371b98b39 100644
--- a/rust/bindgen_parameters
+++ b/rust/bindgen_parameters
@@ -34,3 +34,4 @@
 # We use const helpers to aid bindgen, to avoid conflicts when constants are
 # recognized, block generation of the non-helper constants.
 --blocklist-item ARCH_SLAB_MINALIGN
+--blocklist-item ARCH_KMALLOC_MINALIGN

Re: [PATCH v2 0/2] Take ARCH_KMALLOC_MINALIGN into account for build-time XArray check

[Miguel Ojeda]

On Fri, Aug 15, 2025 at 9:12 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> diff --cc rust/kernel/alloc.rs
> index b39c279236f5,907301334d8c..000000000000
> --- a/rust/kernel/alloc.rs
> +++ b/rust/kernel/alloc.rs
> @@@ -164,7 -137,15 +164,15 @@@ impl NumaNode
>   /// - 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`.
>  +    /// Allocate memory based on `layout`, `flags` and `nid`.
>       ///
>       /// On success, returns a buffer represented as `NonNull<[u8]>` that satisfies the layout
>       /// constraints (i.e. minimum size and alignment as specified by `layout`).

`MIN_ALIGN` is missing in `Cmalloc` -- from `rusttest`:

     error[E0046]: not all trait items implemented, missing: `MIN_ALIGN`
       --> rust/kernel/alloc/allocator_test.rs:48:1
        |
    48  | unsafe impl Allocator for Cmalloc {
        | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing `MIN_ALIGN` in
implementation
        |
       ::: rust/kernel/alloc.rs:173:5
        |
    173 |     const MIN_ALIGN: usize;
        |     ---------------------- `MIN_ALIGN` from trait

i.e. similar to the other one.

Cheers,
Miguel