lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87fr7psijk.fsf@t14s.mail-host-address-is-not-set>
Date: Wed, 28 Jan 2026 19:23:27 +0100
From: Andreas Hindborg <a.hindborg@...nel.org>
To: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
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

"Miguel Ojeda" <miguel.ojeda.sandonis@...il.com> writes:

> 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.

Thanks for the comments, I agree with all.

>
>> +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.

I think I wrote this before `kernel::prelude`. I would put it in the
prelude today instead of here, what do you think about that?


Best regards,
Andreas Hindborg




Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