[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <DALFZ6YG8KPZ.2OQ25ED53ZI5X@nvidia.com>
Date: Fri, 13 Jun 2025 22:32:47 +0900
From: "Alexandre Courbot" <acourbot@...dia.com>
To: "Michal Wilczynski" <m.wilczynski@...sung.com>, "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"
<lossin@...nel.org>, "Andreas Hindborg" <a.hindborg@...nel.org>, "Alice
Ryhl" <aliceryhl@...gle.com>, "Trevor Gross" <tmgross@...ch.edu>, "Danilo
Krummrich" <dakr@...nel.org>, "Marek Szyprowski" <m.szyprowski@...sung.com>
Cc: <linux-kernel@...r.kernel.org>, <rust-for-linux@...r.kernel.org>
Subject: Re: [PATCH] rust: math: Add KernelMathExt trait with a mul_div
helper
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@kernel.org/
> [2] - https://lore.kernel.org/all/CANiq72kVvLogBSVKz0eRg6V4LDB1z7b-6y1WPLSQfXXLW7X3cw@mail.gmail.com/
> [3] - https://lore.kernel.org/all/20250524-rust-next-pwm-working-fan-for-sending-v1-2-bdd2d5094ff7@samsung.com/
>
> Signed-off-by: Michal Wilczynski <m.wilczynski@...sung.com>
> ---
> 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@...sung.com>
> +
> +//! 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.
Powered by blists - more mailing lists