diff mbox series

[7/7] time: remove timespec64 hack

Message ID 20171019111450.3702205-7-arnd@arndb.de
State Superseded
Headers show
Series [1/7] timekeeping: consolidate timekeeping_inject_offset code | expand

Commit Message

Arnd Bergmann Oct. 19, 2017, 11:14 a.m. UTC
This may be a somewhat controversial change, changing 64-bit architectures
to use the same 'struct timespec64' definition that 32-bit architectures
have, and removing a micro-optimization that tries to minimize the
difference between timespec and timespec64.

Starting with gcc-5, the compiler can completely optimize away the
timespec_to_timespec64 and timespec64_to_timespec functions on 64-bit
architectures. With older compilers, we introduce a couple of extra
copies of local variables, but those are easily avoided by using
the timespec64 based interfaces consistently, as we do in most of the
important code paths already.

The main upside of removing the hack is that printing the tv_sec
field of a timespec64 structure can now use the %lld format
string on all architectures without a cast to time64_t. Without
this patch, the field is a 'long' type and would have to be printed
using %ld on 64-bit architectures.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

---
 include/linux/time32.h        | 18 +++--------------
 include/linux/time64.h        |  7 -------
 include/linux/timekeeping32.h | 45 -------------------------------------------
 kernel/time/time.c            |  2 --
 4 files changed, 3 insertions(+), 69 deletions(-)

-- 
2.9.0

Comments

John Stultz Oct. 24, 2017, 8:49 p.m. UTC | #1
On Thu, Oct 19, 2017 at 4:14 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> This may be a somewhat controversial change, changing 64-bit architectures

> to use the same 'struct timespec64' definition that 32-bit architectures

> have, and removing a micro-optimization that tries to minimize the

> difference between timespec and timespec64.

>

> Starting with gcc-5, the compiler can completely optimize away the

> timespec_to_timespec64 and timespec64_to_timespec functions on 64-bit

> architectures. With older compilers, we introduce a couple of extra

> copies of local variables, but those are easily avoided by using

> the timespec64 based interfaces consistently, as we do in most of the

> important code paths already.

>

> The main upside of removing the hack is that printing the tv_sec

> field of a timespec64 structure can now use the %lld format

> string on all architectures without a cast to time64_t. Without

> this patch, the field is a 'long' type and would have to be printed

> using %ld on 64-bit architectures.

>

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>


This one doesn't seem to build for me...  In the meantime, I'm going
to go ahead testing with patches 1-6.

jstultz@buildbox:~/projects/linux/time$ make -j24 bzImage > /dev/null
fs/select.c: In function ‘compat_core_sys_select’:
fs/select.c:1244:27: error: passing argument 3 of ‘do_select’ from
incompatible pointer type [-Werror=incompatible-pointer-types]
  ret = do_select(n, &fds, end_time);
                           ^
fs/select.c:451:12: note: expected ‘struct timespec64 *’ but argument
is of type ‘struct timespec *’
 static int do_select(int n, fd_set_bits *fds, struct timespec64 *end_time)
            ^
