Alexandre Courbot
2025-May-01 12:58 UTC
[PATCH v2 17/21] rust: num: Add an upward alignment helper for usize
From: Joel Fernandes <joelagnelf at nvidia.com> This will be used in the nova-core driver where we need to upward-align the image size to get to the next image in the VBIOS ROM. [acourbot at nvidia.com: handled conflicts due to removal of patch creating num.rs] Signed-off-by: Joel Fernandes <joelagnelf at nvidia.com> Signed-off-by: Alexandre Courbot <acourbot at nvidia.com> --- rust/kernel/lib.rs | 1 + rust/kernel/num.rs | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs index ab0286857061d2de1be0279cbd2cd3490e5a48c3..be75b196aa7a29cf3eed7c902ed8fb98689bbb50 100644 --- a/rust/kernel/lib.rs +++ b/rust/kernel/lib.rs @@ -67,6 +67,7 @@ pub mod miscdevice; #[cfg(CONFIG_NET)] pub mod net; +pub mod num; pub mod of; pub mod page; #[cfg(CONFIG_PCI)] diff --git a/rust/kernel/num.rs b/rust/kernel/num.rs new file mode 100644 index 0000000000000000000000000000000000000000..953c6ab012601efb3c9387b4b299e22233670c4b --- /dev/null +++ b/rust/kernel/num.rs @@ -0,0 +1,21 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! Numerical and binary utilities for primitive types. + +/// A trait providing alignment operations for `usize`. +pub trait UsizeAlign { + /// Aligns `self` upwards to the nearest multiple of `align`. + fn align_up(self, align: usize) -> usize; +} + +impl UsizeAlign for usize { + fn align_up(mut self, align: usize) -> usize { + self = (self + align - 1) & !(align - 1); + self + } +} + +/// Aligns `val` upwards to the nearest multiple of `align`. +pub const fn usize_align_up(val: usize, align: usize) -> usize { + (val + align - 1) & !(align - 1) +} -- 2.49.0
Timur Tabi
2025-May-01 15:19 UTC
[PATCH v2 17/21] rust: num: Add an upward alignment helper for usize
On Thu, 2025-05-01 at 21:58 +0900, Alexandre Courbot wrote:> +impl UsizeAlign for usize { > +??? fn align_up(mut self, align: usize) -> usize { > +??????? self = (self + align - 1) & !(align - 1); > +??????? self > +??? } > +} > + > +/// Aligns `val` upwards to the nearest multiple of `align`. > +pub const fn usize_align_up(val: usize, align: usize) -> usize { > +??? (val + align - 1) & !(align - 1) > +}Why not have usize_align_up() just return "val.align_up(align)"? But why why two versions at all? Is there any context where you could use one and not the other?
Alexandre Courbot
2025-May-02 04:57 UTC
[PATCH v2 17/21] rust: num: Add an upward alignment helper for usize
On Thu May 1, 2025 at 9:58 PM JST, Alexandre Courbot wrote:> From: Joel Fernandes <joelagnelf at nvidia.com> > > This will be used in the nova-core driver where we need to upward-align > the image size to get to the next image in the VBIOS ROM. > > [acourbot at nvidia.com: handled conflicts due to removal of patch creating > num.rs] > > Signed-off-by: Joel Fernandes <joelagnelf at nvidia.com> > Signed-off-by: Alexandre Courbot <acourbot at nvidia.com> > --- > rust/kernel/lib.rs | 1 + > rust/kernel/num.rs | 21 +++++++++++++++++++++ > 2 files changed, 22 insertions(+) > > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs > index ab0286857061d2de1be0279cbd2cd3490e5a48c3..be75b196aa7a29cf3eed7c902ed8fb98689bbb50 100644 > --- a/rust/kernel/lib.rs > +++ b/rust/kernel/lib.rs > @@ -67,6 +67,7 @@ > pub mod miscdevice; > #[cfg(CONFIG_NET)] > pub mod net; > +pub mod num; > pub mod of; > pub mod page; > #[cfg(CONFIG_PCI)] > diff --git a/rust/kernel/num.rs b/rust/kernel/num.rs > new file mode 100644 > index 0000000000000000000000000000000000000000..953c6ab012601efb3c9387b4b299e22233670c4b > --- /dev/null > +++ b/rust/kernel/num.rs > @@ -0,0 +1,21 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +//! Numerical and binary utilities for primitive types. > + > +/// A trait providing alignment operations for `usize`. > +pub trait UsizeAlign { > + /// Aligns `self` upwards to the nearest multiple of `align`. > + fn align_up(self, align: usize) -> usize; > +}As it turns out I will also need the same functionality for u64 in a future patch. :) Could we turn this into a more generic trait? E.g: /// A trait providing alignment operations for `usize`. pub trait Align { /// Aligns `self` upwards to the nearest multiple of `align`. fn align_up(self, align: Self) -> Self; } That way it can be implemented for all basic types. I'd suggest having a local macro that takes an arbitrary number of arguments and generates the impl blocks for each of them, which would be invoked as: impl_align!(i8, u8, i16, u16, ...);> +impl UsizeAlign for usize { > + fn align_up(mut self, align: usize) -> usize { > + self = (self + align - 1) & !(align - 1); > + self > + } > +}Note that there is no need to mutate `self`, the body can just be: (self + align - 1) & !(align - 1) This operation can trigger overflows/underflows, so I guess for safety it should return a `Result` and use `checked_add` and `checked_sub`, after moving `align - 1` into a local variable to avoid checking it twice. There is also the interesting question of, what if `align` is not a power of 2. We should probably document the fact that it is expected to be.> +/// Aligns `val` upwards to the nearest multiple of `align`. > +pub const fn usize_align_up(val: usize, align: usize) -> usize { > + (val + align - 1) & !(align - 1) > +}Let's add a statement in the documentation saying that this exists for use in `const` contexts. The `impl` version can maybe call this one directly to avoid repeating the same operation twice. Actually I'm not sure whether we want the `impl` block at all since this provides the same functionality, albeit in a slightly less cosmetic way. Can core R4L folks provide guidance about that? Cheers, Alex.