2010-08-20 08:13:41

by Zachary Amsden

[permalink] [raw]
Subject: [KVM timekeeping 17/35] Implement getnsboottime kernel API

Add a kernel call to get the number of nanoseconds since boot. This
is generally useful enough to make it a generic call.

Signed-off-by: Zachary Amsden <[email protected]>
---
include/linux/time.h | 1 +
kernel/time/timekeeping.c | 27 +++++++++++++++++++++++++++
2 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/include/linux/time.h b/include/linux/time.h
index ea3559f..5d04108 100644
--- a/include/linux/time.h
+++ b/include/linux/time.h
@@ -145,6 +145,7 @@ extern void getnstimeofday(struct timespec *tv);
extern void getrawmonotonic(struct timespec *ts);
extern void getboottime(struct timespec *ts);
extern void monotonic_to_bootbased(struct timespec *ts);
+extern s64 getnsboottime(void);

extern struct timespec timespec_trunc(struct timespec t, unsigned gran);
extern int timekeeping_valid_for_hres(void);
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index caf8d4d..d250f0a 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -285,6 +285,33 @@ void ktime_get_ts(struct timespec *ts)
}
EXPORT_SYMBOL_GPL(ktime_get_ts);

+
+/**
+ * getnsboottime - get the bootbased clock in nsec format
+ *
+ * The function calculates the bootbased clock from the realtime
+ * clock and the wall_to_monotonic offset and stores the result
+ * in normalized timespec format in the variable pointed to by @ts.
+ */
+s64 getnsboottime(void)
+{
+ unsigned int seq;
+ s64 secs, nsecs;
+
+ WARN_ON(timekeeping_suspended);
+
+ do {
+ seq = read_seqbegin(&xtime_lock);
+ secs = xtime.tv_sec + wall_to_monotonic.tv_sec;
+ secs += total_sleep_time.tv_sec;
+ nsecs = xtime.tv_nsec + wall_to_monotonic.tv_nsec;
+ nsecs += total_sleep_time.tv_nsec + timekeeping_get_ns();
+
+ } while (read_seqretry(&xtime_lock, seq));
+ return nsecs + (secs * NSEC_PER_SEC);
+}
+EXPORT_SYMBOL_GPL(getnsboottime);
+
/**
* do_gettimeofday - Returns the time of day in a timeval
* @tv: pointer to the timeval to be set
--
1.7.1


2010-08-20 18:40:29

by john stultz

[permalink] [raw]
Subject: Re: [KVM timekeeping 17/35] Implement getnsboottime kernel API

On Thu, 2010-08-19 at 22:07 -1000, Zachary Amsden wrote:
> Add a kernel call to get the number of nanoseconds since boot. This
> is generally useful enough to make it a generic call.

Few comments here.

> Signed-off-by: Zachary Amsden <[email protected]>
> ---
> include/linux/time.h | 1 +
> kernel/time/timekeeping.c | 27 +++++++++++++++++++++++++++
> 2 files changed, 28 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/time.h b/include/linux/time.h
> index ea3559f..5d04108 100644
> --- a/include/linux/time.h
> +++ b/include/linux/time.h
> @@ -145,6 +145,7 @@ extern void getnstimeofday(struct timespec *tv);
> extern void getrawmonotonic(struct timespec *ts);
> extern void getboottime(struct timespec *ts);
> extern void monotonic_to_bootbased(struct timespec *ts);
> +extern s64 getnsboottime(void);

So instead of converting the timespec from getboottime, why did you add
a new interface? Also if not a timespec, why did you pick a s64 instead
of a ktime_t?


> extern struct timespec timespec_trunc(struct timespec t, unsigned gran);
> extern int timekeeping_valid_for_hres(void);
> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> index caf8d4d..d250f0a 100644
> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
> @@ -285,6 +285,33 @@ void ktime_get_ts(struct timespec *ts)
> }
> EXPORT_SYMBOL_GPL(ktime_get_ts);
>
> +
> +/**
> + * getnsboottime - get the bootbased clock in nsec format
> + *
> + * The function calculates the bootbased clock from the realtime
> + * clock and the wall_to_monotonic offset and stores the result
> + * in normalized timespec format in the variable pointed to by @ts.
> + */
> +s64 getnsboottime(void)
> +{
> + unsigned int seq;
> + s64 secs, nsecs;
> +
> + WARN_ON(timekeeping_suspended);
> +
> + do {
> + seq = read_seqbegin(&xtime_lock);
> + secs = xtime.tv_sec + wall_to_monotonic.tv_sec;
> + secs += total_sleep_time.tv_sec;
> + nsecs = xtime.tv_nsec + wall_to_monotonic.tv_nsec;
> + nsecs += total_sleep_time.tv_nsec + timekeeping_get_ns();
> +
> + } while (read_seqretry(&xtime_lock, seq));
> + return nsecs + (secs * NSEC_PER_SEC);
> +}
> +EXPORT_SYMBOL_GPL(getnsboottime);

