On Thu May 1, 2025 at 9:58 PM JST, Alexandre Courbot wrote:
> From: Joel Fernandes <joelagn...@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.
>
> [acour...@nvidia.com: handled conflicts due to removal of patch creating
> num.rs]
>
> Signed-off-by: Joel Fernandes <joelagn...@nvidia.com>
> Signed-off-by: Alexandre Courbot <acour...@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.
