Nouveau - May 2025 - [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

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?

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.