Re: [PATCH v4 2/2] rust: time: add Delta::to_jiffies_timeout() for timeout conversion

[Andreas Hindborg <a.hindborg@kernel.org>]

"FUJITA Tomonori" <tomo@flapping.org> writes:

> From: FUJITA Tomonori <fujita.tomonori@gmail.com>
>
> The boundaries of __msecs_to_jiffies() and nsecs_to_jiffies64() depend
> on which HZ branch is compiled. With CONFIG_HZ_300
> nsecs_to_jiffies64() overflows after 64.99 years, which is less than
> the 292 years a Delta can hold. With HZ=1000 a jiffy is a millisecond,
> so __msecs_to_jiffies() returns its argument unchanged and never caps
> it at MAX_JIFFY_OFFSET.
>
> Compute the conversion in Rust with mul_u64_add_u64_div_u64() instead. It
> returns (a * b + c) / d, computing a * b internally in 128 bits, so
> ceil(nanos * HZ / NSEC_PER_SEC) needs no input clamp and rounds once. The
> bound then follows from the arithmetic: with HZ <= NSEC_PER_SEC, which a
> static_assert() checks, the result is at most the nanosecond count.
>
> Unless the result saturates, the value is rounded up, so the timeout is
> never shorter than the requested span. It saturates at zero jiffies for a
> negative span, i.e. an immediate timeout, and at MAX_JIFFY_OFFSET, the
> upper bound the kernel uses for a jiffies span. Since MAX_JIFFY_OFFSET is
> derived from long, only 32 bit can reach it, and a saturated timeout there
> is finite, so it can be shorter than the requested span.
>
> Reviewed-by: Gary Guo <gary@garyguo.net>
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> ---
>  rust/helpers/helpers.c   |   1 +
>  rust/helpers/math.c      |   8 +++
>  rust/kernel/Kconfig.test |  10 ++++
>  rust/kernel/time.rs      | 105 +++++++++++++++++++++++++++++++++++++++
>  4 files changed, 124 insertions(+)
>  create mode 100644 rust/helpers/math.c
>
> diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
> index 440fb7638e3c..08374b163774 100644
> --- a/rust/helpers/helpers.c
> +++ b/rust/helpers/helpers.c
> @@ -73,6 +73,7 @@
>  #include "kunit.c"
>  #include "list.c"
>  #include "maple_tree.c"
> +#include "math.c"
>  #include "mm.c"
>  #include "mutex.c"
>  #include "net/genetlink.c"
> diff --git a/rust/helpers/math.c b/rust/helpers/math.c
> new file mode 100644
> index 000000000000..e2ee29bcce0f
> --- /dev/null
> +++ b/rust/helpers/math.c
> @@ -0,0 +1,8 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/math64.h>
> +
> +__rust_helper u64 rust_helper_mul_u64_add_u64_div_u64(u64 a, u64 b, u64 c, u64 d)
> +{
> +	return mul_u64_add_u64_div_u64(a, b, c, d);
> +}
> diff --git a/rust/kernel/Kconfig.test b/rust/kernel/Kconfig.test
> index e6a5c7a795f0..0087749995d2 100644
> --- a/rust/kernel/Kconfig.test
> +++ b/rust/kernel/Kconfig.test
> @@ -83,4 +83,14 @@ config RUST_BITFIELD_KUNIT_TEST
>
>  	  If unsure, say N.
>
> +config RUST_TIME_KUNIT_TEST
> +	bool "KUnit tests for the Rust time API" if !KUNIT_ALL_TESTS
> +	default KUNIT_ALL_TESTS
> +	help
> +	  This option enables KUnit tests for the Rust time API.
> +	  These are only for development and testing, not for regular
> +	  kernel use cases.
> +
> +	  If unsure, say N.

Why would you gate out the tests? They do not look like they take long
to run or anything.


Best regards,
Andreas Hindborg