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 baolin.wang@linaro.org
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);
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