On 28 April 2015 at 17:42, Arnd Bergmann <[email protected]> wrote:
> 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()
>
>
But the default_timer_get64((struct k_clock *kc, struct k_itimer *timr, struct
itimerspec64 *cur_setting64)
function he suggested can't get the "kc" parameter, cause the "timer_get"
point prototype is
"void (*timer_get64) (struct k_itimer *timr, struct itimerspec64
*cur_setting);".
static int default_timer_get64(struct k_clock *kc, struct k_itimer *timr,
struct itimerspec64 *cur_setting64)
{
struct itimerspec cur_setting;
kc->timer_get(timer, &cur_setting);
return 0;
}
> @@ -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
>
OK.
--
Baolin.wang
Best Regards
_______________________________________________
Y2038 mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/y2038
