* [PATCH] Hook compat_sys_nanosleep up to high res timer code @ 2007-10-14 21:54 Anton Blanchard 2007-10-14 22:28 ` Arnd Bergmann 0 siblings, 1 reply; 9+ messages in thread From: Anton Blanchard @ 2007-10-14 21:54 UTC (permalink / raw) To: mingo, tglx; +Cc: linuxppc-dev, linux-kernel Now we have high res timers on ppc64 I thought Id test them. It turns out compat_sys_nanosleep hasnt been converted to the hrtimer code and so is limited to HZ resolution. The following patch makes compat_sys_nanosleep call hrtimer_nanosleep and uses compat_alloc_user_space to avoid setting KERNEL_DS. Signed-off-by: Anton Blanchard <anton@samba.org> --- diff --git a/kernel/compat.c b/kernel/compat.c index 3bae374..46795ac 100644 --- a/kernel/compat.c +++ b/kernel/compat.c @@ -40,62 +40,29 @@ int put_compat_timespec(const struct timespec *ts, struct compat_timespec __user __put_user(ts->tv_nsec, &cts->tv_nsec)) ? -EFAULT : 0; } -static long compat_nanosleep_restart(struct restart_block *restart) -{ - unsigned long expire = restart->arg0, now = jiffies; - struct compat_timespec __user *rmtp; - - /* Did it expire while we handled signals? */ - if (!time_after(expire, now)) - return 0; - - expire = schedule_timeout_interruptible(expire - now); - if (expire == 0) - return 0; - - rmtp = (struct compat_timespec __user *)restart->arg1; - if (rmtp) { - struct compat_timespec ct; - struct timespec t; - - jiffies_to_timespec(expire, &t); - ct.tv_sec = t.tv_sec; - ct.tv_nsec = t.tv_nsec; - if (copy_to_user(rmtp, &ct, sizeof(ct))) - return -EFAULT; - } - /* The 'restart' block is already filled in */ - return -ERESTART_RESTARTBLOCK; -} - asmlinkage long compat_sys_nanosleep(struct compat_timespec __user *rqtp, - struct compat_timespec __user *rmtp) + struct compat_timespec __user *rmtp) { - struct timespec t; - struct restart_block *restart; - unsigned long expire; + struct timespec tu; + struct timespec __user *rmtp64; + long ret; - if (get_compat_timespec(&t, rqtp)) + if (get_compat_timespec(&tu, rqtp)) return -EFAULT; - if ((t.tv_nsec >= 1000000000L) || (t.tv_nsec < 0) || (t.tv_sec < 0)) + if (!timespec_valid(&tu)) return -EINVAL; - expire = timespec_to_jiffies(&t) + (t.tv_sec || t.tv_nsec); - expire = schedule_timeout_interruptible(expire); - if (expire == 0) - return 0; + rmtp64 = compat_alloc_user_space(sizeof(*rmtp64)); + ret = hrtimer_nanosleep(&tu, rmtp64, HRTIMER_MODE_REL, CLOCK_MONOTONIC); - if (rmtp) { - jiffies_to_timespec(expire, &t); - if (put_compat_timespec(&t, rmtp)) + if (ret) { + if (copy_from_user(&tu, rmtp64, sizeof(*rmtp64)) || + put_compat_timespec(&tu, rmtp)) return -EFAULT; } - restart = ¤t_thread_info()->restart_block; - restart->fn = compat_nanosleep_restart; - restart->arg0 = jiffies + expire; - restart->arg1 = (unsigned long) rmtp; - return -ERESTART_RESTARTBLOCK; + + return ret; } static inline long get_compat_itimerval(struct itimerval *o, ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] Hook compat_sys_nanosleep up to high res timer code 2007-10-14 21:54 [PATCH] Hook compat_sys_nanosleep up to high res timer code Anton Blanchard @ 2007-10-14 22:28 ` Arnd Bergmann 2007-10-14 23:16 ` Anton Blanchard 0 siblings, 1 reply; 9+ messages in thread From: Arnd Bergmann @ 2007-10-14 22:28 UTC (permalink / raw) To: linuxppc-dev; +Cc: mingo, tglx, Anton Blanchard, linux-kernel On Sunday 14 October 2007, Anton Blanchard wrote: > Now we have high res timers on ppc64 I thought Id test them. It turns > out compat_sys_nanosleep hasnt been converted to the hrtimer code and so > is limited to HZ resolution. > > The following patch makes compat_sys_nanosleep call hrtimer_nanosleep > and uses compat_alloc_user_space to avoid setting KERNEL_DS. > > Signed-off-by: Anton Blanchard <anton@samba.org> The code looks correct, but I think it would be nicer to change hrtimer_nanosleep to take a kernel pointer and have all three callers (common_nsleep, sys_nanosleep and compat_sys_nanosleep) do the copy_to_user/put_compat_timespec in the caller. This would also make it possible to get rid of set_fs() in compat_sys_clock_nanosleep(). Arnd <>< ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Hook compat_sys_nanosleep up to high res timer code 2007-10-14 22:28 ` Arnd Bergmann @ 2007-10-14 23:16 ` Anton Blanchard 2007-10-15 6:11 ` Thomas Gleixner 0 siblings, 1 reply; 9+ messages in thread From: Anton Blanchard @ 2007-10-14 23:16 UTC (permalink / raw) To: Arnd Bergmann; +Cc: linuxppc-dev, mingo, tglx, linux-kernel Hi Arnd, > The code looks correct, but I think it would be nicer to change > hrtimer_nanosleep to take a kernel pointer and have all three > callers (common_nsleep, sys_nanosleep and compat_sys_nanosleep) > do the copy_to_user/put_compat_timespec in the caller. Good idea, I had considered that but thought a larger cleanup might run afoul of the merge rules :) Regardless, here it is. Id appreciate a once over since it does affect more code than the previous patch :) Anton -- Now we have high res timers on ppc64 I thought Id test them. It turns out compat_sys_nanosleep hasnt been converted to the hrtimer code and so is limited to HZ resolution. The following patch pulls the copy_to_user out of hrtimer_nanosleep and into the callers (common_nsleep, sys_nanosleep and compat_sys_nanosleep) thus avoiding any set_fs(KERNEL_DS) or compat_alloc_userspace tricks. Signed-off-by: Anton Blanchard <anton@samba.org> --- diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h index 540799b..7a9398e 100644 --- a/include/linux/hrtimer.h +++ b/include/linux/hrtimer.h @@ -300,7 +300,7 @@ hrtimer_forward(struct hrtimer *timer, ktime_t now, ktime_t interval); /* Precise sleep: */ extern long hrtimer_nanosleep(struct timespec *rqtp, - struct timespec __user *rmtp, + struct timespec *rmtp, const enum hrtimer_mode mode, const clockid_t clockid); extern long hrtimer_nanosleep_restart(struct restart_block *restart_block); diff --git a/kernel/compat.c b/kernel/compat.c index 3bae374..44abfce 100644 --- a/kernel/compat.c +++ b/kernel/compat.c @@ -40,62 +40,26 @@ int put_compat_timespec(const struct timespec *ts, struct compat_timespec __user __put_user(ts->tv_nsec, &cts->tv_nsec)) ? -EFAULT : 0; } -static long compat_nanosleep_restart(struct restart_block *restart) -{ - unsigned long expire = restart->arg0, now = jiffies; - struct compat_timespec __user *rmtp; - - /* Did it expire while we handled signals? */ - if (!time_after(expire, now)) - return 0; - - expire = schedule_timeout_interruptible(expire - now); - if (expire == 0) - return 0; - - rmtp = (struct compat_timespec __user *)restart->arg1; - if (rmtp) { - struct compat_timespec ct; - struct timespec t; - - jiffies_to_timespec(expire, &t); - ct.tv_sec = t.tv_sec; - ct.tv_nsec = t.tv_nsec; - if (copy_to_user(rmtp, &ct, sizeof(ct))) - return -EFAULT; - } - /* The 'restart' block is already filled in */ - return -ERESTART_RESTARTBLOCK; -} - asmlinkage long compat_sys_nanosleep(struct compat_timespec __user *rqtp, - struct compat_timespec __user *rmtp) + struct compat_timespec __user *rmtp) { - struct timespec t; - struct restart_block *restart; - unsigned long expire; + struct timespec tu, rmt; + long ret; - if (get_compat_timespec(&t, rqtp)) + if (get_compat_timespec(&tu, rqtp)) return -EFAULT; - if ((t.tv_nsec >= 1000000000L) || (t.tv_nsec < 0) || (t.tv_sec < 0)) + if (!timespec_valid(&tu)) return -EINVAL; - expire = timespec_to_jiffies(&t) + (t.tv_sec || t.tv_nsec); - expire = schedule_timeout_interruptible(expire); - if (expire == 0) - return 0; + ret = hrtimer_nanosleep(&tu, &rmt, HRTIMER_MODE_REL, CLOCK_MONOTONIC); - if (rmtp) { - jiffies_to_timespec(expire, &t); - if (put_compat_timespec(&t, rmtp)) + if (ret) { + if (put_compat_timespec(&rmt, rmtp)) return -EFAULT; } - restart = ¤t_thread_info()->restart_block; - restart->fn = compat_nanosleep_restart; - restart->arg0 = jiffies + expire; - restart->arg1 = (unsigned long) rmtp; - return -ERESTART_RESTARTBLOCK; + + return ret; } static inline long get_compat_itimerval(struct itimerval *o, diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index dc8a445..095e09e 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -1286,8 +1286,7 @@ static int __sched do_nanosleep(struct hrtimer_sleeper *t, enum hrtimer_mode mod long __sched hrtimer_nanosleep_restart(struct restart_block *restart) { struct hrtimer_sleeper t; - struct timespec __user *rmtp; - struct timespec tu; + struct timespec *rmtp; ktime_t time; restart->fn = do_no_restart_syscall; @@ -1298,14 +1297,12 @@ long __sched hrtimer_nanosleep_restart(struct restart_block *restart) if (do_nanosleep(&t, HRTIMER_MODE_ABS)) return 0; - rmtp = (struct timespec __user *) restart->arg1; + rmtp = (struct timespec *)restart->arg1; if (rmtp) { time = ktime_sub(t.timer.expires, t.timer.base->get_time()); if (time.tv64 <= 0) return 0; - tu = ktime_to_timespec(time); - if (copy_to_user(rmtp, &tu, sizeof(tu))) - return -EFAULT; + *rmtp = ktime_to_timespec(time); } restart->fn = hrtimer_nanosleep_restart; @@ -1314,12 +1311,11 @@ long __sched hrtimer_nanosleep_restart(struct restart_block *restart) return -ERESTART_RESTARTBLOCK; } -long hrtimer_nanosleep(struct timespec *rqtp, struct timespec __user *rmtp, +long hrtimer_nanosleep(struct timespec *rqtp, struct timespec *rmtp, const enum hrtimer_mode mode, const clockid_t clockid) { struct restart_block *restart; struct hrtimer_sleeper t; - struct timespec tu; ktime_t rem; hrtimer_init(&t.timer, clockid, mode); @@ -1335,9 +1331,7 @@ long hrtimer_nanosleep(struct timespec *rqtp, struct timespec __user *rmtp, rem = ktime_sub(t.timer.expires, t.timer.base->get_time()); if (rem.tv64 <= 0) return 0; - tu = ktime_to_timespec(rem); - if (copy_to_user(rmtp, &tu, sizeof(tu))) - return -EFAULT; + *rmtp = ktime_to_timespec(rem); } restart = ¤t_thread_info()->restart_block; @@ -1353,7 +1347,8 @@ long hrtimer_nanosleep(struct timespec *rqtp, struct timespec __user *rmtp, asmlinkage long sys_nanosleep(struct timespec __user *rqtp, struct timespec __user *rmtp) { - struct timespec tu; + struct timespec tu, rmt; + int ret; if (copy_from_user(&tu, rqtp, sizeof(tu))) return -EFAULT; @@ -1361,7 +1356,14 @@ sys_nanosleep(struct timespec __user *rqtp, struct timespec __user *rmtp) if (!timespec_valid(&tu)) return -EINVAL; - return hrtimer_nanosleep(&tu, rmtp, HRTIMER_MODE_REL, CLOCK_MONOTONIC); + ret = hrtimer_nanosleep(&tu, &rmt, HRTIMER_MODE_REL, CLOCK_MONOTONIC); + + if (ret) { + if (copy_to_user(rmtp, &rmt, sizeof(*rmtp))) + return -EFAULT; + } + + return ret; } /* diff --git a/kernel/posix-timers.c b/kernel/posix-timers.c index 7a15afb..fc7dac2 100644 --- a/kernel/posix-timers.c +++ b/kernel/posix-timers.c @@ -980,9 +980,19 @@ sys_clock_getres(const clockid_t which_clock, struct timespec __user *tp) static int common_nsleep(const clockid_t which_clock, int flags, struct timespec *tsave, struct timespec __user *rmtp) { - return hrtimer_nanosleep(tsave, rmtp, flags & TIMER_ABSTIME ? + struct timespec rmt; + int ret; + + ret = hrtimer_nanosleep(tsave, &rmt, flags & TIMER_ABSTIME ? HRTIMER_MODE_ABS : HRTIMER_MODE_REL, which_clock); + + if (ret) { + if (copy_to_user(rmtp, &rmt, sizeof(*rmtp))) + return -EFAULT; + } + + return ret; } asmlinkage long ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] Hook compat_sys_nanosleep up to high res timer code 2007-10-14 23:16 ` Anton Blanchard @ 2007-10-15 6:11 ` Thomas Gleixner 2007-10-15 6:38 ` [PATCH] Rework hrtimer_nanosleep to make sys_compat_nanosleep easier Anton Blanchard 0 siblings, 1 reply; 9+ messages in thread From: Thomas Gleixner @ 2007-10-15 6:11 UTC (permalink / raw) To: Anton Blanchard; +Cc: linuxppc-dev, mingo, linux-kernel, Arnd Bergmann On Sun, 14 Oct 2007, Anton Blanchard wrote: > Hi Arnd, > > > The code looks correct, but I think it would be nicer to change > > hrtimer_nanosleep to take a kernel pointer and have all three > > callers (common_nsleep, sys_nanosleep and compat_sys_nanosleep) > > do the copy_to_user/put_compat_timespec in the caller. > > Good idea, I had considered that but thought a larger cleanup might run > afoul of the merge rules :) Looks good, except .... > --- a/kernel/compat.c > +++ b/kernel/compat.c > @@ -40,62 +40,26 @@ int put_compat_timespec(const struct timespec *ts, struct compat_timespec __user > __put_user(ts->tv_nsec, &cts->tv_nsec)) ? -EFAULT : 0; > } Can you put this into a separate patch please ? > --- a/kernel/hrtimer.c > +++ b/kernel/hrtimer.c > restart = ¤t_thread_info()->restart_block; > @@ -1353,7 +1347,8 @@ long hrtimer_nanosleep(struct timespec *rqtp, struct timespec __user *rmtp, > asmlinkage long > sys_nanosleep(struct timespec __user *rqtp, struct timespec __user *rmtp) > { > - struct timespec tu; > + struct timespec tu, rmt; > + int ret; > > if (copy_from_user(&tu, rqtp, sizeof(tu))) > return -EFAULT; > @@ -1361,7 +1356,14 @@ sys_nanosleep(struct timespec __user *rqtp, struct timespec __user *rmtp) > if (!timespec_valid(&tu)) > return -EINVAL; > > - return hrtimer_nanosleep(&tu, rmtp, HRTIMER_MODE_REL, CLOCK_MONOTONIC); > + ret = hrtimer_nanosleep(&tu, &rmt, HRTIMER_MODE_REL, CLOCK_MONOTONIC); > + > + if (ret) { Can you check for rmtp as well ? rmtp is optional and can be NULL > + if (copy_to_user(rmtp, &rmt, sizeof(*rmtp))) > + return -EFAULT; > + } > + > + return ret; > } > > /* > diff --git a/kernel/posix-timers.c b/kernel/posix-timers.c > index 7a15afb..fc7dac2 100644 > --- a/kernel/posix-timers.c > +++ b/kernel/posix-timers.c > @@ -980,9 +980,19 @@ sys_clock_getres(const clockid_t which_clock, struct timespec __user *tp) > static int common_nsleep(const clockid_t which_clock, int flags, > struct timespec *tsave, struct timespec __user *rmtp) > { > - return hrtimer_nanosleep(tsave, rmtp, flags & TIMER_ABSTIME ? > + struct timespec rmt; > + int ret; > + > + ret = hrtimer_nanosleep(tsave, &rmt, flags & TIMER_ABSTIME ? > HRTIMER_MODE_ABS : HRTIMER_MODE_REL, > which_clock); > + > + if (ret) { Ditto. > + if (copy_to_user(rmtp, &rmt, sizeof(*rmtp))) > + return -EFAULT; > + } > + > + return ret; > } Thanks, tglx ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] Rework hrtimer_nanosleep to make sys_compat_nanosleep easier 2007-10-15 6:11 ` Thomas Gleixner @ 2007-10-15 6:38 ` Anton Blanchard 2007-10-15 6:43 ` [PATCH] Hook compat_sys_nanosleep up to high res timer code Anton Blanchard 2007-10-15 7:28 ` [PATCH] Rework hrtimer_nanosleep to make sys_compat_nanosleep easier Arnd Bergmann 0 siblings, 2 replies; 9+ messages in thread From: Anton Blanchard @ 2007-10-15 6:38 UTC (permalink / raw) To: Thomas Gleixner; +Cc: linuxppc-dev, mingo, linux-kernel, Arnd Bergmann Hi Thomas, Thanks for the review, updates to follow. Anton -- Pull the copy_to_user out of hrtimer_nanosleep and into the callers (common_nsleep, sys_nanosleep) in preparation for converting compat_sys_nanosleep to use hrtimers. Signed-off-by: Anton Blanchard <anton@samba.org> --- diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h index 540799b..7a9398e 100644 --- a/include/linux/hrtimer.h +++ b/include/linux/hrtimer.h @@ -300,7 +300,7 @@ hrtimer_forward(struct hrtimer *timer, ktime_t now, ktime_t interval); /* Precise sleep: */ extern long hrtimer_nanosleep(struct timespec *rqtp, - struct timespec __user *rmtp, + struct timespec *rmtp, const enum hrtimer_mode mode, const clockid_t clockid); extern long hrtimer_nanosleep_restart(struct restart_block *restart_block); diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index dc8a445..129fead 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -1286,8 +1286,7 @@ static int __sched do_nanosleep(struct hrtimer_sleeper *t, enum hrtimer_mode mod long __sched hrtimer_nanosleep_restart(struct restart_block *restart) { struct hrtimer_sleeper t; - struct timespec __user *rmtp; - struct timespec tu; + struct timespec *rmtp; ktime_t time; restart->fn = do_no_restart_syscall; @@ -1298,14 +1297,12 @@ long __sched hrtimer_nanosleep_restart(struct restart_block *restart) if (do_nanosleep(&t, HRTIMER_MODE_ABS)) return 0; - rmtp = (struct timespec __user *) restart->arg1; + rmtp = (struct timespec *)restart->arg1; if (rmtp) { time = ktime_sub(t.timer.expires, t.timer.base->get_time()); if (time.tv64 <= 0) return 0; - tu = ktime_to_timespec(time); - if (copy_to_user(rmtp, &tu, sizeof(tu))) - return -EFAULT; + *rmtp = ktime_to_timespec(time); } restart->fn = hrtimer_nanosleep_restart; @@ -1314,12 +1311,11 @@ long __sched hrtimer_nanosleep_restart(struct restart_block *restart) return -ERESTART_RESTARTBLOCK; } -long hrtimer_nanosleep(struct timespec *rqtp, struct timespec __user *rmtp, +long hrtimer_nanosleep(struct timespec *rqtp, struct timespec *rmtp, const enum hrtimer_mode mode, const clockid_t clockid) { struct restart_block *restart; struct hrtimer_sleeper t; - struct timespec tu; ktime_t rem; hrtimer_init(&t.timer, clockid, mode); @@ -1335,9 +1331,7 @@ long hrtimer_nanosleep(struct timespec *rqtp, struct timespec __user *rmtp, rem = ktime_sub(t.timer.expires, t.timer.base->get_time()); if (rem.tv64 <= 0) return 0; - tu = ktime_to_timespec(rem); - if (copy_to_user(rmtp, &tu, sizeof(tu))) - return -EFAULT; + *rmtp = ktime_to_timespec(rem); } restart = ¤t_thread_info()->restart_block; @@ -1353,7 +1347,8 @@ long hrtimer_nanosleep(struct timespec *rqtp, struct timespec __user *rmtp, asmlinkage long sys_nanosleep(struct timespec __user *rqtp, struct timespec __user *rmtp) { - struct timespec tu; + struct timespec tu, rmt; + int ret; if (copy_from_user(&tu, rqtp, sizeof(tu))) return -EFAULT; @@ -1361,7 +1356,14 @@ sys_nanosleep(struct timespec __user *rqtp, struct timespec __user *rmtp) if (!timespec_valid(&tu)) return -EINVAL; - return hrtimer_nanosleep(&tu, rmtp, HRTIMER_MODE_REL, CLOCK_MONOTONIC); + ret = hrtimer_nanosleep(&tu, &rmt, HRTIMER_MODE_REL, CLOCK_MONOTONIC); + + if (ret && rmtp) { + if (copy_to_user(rmtp, &rmt, sizeof(*rmtp))) + return -EFAULT; + } + + return ret; } /* diff --git a/kernel/posix-timers.c b/kernel/posix-timers.c index 57efe04..cce8c75 100644 --- a/kernel/posix-timers.c +++ b/kernel/posix-timers.c @@ -980,9 +980,19 @@ sys_clock_getres(const clockid_t which_clock, struct timespec __user *tp) static int common_nsleep(const clockid_t which_clock, int flags, struct timespec *tsave, struct timespec __user *rmtp) { - return hrtimer_nanosleep(tsave, rmtp, flags & TIMER_ABSTIME ? + struct timespec rmt; + int ret; + + ret = hrtimer_nanosleep(tsave, &rmt, flags & TIMER_ABSTIME ? HRTIMER_MODE_ABS : HRTIMER_MODE_REL, which_clock); + + if (ret && rmtp) { + if (copy_to_user(rmtp, &rmt, sizeof(*rmtp))) + return -EFAULT; + } + + return ret; } asmlinkage long ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH] Hook compat_sys_nanosleep up to high res timer code 2007-10-15 6:38 ` [PATCH] Rework hrtimer_nanosleep to make sys_compat_nanosleep easier Anton Blanchard @ 2007-10-15 6:43 ` Anton Blanchard 2007-10-15 7:28 ` [PATCH] Rework hrtimer_nanosleep to make sys_compat_nanosleep easier Arnd Bergmann 1 sibling, 0 replies; 9+ messages in thread From: Anton Blanchard @ 2007-10-15 6:43 UTC (permalink / raw) To: Thomas Gleixner; +Cc: linuxppc-dev, mingo, linux-kernel, Arnd Bergmann Now we have high res timers on ppc64 I thought Id test them. It turns out compat_sys_nanosleep hasnt been converted to the hrtimer code and so is limited to HZ resolution. The follow patch converts compat_sys_nanosleep to use high res timers. Signed-off-by: Anton Blanchard <anton@samba.org> --- diff --git a/kernel/compat.c b/kernel/compat.c index 3bae374..729f63d 100644 --- a/kernel/compat.c +++ b/kernel/compat.c @@ -40,62 +40,26 @@ int put_compat_timespec(const struct timespec *ts, struct compat_timespec __user __put_user(ts->tv_nsec, &cts->tv_nsec)) ? -EFAULT : 0; } -static long compat_nanosleep_restart(struct restart_block *restart) -{ - unsigned long expire = restart->arg0, now = jiffies; - struct compat_timespec __user *rmtp; - - /* Did it expire while we handled signals? */ - if (!time_after(expire, now)) - return 0; - - expire = schedule_timeout_interruptible(expire - now); - if (expire == 0) - return 0; - - rmtp = (struct compat_timespec __user *)restart->arg1; - if (rmtp) { - struct compat_timespec ct; - struct timespec t; - - jiffies_to_timespec(expire, &t); - ct.tv_sec = t.tv_sec; - ct.tv_nsec = t.tv_nsec; - if (copy_to_user(rmtp, &ct, sizeof(ct))) - return -EFAULT; - } - /* The 'restart' block is already filled in */ - return -ERESTART_RESTARTBLOCK; -} - asmlinkage long compat_sys_nanosleep(struct compat_timespec __user *rqtp, - struct compat_timespec __user *rmtp) + struct compat_timespec __user *rmtp) { - struct timespec t; - struct restart_block *restart; - unsigned long expire; + struct timespec tu, rmt; + long ret; - if (get_compat_timespec(&t, rqtp)) + if (get_compat_timespec(&tu, rqtp)) return -EFAULT; - if ((t.tv_nsec >= 1000000000L) || (t.tv_nsec < 0) || (t.tv_sec < 0)) + if (!timespec_valid(&tu)) return -EINVAL; - expire = timespec_to_jiffies(&t) + (t.tv_sec || t.tv_nsec); - expire = schedule_timeout_interruptible(expire); - if (expire == 0) - return 0; + ret = hrtimer_nanosleep(&tu, &rmt, HRTIMER_MODE_REL, CLOCK_MONOTONIC); - if (rmtp) { - jiffies_to_timespec(expire, &t); - if (put_compat_timespec(&t, rmtp)) + if (ret && rmtp) { + if (put_compat_timespec(&rmt, rmtp)) return -EFAULT; } - restart = ¤t_thread_info()->restart_block; - restart->fn = compat_nanosleep_restart; - restart->arg0 = jiffies + expire; - restart->arg1 = (unsigned long) rmtp; - return -ERESTART_RESTARTBLOCK; + + return ret; } static inline long get_compat_itimerval(struct itimerval *o, ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] Rework hrtimer_nanosleep to make sys_compat_nanosleep easier 2007-10-15 6:38 ` [PATCH] Rework hrtimer_nanosleep to make sys_compat_nanosleep easier Anton Blanchard 2007-10-15 6:43 ` [PATCH] Hook compat_sys_nanosleep up to high res timer code Anton Blanchard @ 2007-10-15 7:28 ` Arnd Bergmann 2007-10-15 21:06 ` Anton Blanchard 2007-10-15 21:13 ` [PATCH] Hook compat_sys_nanosleep up to high res timer code Anton Blanchard 1 sibling, 2 replies; 9+ messages in thread From: Arnd Bergmann @ 2007-10-15 7:28 UTC (permalink / raw) To: linuxppc-dev; +Cc: Thomas Gleixner, mingo, Anton Blanchard, linux-kernel On Monday 15 October 2007, Anton Blanchard wrote: > Pull the copy_to_user out of hrtimer_nanosleep and into the callers > (common_nsleep, sys_nanosleep) in preparation for converting > compat_sys_nanosleep to use hrtimers. Looks good, except for two micro-optimization: > Signed-off-by: Anton Blanchard <anton@samba.org> Acked-by: Arnd Bergmann <arnd@arndb.de> > @@ -1361,7 +1356,14 @@ sys_nanosleep(struct timespec __user *rqtp, struct= timespec __user *rmtp) > =A0=A0=A0=A0=A0=A0=A0=A0if (!timespec_valid(&tu)) > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0return -EINVAL; > =A0 > -=A0=A0=A0=A0=A0=A0=A0return hrtimer_nanosleep(&tu, rmtp, HRTIMER_MODE_RE= L, CLOCK_MONOTONIC); > +=A0=A0=A0=A0=A0=A0=A0ret =3D hrtimer_nanosleep(&tu, &rmt, HRTIMER_MODE_R= EL, CLOCK_MONOTONIC); > + > +=A0=A0=A0=A0=A0=A0=A0if (ret && rmtp) { > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0if (copy_to_user(rmtp, &rmt= , sizeof(*rmtp))) > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0ret= urn -EFAULT; > +=A0=A0=A0=A0=A0=A0=A0} > + > +=A0=A0=A0=A0=A0=A0=A0return ret; > =A0} > =A0 If it's common to call sys_nanosleep with a NULL rmtp argument, we could sa= ve a few cycles using=20 return hrtimer_nanosleep(&tu, rmtp ? &rmp : NULL, HRTIMER_MODE_REL, CLOCK_MONOTONIC); > diff --git a/kernel/posix-timers.c b/kernel/posix-timers.c > index 57efe04..cce8c75 100644 > --- a/kernel/posix-timers.c > +++ b/kernel/posix-timers.c > @@ -980,9 +980,19 @@ sys_clock_getres(const clockid_t which_clock, struct= timespec __user *tp) > =A0static int common_nsleep(const clockid_t which_clock, int flags, > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 = struct timespec *tsave, struct timespec __user *rmtp) > =A0{ > -=A0=A0=A0=A0=A0=A0=A0return hrtimer_nanosleep(tsave, rmtp, flags & TIMER= _ABSTIME ? > +=A0=A0=A0=A0=A0=A0=A0struct timespec rmt; > +=A0=A0=A0=A0=A0=A0=A0int ret; > + > +=A0=A0=A0=A0=A0=A0=A0ret =3D hrtimer_nanosleep(tsave, &rmt, flags & TIME= R_ABSTIME ? > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0 HRTIMER_MODE_ABS : HRTIMER_MODE_REL, > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0 which_clock); > + > +=A0=A0=A0=A0=A0=A0=A0if (ret && rmtp) { > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0if (copy_to_user(rmtp, &rmt= , sizeof(*rmtp))) > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0ret= urn -EFAULT; > +=A0=A0=A0=A0=A0=A0=A0} > + > +=A0=A0=A0=A0=A0=A0=A0return ret; > =A0} I think it would be better here to propagate the move to a kernel *rmtp down to sys_clock_nanosleep so we get the same optimization in compat_sys_clock_nanosleep. That should probably also be a separate patch. I can do one if you don't do it first. Arnd <>< ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] Rework hrtimer_nanosleep to make sys_compat_nanosleep easier 2007-10-15 7:28 ` [PATCH] Rework hrtimer_nanosleep to make sys_compat_nanosleep easier Arnd Bergmann @ 2007-10-15 21:06 ` Anton Blanchard 2007-10-15 21:13 ` [PATCH] Hook compat_sys_nanosleep up to high res timer code Anton Blanchard 1 sibling, 0 replies; 9+ messages in thread From: Anton Blanchard @ 2007-10-15 21:06 UTC (permalink / raw) To: Arnd Bergmann; +Cc: linuxppc-dev, Thomas Gleixner, mingo, linux-kernel Hi, > If it's common to call sys_nanosleep with a NULL rmtp argument, we could > save a few cycles using > > return hrtimer_nanosleep(&tu, rmtp ? &rmp : NULL, HRTIMER_MODE_REL, > CLOCK_MONOTONIC); Good idea, patches updated. > I think it would be better here to propagate the move to a kernel *rmtp > down to sys_clock_nanosleep so we get the same optimization in > compat_sys_clock_nanosleep. That should probably also be a separate > patch. I can do one if you don't do it first. I can get to this later in the week, if you feel the urge in the meantime go for it :) Anton -- Pull the copy_to_user out of hrtimer_nanosleep and into the callers (common_nsleep, sys_nanosleep) in preparation for converting compat_sys_nanosleep to use hrtimers. Signed-off-by: Anton Blanchard <anton@samba.org> Acked-by: Arnd Bergmann <arnd@arndb.de> --- diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h index 540799b..7a9398e 100644 --- a/include/linux/hrtimer.h +++ b/include/linux/hrtimer.h @@ -300,7 +300,7 @@ hrtimer_forward(struct hrtimer *timer, ktime_t now, ktime_t interval); /* Precise sleep: */ extern long hrtimer_nanosleep(struct timespec *rqtp, - struct timespec __user *rmtp, + struct timespec *rmtp, const enum hrtimer_mode mode, const clockid_t clockid); extern long hrtimer_nanosleep_restart(struct restart_block *restart_block); diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index dc8a445..b2b2c2b 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -1286,8 +1286,7 @@ static int __sched do_nanosleep(struct hrtimer_sleeper *t, enum hrtimer_mode mod long __sched hrtimer_nanosleep_restart(struct restart_block *restart) { struct hrtimer_sleeper t; - struct timespec __user *rmtp; - struct timespec tu; + struct timespec *rmtp; ktime_t time; restart->fn = do_no_restart_syscall; @@ -1298,14 +1297,12 @@ long __sched hrtimer_nanosleep_restart(struct restart_block *restart) if (do_nanosleep(&t, HRTIMER_MODE_ABS)) return 0; - rmtp = (struct timespec __user *) restart->arg1; + rmtp = (struct timespec *)restart->arg1; if (rmtp) { time = ktime_sub(t.timer.expires, t.timer.base->get_time()); if (time.tv64 <= 0) return 0; - tu = ktime_to_timespec(time); - if (copy_to_user(rmtp, &tu, sizeof(tu))) - return -EFAULT; + *rmtp = ktime_to_timespec(time); } restart->fn = hrtimer_nanosleep_restart; @@ -1314,12 +1311,11 @@ long __sched hrtimer_nanosleep_restart(struct restart_block *restart) return -ERESTART_RESTARTBLOCK; } -long hrtimer_nanosleep(struct timespec *rqtp, struct timespec __user *rmtp, +long hrtimer_nanosleep(struct timespec *rqtp, struct timespec *rmtp, const enum hrtimer_mode mode, const clockid_t clockid) { struct restart_block *restart; struct hrtimer_sleeper t; - struct timespec tu; ktime_t rem; hrtimer_init(&t.timer, clockid, mode); @@ -1335,9 +1331,7 @@ long hrtimer_nanosleep(struct timespec *rqtp, struct timespec __user *rmtp, rem = ktime_sub(t.timer.expires, t.timer.base->get_time()); if (rem.tv64 <= 0) return 0; - tu = ktime_to_timespec(rem); - if (copy_to_user(rmtp, &tu, sizeof(tu))) - return -EFAULT; + *rmtp = ktime_to_timespec(rem); } restart = ¤t_thread_info()->restart_block; @@ -1353,7 +1347,8 @@ long hrtimer_nanosleep(struct timespec *rqtp, struct timespec __user *rmtp, asmlinkage long sys_nanosleep(struct timespec __user *rqtp, struct timespec __user *rmtp) { - struct timespec tu; + struct timespec tu, rmt; + int ret; if (copy_from_user(&tu, rqtp, sizeof(tu))) return -EFAULT; @@ -1361,7 +1356,15 @@ sys_nanosleep(struct timespec __user *rqtp, struct timespec __user *rmtp) if (!timespec_valid(&tu)) return -EINVAL; - return hrtimer_nanosleep(&tu, rmtp, HRTIMER_MODE_REL, CLOCK_MONOTONIC); + ret = hrtimer_nanosleep(&tu, rmtp ? &rmt : NULL, HRTIMER_MODE_REL, + CLOCK_MONOTONIC); + + if (ret && rmtp) { + if (copy_to_user(rmtp, &rmt, sizeof(*rmtp))) + return -EFAULT; + } + + return ret; } /* diff --git a/kernel/posix-timers.c b/kernel/posix-timers.c index 57efe04..56b3d86 100644 --- a/kernel/posix-timers.c +++ b/kernel/posix-timers.c @@ -980,9 +980,20 @@ sys_clock_getres(const clockid_t which_clock, struct timespec __user *tp) static int common_nsleep(const clockid_t which_clock, int flags, struct timespec *tsave, struct timespec __user *rmtp) { - return hrtimer_nanosleep(tsave, rmtp, flags & TIMER_ABSTIME ? - HRTIMER_MODE_ABS : HRTIMER_MODE_REL, - which_clock); + struct timespec rmt; + int ret; + + ret = hrtimer_nanosleep(tsave, rmtp ? &rmt : NULL, + flags & TIMER_ABSTIME ? + HRTIMER_MODE_ABS : HRTIMER_MODE_REL, + which_clock); + + if (ret && rmtp) { + if (copy_to_user(rmtp, &rmt, sizeof(*rmtp))) + return -EFAULT; + } + + return ret; } asmlinkage long ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH] Hook compat_sys_nanosleep up to high res timer code 2007-10-15 7:28 ` [PATCH] Rework hrtimer_nanosleep to make sys_compat_nanosleep easier Arnd Bergmann 2007-10-15 21:06 ` Anton Blanchard @ 2007-10-15 21:13 ` Anton Blanchard 1 sibling, 0 replies; 9+ messages in thread From: Anton Blanchard @ 2007-10-15 21:13 UTC (permalink / raw) To: Arnd Bergmann; +Cc: linuxppc-dev, Thomas Gleixner, mingo, linux-kernel Now we have high res timers on ppc64 I thought Id test them. It turns out compat_sys_nanosleep hasnt been converted to the hrtimer code and so is limited to HZ resolution. The follow patch converts compat_sys_nanosleep to use high res timers. Signed-off-by: Anton Blanchard <anton@samba.org> --- diff --git a/kernel/compat.c b/kernel/compat.c index 3bae374..252a446 100644 --- a/kernel/compat.c +++ b/kernel/compat.c @@ -40,62 +40,27 @@ int put_compat_timespec(const struct timespec *ts, struct compat_timespec __user __put_user(ts->tv_nsec, &cts->tv_nsec)) ? -EFAULT : 0; } -static long compat_nanosleep_restart(struct restart_block *restart) -{ - unsigned long expire = restart->arg0, now = jiffies; - struct compat_timespec __user *rmtp; - - /* Did it expire while we handled signals? */ - if (!time_after(expire, now)) - return 0; - - expire = schedule_timeout_interruptible(expire - now); - if (expire == 0) - return 0; - - rmtp = (struct compat_timespec __user *)restart->arg1; - if (rmtp) { - struct compat_timespec ct; - struct timespec t; - - jiffies_to_timespec(expire, &t); - ct.tv_sec = t.tv_sec; - ct.tv_nsec = t.tv_nsec; - if (copy_to_user(rmtp, &ct, sizeof(ct))) - return -EFAULT; - } - /* The 'restart' block is already filled in */ - return -ERESTART_RESTARTBLOCK; -} - asmlinkage long compat_sys_nanosleep(struct compat_timespec __user *rqtp, - struct compat_timespec __user *rmtp) + struct compat_timespec __user *rmtp) { - struct timespec t; - struct restart_block *restart; - unsigned long expire; + struct timespec tu, rmt; + long ret; - if (get_compat_timespec(&t, rqtp)) + if (get_compat_timespec(&tu, rqtp)) return -EFAULT; - if ((t.tv_nsec >= 1000000000L) || (t.tv_nsec < 0) || (t.tv_sec < 0)) + if (!timespec_valid(&tu)) return -EINVAL; - expire = timespec_to_jiffies(&t) + (t.tv_sec || t.tv_nsec); - expire = schedule_timeout_interruptible(expire); - if (expire == 0) - return 0; + ret = hrtimer_nanosleep(&tu, rmtp ? &rmt : NULL, HRTIMER_MODE_REL, + CLOCK_MONOTONIC); - if (rmtp) { - jiffies_to_timespec(expire, &t); - if (put_compat_timespec(&t, rmtp)) + if (ret && rmtp) { + if (put_compat_timespec(&rmt, rmtp)) return -EFAULT; } - restart = ¤t_thread_info()->restart_block; - restart->fn = compat_nanosleep_restart; - restart->arg0 = jiffies + expire; - restart->arg1 = (unsigned long) rmtp; - return -ERESTART_RESTARTBLOCK; + + return ret; } static inline long get_compat_itimerval(struct itimerval *o, ^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2007-10-15 21:13 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2007-10-14 21:54 [PATCH] Hook compat_sys_nanosleep up to high res timer code Anton Blanchard 2007-10-14 22:28 ` Arnd Bergmann 2007-10-14 23:16 ` Anton Blanchard 2007-10-15 6:11 ` Thomas Gleixner 2007-10-15 6:38 ` [PATCH] Rework hrtimer_nanosleep to make sys_compat_nanosleep easier Anton Blanchard 2007-10-15 6:43 ` [PATCH] Hook compat_sys_nanosleep up to high res timer code Anton Blanchard 2007-10-15 7:28 ` [PATCH] Rework hrtimer_nanosleep to make sys_compat_nanosleep easier Arnd Bergmann 2007-10-15 21:06 ` Anton Blanchard 2007-10-15 21:13 ` [PATCH] Hook compat_sys_nanosleep up to high res timer code Anton Blanchard
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).