Re: [PATCH] rust: math: Add KernelMathExt trait with a mul_div helper

[Alexandre Courbot]

On Tue Jun 10, 2025 at 6:53 AM JST, Michal Wilczynski wrote:
> The PWM subsystem and other kernel modules often need to perform a
> 64 by 64-bit multiplication followed by a 64-bit division. Performing
> this naively in Rust risks overflow on the intermediate multiplication.
> The kernel provides the C helper 'mul_u64_u64_div_u64' for this exact
> purpose.
>
> Introduce a safe Rust wrapper for this function to make it available to
> Rust drivers.
>
> Following feedback from the mailing list [1], this functionality is
> provided via a 'KernelMathExt' extension trait. This allows for
> idiomatic, method style calls (e.g. val.mul_div()) and provides a
> scalable pattern for adding helpers for other integer types in the
> future.
>
> The safe wrapper is named 'mul_div' and not 'mul_u64_u64_div_u64' [2]
> because its behavior differs from the underlying C function. The C
> helper traps on a division by zero, whereas this safe wrapper returns
> `None`, thus exhibiting different and safer behavior.
>
> This is required for the Rust PWM TH1520 driver [3].
>
> [1] - https://lore.kernel.org/all/DAFQ19RBBSQL.3OGUXOQ0PA9YH@xxxxxxxxxx/
> [2] - https://lore.kernel.org/all/CANiq72kVvLogBSVKz0eRg6V4LDB1z7b-6y1WPLSQfXXLW7X3cw@xxxxxxxxxxxxxx/
> [3] - https://lore.kernel.org/all/20250524-rust-next-pwm-working-fan-for-sending-v1-2-bdd2d5094ff7@xxxxxxxxxxx/
>
> Signed-off-by: Michal Wilczynski <m.wilczynski@xxxxxxxxxxx>
> ---
>  rust/kernel/lib.rs  |  1 +
>  rust/kernel/math.rs | 34 ++++++++++++++++++++++++++++++++++
>  2 files changed, 35 insertions(+)
>
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 6b4774b2b1c37f4da1866e993be6230bc6715841..d652c92633b82525f37e5cd8a040d268e0c191d1 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -85,6 +85,7 @@
>  #[cfg(CONFIG_KUNIT)]
>  pub mod kunit;
>  pub mod list;
> +pub mod math;
>  pub mod miscdevice;
>  pub mod mm;
>  #[cfg(CONFIG_NET)]
> diff --git a/rust/kernel/math.rs b/rust/kernel/math.rs
> new file mode 100644
> index 0000000000000000000000000000000000000000..b89e23f9266117dcf96561fbf13b1c66a4851b48
> --- /dev/null
> +++ b/rust/kernel/math.rs
> @@ -0,0 +1,34 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2025 Samsung Electronics Co., Ltd.
> +// Author: Michal Wilczynski <m.wilczynski@xxxxxxxxxxx>
> +
> +//! Safe wrappers for kernel math helpers.
> +//!
> +//! This module provides safe, idiomatic Rust wrappers for C functions, whose
> +//! FFI bindings are auto-generated in the `bindings` crate.
> +
> +use crate::bindings;
> +
> +/// An extension trait that provides access to kernel math helpers on primitive integer types.
> +pub trait KernelMathExt: Sized {
> +    /// Multiplies self by `multiplier and divides by divisor.
> +    ///
> +    /// This wrapper around the kernel's `mul_u64_u64_div_u64` C helper ensures that no
> +    /// overflow occurs during the intermediate multiplication.
> +    ///
> +    /// # Returns
> +    /// * Some(result) if the division is successful.
> +    /// * None if the divisor is zero.
> +    fn mul_div(self, multiplier: Self, divisor: Self) -> Option<Self>;
> +}
> +
> +impl KernelMathExt for u64 {
> +    fn mul_div(self, multiplier: u64, divisor: u64) -> Option<u64> {
> +        if divisor == 0 {
> +            return None;
> +        }

Would it make sense to turn `divisor` into a `NonZero<Self>` and return
`u64`? This could remove the need for the caller to perform a check if
it can statically infer that the divisor is not zero.