Message-Id: <20250429.221733.2034231929519765445.fujita.tomonori@gmail.com>
Date: Tue, 29 Apr 2025 22:17:33 +0900 (JST)
From: FUJITA Tomonori <fujita.tomonori@...il.com>
To: a.hindborg@...nel.org
Cc: fujita.tomonori@...il.com, rust-for-linux@...r.kernel.org,
 gary@...yguo.net, aliceryhl@...gle.com, me@...enk.dev,
 daniel.almeida@...labora.com, linux-kernel@...r.kernel.org,
 netdev@...r.kernel.org, andrew@...n.ch, hkallweit1@...il.com,
 tmgross@...ch.edu, ojeda@...nel.org, alex.gaynor@...il.com,
 bjorn3_gh@...tonmail.com, benno.lossin@...ton.me, a.hindborg@...sung.com,
 anna-maria@...utronix.de, frederic@...nel.org, tglx@...utronix.de,
 arnd@...db.de, jstultz@...gle.com, sboyd@...nel.org, mingo@...hat.com,
 peterz@...radead.org, juri.lelli@...hat.com, vincent.guittot@...aro.org,
 dietmar.eggemann@....com, rostedt@...dmis.org, bsegall@...gle.com,
 mgorman@...e.de, vschneid@...hat.com, tgunders@...hat.com,
 david.laight.linux@...il.com, boqun.feng@...il.com, pbonzini@...hat.com,
 jfalempe@...hat.com, linux@...linux.org.uk, chrisi.schrefl@...il.com,
 arnd@...db.de, linus.walleij@...aro.org
Subject: Re: [PATCH v15 5/6] rust: time: Add wrapper for fsleep() function

On Mon, 28 Apr 2025 20:16:47 +0200
Andreas Hindborg <a.hindborg@...nel.org> wrote:

> Hi Tomonori,
> 
> "FUJITA Tomonori" <fujita.tomonori@...il.com> writes:
> 
>> Add a wrapper for fsleep(), flexible sleep functions in
>> include/linux/delay.h which typically deals with hardware delays.
>>
>> The kernel supports several sleep functions to handle various lengths
>> of delay. This adds fsleep(), automatically chooses the best sleep
>> method based on a duration.
>>
>> sleep functions including fsleep() belongs to TIMERS, not
>> TIMEKEEPING. They are maintained separately. rust/kernel/time.rs is an
>> abstraction for TIMEKEEPING. To make Rust abstractions match the C
>> side, add rust/kernel/time/delay.rs for this wrapper.
>>
>> fsleep() can only be used in a nonatomic context. This requirement is
>> not checked by these abstractions, but it is intended that klint [1]
>> or a similar tool will be used to check it in the future.
> 
> I get an error when building this patch for arm32:
> 
>   + kernel-make -j 96 O=/home/aeh/src/linux-rust/test-build-arm-1.78.0 vmlinux modules
>   ld.lld: error: undefined symbol: __aeabi_uldivmod
>   >>> referenced by kernel.df165ca450b1fd1-cgu.0
>   >>>               rust/kernel.o:(kernel::time::delay::fsleep) in archive vmlinux.a
>   >>> did you mean: __aeabi_uidivmod
>   >>> defined in: vmlinux.a(arch/arm/lib/lib1funcs.o)
> 
> Looks like a division function of some sort is not defined. Can you
> reproduce?

Ah, 64-bit integer division on 32-bit architectures.

I think that the DRM QR driver has the same problem:

https://lore.kernel.org/rust-for-linux/CANiq72ke45eOwckMhWHvmwxc03dxr4rnxxKvx+HvWdBLopZfrQ@mail.gmail.com/

It appears that there is still no consensus on how to resolve it. CC
the participants in the above thread.

I think that we can drop this patch and better to focus on Instant and
Delta types in this merge window.

With the patch below, this issue could be resolved like the C side,
but I'm not sure whether we can reach a consensus quickly.

diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index 48143cdd26b3..c44d45960eb1 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -19,6 +19,7 @@
 #include "io.c"
 #include "jump_label.c"
 #include "kunit.c"
+#include "math64.c"
 #include "mutex.c"
 #include "page.c"
 #include "platform.c"
diff --git a/rust/helpers/math64.c b/rust/helpers/math64.c
new file mode 100644
index 000000000000..f94708cf8fcb
--- /dev/null
+++ b/rust/helpers/math64.c
@@ -0,0 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/math64.h>
+
+s64 rust_helper_div64_s64(s64 dividend, s64 divisor)
+{
+	return div64_s64(dividend, divisor);
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index de07aadd1ff5..d272e0b0b05d 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -60,6 +60,7 @@
 #[cfg(CONFIG_KUNIT)]
 pub mod kunit;
 pub mod list;
+pub mod math64;
 pub mod miscdevice;
 #[cfg(CONFIG_NET)]
 pub mod net;
diff --git a/rust/kernel/math64.rs b/rust/kernel/math64.rs
new file mode 100644
index 000000000000..523e47911859
--- /dev/null
+++ b/rust/kernel/math64.rs
@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! 64-bit integer arithmetic helpers.
+//!
+//! C header: [`include/linux/math64.h`](srctree/include/linux/math64.h)
+
+/// Divide a signed 64-bit integer by another signed 64-bit integer.
+#[inline]
+pub fn div64_s64(dividend: i64, divisor: i64) -> i64 {
+    // SAFETY: Calling `div64_s64()` is safe as long as `divisor` is non-zero.
+    unsafe { bindings::div64_s64(dividend, divisor) }
+}
diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
index 863385905029..7b5255893929 100644
--- a/rust/kernel/time.rs
+++ b/rust/kernel/time.rs
@@ -24,6 +24,8 @@
 //! C header: [`include/linux/jiffies.h`](srctree/include/linux/jiffies.h).
 //! C header: [`include/linux/ktime.h`](srctree/include/linux/ktime.h).
 
+use crate::math64;
+
 pub mod delay;
 pub mod hrtimer;
 
@@ -229,13 +231,16 @@ pub const fn as_nanos(self) -> i64 {
     /// Return the smallest number of microseconds greater than or equal
     /// to the value in the [`Delta`].
     #[inline]
-    pub const fn as_micros_ceil(self) -> i64 {
-        self.as_nanos().saturating_add(NSEC_PER_USEC - 1) / NSEC_PER_USEC
+    pub fn as_micros_ceil(self) -> i64 {
+        math64::div64_s64(
+            self.as_nanos().saturating_add(NSEC_PER_USEC - 1),
+            NSEC_PER_USEC,
+        )
     }
 
     /// Return the number of milliseconds in the [`Delta`].
     #[inline]
-    pub const fn as_millis(self) -> i64 {
-        self.as_nanos() / NSEC_PER_MSEC
+    pub fn as_millis(self) -> i64 {
+        math64::div64_s64(self.as_nanos(), NSEC_PER_MSEC)
     }
 }

base-commit: da37ddd3f607897d039d82e6621671c3f7baa886
-- 
2.43.0

Powered by blists - more mailing lists