| Message ID | 1435587807-10008-2-git-send-email-bamvor.zhangjian@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Mon, Jun 29, 2015 at 7:23 AM, Bamvor Zhang Jian <bamvor.zhangjian@linaro.org> wrote: > +int get_timeval64(struct timeval64 *tv, > + const struct __kernel_timeval __user *utv) > +{ > + struct __kernel_timeval ktv; > + int ret; > + > + ret = copy_from_user(&ktv, utv, sizeof(ktv)); > + if (ret) > + return -EFAULT; > + > + tv->tv_sec = ktv.tv_sec; > + if (!IS_ENABLED(CONFIG_64BIT) > +#ifdef CONFIG_COMPAT > + || is_compat_task() > +#endif These sorts of ifdefs are to be avoided inside of functions. Instead, it seems is_compat_task() should be defined to 0 in the !CONFIG_COMPAT case, so you can avoid the ifdefs and the compiler can still optimize it out. Otherwise this looks similar to a patch Baolin (cc'ed) has been working on. thanks -john -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Hi, John On 07/09/2015 04:09 AM, John Stultz wrote: > On Mon, Jun 29, 2015 at 7:23 AM, Bamvor Zhang Jian > <bamvor.zhangjian@linaro.org> wrote: >> +int get_timeval64(struct timeval64 *tv, >> + const struct __kernel_timeval __user *utv) >> +{ >> + struct __kernel_timeval ktv; >> + int ret; >> + >> + ret = copy_from_user(&ktv, utv, sizeof(ktv)); >> + if (ret) >> + return -EFAULT; >> + >> + tv->tv_sec = ktv.tv_sec; >> + if (!IS_ENABLED(CONFIG_64BIT) >> +#ifdef CONFIG_COMPAT >> + || is_compat_task() >> +#endif > > These sorts of ifdefs are to be avoided inside of functions. > Instead, it seems is_compat_task() should be defined to 0 in the > !CONFIG_COMPAT case, so you can avoid the ifdefs and the compiler can > still optimize it out. I add this ifdef because I got compile failure on arm platform. This file do not include the <linux/compat.h> directly. And in arm64, compat.h is included implicitily. So, I am not sure what I should do here. Include <linux/compat.h> in this file directly or add a this check at the beginning of this file? #ifndef is_compat_task #define is_compat_task() (0) #endif > Otherwise this looks similar to a patch Baolin (cc'ed) has been working on. Yes. regards bamvor > > thanks > -john > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Hi, Arnd On 07/09/2015 06:26 PM, Arnd Bergmann wrote: > On Thursday 09 July 2015 17:02:47 Bamvor Zhang Jian wrote: >> On 07/09/2015 04:09 AM, John Stultz wrote: >>> On Mon, Jun 29, 2015 at 7:23 AM, Bamvor Zhang Jian >>> <bamvor.zhangjian@linaro.org> wrote: >>>> +int get_timeval64(struct timeval64 *tv, >>>> + const struct __kernel_timeval __user *utv) >>>> +{ >>>> + struct __kernel_timeval ktv; >>>> + int ret; >>>> + >>>> + ret = copy_from_user(&ktv, utv, sizeof(ktv)); >>>> + if (ret) >>>> + return -EFAULT; >>>> + >>>> + tv->tv_sec = ktv.tv_sec; >>>> + if (!IS_ENABLED(CONFIG_64BIT) >>>> +#ifdef CONFIG_COMPAT >>>> + || is_compat_task() >>>> +#endif >>> >>> These sorts of ifdefs are to be avoided inside of functions. >> >>> Instead, it seems is_compat_task() should be defined to 0 in the >>> !CONFIG_COMPAT case, so you can avoid the ifdefs and the compiler can >>> still optimize it out. >> I add this ifdef because I got compile failure on arm platform. This >> file do not include the <linux/compat.h> directly. And in arm64, >> compat.h is included implicitily. >> So, I am not sure what I should do here. Include <linux/compat.h> in >> this file directly or add a this check at the beginning of this file? >> >> #ifndef is_compat_task >> #define is_compat_task() (0) >> #endif >> > > Actually I think we can completely skip this test here: Unlike > timespec, timeval is defined in a way that always lets user space > use a 64-bit type for the microsecond portion (suseconds_t tv_usec). I do not familar with this type. I grep the suseconds_t in glibc, it seems that suseconds_t(__SUSECONDS_T_TYPE) is defined as __SYSCALL_SLONG_TYPE which is __SLONGWORD_TYPE(32bit on 32bit architecture). > I think we should simplify this case and just assume that user space > does exactly that, and treat a tv_usec value with a nonzero upper > half as an error. > > I would also keep this function local to the ppdev driver, in order > to not proliferate this to generic kernel code, but that is something > we can debate, based on what other drivers need. For core kernel > code, we should not need a get_timeval64 function because all system > calls that pass a timeval structure are obsolete and we don't need > to provide 64-bit time_t variants of them. Got it. regards bamvor > > Arnd > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
diff --git a/include/linux/time64.h b/include/linux/time64.h index 77b5df2..2ca4f85 100644 --- a/include/linux/time64.h +++ b/include/linux/time64.h @@ -1,24 +1,35 @@ #ifndef _LINUX_TIME64_H #define _LINUX_TIME64_H -#include <uapi/linux/time.h> -#include <linux/math64.h> typedef __s64 time64_t; +#ifndef CONFIG_COMPAT_TIME +# define __kernel_timeval timeval +#endif + /* * This wants to go into uapi/linux/time.h once we agreed about the * userspace interfaces. */ #if __BITS_PER_LONG == 64 # define timespec64 timespec +# define timeval64 timeval #else struct timespec64 { time64_t tv_sec; /* seconds */ long tv_nsec; /* nanoseconds */ }; + +struct timeval64 { + time64_t tv_sec; /* seconds */ + __kernel_suseconds_t tv_usec; /* microseconds */ +}; #endif +#include <uapi/linux/time.h> +#include <linux/math64.h> + /* Parameters used to convert the timespec values: */ #define MSEC_PER_SEC 1000L #define USEC_PER_MSEC 1000L @@ -189,4 +200,9 @@ static __always_inline void timespec64_add_ns(struct timespec64 *a, u64 ns) #endif +extern int get_timeval64(struct timeval64 *tv, + const struct __kernel_timeval __user *utv); +extern int put_timeval64(const struct timeval64 *tv, + struct __kernel_timeval __user *utv); + #endif /* _LINUX_TIME64_H */ diff --git a/include/uapi/linux/time.h b/include/uapi/linux/time.h index e75e1b6..2ca6a31 100644 --- a/include/uapi/linux/time.h +++ b/include/uapi/linux/time.h @@ -66,4 +66,14 @@ struct itimerval { */ #define TIMER_ABSTIME 0x01 +/* types based on 64-bit time_t */ +#ifndef __kernel_timeval +typedef __s64 __kernel_time64_t; + +struct __kernel_timeval { + __kernel_time64_t tv_sec; + __s64 tv_usec; +}; +#endif + #endif /* _UAPI_LINUX_TIME_H */ diff --git a/kernel/time/time.c b/kernel/time/time.c index 85d5bb1..adf0455 100644 --- a/kernel/time/time.c +++ b/kernel/time/time.c @@ -37,6 +37,7 @@ #include <linux/fs.h> #include <linux/math64.h> #include <linux/ptrace.h> +#include <linux/time64.h> #include <asm/uaccess.h> #include <asm/unistd.h> @@ -759,3 +760,38 @@ struct timespec timespec_add_safe(const struct timespec lhs, return res; } + +int get_timeval64(struct timeval64 *tv, + const struct __kernel_timeval __user *utv) +{ + struct __kernel_timeval ktv; + int ret; + + ret = copy_from_user(&ktv, utv, sizeof(ktv)); + if (ret) + return -EFAULT; + + tv->tv_sec = ktv.tv_sec; + if (!IS_ENABLED(CONFIG_64BIT) +#ifdef CONFIG_COMPAT + || is_compat_task() +#endif + ) + ktv.tv_usec &= 0xfffffffful; + tv->tv_usec = ktv.tv_usec; + + return 0; +} +EXPORT_SYMBOL_GPL(get_timeval64); + +int put_timeval64(const struct timeval64 *tv, + struct __kernel_timeval __user *utv) +{ + struct __kernel_timeval ktv = { + .tv_sec = tv->tv_sec, + .tv_usec = tv->tv_usec + }; + return copy_to_user(utv, &utv, sizeof(ktv)) ? -EFAULT : 0; +} +EXPORT_SYMBOL_GPL(put_timeval64); +
Add timeval64 structure and copy_(from)|(to)_user functions. Add __kernel_timeval for syscalls. This changes follow the similar way in the followings commit: 361a3bf time64: Add time64.h header and define struct timespec64 49cd6f8 time: More core infrastructure for timespec64 ca2c9c5 y2038: introduce struct __kernel_timespec[1]. [1] http://git.kernel.org/cgit/linux/kernel/git/arnd/playground.git/commit/?h=y2038-syscalls&id=9005d4f4a44fc56bd0a1fe7c08e8e3f13eb75de7 Signed-off-by: Bamvor Zhang Jian <bamvor.zhangjian@linaro.org> --- include/linux/time64.h | 20 ++++++++++++++++++-- include/uapi/linux/time.h | 10 ++++++++++ kernel/time/time.c | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 2 deletions(-)