[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20110423175922.GB484@redhat.com>
Date: Sat, 23 Apr 2011 19:59:22 +0200
From: Oleg Nesterov <oleg@...hat.com>
To: Linus Torvalds <torvalds@...ux-foundation.org>,
Andrew Morton <akpm@...ux-foundation.org>
Cc: Tejun Heo <tj@...nel.org>,
"Nikita V. Youshchenko" <nyoushchenko@...sta.com>,
Matt Fleming <matt@...sole-pimps.org>,
linux-kernel@...r.kernel.org
Subject: [PATCH 1/3] signal: sys_rt_sigtimedwait: simplify the timeout logic
No functional changes, cleanup compat_sys_rt_sigtimedwait() and
sys_rt_sigtimedwait().
Calculate the timeout before we take ->siglock, this simplifies and
lessens the code. Use timespec_valid() to check the timespec.
I don't understand why we are adding (ts.tv_sec || ts.tv_nsec) to
timespec_to_jiffies(&ts). Perhaps to ensure we will sleep at least
one jiffy if ts != 0? But in this case we should only check tv_nsec,
I don't think timespec_to_jiffies() can return zero if tv_sec != 0.
In fact I suspect timespec_to_jiffies() can only return zero if
tv_sec == tv_nsec == 0 because we add "TICK_NSEC - 1", but I am not
sure I understand correctly this math.
Signed-off-by: Oleg Nesterov <oleg@...hat.com>
---
kernel/signal.c | 46 ++++++++++++++++++++--------------------------
kernel/compat.c | 38 ++++++++++++++++----------------------
2 files changed, 36 insertions(+), 48 deletions(-)
--- sigprocmask/kernel/signal.c~1_sigtimedwait_to 2011-04-22 15:48:33.000000000 +0200
+++ sigprocmask/kernel/signal.c 2011-04-22 19:05:51.000000000 +0200
@@ -2327,7 +2327,7 @@ SYSCALL_DEFINE4(rt_sigtimedwait, const s
sigset_t these;
struct timespec ts;
siginfo_t info;
- long timeout = 0;
+ long timeout;
/* XXX: Don't preclude handling different sized sigset_t's. */
if (sigsetsize != sizeof(sigset_t))
@@ -2343,41 +2343,35 @@ SYSCALL_DEFINE4(rt_sigtimedwait, const s
sigdelsetmask(&these, sigmask(SIGKILL)|sigmask(SIGSTOP));
signotset(&these);
+ timeout = MAX_SCHEDULE_TIMEOUT;
if (uts) {
if (copy_from_user(&ts, uts, sizeof(ts)))
return -EFAULT;
- if (ts.tv_nsec >= 1000000000L || ts.tv_nsec < 0
- || ts.tv_sec < 0)
+ if (!timespec_valid(&ts))
return -EINVAL;
+ timeout = timespec_to_jiffies(&ts) + (ts.tv_sec || ts.tv_nsec);
}
spin_lock_irq(¤t->sighand->siglock);
sig = dequeue_signal(current, &these, &info);
- if (!sig) {
- timeout = MAX_SCHEDULE_TIMEOUT;
- if (uts)
- timeout = (timespec_to_jiffies(&ts)
- + (ts.tv_sec || ts.tv_nsec));
-
- if (timeout) {
- /*
- * None ready -- temporarily unblock those we're
- * interested while we are sleeping in so that we'll
- * be awakened when they arrive.
- */
- current->real_blocked = current->blocked;
- sigandsets(¤t->blocked, ¤t->blocked, &these);
- recalc_sigpending();
- spin_unlock_irq(¤t->sighand->siglock);
+ if (!sig && timeout) {
+ /*
+ * None ready -- temporarily unblock those we're
+ * interested while we are sleeping in so that we'll
+ * be awakened when they arrive.
+ */
+ current->real_blocked = current->blocked;
+ sigandsets(¤t->blocked, ¤t->blocked, &these);
+ recalc_sigpending();
+ spin_unlock_irq(¤t->sighand->siglock);
- timeout = schedule_timeout_interruptible(timeout);
+ timeout = schedule_timeout_interruptible(timeout);
- spin_lock_irq(¤t->sighand->siglock);
- sig = dequeue_signal(current, &these, &info);
- current->blocked = current->real_blocked;
- siginitset(¤t->real_blocked, 0);
- recalc_sigpending();
- }
+ spin_lock_irq(¤t->sighand->siglock);
+ sig = dequeue_signal(current, &these, &info);
+ current->blocked = current->real_blocked;
+ siginitset(¤t->real_blocked, 0);
+ recalc_sigpending();
}
spin_unlock_irq(¤t->sighand->siglock);
--- sigprocmask/kernel/compat.c~1_sigtimedwait_to 2011-04-14 21:23:22.000000000 +0200
+++ sigprocmask/kernel/compat.c 2011-04-22 19:19:39.000000000 +0200
@@ -893,7 +893,7 @@ compat_sys_rt_sigtimedwait (compat_sigse
int sig;
struct timespec t;
siginfo_t info;
- long ret, timeout = 0;
+ long ret, timeout;
if (sigsetsize != sizeof(sigset_t))
return -EINVAL;
@@ -904,36 +904,30 @@ compat_sys_rt_sigtimedwait (compat_sigse
sigdelsetmask(&s,sigmask(SIGKILL)|sigmask(SIGSTOP));
signotset(&s);
+ timeout = MAX_SCHEDULE_TIMEOUT;
if (uts) {
if (get_compat_timespec (&t, uts))
return -EFAULT;
- if (t.tv_nsec >= 1000000000L || t.tv_nsec < 0
- || t.tv_sec < 0)
+ if (!timespec_valid(&t))
return -EINVAL;
+ timeout = timespec_to_jiffies(&t) + (t.tv_sec || t.tv_nsec);
}
spin_lock_irq(¤t->sighand->siglock);
sig = dequeue_signal(current, &s, &info);
- if (!sig) {
- timeout = MAX_SCHEDULE_TIMEOUT;
- if (uts)
- timeout = timespec_to_jiffies(&t)
- +(t.tv_sec || t.tv_nsec);
- if (timeout) {
- current->real_blocked = current->blocked;
- sigandsets(¤t->blocked, ¤t->blocked, &s);
-
- recalc_sigpending();
- spin_unlock_irq(¤t->sighand->siglock);
+ if (!sig && timeout) {
+ current->real_blocked = current->blocked;
+ sigandsets(¤t->blocked, ¤t->blocked, &s);
+ recalc_sigpending();
+ spin_unlock_irq(¤t->sighand->siglock);
- timeout = schedule_timeout_interruptible(timeout);
+ timeout = schedule_timeout_interruptible(timeout);
- spin_lock_irq(¤t->sighand->siglock);
- sig = dequeue_signal(current, &s, &info);
- current->blocked = current->real_blocked;
- siginitset(¤t->real_blocked, 0);
- recalc_sigpending();
- }
+ spin_lock_irq(¤t->sighand->siglock);
+ sig = dequeue_signal(current, &s, &info);
+ current->blocked = current->real_blocked;
+ siginitset(¤t->real_blocked, 0);
+ recalc_sigpending();
}
spin_unlock_irq(¤t->sighand->siglock);
@@ -943,7 +937,7 @@ compat_sys_rt_sigtimedwait (compat_sigse
if (copy_siginfo_to_user32(uinfo, &info))
ret = -EFAULT;
}
- }else {
+ } else {
ret = timeout?-EINTR:-EAGAIN;
}
return ret;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists