[PATCH 3/3] signal: do_sigtimedwait() needsretarget_shared_pending()

[Oleg Nesterov]

do_sigtimedwait() changes current->blocked and thus it needs
set_current_bloked()->retarget_shared_pending().

We could use set_current_bloked() directly. It is fine to change
->real_blocked from all-zeroes to ->blocked and vice versa lockless,
but this is not immediately clear, looks racy, and needs a huge
comment to explain why this is correct.

To keep the things simple this patch adds the new static helper,
__set_task_blocked() which should be called with ->siglock held. This
way we can change both ->real_blocked and ->blocked atomically under
->siglock as the current code does. This is more understandable.

Signed-off-by: Oleg Nesterov <oleg@xxxxxxxxxx>
---

 kernel/signal.c |   21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

--- sigprocmask/kernel/signal.c~3_sigtimedwait_retarget	2011-04-22 20:57:06.000000000 +0200
+++ sigprocmask/kernel/signal.c	2011-04-22 21:49:11.000000000 +0200
@@ -2115,11 +2115,8 @@ long do_no_restart_syscall(struct restar
 	return -EINTR;
 }
 
-void set_current_blocked(const sigset_t *newset)
+static void __set_task_blocked(struct task_struct *tsk, const sigset_t *newset)
 {
-	struct task_struct *tsk = current;
-
-	spin_lock_irq(&tsk->sighand->siglock);
 	if (signal_pending(tsk) && !thread_group_empty(tsk)) {
 		sigset_t newblocked;
 		signandsets(&newblocked, newset, &current->blocked);
@@ -2127,6 +2124,14 @@ void set_current_blocked(const sigset_t 
 	}
 	tsk->blocked = *newset;
 	recalc_sigpending();
+}
+
+void set_current_blocked(const sigset_t *newset)
+{
+	struct task_struct *tsk = current;
+
+	spin_lock_irq(&tsk->sighand->siglock);
+	__set_task_blocked(tsk, newset);
 	spin_unlock_irq(&tsk->sighand->siglock);
 }
 
@@ -2322,7 +2327,8 @@ int do_sigtimedwait(sigset_t *these, sig
 		/*
 		 * None ready -- temporarily unblock those we're
 		 * interested while we are sleeping in so that we'll
-		 * be awakened when they arrive.
+		 * be awakened when they arrive. Unblocking is always
+		 * fine, we can avoid set_current_blocked().
 		 */
 		tsk->real_blocked = tsk->blocked;
 		sigandsets(&tsk->blocked, &tsk->blocked, these);
@@ -2332,10 +2338,9 @@ int do_sigtimedwait(sigset_t *these, sig
 		timeout = schedule_timeout_interruptible(timeout);
 
 		spin_lock_irq(&tsk->sighand->siglock);
-		sig = dequeue_signal(tsk, these, info);
-		tsk->blocked = tsk->real_blocked;
+		__set_task_blocked(tsk, &tsk->real_blocked);
 		siginitset(&tsk->real_blocked, 0);
-		recalc_sigpending();
+		sig = dequeue_signal(tsk, these, info);
 	}
 	spin_unlock_irq(&tsk->sighand->siglock);
 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/