On Tuesday 28 April 2015 16:05:47 Baolin Wang wrote:
> This patch converts to the 64bit methods with timespec64/itimerspec64 type,
> and changes the syscall implementation according to the CONFIG_64BIT macro.
>
> Signed-off-by: Baolin Wang <[email protected]>
> ---
> kernel/time/posix-timers.c | 103
> +++++++++++++++++++++++++++++++++-----------
> 1 file changed, 78 insertions(+), 25 deletions(-)
>
> diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c
> index 8564b88..7109688 100644
> --- a/kernel/time/posix-timers.c
> +++ b/kernel/time/posix-timers.c
> @@ -522,13 +522,13 @@ void posix_timers_register_clock(const clockid_t
> clock_id,
> return;
> }
>
> - if (!new_clock->clock_get) {
> - printk(KERN_WARNING "POSIX clock id %d lacks clock_get()\n",
> + if (!new_clock->clock_get && !new_clock->clock_get64) {
> + printk(KERN_WARNING "POSIX clock id %d lacks clock_get() and
> clock_get64()\n",
> clock_id);
> return;
> }
Here you are missing a step that Thomas suggested in the previous review:
add a default clock_get64() implementation that calls clock_get()
> @@ -766,7 +767,7 @@ common_timer_get(struct k_itimer *timr, struct itimerspec
> *cur_setting)
> cur_setting->it_value = ktime_to_timespec(remaining);
> }
>
> -static int __timer_gettime(timer_t timer_id, struct itimerspec *cur_setting)
> +static int __timer_gettime(timer_t timer_id, struct itimerspec64
> *cur_setting)
> {
> struct k_itimer *timr;
> struct k_clock *kc;
> @@ -778,10 +779,10 @@ static int __timer_gettime(timer_t timer_id, struct
> itimerspec *cur_setting)
> return -EINVAL;
>
> kc = clockid_to_kclock(timr->it_clock);
> - if (WARN_ON_ONCE(!kc || !kc->timer_get))
> + if (WARN_ON_ONCE(!kc || !kc->timer_get64))
> ret = -EINVAL;
> else
> - kc->timer_get(timr, cur_setting);
> + kc->timer_get64(timr, cur_setting);
>
> unlock_timer(timr, flags);
> return ret;
Without that change, you are now breaking all implementations of timer_get()
because the kernel tries to call timer_get64, which is a NULL pointer. The
same thing applies to all the system calls of course.
> @@ -791,8 +792,17 @@ static int __timer_gettime(timer_t timer_id, struct
> itimerspec *cur_setting)
> SYSCALL_DEFINE2(timer_gettime, timer_t, timer_id,
> struct itimerspec __user *, setting)
> {
> - struct itimerspec cur_setting;
> +#ifdef CONFIG_64BIT
> + struct itimerspec64 cur_setting;
> int ret = __timer_gettime(timer_id, &cur_setting);
> +#else
> + struct itimerspec64 cur_setting64;
> + struct itimerspec cur_setting;
> + int ret = __timer_gettime(timer_id, &cur_setting64);
> +
> + if (!ret)
> + cur_setting = itimerspec64_to_itimerspec(&cur_setting64);
> +#endif
>
> if (!ret && copy_to_user(setting, &cur_setting, sizeof (cur_setting)))
> return -EFAULT;
At this point, you are overlapping a bit with the series that I'm doing, but
I guess you can leave it like this for now.
What I want to do here is to introduce a get_itimerspec64()/put_itimerspec64()
function that copies an itimerspec structure from user space and puts it into
a local variable, with different implementations for 32-bit and 64-bit,
in order to avoid the #ifdef. I'm hoping to have a first cut at my patch series
soon, and we can coordinate this part then.
Arnd
_______________________________________________
Y2038 mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/y2038