You forgot to include the boottime.tv_sec/nsec offset in this. Take a
look again at getboottime()

thanks
-john

2010-08-20 23:38:14

by Zachary Amsden

[permalink] [raw]
Subject: Re: [KVM timekeeping 17/35] Implement getnsboottime kernel API

On 08/20/2010 08:39 AM, john stultz wrote:
> On Thu, 2010-08-19 at 22:07 -1000, Zachary Amsden wrote:
>
>> Add a kernel call to get the number of nanoseconds since boot. This
>> is generally useful enough to make it a generic call.
>>
> Few comments here.
>
>
>> Signed-off-by: Zachary Amsden<[email protected]>
>> ---
>> include/linux/time.h | 1 +
>> kernel/time/timekeeping.c | 27 +++++++++++++++++++++++++++
>> 2 files changed, 28 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/linux/time.h b/include/linux/time.h
>> index ea3559f..5d04108 100644
>> --- a/include/linux/time.h
>> +++ b/include/linux/time.h
>> @@ -145,6 +145,7 @@ extern void getnstimeofday(struct timespec *tv);
>> extern void getrawmonotonic(struct timespec *ts);
>> extern void getboottime(struct timespec *ts);
>> extern void monotonic_to_bootbased(struct timespec *ts);
>> +extern s64 getnsboottime(void);
>>
> So instead of converting the timespec from getboottime, why did you add
> a new interface? Also if not a timespec, why did you pick a s64 instead
> of a ktime_t?
>

The new interface was suggested several times, so I'm proposing it. I'm
indifferent to putting it the kernel API or making it internal to KVM.
KVM doesn't want to deal with conversions to / from ktime_t; this code
uses a lot (too much) math, and it's easy to get wrong when splitting
sec / nsec fields. So s64 seems a natural type for ns values. I
realize it's not entirely consistent with the kernel API, but s64
representation for ns seems to be creeping in.

>
>
>> extern struct timespec timespec_trunc(struct timespec t, unsigned gran);
>> extern int timekeeping_valid_for_hres(void);
>> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
>> index caf8d4d..d250f0a 100644
>> --- a/kernel/time/timekeeping.c
>> +++ b/kernel/time/timekeeping.c
>> @@ -285,6 +285,33 @@ void ktime_get_ts(struct timespec *ts)
>> }
>> EXPORT_SYMBOL_GPL(ktime_get_ts);
>>
>> +
>> +/**
>> + * getnsboottime - get the bootbased clock in nsec format
>> + *
>> + * The function calculates the bootbased clock from the realtime
>> + * clock and the wall_to_monotonic offset and stores the result
>> + * in normalized timespec format in the variable pointed to by @ts.
>> + */
>> +s64 getnsboottime(void)
>> +{
>> + unsigned int seq;
>> + s64 secs, nsecs;
>> +
>> + WARN_ON(timekeeping_suspended);
>> +
>> + do {
>> + seq = read_seqbegin(&xtime_lock);
>> + secs = xtime.tv_sec + wall_to_monotonic.tv_sec;
>> + secs += total_sleep_time.tv_sec;
>> + nsecs = xtime.tv_nsec + wall_to_monotonic.tv_nsec;
>> + nsecs += total_sleep_time.tv_nsec + timekeeping_get_ns();
>> +
>> + } while (read_seqretry(&xtime_lock, seq));
>> + return nsecs + (secs * NSEC_PER_SEC);
>> +}
>> +EXPORT_SYMBOL_GPL(getnsboottime);
>>
> You forgot to include the boottime.tv_sec/nsec offset in this. Take a
> look again at getboottime()
>

