Message-Id: <DG0ANMHJMJ40.3J7GZJ5H6WRMA@garyguo.net>
Date: Wed, 28 Jan 2026 14:32:27 +0000
From: "Gary Guo" <gary@...yguo.net>
To: "Andreas Hindborg" <a.hindborg@...nel.org>, "Miguel Ojeda"
 <ojeda@...nel.org>, "Boqun Feng" <boqun.feng@...il.com>, "Gary Guo"
 <gary@...yguo.net>, Björn Roy Baron
 <bjorn3_gh@...tonmail.com>, "Benno Lossin" <lossin@...nel.org>, "Alice
 Ryhl" <aliceryhl@...gle.com>, "Trevor Gross" <tmgross@...ch.edu>, "Danilo
 Krummrich" <dakr@...nel.org>
Cc: <linux-kernel@...r.kernel.org>, <rust-for-linux@...r.kernel.org>
Subject: Re: [PATCH] rust: add `CacheAligned` for easy cache line alignment
 of values

On Wed Jan 28, 2026 at 2:05 PM GMT, Andreas Hindborg wrote:
> `CacheAligned` allows to easily align values to a 64 byte boundary.
>
> An example use case is the kernel `struct spinlock`. This struct is 4 bytes
> on x86 when lockdep is not enabled. The structure is not padded to fit a
> cache line. The effect of this for `SpinLock` is that the lock variable and
> the value protected by the lock might share a cache line, depending on the
> alignment requirements of the protected value. Wrapping the value in
> `CacheAligned` to get a `SpinLock<CacheAligned<T>>` solves this problem.

Do you mean `CacheAligned<SpinLock<T>>`?

>
> Signed-off-by: Andreas Hindborg <a.hindborg@...sung.com>
> ---
> Signed-off-by: Andreas Hindborg <a.hindborg@...nel.org>

Double SOB

> ---
>  rust/kernel/cache_aligned.rs | 59 ++++++++++++++++++++++++++++++++++++++++++++
>  rust/kernel/lib.rs           |  2 ++
>  2 files changed, 61 insertions(+)
>
> diff --git a/rust/kernel/cache_aligned.rs b/rust/kernel/cache_aligned.rs
> new file mode 100644
> index 0000000000000..9c33b8613c077
> --- /dev/null
> +++ b/rust/kernel/cache_aligned.rs
> @@ -0,0 +1,59 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +use kernel::try_pin_init;
> +use pin_init::{
> +    pin_data,
> +    pin_init,
> +    PinInit, //
> +};
> +
> +/// Wrapper type that alings content to a 64 byte cache line.
> +#[repr(align(64))]
> +#[pin_data]
> +pub struct CacheAligned<T: ?Sized> {
> +    #[pin]
> +    value: T,
> +}
> +
> +impl<T> CacheAligned<T> {
> +    /// Creates an initializer for `CacheAligned<T>` form an initalizer for `T`
> +    pub fn new(t: impl PinInit<T>) -> impl PinInit<CacheAligned<T>> {
> +        pin_init!( CacheAligned {
> +            value <- t
> +        })
> +    }
> +
> +    /// Creates a fallible initializer for `CacheAligned<T>` form a fallible
> +    /// initalizer for `T`
> +    pub fn try_new(
> +        t: impl PinInit<T, crate::error::Error>,
> +    ) -> impl PinInit<CacheAligned<T>, crate::error::Error> {
> +        try_pin_init!( CacheAligned {
> +            value <- t
> +        }? crate::error::Error )
> +    }

You don't need two methods. You can have a single `new` method that's generic
over error type.

> +
> +    /// Get a pointer to the contained value without creating a reference.
> +    ///
> +    /// # Safety
> +    ///
> +    /// - `ptr` must be dereferenceable.
> +    pub const unsafe fn raw_get(ptr: *mut Self) -> *mut T {
> +        // SAFETY: by function safety requirements `ptr` is valid for read
> +        unsafe { &raw mut ((*ptr).value) }
> +    }

Have you had a case where you need this? Most wrapper types shouldn't need this.

> +}
> +
> +impl<T: ?Sized> core::ops::Deref for CacheAligned<T> {
> +    type Target = T;
> +
> +    fn deref(&self) -> &T {
> +        &self.value
> +    }
> +}
> +
> +impl<T: ?Sized> core::ops::DerefMut for CacheAligned<T> {
> +    fn deref_mut(&mut self) -> &mut T {
> +        &mut self.value
> +    }
> +}
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index f812cf1200428..af6d48b078428 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -75,6 +75,7 @@
>  pub mod bug;
>  #[doc(hidden)]
>  pub mod build_assert;
> +mod cache_aligned;
>  pub mod clk;
>  #[cfg(CONFIG_CONFIGFS_FS)]
>  pub mod configfs;
> @@ -156,6 +157,7 @@
>  
>  #[doc(hidden)]
>  pub use bindings;
> +pub use cache_aligned::CacheAligned;

Let's not expose this from top-level of kernel crate.

I have been thinking about a good namespace for these auxillary types. For my
own project I would chuck them to `crate::utils`, but that won't be a very
descriptive name.

I wonder for this and other types that tweak the memory layout, we could have a
`kernel::layout` which contains utilities for precisely controlling the layout?

Best,
Gary

>  pub use macros;
>  pub use uapi;
>  
>
> ---
> base-commit: 63804fed149a6750ffd28610c5c1c98cce6bd377
> change-id: 20260128-cache-aligned-c4c0acf870ff
>
> Best regards,

Powered by blists - more mailing lists