[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CANiq72=nMQb_zKOngmKWQQeESGAovC+pE2wiD7Njxxix-BQ3Fw@mail.gmail.com>
Date: Wed, 28 Jan 2026 15:26:29 +0100
From: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
To: Andreas Hindborg <a.hindborg@...nel.org>
Cc: 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>, 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 3:05 PM Andreas Hindborg <a.hindborg@...nel.org> wrote:
>
> Signed-off-by: Andreas Hindborg <a.hindborg@...sung.com>
> ---
> Signed-off-by: Andreas Hindborg <a.hindborg@...nel.org>
Something strange is going on with the tags.
> +// SPDX-License-Identifier: GPL-2.0
Please give a title to the module even if it is not public.
> +use kernel::try_pin_init;
> +use pin_init::{
> + pin_data,
> + pin_init,
> + PinInit, //
> +};
> +
> +/// Wrapper type that alings content to a 64 byte cache line.
Typo.
More importantly, please add some docs and examples -- the commit
message has more documentation than the code... :)
Also, it would be nice to show a user in a second patch.
> +#[repr(align(64))]
Even if 64 bytes is common, wouldn't this depend on the system?
> + /// Creates a fallible initializer for `CacheAligned<T>` form a fallible
> + /// initalizer for `T`
Two typos and missing period at the end.
Also just using [`CacheAligned`] would probably be simpler.
> + // SAFETY: by function safety requirements `ptr` is valid for read
// SAFETY: By the the safety requirement, `ptr` is valid for read.
> + try_pin_init!( CacheAligned {
This and another one is formatted differently than we usually do.
> +pub use cache_aligned::CacheAligned;
It seems you want this short, in which case it should perhaps go in
that case, but I think it is best to leave an extra level otherwise
and let users import it.
Thanks!
Cheers,
Miguel
Powered by blists - more mailing lists