I don't think so... boottime is internal to getboottime, and it's just
wall_to_monotonic + total_sleep_time -- right?

Perhaps I've named the function badly. What I want is the monotonic
clock, adjusted for sleep time - i.e. a clock that counts elapsed real
time without accounting for wall clock changes due to time zone, which
never goes backwards.

Zach

2010-08-21 00:02:47

by john stultz

[permalink] [raw]
Subject: Re: [KVM timekeeping 17/35] Implement getnsboottime kernel API

On Fri, 2010-08-20 at 13:37 -1000, Zachary Amsden wrote:
> On 08/20/2010 08:39 AM, john stultz wrote:
> > On Thu, 2010-08-19 at 22:07 -1000, Zachary Amsden wrote:
> >
> >> Add a kernel call to get the number of nanoseconds since boot. This
> >> is generally useful enough to make it a generic call.
> >>
> > Few comments here.
> >
> >
> >> Signed-off-by: Zachary Amsden<[email protected]>
> >> ---
> >> include/linux/time.h | 1 +
> >> kernel/time/timekeeping.c | 27 +++++++++++++++++++++++++++
> >> 2 files changed, 28 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/include/linux/time.h b/include/linux/time.h
> >> index ea3559f..5d04108 100644
> >> --- a/include/linux/time.h
> >> +++ b/include/linux/time.h
> >> @@ -145,6 +145,7 @@ extern void getnstimeofday(struct timespec *tv);
> >> extern void getrawmonotonic(struct timespec *ts);
> >> extern void getboottime(struct timespec *ts);
> >> extern void monotonic_to_bootbased(struct timespec *ts);
> >> +extern s64 getnsboottime(void);
> >>
> > So instead of converting the timespec from getboottime, why did you add
> > a new interface? Also if not a timespec, why did you pick a s64 instead
> > of a ktime_t?
> >
>
> The new interface was suggested several times, so I'm proposing it. I'm
> indifferent to putting it the kernel API or making it internal to KVM.
> KVM doesn't want to deal with conversions to / from ktime_t; this code
> uses a lot (too much) math, and it's easy to get wrong when splitting
> sec / nsec fields. So s64 seems a natural type for ns values. I
> realize it's not entirely consistent with the kernel API, but s64
> representation for ns seems to be creeping in.

I can understand wanting that, way back I was pushing for s64 ns
representations for most time values, but the ktime_t was considered a
reasonable compromise to avoid costly 64bit divides to split (sec,nsec)
on 32bit arches.

Maybe call it getboottime_ns() just to distinguish it from
getnstimeofday() which returns a timespec?


> >> extern struct timespec timespec_trunc(struct timespec t, unsigned gran);
> >> extern int timekeeping_valid_for_hres(void);
> >> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> >> index caf8d4d..d250f0a 100644
> >> --- a/kernel/time/timekeeping.c
> >> +++ b/kernel/time/timekeeping.c
> >> @@ -285,6 +285,33 @@ void ktime_get_ts(struct timespec *ts)
> >> }
> >> EXPORT_SYMBOL_GPL(ktime_get_ts);
> >>
> >> +
> >> +/**
> >> + * getnsboottime - get the bootbased clock in nsec format
> >> + *
> >> + * The function calculates the bootbased clock from the realtime
> >> + * clock and the wall_to_monotonic offset and stores the result
> >> + * in normalized timespec format in the variable pointed to by @ts.
> >> + */
> >> +s64 getnsboottime(void)
> >> +{
> >> + unsigned int seq;
> >> + s64 secs, nsecs;
> >> +
> >> + WARN_ON(timekeeping_suspended);
> >> +
> >> + do {
> >> + seq = read_seqbegin(&xtime_lock);
> >> + secs = xtime.tv_sec + wall_to_monotonic.tv_sec;
> >> + secs += total_sleep_time.tv_sec;
> >> + nsecs = xtime.tv_nsec + wall_to_monotonic.tv_nsec;
> >> + nsecs += total_sleep_time.tv_nsec + timekeeping_get_ns();
> >> +
> >> + } while (read_seqretry(&xtime_lock, seq));
> >> + return nsecs + (secs * NSEC_PER_SEC);
> >> +}
> >> +EXPORT_SYMBOL_GPL(getnsboottime);
> >>
> > You forgot to include the boottime.tv_sec/nsec offset in this. Take a
> > look again at getboottime()
> >
>
> I don't think so... boottime is internal to getboottime, and it's just
> wall_to_monotonic + total_sleep_time -- right?

