mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Gary Guo" <gary@garyguo.net>
To: "Andreas Hindborg" <a.hindborg@kernel.org>,
	"Gary Guo" <gary@garyguo.net>,
	"Alexandre Courbot" <acourbot@nvidia.com>
Cc: "Miguel Ojeda" <ojeda@kernel.org>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Danilo Krummrich" <dakr@kernel.org>,
	linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org
Subject: Re: [PATCH] rust: add `CacheAligned` for easy cache line alignment of values
Date: Tue, 19 May 2026 11:35:13 +0100	[thread overview]
Message-ID: <DIML4GDC87AA.116PZJDMV23IC@garyguo.net> (raw)
In-Reply-To: <878q9f95j0.fsf@t14s.mail-host-address-is-not-set>

On Tue May 19, 2026 at 9:18 AM BST, Andreas Hindborg wrote:
> "Gary Guo" <gary@garyguo.net> writes:
>
>> On Mon May 18, 2026 at 2:41 PM BST, Andreas Hindborg wrote:
>>> "Gary Guo" <gary@garyguo.net> writes:
>>>
>>>> On Wed Jan 28, 2026 at 2:25 PM GMT, Alexandre Courbot wrote:
>>>>> On Wed Jan 28, 2026 at 11:05 PM JST, 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.
>>>>>>
>>>>>> Signed-off-by: Andreas Hindborg <a.hindborg@samsung.com>
>>>>>> ---
>>>>>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>>>>>> ---
>>>>>>  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.
>>>>>
>>>>> nit: s/alings/aligns
>>>>>
>>>>>> +#[repr(align(64))]
>>>>>
>>>>> While 64 bytes is the most common cache line size, AFAIK this is not
>>>>> a universal value? Can we expose and use `L1_CACHE_BYTES` here?
>>>>
>>>> Unfortunately `repr(align())` does not accept expression or macro invocations.
>>>> It's still possible with code-generation, but it'll be more tricky.
>>>>
>>>> On all archs that we do support today, I think the value is always 64. However
>>>> it'd worth putting a FIXME or TODO (or assertion, maybe?) in case new archs gets
>>>> addded where this isn't true.
>>>
>>> I was looking into how to implement this properly. Apparently, we don't
>>> have a config item that specifies the L1 cache line size for all
>>> architectures. Each architectures defines the cache line size as C
>>> define in a header. For x86 we have
>>>
>>>   #define L1_CACHE_SHIFT	(CONFIG_X86_L1_CACHE_SHIFT)
>>>
>>> and for arm64
>>>
>>>   #define L1_CACHE_SHIFT		(6)
>>>
>>> and so on.
>>>
>>> One thing we could do is run the C preprocessor on a small snippet to
>>> expand the `L1_CACHE_SHIFT` symbol at some point before invoking
>>> `rustc`. Then we can pass the value to `rustc` via environment variable
>>> when building the `macros` crate. This is similar to how we pass
>>> `RUST_MODFILE` to `rustc`, sans the cpp invocation.
>>>
>>> Otherwise we have to convince all architectures that support Rust to
>>> emit a config that we can rely on, like `CONFIG_L1_CACHE_SHIFT`.
>>>
>>> The latter options is probably the better one, what do you all think?
>>>
>>> Best regards,
>>> Andreas Hindborg
>>
>> You can implement this with generics alone using associative types.
>>
>> mod sealed {
>>     pub trait Sealed {
>>         type Repr;
>>     }
>> }
>>
>> trait Alignment: sealed::Sealed {}
>>
>> #[repr(transparent)]
>> struct Align<const N: usize>([<Self as sealed::Sealed>::Repr; 0])
>> where
>>     Self: Alignment;
>>
>> impl<const N: usize> Align<N>
>> where
>>     Self: Alignment,
>> {
>>     #[inline]
>>     pub fn new() -> Self {
>>         Align([])
>>     }
>> }
>>
>> macro_rules! impl_align {
>>     () => {};
>>     ($a:literal $($rest:literal)*) => {
>>         const _: () = {
>>             #[repr(align($a))]
>>             struct Repr;
>>
>>             impl sealed::Sealed for Align<$a> {
>>                 type Repr = Repr;
>>             }
>>             
>>             impl Alignment for Align<$a> {}
>>         };
>>         
>>         impl_align!($($rest)*);
>>     }
>> }
>>
>> impl_align!(32 64 128 256);
>
> Right, that is great for avoiding code duplication in case we want to
> have `Align<16>`, `Align<32>`, etc.
>
> My discussion was about having just `CacheAligned` and having the
> alignment of that type automatically be the `1 << L1_CACHE_SHIFT`.

#[repr(transparent)]
struct CacheAligned(Align<{ 1 << L1_CACHE_SHIFT }>);

Best,
Gary

  reply	other threads:[~2026-05-19 10:35 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-28 14:05 Andreas Hindborg
2026-01-28 14:25 ` Alexandre Courbot
2026-01-28 14:41   ` Gary Guo
2026-01-28 14:45     ` Miguel Ojeda
2026-01-28 18:19       ` Andreas Hindborg
2026-01-28 14:46     ` Alice Ryhl
2026-01-28 15:05       ` Gary Guo
2026-05-18 13:41     ` Andreas Hindborg
2026-05-18 16:22       ` Gary Guo
2026-05-19  8:18         ` Andreas Hindborg
2026-05-19 10:35           ` Gary Guo [this message]
2026-05-19 11:28             ` Andreas Hindborg
2026-01-28 14:26 ` Miguel Ojeda
2026-01-28 14:34   ` Miguel Ojeda
2026-01-28 18:23   ` Andreas Hindborg
2026-01-28 18:41     ` Miguel Ojeda
2026-01-28 14:32 ` Gary Guo
2026-01-28 18:27   ` Andreas Hindborg
2026-05-19  9:16   ` Andreas Hindborg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DIML4GDC87AA.116PZJDMV23IC@garyguo.net \
    --to=gary@garyguo.net \
    --cc=a.hindborg@kernel.org \
    --cc=acourbot@nvidia.com \
    --cc=aliceryhl@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=dakr@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tmgross@umich.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®