* [PATCH] rust: math: Add KernelMathExt trait with a mul_div helper [not found] <CGME20250609215523eucas1p22f6d7d84b403badfb77af7df973b97a9@eucas1p2.samsung.com> @ 2025-06-09 21:53 ` Michal Wilczynski 2025-06-09 22:21 ` Benno Lossin ` (2 more replies) 0 siblings, 3 replies; 6+ messages in thread From: Michal Wilczynski @ 2025-06-09 21:53 UTC (permalink / raw) To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich, Marek Szyprowski Cc: linux-kernel, rust-for-linux, Michal Wilczynski 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@samsung.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@samsung.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; + } + // SAFETY: The C function `mul_u64_u64_div_u64` is safe to call because the divisor + // is guaranteed to be non-zero. The FFI bindings use `u64`, matching our types. + Some(unsafe { bindings::mul_u64_u64_div_u64(self, multiplier, divisor) }) + } +} --- base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494 change-id: 20250609-math-rust-v1-d3989515e32e Best regards, -- Michal Wilczynski <m.wilczynski@samsung.com> ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] rust: math: Add KernelMathExt trait with a mul_div helper 2025-06-09 21:53 ` [PATCH] rust: math: Add KernelMathExt trait with a mul_div helper Michal Wilczynski @ 2025-06-09 22:21 ` Benno Lossin 2025-06-12 11:00 ` Michal Wilczynski 2025-06-10 13:09 ` kernel test robot 2025-06-13 13:32 ` Alexandre Courbot 2 siblings, 1 reply; 6+ messages in thread From: Benno Lossin @ 2025-06-09 22:21 UTC (permalink / raw) To: Michal Wilczynski, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich, Marek Szyprowski Cc: linux-kernel, rust-for-linux On Mon Jun 9, 2025 at 11:53 PM CEST, 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 You could turn this into a `Suggested-by`. > 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/ Please use the `Link: https://... [.]` format. > > Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.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@samsung.com> > + > +//! Safe wrappers for kernel math helpers. I wouldn't stress the fact that these are safe, they better be! > +//! > +//! 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 { We should add this trait to the prelude. > + /// Multiplies self by `multiplier and divides by divisor. > + /// > + /// This wrapper around the kernel's `mul_u64_u64_div_u64` C helper ensures that no The trait shouldn't make a reference to `u64`. > + /// overflow occurs during the intermediate multiplication. > + /// > + /// # Returns > + /// * Some(result) if the division is successful. Use backtics (`) for code in documentation and comments. > + /// * None if the divisor is zero. > + fn mul_div(self, multiplier: Self, divisor: Self) -> Option<Self>; > +} > + > +impl KernelMathExt for u64 { Can you also implement this for the other types that have a `mul_..._div` function in the kernel? > + fn mul_div(self, multiplier: u64, divisor: u64) -> Option<u64> { > + if divisor == 0 { > + return None; > + } > + // SAFETY: The C function `mul_u64_u64_div_u64` is safe to call because the divisor > + // is guaranteed to be non-zero. The FFI bindings use `u64`, matching our types. Let's not talk about "safe to call", but rather say it like this: // SAFETY: `mul_u64_u64_div_u64` requires the divisor to be non-zero // which is checked above. It has no other requirements. --- Cheers, Benno > + Some(unsafe { bindings::mul_u64_u64_div_u64(self, multiplier, divisor) }) > + } > +} > > --- > base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494 > change-id: 20250609-math-rust-v1-d3989515e32e > > Best regards, ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rust: math: Add KernelMathExt trait with a mul_div helper 2025-06-09 22:21 ` Benno Lossin @ 2025-06-12 11:00 ` Michal Wilczynski 2025-06-12 11:10 ` Benno Lossin 0 siblings, 1 reply; 6+ messages in thread From: Michal Wilczynski @ 2025-06-12 11:00 UTC (permalink / raw) To: Benno Lossin, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich, Marek Szyprowski, Uwe Kleine-König Cc: linux-kernel, rust-for-linux On 6/10/25 00:21, Benno Lossin wrote: > On Mon Jun 9, 2025 at 11:53 PM CEST, 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 > > You could turn this into a `Suggested-by`. > >> 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/ > > Please use the `Link: https://... [.]` format. > >> >> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.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@samsung.com> >> + >> +//! Safe wrappers for kernel math helpers. > > I wouldn't stress the fact that these are safe, they better be! > >> +//! >> +//! 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 { > > We should add this trait to the prelude. > >> + /// Multiplies self by `multiplier and divides by divisor. >> + /// >> + /// This wrapper around the kernel's `mul_u64_u64_div_u64` C helper ensures that no > > The trait shouldn't make a reference to `u64`. > >> + /// overflow occurs during the intermediate multiplication. >> + /// >> + /// # Returns >> + /// * Some(result) if the division is successful. > > Use backtics (`) for code in documentation and comments. > >> + /// * None if the divisor is zero. >> + fn mul_div(self, multiplier: Self, divisor: Self) -> Option<Self>; >> +} >> + >> +impl KernelMathExt for u64 { > > Can you also implement this for the other types that have a > `mul_..._div` function in the kernel? +Uwe Hi Benno, Thank you for the review and feedback on the patch. Regarding your suggestion to implement the trait for other types, I've taken a closer look at the existing kernel helpers (e.g., in lib/math/div64.c). My finding is that while some signed division helpers exist, there don't appear to be standard, exported mul_sX_sX_div_sX functions that are direct equivalents of the u64 version. This makes the generic trait idea less broadly applicable than I initially hoped. Furthermore, a more significant issue has come to my attention regarding the u64 C function itself. The x86-specific static inline implementation uses assembly that triggers a #DE (Divide Error) exception if the final quotient overflows the 64-bit result. This would lead to a kernel panic. /* * Will generate an #DE when the result doesn't fit u64, could fix with an * __ex_table[] entry when it becomes an issue. */ static inline u64 mul_u64_u64_div_u64(...) Given that a direct FFI binding would expose this potentially panicking behavior to safe Rust, I am now reconsidering if binding this function is the right approach. Perhaps this should be handled in pure Rust. For my specific case with the PWM driver, it's likely that a simple checked_mul would be sufficient. In practice, many drivers use this function for calculations like the following, where one of the multiplicands is not a full 64-bit value: wf->duty_offset_ns = DIV64_U64_ROUND_UP((u64)wfhw->duty_offset_cnt * NSEC_PER_SEC, ddata->clk_rate_hz); In this common pattern, the intermediate multiplication often does not risk overflowing a u64. Using checked_mul would be completely safe and avoid the FFI complexity and the overflow risk entirely. Given these points, do you still think pursuing the general-purpose KernelMathExt trait via an FFI wrapper is the desired direction? Or would you prefer that I pivot to a simpler, safer, pure-Rust approach using checked_mul directly within the PWM driver where it is needed? > >> + fn mul_div(self, multiplier: u64, divisor: u64) -> Option<u64> { >> + if divisor == 0 { >> + return None; >> + } >> + // SAFETY: The C function `mul_u64_u64_div_u64` is safe to call because the divisor >> + // is guaranteed to be non-zero. The FFI bindings use `u64`, matching our types. > > Let's not talk about "safe to call", but rather say it like this: > > // SAFETY: `mul_u64_u64_div_u64` requires the divisor to be non-zero > // which is checked above. It has no other requirements. > > --- > Cheers, > Benno > >> + Some(unsafe { bindings::mul_u64_u64_div_u64(self, multiplier, divisor) }) >> + } >> +} >> >> --- >> base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494 >> change-id: 20250609-math-rust-v1-d3989515e32e >> >> Best regards, > > Best regards, -- Michal Wilczynski <m.wilczynski@samsung.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rust: math: Add KernelMathExt trait with a mul_div helper 2025-06-12 11:00 ` Michal Wilczynski @ 2025-06-12 11:10 ` Benno Lossin 0 siblings, 0 replies; 6+ messages in thread From: Benno Lossin @ 2025-06-12 11:10 UTC (permalink / raw) To: Michal Wilczynski, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich, Marek Szyprowski, Uwe Kleine-König Cc: linux-kernel, rust-for-linux On Thu Jun 12, 2025 at 1:00 PM CEST, Michal Wilczynski wrote: > On 6/10/25 00:21, Benno Lossin wrote: >> On Mon Jun 9, 2025 at 11:53 PM CEST, Michal Wilczynski wrote: >>> + /// * None if the divisor is zero. >>> + fn mul_div(self, multiplier: Self, divisor: Self) -> Option<Self>; >>> +} >>> + >>> +impl KernelMathExt for u64 { >> >> Can you also implement this for the other types that have a >> `mul_..._div` function in the kernel? > > +Uwe > > Hi Benno, > > Thank you for the review and feedback on the patch. > > Regarding your suggestion to implement the trait for other types, I've > taken a closer look at the existing kernel helpers (e.g., in > lib/math/div64.c). My finding is that while some signed division helpers > exist, there don't appear to be standard, exported mul_sX_sX_div_sX > functions that are direct equivalents of the u64 version. This makes the > generic trait idea less broadly applicable than I initially hoped. > > Furthermore, a more significant issue has come to my attention regarding > the u64 C function itself. The x86-specific static inline implementation > uses assembly that triggers a #DE (Divide Error) exception if the final > quotient overflows the 64-bit result. This would lead to a kernel panic. > > /* > * Will generate an #DE when the result doesn't fit u64, could fix with an > * __ex_table[] entry when it becomes an issue. > */ > static inline u64 mul_u64_u64_div_u64(...) > > Given that a direct FFI binding would expose this potentially panicking > behavior to safe Rust, I am now reconsidering if binding this function > is the right approach. > > Perhaps this should be handled in pure Rust. For my specific case with > the PWM driver, it's likely that a simple checked_mul would be > sufficient. In practice, many drivers use this function for calculations > like the following, where one of the multiplicands is not a full 64-bit > value: > wf->duty_offset_ns = DIV64_U64_ROUND_UP((u64)wfhw->duty_offset_cnt * NSEC_PER_SEC, > ddata->clk_rate_hz); > > In this common pattern, the intermediate multiplication often does not > risk overflowing a u64. Using checked_mul would be completely safe and > avoid the FFI complexity and the overflow risk entirely. > > Given these points, do you still think pursuing the general-purpose > KernelMathExt trait via an FFI wrapper is the desired direction? Or > would you prefer that I pivot to a simpler, safer, pure-Rust approach > using checked_mul directly within the PWM driver where it is needed? My understanding was that your use-case required the multiplication & division operation to work even if `a * b` would overflow and only the division by `c` would bring it back into u64 range. If you don't need that, then I would just use `checked_mul` as it is much simpler. We can always reintroduce a `KernelMathExt` trait later when the need arises. --- Cheers, Benno ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rust: math: Add KernelMathExt trait with a mul_div helper 2025-06-09 21:53 ` [PATCH] rust: math: Add KernelMathExt trait with a mul_div helper Michal Wilczynski 2025-06-09 22:21 ` Benno Lossin @ 2025-06-10 13:09 ` kernel test robot 2025-06-13 13:32 ` Alexandre Courbot 2 siblings, 0 replies; 6+ messages in thread From: kernel test robot @ 2025-06-10 13:09 UTC (permalink / raw) To: Michal Wilczynski, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich, Marek Szyprowski Cc: oe-kbuild-all, linux-kernel, rust-for-linux, Michal Wilczynski Hi Michal, kernel test robot noticed the following build errors: [auto build test ERROR on 19272b37aa4f83ca52bdf9c16d5d81bdd1354494] url: https://github.com/intel-lab-lkp/linux/commits/Michal-Wilczynski/rust-math-Add-KernelMathExt-trait-with-a-mul_div-helper/20250610-055722 base: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494 patch link: https://lore.kernel.org/r/20250609-math-rust-v1-v1-1-285fac00031f%40samsung.com patch subject: [PATCH] rust: math: Add KernelMathExt trait with a mul_div helper config: x86_64-rhel-9.4-rust (https://download.01.org/0day-ci/archive/20250610/202506102043.Tvsj0sXM-lkp@intel.com/config) compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff) rustc: rustc 1.78.0 (9b00956e5 2024-04-29) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250610/202506102043.Tvsj0sXM-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202506102043.Tvsj0sXM-lkp@intel.com/ All errors (new ones prefixed by >>): >> error[E0425]: cannot find function `mul_u64_u64_div_u64` in crate `bindings` --> rust/kernel/math.rs:32:33 | 32 | Some(unsafe { bindings::mul_u64_u64_div_u64(self, multiplier, divisor) }) | ^^^^^^^^^^^^^^^^^^^ not found in `bindings` -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rust: math: Add KernelMathExt trait with a mul_div helper 2025-06-09 21:53 ` [PATCH] rust: math: Add KernelMathExt trait with a mul_div helper Michal Wilczynski 2025-06-09 22:21 ` Benno Lossin 2025-06-10 13:09 ` kernel test robot @ 2025-06-13 13:32 ` Alexandre Courbot 2 siblings, 0 replies; 6+ messages in thread From: Alexandre Courbot @ 2025-06-13 13:32 UTC (permalink / raw) To: Michal Wilczynski, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich, Marek Szyprowski Cc: linux-kernel, rust-for-linux 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@samsung.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@samsung.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. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-06-13 13:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20250609215523eucas1p22f6d7d84b403badfb77af7df973b97a9@eucas1p2.samsung.com>
2025-06-09 21:53 ` [PATCH] rust: math: Add KernelMathExt trait with a mul_div helper Michal Wilczynski
2025-06-09 22:21 ` Benno Lossin
2025-06-12 11:00 ` Michal Wilczynski
2025-06-12 11:10 ` Benno Lossin
2025-06-10 13:09 ` kernel test robot
2025-06-13 13:32 ` Alexandre Courbot
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.