Date: Fri, 22 Apr 2011 13:46:16 +0100
From: Matt Fleming <matt@console-pimps.org>
To: Oleg Nesterov <oleg@redhat.com>
Cc: Tejun Heo <tj@kernel.org>, Linus Torvalds <torvalds@linux-foundation.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        "Nikita V. Youshchenko" <nyoushchenko@mvista.com>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 5/7] signal: sigprocmask() should do
 retarget_shared_pending()
Message-ID: <20110422134616.7447e612@mfleming-mobl1.ger.corp.intel.com>
In-Reply-To: <20110418134557.GF15951@redhat.com>
References: <20110418134421.GA15951@redhat.com>
	<20110418134557.GF15951@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1717
Lines: 38

On Mon, 18 Apr 2011 15:45:57 +0200
Oleg Nesterov <oleg@redhat.com> wrote:

> In short, almost every changing of current->blocked is wrong, or at least
> can lead to the unexpected results.
> 
> For example. Two threads T1 and T2, T1 sleeps in sigtimedwait/pause/etc.
> kill(tgid, SIG) can pick T2 for TIF_SIGPENDING. If T2 calls sigprocmask()
> and blocks SIG before it notices the pending signal, nobody else can handle
> this pending shared signal.
> 
> I am not sure this is bug, but at least this looks strange imho. T1 should
> not sleep forever, there is a signal which should wake it up.
> 
> This patch moves the code which actually changes ->blocked into the new
> helper, set_current_blocked() and changes this code to call
> retarget_shared_pending() as exit_signals() does. We should only care about
> the signals we just blocked, we use "newset & ~current->blocked" as a mask.
> 
> We do not check !sigisemptyset(newblocked), retarget_shared_pending() is
> cheap unless mask & shared_pending.
> 
> Note: for this particular case we could simply change sigprocmask() to
> return -EINTR if signal_pending(), but then we should change other callers
> and, more importantly, if we need this fix then set_current_blocked() will
> have more callers and some of them can't restart. See the next patch as a
> random example.
> 
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>

This looks much simpler to me.

Reviewed-by: Matt Fleming <matt.fleming@linux.intel.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/