| 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
| ||
|
Message-Id: <DAKL0KOWUB1G.1DSJPRWFYC43O@nvidia.com> Date: Thu, 12 Jun 2025 22:17:01 +0900 From: "Alexandre Courbot" <acourbot@...dia.com> To: "Benno Lossin" <lossin@...nel.org>, "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 Wed Jun 4, 2025 at 4:18 PM JST, Benno Lossin wrote: > On Wed Jun 4, 2025 at 2:05 AM CEST, Alexandre Courbot wrote: >> On Wed Jun 4, 2025 at 8:02 AM JST, Benno Lossin wrote: >>> 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? >> >> Say you have an IOMMU with support for different pages sizes, the size >> of a particular page can be decided at runtime. >> >>> >>> 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 :) >> >> I think the `Option` (and subsequent `unwrap`) is something we want to >> avoid on such a common operation. > > Makes sense. > >>> 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? >> >> It will just return `(self + (self - 1)) & (alignment - 1)`, which will >> likely be a value you don't want. > > So wouldn't it be better to make users validate that they gave a > power-of-2 alignment? > >> So yes, for this particular operation we would prefer to only use powers >> of 2 as inputs - if we can ensure that then it solves most of our >> problems (can use `next_multiple_of`, no `Option`, etc). >> >> Maybe we can introduce a new integer type that, similarly to `NonZero`, >> guarantees that the value it stores is a power of 2? Users with const >> values (90+% of uses) won't see any difference, and if working with a >> runtime-generated value we will want to validate it anyway... > > I like this idea. But it will mean that we have to have a custom > function that is either standalone and const or in an extension trait :( > But for this one we can use the name `align_up` :) > > Here is a cool idea for the implementation: https://godbolt.org/z/x6navM5WK Yeah that's close to what I had in mind. Actually, we can also define `align_up` and `align_down` within this new type, and these methods can now be const since they are not implemented via a trait!
Powered by blists - more mailing lists