Right, sorry, some architectures refine boot time even further,
providing an offset from when the machine was actually powered on to
when the timekeeping code was initialized. But that's already adjusted
into wall_to_monotonic at startup. I thought we kept it separately.


> Perhaps I've named the function badly. What I want is the monotonic
> clock, adjusted for sleep time - i.e. a clock that counts elapsed real
> time without accounting for wall clock changes due to time zone, which
> never goes backwards.

That looks fine then. Its a little confusing since getboottime()
returns a timespec with the absolute time that the system booted. Where
as your interface is providing the time since boot.

Maybe gettimefromboot_ns() would be clearer?

thanks
-john





2010-08-21 00:52:59

by Zachary Amsden

[permalink] [raw]
Subject: Re: [KVM timekeeping 17/35] Implement getnsboottime kernel API

On 08/20/2010 02:02 PM, john stultz wrote:
> On Fri, 2010-08-20 at 13:37 -1000, Zachary Amsden wrote:
>
>> On 08/20/2010 08:39 AM, john stultz wrote:
>>
>>> On Thu, 2010-08-19 at 22:07 -1000, Zachary Amsden wrote:
>>>
>>>
>>>> Add a kernel call to get the number of nanoseconds since boot. This
>>>> is generally useful enough to make it a generic call.
>>>>
>>>>
>>> Few comments here.
>>>
>>>
>>>
>>>> Signed-off-by: Zachary Amsden<[email protected]>
>>>> ---
>>>> include/linux/time.h | 1 +
>>>> kernel/time/timekeeping.c | 27 +++++++++++++++++++++++++++
>>>> 2 files changed, 28 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/include/linux/time.h b/include/linux/time.h
>>>> index ea3559f..5d04108 100644
>>>> --- a/include/linux/time.h
>>>> +++ b/include/linux/time.h
>>>> @@ -145,6 +145,7 @@ extern void getnstimeofday(struct timespec *tv);
>>>> extern void getrawmonotonic(struct timespec *ts);
>>>> extern void getboottime(struct timespec *ts);
>>>> extern void monotonic_to_bootbased(struct timespec *ts);
>>>> +extern s64 getnsboottime(void);
>>>>
>>>>
>>> So instead of converting the timespec from getboottime, why did you add
>>> a new interface? Also if not a timespec, why did you pick a s64 instead
>>> of a ktime_t?
>>>
>>>
>> The new interface was suggested several times, so I'm proposing it. I'm
>> indifferent to putting it the kernel API or making it internal to KVM.
>> KVM doesn't want to deal with conversions to / from ktime_t; this code
>> uses a lot (too much) math, and it's easy to get wrong when splitting
>> sec / nsec fields. So s64 seems a natural type for ns values. I
>> realize it's not entirely consistent with the kernel API, but s64
>> representation for ns seems to be creeping in.
>>
> I can understand wanting that, way back I was pushing for s64 ns
> representations for most time values, but the ktime_t was considered a
> reasonable compromise to avoid costly 64bit divides to split (sec,nsec)
> on 32bit arches.
>

We want time in simply parseable formats, so we always end up with sec /
msec, sec / usec, sec / nsec. This is simply a convenient
representation for humans. Programmers always end up copying this model
and it causes so many lovely bugs. How many times can you race while
reading CMOS Y/M/D/H/S?

Fortunately now that 64-bit computing is nearly pervasive, we can make
most of these problems go away.

I think gettimefromboot_ns() is a good descriptive name, but slightly
too long - it would ruin my indentation. Perhaps getrealtime_ns()?

Zach

2010-08-21 01:04:25

by john stultz

[permalink] [raw]
Subject: Re: [KVM timekeeping 17/35] Implement getnsboottime kernel API

