[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87cy2tsicw.fsf@t14s.mail-host-address-is-not-set>
Date: Wed, 28 Jan 2026 19:27:27 +0100
From: Andreas Hindborg <a.hindborg@...nel.org>
To: Gary Guo <gary@...yguo.net>, 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
"Gary Guo" <gary@...yguo.net> writes:
> 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
`b4` did this! It only has the @samsung SOB in my tree.
>
>> ---
>> 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.
Ok, cool.
>
>> +
>> + /// 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.
I did, but I do not currently. I'll drop it.
>
>> +}
>> +
>> +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.
Right. As I told Miguel, I would put this in the prelude.
>
> 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?
How about `kernel::mem` since it is memory related?
Best regards,
Andreas Hindborg
Powered by blists - more mailing lists