linux-kernel - Re: [KVM timekeeping 17/35] Implement getnsboottime kernel API

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4C784EBF.2010908@redhat.com>
Date:	Fri, 27 Aug 2010 13:48:15 -1000
From:	Zachary Amsden <zamsden@...hat.com>
To:	Jan Kiszka <jan.kiszka@...mens.com>
CC:	kvm@...r.kernel.org, Avi Kivity <avi@...hat.com>,
	Marcelo Tosatti <mtosatti@...hat.com>,
	Glauber Costa <glommer@...hat.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	John Stultz <johnstul@...ibm.com>, linux-kernel@...r.kernel.org
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<zamsden@...hat.com>
>> ---
>>   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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/