fs/select.c: In function ‘C_SYSC_select’:
fs/select.c:1279:31: error: passing argument 1 of
‘poll_select_set_timeout’ from incompatible pointer type
[-Werror=incompatible-pointer-types]
   if (poll_select_set_timeout(to,
                               ^
fs/select.c:273:5: note: expected ‘struct timespec64 *’ but argument
is of type ‘struct timespec *’
 int poll_select_set_timeout(struct timespec64 *to, time64_t sec, long nsec)
     ^
fs/select.c: In function ‘do_compat_pselect’:
fs/select.c:1325:31: error: passing argument 1 of
‘poll_select_set_timeout’ from incompatible pointer type
[-Werror=incompatible-pointer-types]
   if (poll_select_set_timeout(to, ts.tv_sec, ts.tv_nsec))
                               ^
fs/select.c:273:5: note: expected ‘struct timespec64 *’ but argument
is of type ‘struct timespec *’
 int poll_select_set_timeout(struct timespec64 *to, time64_t sec, long nsec)
     ^
fs/select.c: In function ‘C_SYSC_ppoll’:
fs/select.c:1394:31: error: passing argument 1 of
‘poll_select_set_timeout’ from incompatible pointer type
[-Werror=incompatible-pointer-types]
   if (poll_select_set_timeout(to, ts.tv_sec, ts.tv_nsec))
                               ^
fs/select.c:273:5: note: expected ‘struct timespec64 *’ but argument
is of type ‘struct timespec *’
 int poll_select_set_timeout(struct timespec64 *to, time64_t sec, long nsec)
     ^
fs/select.c:1409:32: error: passing argument 3 of ‘do_sys_poll’ from
incompatible pointer type [-Werror=incompatible-pointer-types]
  ret = do_sys_poll(ufds, nfds, to);
                                ^
fs/select.c:928:12: note: expected ‘struct timespec64 *’ but argument
is of type ‘struct timespec *’
 static int do_sys_poll(struct pollfd __user *ufds, unsigned int nfds,
            ^
cc1: some warnings being treated as errors


thanks
-john
Arnd Bergmann Nov. 6, 2017, 9:02 p.m. UTC | #2
On Tue, Oct 24, 2017 at 10:49 PM, John Stultz <john.stultz@linaro.org> wrote:
> On Thu, Oct 19, 2017 at 4:14 AM, Arnd Bergmann <arnd@arndb.de> wrote:


>

> This one doesn't seem to build for me...  In the meantime, I'm going

> to go ahead testing with patches 1-6.

>

> jstultz@buildbox:~/projects/linux/time$ make -j24 bzImage > /dev/null

> fs/select.c: In function ‘compat_core_sys_select’:

> fs/select.c:1244:27: error: passing argument 3 of ‘do_select’ from

> incompatible pointer type [-Werror=incompatible-pointer-types]

>   ret = do_select(n, &fds, end_time);


I looked into this again, as I have some more patches on top now that
I wan to submit soon. The problem you ran into was fixed by Deepa's
"io_getevents: Use timespec64 to represent timeouts" patch that Al
Viro merged into vfs/for-next. It's currently in linux-net but not in
mainline, and I didn't notice the problem because I was building on
linux-next.

If you agree with this patch in principle, I'll resend it after the merge
window, applying this one will help with my "x86: convert x86_platform_ops
to timespec64" that needed a rather ugly workaround otherwise.

      Arnd
diff mbox series

Patch

diff --git a/include/linux/time32.h b/include/linux/time32.h
index 305f81dd5429..bd53e66bab1f 100644
--- a/include/linux/time32.h
+++ b/include/linux/time32.h
@@ -18,25 +18,14 @@ 
 /* timespec64 is defined as timespec here */
 static inline struct timespec timespec64_to_timespec(const struct timespec64 ts64)
 {
-	return ts64;
+	return *(const struct timespec *)&ts64;
 }
 
 static inline struct timespec64 timespec_to_timespec64(const struct timespec ts)
 {
-	return ts;
+	return *(const struct timespec64 *)&ts;
 }
 
-# define timespec_equal			timespec64_equal
-# define timespec_compare		timespec64_compare
-# define set_normalized_timespec	set_normalized_timespec64
-# define timespec_add			timespec64_add
-# define timespec_sub			timespec64_sub
-# define timespec_valid			timespec64_valid
-# define timespec_valid_strict		timespec64_valid_strict
-# define timespec_to_ns			timespec64_to_ns
-# define ns_to_timespec			ns_to_timespec64
-# define timespec_add_ns		timespec64_add_ns
-
 #else
 static inline struct timespec timespec64_to_timespec(const struct timespec64 ts64)
 {
@@ -55,6 +44,7 @@  static inline struct timespec64 timespec_to_timespec64(const struct timespec ts)
 	ret.tv_nsec = ts.tv_nsec;
 	return ret;
 }
+#endif
 
 static inline int timespec_equal(const struct timespec *a,
                                  const struct timespec *b)
@@ -157,8 +147,6 @@  static __always_inline void timespec_add_ns(struct timespec *a, u64 ns)
 	a->tv_nsec = ns;
 }
 
-#endif
-
 /**
  * time_to_tm - converts the calendar time to local broken-down time
  *
diff --git a/include/linux/time64.h b/include/linux/time64.h
index ec1888cf5378..b6b5650d8fab 100644
--- a/include/linux/time64.h
+++ b/include/linux/time64.h
@@ -7,11 +7,6 @@ 
 typedef __s64 time64_t;
 typedef __u64 timeu64_t;
 
-#if __BITS_PER_LONG == 64
-/* this trick allows us to optimize out timespec64_to_timespec */
-# define timespec64 timespec
-#define itimerspec64 itimerspec
-#else
 struct timespec64 {
 	time64_t	tv_sec;			/* seconds */
 	long		tv_nsec;		/* nanoseconds */
@@ -22,8 +17,6 @@  struct itimerspec64 {
 	struct timespec64 it_value;
 };
 
-#endif
-
 /* Parameters used to convert the timespec values: */
 #define MSEC_PER_SEC	1000L
 #define USEC_PER_MSEC	1000L
diff --git a/include/linux/timekeeping32.h b/include/linux/timekeeping32.h
index af4114d5dc17..f707ddb4dad7 100644
--- a/include/linux/timekeeping32.h
+++ b/include/linux/timekeeping32.h
@@ -19,50 +19,6 @@  static inline struct timespec current_kernel_time(void)
 	return timespec64_to_timespec(now);
 }
 
-#if BITS_PER_LONG == 64
-/**
- * Deprecated. Use do_settimeofday64().
- */
-static inline int do_settimeofday(const struct timespec *ts)
-{
-	return do_settimeofday64(ts);
-}
-
-static inline int __getnstimeofday(struct timespec *ts)
-{
-	return __getnstimeofday64(ts);
-}
-
-static inline void getnstimeofday(struct timespec *ts)
-{
-	getnstimeofday64(ts);
-}
-
-static inline void ktime_get_ts(struct timespec *ts)
-{
-	ktime_get_ts64(ts);
-}
-
-static inline void ktime_get_real_ts(struct timespec *ts)
-{
-	getnstimeofday64(ts);
-}
-
-static inline void getrawmonotonic(struct timespec *ts)
-{
-	getrawmonotonic64(ts);
-}
-
-static inline struct timespec get_monotonic_coarse(void)
-{
-	return get_monotonic_coarse64();
-}
-
-static inline void getboottime(struct timespec *ts)
-{
-	return getboottime64(ts);
-}
-#else
 /**
  * Deprecated. Use do_settimeofday64().
  */
@@ -127,7 +83,6 @@  static inline void getboottime(struct timespec *ts)
 	getboottime64(&ts64);
 	*ts = timespec64_to_timespec(ts64);
 }
-#endif
 
 /*
  * Timespec interfaces utilizing the ktime based ones
diff --git a/kernel/time/time.c b/kernel/time/time.c
index fe60ebd301cf..4ad81b020c3e 100644
--- a/kernel/time/time.c
+++ b/kernel/time/time.c
@@ -407,7 +407,6 @@  time64_t mktime64(const unsigned int year0, const unsigned int mon0,
 }
 EXPORT_SYMBOL(mktime64);
 
-#if __BITS_PER_LONG == 32
 /**
  * set_normalized_timespec - set timespec sec and nsec parts and normalize
  *
@@ -468,7 +467,6 @@  struct timespec ns_to_timespec(const s64 nsec)
 	return ts;
 }
 EXPORT_SYMBOL(ns_to_timespec);
-#endif
 
 /**
  * ns_to_timeval - Convert nanoseconds to timeval