On Fri, 2010-08-20 at 14:52 -1000, Zachary Amsden wrote:
> I think gettimefromboot_ns() is a good descriptive name, but slightly
> too long - it would ruin my indentation. Perhaps getrealtime_ns()?

Sigh... So getrealtime_ns would probably be confused with
CLOCK_REALTIME, which is wall time. :P

At this point it feels too nitpicky to suggest anything else, so go
ahead and use boottime_ns and we'll refine things if anyone actually
trips up on it.

thanks
-john

2010-08-21 01:22:42

by Zachary Amsden

[permalink] [raw]
Subject: Re: [KVM timekeeping 17/35] Implement getnsboottime kernel API

On 08/20/2010 03:04 PM, john stultz wrote:
> On Fri, 2010-08-20 at 14:52 -1000, Zachary Amsden wrote:
>
>> I think gettimefromboot_ns() is a good descriptive name, but slightly
>> too long - it would ruin my indentation. Perhaps getrealtime_ns()?
>>
> Sigh... So getrealtime_ns would probably be confused with
> CLOCK_REALTIME, which is wall time. :P
>
> At this point it feels too nitpicky to suggest anything else, so go
> ahead and use boottime_ns and we'll refine things if anyone actually
> trips up on it.
>

As long as the prototype is adequately commented, there needn't be any
confusion ;)

Zach

2010-08-27 18:05:38

by Jan Kiszka

[permalink] [raw]
Subject: Re: [KVM timekeeping 17/35] Implement getnsboottime kernel API

Zachary Amsden wrote:
> Add a kernel call to get the number of nanoseconds since boot. This
> is generally useful enough to make it a generic call.
>
> Signed-off-by: Zachary Amsden <[email protected]>
> ---
> include/linux/time.h | 1 +
> kernel/time/timekeeping.c | 27 +++++++++++++++++++++++++++
> 2 files changed, 28 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/time.h b/include/linux/time.h
> index ea3559f..5d04108 100644
> --- a/include/linux/time.h
> +++ b/include/linux/time.h
> @@ -145,6 +145,7 @@ extern void getnstimeofday(struct timespec *tv);
> extern void getrawmonotonic(struct timespec *ts);
> extern void getboottime(struct timespec *ts);
> extern void monotonic_to_bootbased(struct timespec *ts);
> +extern s64 getnsboottime(void);
>
> extern struct timespec timespec_trunc(struct timespec t, unsigned gran);
> extern int timekeeping_valid_for_hres(void);
> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> index caf8d4d..d250f0a 100644
> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
> @@ -285,6 +285,33 @@ void ktime_get_ts(struct timespec *ts)
> }
> EXPORT_SYMBOL_GPL(ktime_get_ts);
>
> +
> +/**
> + * getnsboottime - get the bootbased clock in nsec format
> + *
> + * The function calculates the bootbased clock from the realtime
> + * clock and the wall_to_monotonic offset and stores the result
> + * in normalized timespec format in the variable pointed to by @ts.
> + */

This thing is not returning anything in some ts variable. And I also had
a hard time spotting the key difference to getboottime - the name is
really confusing.

Besides this, if you have good suggestion how to provide a compat
version for older kernels, I'm all ears. Please also have a careful look
at kvm-kmod's kvm_getboottime again, right now I'm a bit confused about
what it is supposed to return and what it actually does (note that
kvm-kmod cannot account for time spent in suspend state).

Thanks!
Jan

