diff mbox

[RFC,v2,09/11] time: Convert pvclock_read_wallclock() to use timespec64

Message ID 1414667745-7703-10-git-send-email-pang.xunlei@linaro.org
State Superseded
Headers show

Commit Message

pang.xunlei Oct. 30, 2014, 11:15 a.m. UTC
As part of addressing 2038 safety for in-kernel uses, this patch
creates no functional change, converts pvclock_read_wallclock()
to use timespec64 instead of timespec.

Signed-off-by: pang.xunlei <pang.xunlei@linaro.org>
---
 arch/x86/include/asm/pvclock.h |    2 +-
 arch/x86/kernel/kvmclock.c     |    7 +++++--
 arch/x86/kernel/pvclock.c      |    9 +++++----
 arch/x86/xen/time.c            |    7 ++-----
 4 files changed, 13 insertions(+), 12 deletions(-)

Comments

David Vrabel Oct. 30, 2014, 11:37 a.m. UTC | #1
On 30/10/14 11:15, pang.xunlei wrote:
> As part of addressing 2038 safety for in-kernel uses, this patch
> creates no functional change, converts pvclock_read_wallclock()
> to use timespec64 instead of timespec.

Acked-by: David Vrabel <david.vrabel@citrix.com>

With one minor comment:

>  
>  	/* get wallclock at system boot */
>  	do {
>  		version = wall_clock->version;
>  		rmb();		/* fetch version before time */
> -		now.tv_sec  = wall_clock->sec;
> +		/* TODO: [2038 safety] wall_clock->sec uses time64_t */
> +		now.tv_sec  = (time64_t)wall_clock->sec;

The size of wall_clock->sec is fixed as part of the hypervisor ABI so
there's nothing TODO here (it would require extensions to the hypervisor
ABI and thus a new pvclock_read_wallclock64() call or similar).

David
Thomas Gleixner Oct. 30, 2014, 2:57 p.m. UTC | #2
On Thu, 30 Oct 2014, pang.xunlei wrote:
>  	/* get wallclock at system boot */
>  	do {
>  		version = wall_clock->version;
>  		rmb();		/* fetch version before time */
> -		now.tv_sec  = wall_clock->sec;
> +		/* TODO: [2038 safety] wall_clock->sec uses time64_t */

Wrong. This is part of the hypervisor ABI and you can add as many
TODOs here as you want. They will not change the ABI ever. If XEN
wants to fix that it needs to implement a new hypervisor interface.

> +		now.tv_sec  = (time64_t)wall_clock->sec;

copy & paste does not solve all these problems magically.

Thanks,

	tglx
diff mbox

Patch

diff --git a/arch/x86/include/asm/pvclock.h b/arch/x86/include/asm/pvclock.h
index d6b078e..3323413 100644
--- a/arch/x86/include/asm/pvclock.h
+++ b/arch/x86/include/asm/pvclock.h
@@ -11,7 +11,7 @@  void pvclock_set_flags(u8 flags);
 unsigned long pvclock_tsc_khz(struct pvclock_vcpu_time_info *src);
 void pvclock_read_wallclock(struct pvclock_wall_clock *wall,
 			    struct pvclock_vcpu_time_info *vcpu,
-			    struct timespec *ts);
+			    struct timespec64 *ts);
 void pvclock_resume(void);
 
 void pvclock_touch_watchdogs(void);
diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index d9156ce..7cd3511 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -46,10 +46,12 @@  static struct pvclock_wall_clock wall_clock;
 /*
  * The wallclock is the time of day when we booted. Since then, some time may
  * have elapsed since the hypervisor wrote the data. So we try to account for
- * that with system time
+ * that with system time.
+ * TODO: [2038 safety] kvm_get_wallclock() should be fixed to use timespec64.
  */
 static void kvm_get_wallclock(struct timespec *now)
 {
+	struct timespec64 now64;
 	struct pvclock_vcpu_time_info *vcpu_time;
 	int low, high;
 	int cpu;
@@ -63,7 +65,8 @@  static void kvm_get_wallclock(struct timespec *now)
 	cpu = smp_processor_id();
 
 	vcpu_time = &hv_clock[cpu].pvti;
-	pvclock_read_wallclock(&wall_clock, vcpu_time, now);
+	pvclock_read_wallclock(&wall_clock, vcpu_time, &now64);
+	*now = timespec64_to_timespec(now64);
 
 	preempt_enable();
 }
diff --git a/arch/x86/kernel/pvclock.c b/arch/x86/kernel/pvclock.c
index 2f355d2..8e9bbe9 100644
--- a/arch/x86/kernel/pvclock.c
+++ b/arch/x86/kernel/pvclock.c
@@ -117,17 +117,18 @@  cycle_t pvclock_clocksource_read(struct pvclock_vcpu_time_info *src)
 
 void pvclock_read_wallclock(struct pvclock_wall_clock *wall_clock,
 			    struct pvclock_vcpu_time_info *vcpu_time,
-			    struct timespec *ts)
+			    struct timespec64 *ts)
 {
 	u32 version;
 	u64 delta;
-	struct timespec now;
+	struct timespec64 now;
 
 	/* get wallclock at system boot */
 	do {
 		version = wall_clock->version;
 		rmb();		/* fetch version before time */
-		now.tv_sec  = wall_clock->sec;
+		/* TODO: [2038 safety] wall_clock->sec uses time64_t */
+		now.tv_sec  = (time64_t)wall_clock->sec;
 		now.tv_nsec = wall_clock->nsec;
 		rmb();		/* fetch time before checking version */
 	} while ((wall_clock->version & 1) || (version != wall_clock->version));
@@ -138,7 +139,7 @@  void pvclock_read_wallclock(struct pvclock_wall_clock *wall_clock,
 	now.tv_nsec = do_div(delta, NSEC_PER_SEC);
 	now.tv_sec = delta;
 
-	set_normalized_timespec(ts, now.tv_sec, now.tv_nsec);
+	set_normalized_timespec64(ts, now.tv_sec, now.tv_nsec);
 }
 
 #ifdef CONFIG_X86_64
diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
index 4e14439..79053ee 100644
--- a/arch/x86/xen/time.c
+++ b/arch/x86/xen/time.c
@@ -171,15 +171,12 @@  static cycle_t xen_clocksource_get_cycles(struct clocksource *cs)
 
 static void xen_read_wallclock(struct timespec64 *ts)
 {
-	struct timespec ts_unsafe;
 	struct shared_info *s = HYPERVISOR_shared_info;
 	struct pvclock_wall_clock *wall_clock = &(s->wc);
-        struct pvclock_vcpu_time_info *vcpu_time;
+	struct pvclock_vcpu_time_info *vcpu_time;
 
 	vcpu_time = &get_cpu_var(xen_vcpu)->time;
-	/* TODO: [2038 safety] pvclock_read_wallclock() uses timespec64 */
-	pvclock_read_wallclock(wall_clock, vcpu_time, &ts_unsafe);
-	*ts = timespec_to_timespec64(ts_unsafe);
+	pvclock_read_wallclock(wall_clock, vcpu_time, ts);
 	put_cpu_var(xen_vcpu);
 }