linux-kernel - Re: [PATCH v4 04/20] rust: add new `num` module with useful integer operations

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: <DAD9TNUBUGPN.1ED519FYR29U4@kernel.org>
Date: Wed, 04 Jun 2025 01:02:08 +0200
From: "Benno Lossin" <lossin@...nel.org>
To: "Alexandre Courbot" <acourbot@...dia.com>, "Miguel Ojeda"
 <ojeda@...nel.org>, "Alex Gaynor" <alex.gaynor@...il.com>, "Boqun Feng"
 <boqun.feng@...il.com>, "Gary Guo" <gary@...yguo.net>,
 Björn Roy Baron <bjorn3_gh@...tonmail.com>, "Benno Lossin"
 <benno.lossin@...ton.me>, "Andreas Hindborg" <a.hindborg@...nel.org>,
 "Alice Ryhl" <aliceryhl@...gle.com>, "Trevor Gross" <tmgross@...ch.edu>,
 "Danilo Krummrich" <dakr@...nel.org>, "David Airlie" <airlied@...il.com>,
 "Simona Vetter" <simona@...ll.ch>, "Maarten Lankhorst"
 <maarten.lankhorst@...ux.intel.com>, "Maxime Ripard" <mripard@...nel.org>,
 "Thomas Zimmermann" <tzimmermann@...e.de>
Cc: "John Hubbard" <jhubbard@...dia.com>, "Ben Skeggs" <bskeggs@...dia.com>,
 "Joel Fernandes" <joelagnelf@...dia.com>, "Timur Tabi" <ttabi@...dia.com>,
 "Alistair Popple" <apopple@...dia.com>, <linux-kernel@...r.kernel.org>,
 <rust-for-linux@...r.kernel.org>, <nouveau@...ts.freedesktop.org>,
 <dri-devel@...ts.freedesktop.org>
Subject: Re: [PATCH v4 04/20] rust: add new `num` module with useful integer
 operations

On Mon Jun 2, 2025 at 3:09 PM CEST, Alexandre Courbot wrote:
> On Thu May 29, 2025 at 4:27 PM JST, Benno Lossin wrote:
>> On Thu May 29, 2025 at 3:18 AM CEST, Alexandre Courbot wrote:
>>> On Thu May 29, 2025 at 5:17 AM JST, Benno Lossin wrote:
>>>> On Wed May 21, 2025 at 8:44 AM CEST, Alexandre Courbot wrote:
>>>>> +    /// Align `self` up to `alignment`.
>>>>> +    ///
>>>>> +    /// `alignment` must be a power of 2 for accurate results.
>>>>> +    ///
>>>>> +    /// Wraps around to `0` if the requested alignment pushes the result above the type's limits.
>>>>> +    ///
>>>>> +    /// # Examples
>>>>> +    ///
>>>>> +    /// ```
>>>>> +    /// use kernel::num::NumExt;
>>>>> +    ///
>>>>> +    /// assert_eq!(0x4fffu32.align_up(0x1000), 0x5000);
>>>>> +    /// assert_eq!(0x4000u32.align_up(0x1000), 0x4000);
>>>>> +    /// assert_eq!(0x0u32.align_up(0x1000), 0x0);
>>>>> +    /// assert_eq!(0xffffu16.align_up(0x100), 0x0);
>>>>> +    /// assert_eq!(0x4fffu32.align_up(0x0), 0x0);
>>>>> +    /// ```
>>>>> +    fn align_up(self, alignment: Self) -> Self;
>>>>
>>>> Isn't this `next_multiple_of` [1] (it also allows non power of 2
>>>> inputs).
>>>>
>>>> [1]: https://doc.rust-lang.org/std/primitive.u32.html#method.next_multiple_of
>>>
>>> It is, however the fact that `next_multiple_of` works with non powers of
>>> two also means it needs to perform a modulo operation. That operation
>>> might well be optimized away by the compiler, but ACAICT we have no way
>>> of proving it will always be the case, hence the always-optimal
>>> implementation here.
>>
>> When you use a power of 2 constant, then I'm very sure that it will get
>> optimized [1]. Even with non-powers of 2, you don't get a division [2].
>> If you find some code that is not optimized, then sure add a custom
>> function.
>>
>> [1]: https://godbolt.org/z/57M9e36T3
>> [2]: https://godbolt.org/z/9P4P8zExh
>
> That's impressive and would definitely work well with a constant. But
> when the value is not known at compile-time, the division does occur
> unfortunately: https://godbolt.org/z/WK1bPMeEx
>
> So I think we will still need a kernel-optimized version of these
> alignment functions.

Hmm what exactly is the use-case for a variable align amount? Could you
store it in const generics?

If not, there are also these two variants that are more efficient:

* option: https://godbolt.org/z/ecnb19zaM
* unsafe: https://godbolt.org/z/EqTaGov71

So if the compiler can infer it from context it still optimizes it :)

But yeah to be extra sure, you need your version. By the way, what
happens if `align` is not a power of 2 in your version?

>>> Also in the kernel we tend to use the `align` nomenclature and I think we
>>> should preserve that for clarity.
>>
>> That's also fair, but we lose the constness of `next_multiple_of`, so
>> you can't use `align_up` in a const function. That might confuse people
>> and then they write their own const helper function... I'd prefer we use
>> all functions that are available in the stdlib.
>
> We definitely want const variants of these, one way or the other (const
> methods in traits are not available yet unfortunately). And yes, on
> principle I am aligned (haha) with using stdlib functions when possible.

Then I'd recommend getting in touch with upstream Rust :) And choose
rusty names for our trait so we have an easy switch.

---
Cheers,
Benno