> +s64 getnsboottime(void)
> +{
> + unsigned int seq;
> + s64 secs, nsecs;
> +
> + WARN_ON(timekeeping_suspended);
> +
> + do {
> + seq = read_seqbegin(&xtime_lock);
> + secs = xtime.tv_sec + wall_to_monotonic.tv_sec;
> + secs += total_sleep_time.tv_sec;
> + nsecs = xtime.tv_nsec + wall_to_monotonic.tv_nsec;
> + nsecs += total_sleep_time.tv_nsec + timekeeping_get_ns();
> +
> + } while (read_seqretry(&xtime_lock, seq));
> + return nsecs + (secs * NSEC_PER_SEC);
> +}
> +EXPORT_SYMBOL_GPL(getnsboottime);
> +
> /**
> * do_gettimeofday - Returns the time of day in a timeval
> * @tv: pointer to the timeval to be set

--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

2010-08-27 23:48:26

by Zachary Amsden

[permalink] [raw]
Subject: Re: [KVM timekeeping 17/35] Implement getnsboottime kernel API

On 08/27/2010 08:05 AM, Jan Kiszka wrote:
> Zachary Amsden wrote:
>
>> Add a kernel call to get the number of nanoseconds since boot. This
>> is generally useful enough to make it a generic call.
>>
>> Signed-off-by: Zachary Amsden<[email protected]>
>> ---
>> include/linux/time.h | 1 +
>> kernel/time/timekeeping.c | 27 +++++++++++++++++++++++++++
>> 2 files changed, 28 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/linux/time.h b/include/linux/time.h
>> index ea3559f..5d04108 100644
>> --- a/include/linux/time.h
>> +++ b/include/linux/time.h
>> @@ -145,6 +145,7 @@ extern void getnstimeofday(struct timespec *tv);
>> extern void getrawmonotonic(struct timespec *ts);
>> extern void getboottime(struct timespec *ts);
>> extern void monotonic_to_bootbased(struct timespec *ts);
>> +extern s64 getnsboottime(void);
>>
>> extern struct timespec timespec_trunc(struct timespec t, unsigned gran);
>> extern int timekeeping_valid_for_hres(void);
>> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
>> index caf8d4d..d250f0a 100644
>> --- a/kernel/time/timekeeping.c
>> +++ b/kernel/time/timekeeping.c
>> @@ -285,6 +285,33 @@ void ktime_get_ts(struct timespec *ts)
>> }
>> EXPORT_SYMBOL_GPL(ktime_get_ts);
>>
>> +
>> +/**
>> + * getnsboottime - get the bootbased clock in nsec format
>> + *
>> + * The function calculates the bootbased clock from the realtime
>> + * clock and the wall_to_monotonic offset and stores the result
>> + * in normalized timespec format in the variable pointed to by @ts.
>> + */
>>
> This thing is not returning anything in some ts variable. And I also had
> a hard time spotting the key difference to getboottime - the name is
> really confusing.
>
> Besides this, if you have good suggestion how to provide a compat
> version for older kernels, I'm all ears. Please also have a careful look
> at kvm-kmod's kvm_getboottime again, right now I'm a bit confused about
> what it is supposed to return and what it actually does (note that
> kvm-kmod cannot account for time spent in suspend state).
>
> Thanks!
> Jan
>
>
>> +s64 getnsboottime(void)
>> +{
>> + unsigned int seq;
>> + s64 secs, nsecs;
>> +
>> + WARN_ON(timekeeping_suspended);
>> +
>> + do {
>> + seq = read_seqbegin(&xtime_lock);
>> + secs = xtime.tv_sec + wall_to_monotonic.tv_sec;
>> + secs += total_sleep_time.tv_sec;
>> + nsecs = xtime.tv_nsec + wall_to_monotonic.tv_nsec;
>> + nsecs += total_sleep_time.tv_nsec + timekeeping_get_ns();
>> +
>> + } while (read_seqretry(&xtime_lock, seq));
>> + return nsecs + (secs * NSEC_PER_SEC);
>> +}
>> +EXPORT_SYMBOL_GPL(getnsboottime);
>> +
>> /**
>> * do_gettimeofday - Returns the time of day in a timeval
>> * @tv: pointer to the timeval to be set
>>
>

Yes, we should probably change the name before making this an actual
kernel API, John made some better suggestions.

For kvm-kmod, the following conversion should work:

static inline u64 get_kernel_ns(void)
{
struct timespec ts;

WARN_ON(preemptible());
ktime_get_ts(&ts);
monotonic_to_bootbased(&ts);
return timespec_to_ns(&ts);
}

The only real point to getnsboottime is to stop the unnecessary
conversions, but looking at it now, it doesn't appear to actually save
any, does it? Maybe it is better to just drop the thing all-together.

Zach

2010-08-30 18:08:08

by Jan Kiszka

[permalink] [raw]
Subject: Re: [KVM timekeeping 17/35] Implement getnsboottime kernel API

Zachary Amsden wrote:
> On 08/27/2010 08:05 AM, Jan Kiszka wrote:
>> Zachary Amsden wrote:
>>
>>> Add a kernel call to get the number of nanoseconds since boot. This
>>> is generally useful enough to make it a generic call.
>>>
>>> Signed-off-by: Zachary Amsden<[email protected]>
>>> ---
>>> include/linux/time.h | 1 +
>>> kernel/time/timekeeping.c | 27 +++++++++++++++++++++++++++
>>> 2 files changed, 28 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/include/linux/time.h b/include/linux/time.h
>>> index ea3559f..5d04108 100644
>>> --- a/include/linux/time.h
>>> +++ b/include/linux/time.h
>>> @@ -145,6 +145,7 @@ extern void getnstimeofday(struct timespec *tv);
>>> extern void getrawmonotonic(struct timespec *ts);
>>> extern void getboottime(struct timespec *ts);
>>> extern void monotonic_to_bootbased(struct timespec *ts);
>>> +extern s64 getnsboottime(void);
>>>
>>> extern struct timespec timespec_trunc(struct timespec t, unsigned gran);
>>> extern int timekeeping_valid_for_hres(void);
>>> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
>>> index caf8d4d..d250f0a 100644
>>> --- a/kernel/time/timekeeping.c
>>> +++ b/kernel/time/timekeeping.c
>>> @@ -285,6 +285,33 @@ void ktime_get_ts(struct timespec *ts)
>>> }
>>> EXPORT_SYMBOL_GPL(ktime_get_ts);
>>>
>>> +
>>> +/**
>>> + * getnsboottime - get the bootbased clock in nsec format
>>> + *
>>> + * The function calculates the bootbased clock from the realtime
>>> + * clock and the wall_to_monotonic offset and stores the result
>>> + * in normalized timespec format in the variable pointed to by @ts.
>>> + */
>>>
>> This thing is not returning anything in some ts variable. And I also had
>> a hard time spotting the key difference to getboottime - the name is
>> really confusing.
>>
>> Besides this, if you have good suggestion how to provide a compat
>> version for older kernels, I'm all ears. Please also have a careful look
>> at kvm-kmod's kvm_getboottime again, right now I'm a bit confused about
>> what it is supposed to return and what it actually does (note that
>> kvm-kmod cannot account for time spent in suspend state).
>>
>> Thanks!
>> Jan
>>
>>
>>> +s64 getnsboottime(void)
>>> +{
>>> + unsigned int seq;
>>> + s64 secs, nsecs;
>>> +
>>> + WARN_ON(timekeeping_suspended);
>>> +
>>> + do {
>>> + seq = read_seqbegin(&xtime_lock);
>>> + secs = xtime.tv_sec + wall_to_monotonic.tv_sec;
>>> + secs += total_sleep_time.tv_sec;
>>> + nsecs = xtime.tv_nsec + wall_to_monotonic.tv_nsec;
>>> + nsecs += total_sleep_time.tv_nsec + timekeeping_get_ns();
>>> +
>>> + } while (read_seqretry(&xtime_lock, seq));
>>> + return nsecs + (secs * NSEC_PER_SEC);
>>> +}
>>> +EXPORT_SYMBOL_GPL(getnsboottime);
>>> +
>>> /**
>>> * do_gettimeofday - Returns the time of day in a timeval
>>> * @tv: pointer to the timeval to be set
>>>
>>
>
> Yes, we should probably change the name before making this an actual
> kernel API, John made some better suggestions.
>
> For kvm-kmod, the following conversion should work:
>
> static inline u64 get_kernel_ns(void)
> {
> struct timespec ts;
>
> WARN_ON(preemptible());
> ktime_get_ts(&ts);
> monotonic_to_bootbased(&ts);
> return timespec_to_ns(&ts);
> }
>
> The only real point to getnsboottime is to stop the unnecessary
> conversions, but looking at it now, it doesn't appear to actually save
> any, does it? Maybe it is better to just drop the thing all-together.

It looks like. Will you post a removal patch kvm.git? I'll better wait
with updating kvm-kmod then.

Jan

--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux