[OPW,kernel,v6] timekeeping: Added a function to return tv_sec portion of ktime_get_ts64()
diff mbox

Message ID 20141029054746.GA14441@heena-HP-Compaq-8200-Elite-MT-PC
State New, archived
Headers show

Commit Message

Heena Sirwani Oct. 29, 2014, 5:47 a.m. UTC
The following patch adds a function to return tv_sec portion of
ktime_get_ts64() function in order to have a function that returns
seconds as 64-bit integers instead of 32-bit integers to address the
y2038 problem.

Since we are interested only in the seconds portion of ktime_get_ts64()
and require this to be as fast as possible, we take the implementation
of ktime_get_ts64() as it is and remove everything pertaining to the
nanoseconds portion. We only add to the seconds calculation if the
calculation of nanoseconds is more than one billion nanoseconds. For all
this calculation, we add a new field to the timekeeper struct,
ktime_sec. We update this field in the function tk_update_ktime_data()
by including the calculation of monotonic clock time in seconds and
including the tk_xtime() implementation that gives the nanoseconds value of
last timer click  instead of using timekeeping_get_ns() which is
expensive and we do not require such fine resolution for nanoseconds.

Signed-off-by: Heena Sirwani <heenasirwani@gmail.com>
---
Changes in v6:
	- Added a new field in timekeeper struct.
	- Updated the field in tk_update_ktime_data()

 include/linux/timekeeper_internal.h |  2 ++
 include/linux/timekeeping.h         |  1 +
 kernel/time/timekeeping.c           | 26 ++++++++++++++++++++++++++
 3 files changed, 29 insertions(+)

Comments

Arnd Bergmann Oct. 29, 2014, 7:24 a.m. UTC | #1
On Wednesday 29 October 2014 11:17:46 Heena Sirwani wrote:
> The following patch adds a function to return tv_sec portion of
> ktime_get_ts64() function in order to have a function that returns
> seconds as 64-bit integers instead of 32-bit integers to address the
> y2038 problem.
> 
> Since we are interested only in the seconds portion of ktime_get_ts64()
> and require this to be as fast as possible, we take the implementation
> of ktime_get_ts64() as it is and remove everything pertaining to the
> nanoseconds portion. We only add to the seconds calculation if the
> calculation of nanoseconds is more than one billion nanoseconds. For all
> this calculation, we add a new field to the timekeeper struct,
> ktime_sec. We update this field in the function tk_update_ktime_data()
> by including the calculation of monotonic clock time in seconds and
> including the tk_xtime() implementation that gives the nanoseconds value of
> last timer click  instead of using timekeeping_get_ns() which is
> expensive and we do not require such fine resolution for nanoseconds.
> 
> Signed-off-by: Heena Sirwani <heenasirwani@gmail.com>

This implementation looks correct to me, very nice!

There are two small optimizations you can make on top:

> diff --git a/include/linux/timekeeper_internal.h b/include/linux/timekeeper_internal.h
> index 95640dc..92e5e9d 100644
> --- a/include/linux/timekeeper_internal.h
> +++ b/include/linux/timekeeper_internal.h
> @@ -77,6 +78,7 @@ struct tk_read_base {
>  struct timekeeper {
>  	struct tk_read_base	tkr;
>  	u64			xtime_sec;
> +	u64			ktime_sec;
>  	struct timespec64	wall_to_monotonic;
>  	ktime_t			offs_real;
>  	ktime_t			offs_boot;

For the monotonic clock, we know for sure that it won't ever
overrun a 32-bit number, so we could store it in a 'unsigned long'
or 'u32' here, while still returning a time64_t from ktime_get_seconds().

This would let us skip the lock in ktime_get_seconds and let it just
return tk->ktime_sec.

Thomas might have an opinion one way or the other, my preference
would be for the slightly more efficient version without the lock.

You could also post both versions and let him decide ;-)

> @@ -427,12 +429,18 @@ static inline void tk_update_ktime_data(struct timekeeper *tk)
>  	 * ==> base_mono = (xtime_sec + wtm_sec) * 1e9 + wtm_nsec
>  	 */
>  	nsec = (s64)(tk->xtime_sec + tk->wall_to_monotonic.tv_sec);
> +	seconds = nsec;
>  	nsec *= NSEC_PER_SEC;
>  	nsec += tk->wall_to_monotonic.tv_nsec;
>  	tk->tkr.base_mono = ns_to_ktime(nsec);
>  
>  	/* Update the monotonic raw base */
>  	tk->base_raw = timespec64_to_ktime(tk->raw_time);
> +
> +	tkr_nsec = (long)(tk->tkr.xtime_nsec >> tk->tkr.shift);
> +	if ((tkr_nsec + tk->wall_to_monotonic.tv_nsec) >= NSEC_PER_SEC)
> +		seconds += 1;
> +	tk->ktime_sec = seconds;
>  }
>  
>  /* must hold timekeeper_lock */

If you slightly reorder the assignments, you can make it more readable
and avoid the extra temporary variable for tkr_nsec.
Just start with

	seconds = (s64)(tk->xtime_sec + tk->wall_to_monotonic.tv_sec);
	nsec = tk->wall_to_monotonic.tv_nsec;

then derive everything else from these.

	Arnd

Patch
diff mbox

diff --git a/include/linux/timekeeper_internal.h b/include/linux/timekeeper_internal.h
index 95640dc..92e5e9d 100644
--- a/include/linux/timekeeper_internal.h
+++ b/include/linux/timekeeper_internal.h
@@ -42,6 +42,7 @@  struct tk_read_base {
  * struct timekeeper - Structure holding internal timekeeping values.
  * @tkr:		The readout base structure
  * @xtime_sec:		Current CLOCK_REALTIME time in seconds
+ * @ktime_sec:		Current CLOCK_MONOTONIC time in seconds
  * @wall_to_monotonic:	CLOCK_REALTIME to CLOCK_MONOTONIC offset
  * @offs_real:		Offset clock monotonic -> clock realtime
  * @offs_boot:		Offset clock monotonic -> clock boottime
@@ -77,6 +78,7 @@  struct tk_read_base {
 struct timekeeper {
 	struct tk_read_base	tkr;
 	u64			xtime_sec;
+	u64			ktime_sec;
 	struct timespec64	wall_to_monotonic;
 	ktime_t			offs_real;
 	ktime_t			offs_boot;
diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h
index 1caa6b0..115d55e 100644
--- a/include/linux/timekeeping.h
+++ b/include/linux/timekeeping.h
@@ -28,6 +28,7 @@  struct timespec __current_kernel_time(void);
 struct timespec get_monotonic_coarse(void);
 extern void getrawmonotonic(struct timespec *ts);
 extern void ktime_get_ts64(struct timespec64 *ts);
+extern time64_t ktime_get_seconds(void);
 
 extern int __getnstimeofday64(struct timespec64 *tv);
 extern void getnstimeofday64(struct timespec64 *tv);
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index ec1791f..703130b 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -418,6 +418,8 @@  EXPORT_SYMBOL_GPL(pvclock_gtod_unregister_notifier);
 static inline void tk_update_ktime_data(struct timekeeper *tk)
 {
 	s64 nsec;
+	u64 seconds;
+	s32 tkr_nsec;
 
 	/*
 	 * The xtime based monotonic readout is:
@@ -427,12 +429,18 @@  static inline void tk_update_ktime_data(struct timekeeper *tk)
 	 * ==> base_mono = (xtime_sec + wtm_sec) * 1e9 + wtm_nsec
 	 */
 	nsec = (s64)(tk->xtime_sec + tk->wall_to_monotonic.tv_sec);
+	seconds = nsec;
 	nsec *= NSEC_PER_SEC;
 	nsec += tk->wall_to_monotonic.tv_nsec;
 	tk->tkr.base_mono = ns_to_ktime(nsec);
 
 	/* Update the monotonic raw base */
 	tk->base_raw = timespec64_to_ktime(tk->raw_time);
+
+	tkr_nsec = (long)(tk->tkr.xtime_nsec >> tk->tkr.shift);
+	if ((tkr_nsec + tk->wall_to_monotonic.tv_nsec) >= NSEC_PER_SEC)
+		seconds += 1;
+	tk->ktime_sec = seconds;
 }
 
 /* must hold timekeeper_lock */
@@ -648,6 +656,24 @@  void ktime_get_ts64(struct timespec64 *ts)
 }
 EXPORT_SYMBOL_GPL(ktime_get_ts64);
 
+time64_t ktime_get_seconds(void)
+{
+	time64_t seconds;
+	struct timekeeper *tk = &tk_core.timekeeper;
+	unsigned int seq;
+
+	WARN_ON(timekeeping_suspended);
+
+	do {
+		seq = read_seqcount_begin(&tk_core.seq);
+		seconds = tk->ktime_sec;
+
+	} while (read_seqcount_retry(&tk_core.seq, seq));
+
+	return seconds;
+}
+EXPORT_SYMBOL_GPL(ktime_get_seconds);
+
 #ifdef CONFIG_NTP_PPS
 
 /**