mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Chris Metcalf <cmetcalf@mellanox.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Gilad Ben Yossef <giladb@ezchip.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ingo Molnar <mingo@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Rik van Riel <riel@redhat.com>, Tejun Heo <tj@kernel.org>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
	Christoph Lameter <cl@linux.com>,
	Viresh Kumar <viresh.kumar@linaro.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	Andy Lutomirski <luto@amacapital.net>,
	Michal Hocko <mhocko@suse.com>, <linux-mm@kvack.org>,
	<linux-doc@vger.kernel.org>, <linux-api@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v12 04/13] task_isolation: add initial support
Date: Wed, 18 May 2016 12:34:22 -0400	[thread overview]
Message-ID: <8e8b24ec-abc6-e599-ad50-218e350213ce@mellanox.com> (raw)
In-Reply-To: <20160518133420.GG3193@twins.programming.kicks-ass.net>

On 5/18/2016 9:34 AM, Peter Zijlstra wrote:
> On Tue, Apr 05, 2016 at 01:38:33PM -0400, Chris Metcalf wrote:
>> diff --git a/kernel/signal.c b/kernel/signal.c
>> index aa9bf00749c1..53e4e62f2778 100644
>> --- a/kernel/signal.c
>> +++ b/kernel/signal.c
>> @@ -34,6 +34,7 @@
>>   #include <linux/compat.h>
>>   #include <linux/cn_proc.h>
>>   #include <linux/compiler.h>
>> +#include <linux/isolation.h>
>>   
>>   #define CREATE_TRACE_POINTS
>>   #include <trace/events/signal.h>
>> @@ -2213,6 +2214,9 @@ relock:
>>   		/* Trace actually delivered signals. */
>>   		trace_signal_deliver(signr, &ksig->info, ka);
>>   
>> +		/* Disable task isolation when delivering a signal. */
> Why !? Changelog is quiet on this.

There are really two reasons.

1. If the task is receiving a signal, it will know it's not isolated
    any more, so we don't need to worry about notifying it explicitly.
    This behavior is easy to document and allows the application to decide
    if the signal is unexpected and it should go straight to its error
    handling path (likely outcome, and in that case you want task isolation
    off anyway) or if it thinks it can plausibly re-enable isolation and
    return to where the signal interrupted you at (hard to imagine how this
    would ever make sense, but you could if you wanted to).

2. When we are delivering a signal we may already be holding the lock
    for the signal subsystem, and it gets hard to figure out whether it's
    safe to send another signal to the application as a "task isolation
    broken" notification.  For example, sending a signal to a task on
    another core involves doing an IPI to that core to kick it; the IPI
    normally is a generic point for notifying the remote core of broken
    task isolation and sending a signal - except that at the point where
    we would do that on the signal path we are already holding the lock,
    so we end up deadlocked.  We could no doubt work around that, but it
    seemed cleaner to decouple the existing signal mechanism from the
    signal delivery for task isolation.

I will add more discussion of the rationale to the commit message.

-- 
Chris Metcalf, Mellanox Technologies
http://www.mellanox.com

  reply	other threads:[~2016-05-18 16:34 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-05 17:38 [PATCH v12 00/13] support "task_isolation" mode Chris Metcalf
2016-04-05 17:38 ` [PATCH v12 01/13] vmstat: add quiet_vmstat_sync function Chris Metcalf
2016-04-05 17:38 ` [PATCH v12 02/13] vmstat: add vmstat_idle function Chris Metcalf
2016-04-05 17:38 ` [PATCH v12 03/13] lru_add_drain_all: factor out lru_add_drain_needed Chris Metcalf
2016-04-05 17:38 ` [PATCH v12 04/13] task_isolation: add initial support Chris Metcalf
2016-05-18 13:34   ` Peter Zijlstra
2016-05-18 16:34     ` Chris Metcalf [this message]
2016-05-18 17:09       ` Peter Zijlstra
2016-04-05 17:38 ` [PATCH v12 05/13] task_isolation: support CONFIG_TASK_ISOLATION_ALL Chris Metcalf
2016-05-18 13:35   ` Peter Zijlstra
2016-05-18 16:34     ` Chris Metcalf
2016-04-05 17:38 ` [PATCH v12 06/13] task_isolation: support PR_TASK_ISOLATION_STRICT mode Chris Metcalf
2016-05-18 13:44   ` Peter Zijlstra
2016-05-18 16:34     ` Chris Metcalf
2016-04-05 17:38 ` [PATCH v12 07/13] task_isolation: add debug boot flag Chris Metcalf
2016-05-18 13:56   ` Peter Zijlstra
2016-05-18 16:36     ` Chris Metcalf
     [not found]     ` <684587d7-3653-7570-215f-37d3e9e786bc@mellanox.com>
2016-05-18 17:06       ` Peter Zijlstra
2016-05-19 15:12         ` Chris Metcalf
     [not found]         ` <b22cf841-9555-fff3-435b-646c4fd95bdc@mellanox.com>
2016-05-19 17:54           ` Peter Zijlstra
2016-05-19 18:05             ` Chris Metcalf
2016-04-05 17:38 ` [PATCH v12 08/13] task_isolation: add PR_TASK_ISOLATION_ONE_SHOT flag Chris Metcalf
2016-04-05 17:38 ` [PATCH v12 09/13] arm, tile: turn off timer tick for oneshot_stopped state Chris Metcalf
2016-04-07 16:58   ` Daniel Lezcano
2016-04-05 17:38 ` [PATCH v12 10/13] arch/x86: enable task isolation functionality Chris Metcalf
2016-05-18 16:23   ` Peter Zijlstra
2016-05-18 16:35     ` Chris Metcalf
2016-05-18 17:10       ` Peter Zijlstra
2016-04-05 17:38 ` [PATCH v12 11/13] arch/tile: " Chris Metcalf
2016-04-05 17:38 ` [PATCH v12 12/13] arm64: factor work_pending state machine to C Chris Metcalf
2016-07-08 15:43   ` [PATCH]: " Chris Metcalf
2016-07-08 15:49     ` Will Deacon
2016-07-08 16:11       ` Chris Metcalf
2016-07-11 11:42       ` Will Deacon
2016-07-11 12:19         ` Mark Rutland
2016-07-29 18:16           ` Chris Metcalf
2016-08-09 10:21             ` Will Deacon
2016-04-05 17:38 ` [PATCH v12 13/13] arch/arm64: enable task isolation functionality Chris Metcalf
2016-05-12 18:26 ` [PATCH v12 00/13] support "task_isolation" mode Chris Metcalf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8e8b24ec-abc6-e599-ad50-218e350213ce@mellanox.com \
    --to=cmetcalf@mellanox.com \
    --cc=akpm@linux-foundation.org \
    --cc=catalin.marinas@arm.com \
    --cc=cl@linux.com \
    --cc=fweisbec@gmail.com \
    --cc=giladb@ezchip.com \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=luto@amacapital.net \
    --cc=mhocko@suse.com \
    --cc=mingo@kernel.org \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=peterz@infradead.org \
    --cc=riel@redhat.com \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=tj@kernel.org \
    --cc=viresh.kumar@linaro.org \
    --cc=will.deacon@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®