* Re: [PATCH] rust: time: implement `Display` for `Delta`
[not found] ` <aZLWjBWaBz4h4_SB@google.com>
@ 2026-05-11 14:22 ` Andreas Hindborg
2026-05-11 14:34 ` Gary Guo
0 siblings, 1 reply; 2+ messages in thread
From: Andreas Hindborg @ 2026-05-11 14:22 UTC (permalink / raw)
To: Alice Ryhl
Cc: Boqun Feng, FUJITA Tomonori, Frederic Weisbecker, Lyude Paul,
Thomas Gleixner, Anna-Maria Behnsen, John Stultz, Stephen Boyd,
Miguel Ojeda, Gary Guo, Björn Roy Baron, Benno Lossin,
Trevor Gross, Danilo Krummrich, rust-for-linux, linux-kernel
Alice Ryhl <aliceryhl@google.com> writes:
> On Sun, Feb 15, 2026 at 09:14:05PM +0100, Andreas Hindborg wrote:
>> Implement the `Display` trait for `Delta` so that it can be displayed by a
>> formatter.
>>
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>> ---
>> rust/kernel/time.rs | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
>> index 6ea98dfcd0278..e18fd17f38c5a 100644
>> --- a/rust/kernel/time.rs
>> +++ b/rust/kernel/time.rs
>> @@ -474,3 +474,9 @@ pub fn rem_nanos(self, dividend: i32) -> Self {
>> }
>> }
>> }
>> +
>> +impl kernel::fmt::Display for Delta {
>> + fn fmt(&self, f: &mut kernel::fmt::Formatter<'_>) -> kernel::fmt::Result {
>> + f.write_fmt(kernel::prelude::fmt!("{}", self.as_nanos()))
>
> I think this can just be
>
> self.as_nanos().fmt(f)
>
> Though I think it would be nicer to include an 'ns' suffix here so it's
> not just the number?
I was planning to use this to format a delta to output via a configfs in
rnull. The existing C driver does not include the "ns" suffix, which is
why I did not either.
I am not opposed to adding the suffix for this particular
implementation, but that would mean my user for this code will no longer
be a user, and I would drop the patch in favor of another solution at
the call site.
Please advise which of the following make most sense:
- Merge this patch without a suffix.
- Iterate on this patch and merge it without a user.
- Drop this patch for now.
Best regards,
Andreas Hindborg
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] rust: time: implement `Display` for `Delta`
2026-05-11 14:22 ` [PATCH] rust: time: implement `Display` for `Delta` Andreas Hindborg
@ 2026-05-11 14:34 ` Gary Guo
0 siblings, 0 replies; 2+ messages in thread
From: Gary Guo @ 2026-05-11 14:34 UTC (permalink / raw)
To: Andreas Hindborg, Alice Ryhl
Cc: Boqun Feng, FUJITA Tomonori, Frederic Weisbecker, Lyude Paul,
Thomas Gleixner, Anna-Maria Behnsen, John Stultz, Stephen Boyd,
Miguel Ojeda, Gary Guo, Björn Roy Baron, Benno Lossin,
Trevor Gross, Danilo Krummrich, rust-for-linux, linux-kernel
On Mon May 11, 2026 at 3:22 PM BST, Andreas Hindborg wrote:
> Alice Ryhl <aliceryhl@google.com> writes:
>
>> On Sun, Feb 15, 2026 at 09:14:05PM +0100, Andreas Hindborg wrote:
>>> Implement the `Display` trait for `Delta` so that it can be displayed by a
>>> formatter.
>>>
>>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>>> ---
>>> rust/kernel/time.rs | 6 ++++++
>>> 1 file changed, 6 insertions(+)
>>>
>>> diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
>>> index 6ea98dfcd0278..e18fd17f38c5a 100644
>>> --- a/rust/kernel/time.rs
>>> +++ b/rust/kernel/time.rs
>>> @@ -474,3 +474,9 @@ pub fn rem_nanos(self, dividend: i32) -> Self {
>>> }
>>> }
>>> }
>>> +
>>> +impl kernel::fmt::Display for Delta {
>>> + fn fmt(&self, f: &mut kernel::fmt::Formatter<'_>) -> kernel::fmt::Result {
>>> + f.write_fmt(kernel::prelude::fmt!("{}", self.as_nanos()))
>>
>> I think this can just be
>>
>> self.as_nanos().fmt(f)
>>
>> Though I think it would be nicer to include an 'ns' suffix here so it's
>> not just the number?
>
> I was planning to use this to format a delta to output via a configfs in
> rnull. The existing C driver does not include the "ns" suffix, which is
> why I did not either.
>
> I am not opposed to adding the suffix for this particular
> implementation, but that would mean my user for this code will no longer
> be a user, and I would drop the patch in favor of another solution at
> the call site.
>
> Please advise which of the following make most sense:
>
> - Merge this patch without a suffix.
> - Iterate on this patch and merge it without a user.
> - Drop this patch for now.
Display is supposed to be user-facing and readable. So to me it's weird that the
"Display" impl would format "Delta" as nanoseconds without a suffix. I think if
you need to depend on its behaviour to be just nanosecond numbers, you should
perhaps just use `delta.as_nanos()` on the print side.
Best,
Gary
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-05-11 14:34 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20260215-time-delta-display-v1-1-bdd2d6e98ebf@kernel.org>
[not found] ` <aZLWjBWaBz4h4_SB@google.com>
2026-05-11 14:22 ` [PATCH] rust: time: implement `Display` for `Delta` Andreas Hindborg
2026-05-11 14:34 ` Gary Guo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox