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

[Oleg Nesterov]

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

We could use set_current_blocked() 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>
Acked-by: Tejun Heo <tj@xxxxxxxxxx>
Reviewed-by: Matt Fleming <matt.fleming@xxxxxxxxxxxxxxx>
---

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

--- sigprocmask/kernel/signal.c~3_sigtimedwait_retarget	2011-04-26 19:53:42.000000000 +0200
+++ sigprocmask/kernel/signal.c	2011-04-26 19:53:55.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);
 }
 
@@ -2343,7 +2348,8 @@ int do_sigtimedwait(const sigset_t *whic
 		/*
 		 * None ready, temporarily unblock those we're interested
 		 * while we are sleeping in so that we'll be awakened when
-		 * they arrive.
+		 * they arrive. Unblocking is always fine, we can avoid
+		 * set_current_blocked().
 		 */
 		tsk->real_blocked = tsk->blocked;
 		sigandsets(&tsk->blocked, &tsk->blocked, &mask);
@@ -2353,10 +2359,9 @@ int do_sigtimedwait(const sigset_t *whic
 		timeout = schedule_timeout_interruptible(timeout);
 
 		spin_lock_irq(&tsk->sighand->siglock);
-		sig = dequeue_signal(tsk, &mask, info);
-		tsk->blocked = tsk->real_blocked;
+		__set_task_blocked(tsk, &tsk->real_blocked);
 		siginitset(&tsk->real_blocked, 0);
-		recalc_sigpending();
+		sig = dequeue_signal(tsk, &mask, 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/