All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chris Metcalf <cmetcalf@ezchip.com>
To: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Gilad Ben Yossef <giladb@ezchip.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ingo Molnar <mingo@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Rik van Riel <riel@redhat.com>, Tejun Heo <tj@kernel.org>,
	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>,
	<linux-doc@vger.kernel.org>, <linux-api@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v9 04/13] task_isolation: add initial support
Date: Fri, 29 Jan 2016 13:18:05 -0500	[thread overview]
Message-ID: <56ABACDD.5090500@ezchip.com> (raw)
In-Reply-To: <20160128002801.GA14313@lerouge>

On 01/27/2016 07:28 PM, Frederic Weisbecker wrote:
> On Tue, Jan 19, 2016 at 03:45:04PM -0500, Chris Metcalf wrote:
>> You asked what happens if nohz_full= is given as well, which is a very
>> good question.  Perhaps the right answer is to have an early_initcall
>> that suppresses task isolation on any cores that lost their nohz_full
>> or isolcpus status due to later boot command line arguments (and
>> generate a console warning, obviously).
> I'd rather imagine that the final nohz full cpumask is "nohz_full=" | "task_isolation="
> That's the easiest way to deal with and both nohz and task isolation can call
> a common initializer that takes care of the allocation and add the cpus to the mask.

I like it!

And by the same token, the final isolcpus cpumask is "isolcpus=" | 
"task_isolation="?
That seems like we'd want to do it to keep things parallel.

>>>> +bool _task_isolation_ready(void)
>>>> +{
>>>> +	WARN_ON_ONCE(!irqs_disabled());
>>>> +
>>>> +	/* If we need to drain the LRU cache, we're not ready. */
>>>> +	if (lru_add_drain_needed(smp_processor_id()))
>>>> +		return false;
>>>> +
>>>> +	/* If vmstats need updating, we're not ready. */
>>>> +	if (!vmstat_idle())
>>>> +		return false;
>>>> +
>>>> +	/* Request rescheduling unless we are in full dynticks mode. */
>>>> +	if (!tick_nohz_tick_stopped()) {
>>>> +		set_tsk_need_resched(current);
>>> I'm not sure doing this will help getting the tick to get stopped.
>> Well, I don't know that there is anything else we CAN do, right?  If there's
>> another task that can run, great - it may be that that's why full dynticks
>> isn't happening yet.  Or, it might be that we're waiting for an RCU tick and
>> there's nothing else we can do, in which case we basically spend our time
>> going around through the scheduler code and back out to the
>> task_isolation_ready() test, but again, there's really nothing else more
>> useful we can be doing at this point.  Once the RCU tick fires (or whatever
>> it was that was preventing full dynticks from engaging), we will pass this
>> test and return to user space.
> There is nothing at all you can do and setting TIF_RESCHED won't help either.
> If there is another task that can run, the scheduler takes care of resched
> by itself :-)

The problem is that the scheduler will only take care of resched at a
later time, typically when we get a timer interrupt later.  By invoking the
scheduler here, we allow any tasks that are ready to run to run
immediately, rather than waiting for an interrupt to wake the scheduler.
Plenty of places in the kernel just call schedule() directly when they are
waiting.  Since we're waiting here regardless, we might as well
immediately get any other runnable tasks dealt with.

We could also just return "false" in _task_isolation_ready(), and then
check tick_nohz_tick_stopped() in _task_isolation_enter() and if false,
call schedule() explicitly there, but that seems a little more roundabout.
Admittedly it's more usual to see kernel code call schedule() directly
to yield the processor, but in this case I'm not convinced it's cleaner
given we're already in a loop where the caller is checking TIF_RESCHED
and then calling schedule() when it's set.

-- 
Chris Metcalf, EZChip Semiconductor
http://www.ezchip.com

WARNING: multiple messages have this Message-ID (diff)
From: Chris Metcalf <cmetcalf@ezchip.com>
To: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Gilad Ben Yossef <giladb@ezchip.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ingo Molnar <mingo@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Rik van Riel <riel@redhat.com>, Tejun Heo <tj@kernel.org>,
	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>,
	linux-doc@vger.kernel.org, linux-api@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v9 04/13] task_isolation: add initial support
Date: Fri, 29 Jan 2016 13:18:05 -0500	[thread overview]
Message-ID: <56ABACDD.5090500@ezchip.com> (raw)
In-Reply-To: <20160128002801.GA14313@lerouge>

On 01/27/2016 07:28 PM, Frederic Weisbecker wrote:
> On Tue, Jan 19, 2016 at 03:45:04PM -0500, Chris Metcalf wrote:
>> You asked what happens if nohz_full= is given as well, which is a very
>> good question.  Perhaps the right answer is to have an early_initcall
>> that suppresses task isolation on any cores that lost their nohz_full
>> or isolcpus status due to later boot command line arguments (and
>> generate a console warning, obviously).
> I'd rather imagine that the final nohz full cpumask is "nohz_full=" | "task_isolation="
> That's the easiest way to deal with and both nohz and task isolation can call
> a common initializer that takes care of the allocation and add the cpus to the mask.

I like it!

And by the same token, the final isolcpus cpumask is "isolcpus=" | 
"task_isolation="?
That seems like we'd want to do it to keep things parallel.

>>>> +bool _task_isolation_ready(void)
>>>> +{
>>>> +	WARN_ON_ONCE(!irqs_disabled());
>>>> +
>>>> +	/* If we need to drain the LRU cache, we're not ready. */
>>>> +	if (lru_add_drain_needed(smp_processor_id()))
>>>> +		return false;
>>>> +
>>>> +	/* If vmstats need updating, we're not ready. */
>>>> +	if (!vmstat_idle())
>>>> +		return false;
>>>> +
>>>> +	/* Request rescheduling unless we are in full dynticks mode. */
>>>> +	if (!tick_nohz_tick_stopped()) {
>>>> +		set_tsk_need_resched(current);
>>> I'm not sure doing this will help getting the tick to get stopped.
>> Well, I don't know that there is anything else we CAN do, right?  If there's
>> another task that can run, great - it may be that that's why full dynticks
>> isn't happening yet.  Or, it might be that we're waiting for an RCU tick and
>> there's nothing else we can do, in which case we basically spend our time
>> going around through the scheduler code and back out to the
>> task_isolation_ready() test, but again, there's really nothing else more
>> useful we can be doing at this point.  Once the RCU tick fires (or whatever
>> it was that was preventing full dynticks from engaging), we will pass this
>> test and return to user space.
> There is nothing at all you can do and setting TIF_RESCHED won't help either.
> If there is another task that can run, the scheduler takes care of resched
> by itself :-)

The problem is that the scheduler will only take care of resched at a
later time, typically when we get a timer interrupt later.  By invoking the
scheduler here, we allow any tasks that are ready to run to run
immediately, rather than waiting for an interrupt to wake the scheduler.
Plenty of places in the kernel just call schedule() directly when they are
waiting.  Since we're waiting here regardless, we might as well
immediately get any other runnable tasks dealt with.

We could also just return "false" in _task_isolation_ready(), and then
check tick_nohz_tick_stopped() in _task_isolation_enter() and if false,
call schedule() explicitly there, but that seems a little more roundabout.
Admittedly it's more usual to see kernel code call schedule() directly
to yield the processor, but in this case I'm not convinced it's cleaner
given we're already in a loop where the caller is checking TIF_RESCHED
and then calling schedule() when it's set.

-- 
Chris Metcalf, EZChip Semiconductor
http://www.ezchip.com

  reply	other threads:[~2016-01-29 18:18 UTC|newest]

Thread overview: 92+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-04 19:34 [PATCH v9 00/13] support "task_isolation" mode for nohz_full Chris Metcalf
2016-01-04 19:34 ` Chris Metcalf
2016-01-04 19:34 ` [PATCH v9 01/13] vmstat: provide a function to quiet down the diff processing Chris Metcalf
2016-01-04 19:34 ` [PATCH v9 02/13] vmstat: add vmstat_idle function Chris Metcalf
2016-01-04 19:34 ` [PATCH v9 03/13] lru_add_drain_all: factor out lru_add_drain_needed Chris Metcalf
2016-01-04 19:34   ` Chris Metcalf
2016-01-04 19:34 ` [PATCH v9 04/13] task_isolation: add initial support Chris Metcalf
2016-01-04 19:34   ` Chris Metcalf
2016-01-19 15:42   ` Frederic Weisbecker
2016-01-19 20:45     ` Chris Metcalf
2016-01-19 20:45       ` Chris Metcalf
2016-01-28  0:28       ` Frederic Weisbecker
2016-01-29 18:18         ` Chris Metcalf [this message]
2016-01-29 18:18           ` Chris Metcalf
2016-01-30 21:11           ` Frederic Weisbecker
2016-01-30 21:11             ` Frederic Weisbecker
2016-02-11 19:24             ` Chris Metcalf
2016-02-11 19:24               ` Chris Metcalf
2016-03-04 12:56               ` Frederic Weisbecker
2016-03-09 19:39                 ` Chris Metcalf
2016-03-09 19:39                   ` Chris Metcalf
2016-04-08 13:56                   ` Frederic Weisbecker
2016-04-08 16:34                     ` Chris Metcalf
2016-04-08 16:34                       ` Chris Metcalf
2016-04-12 18:41                       ` Chris Metcalf
2016-04-12 18:41                         ` Chris Metcalf
2016-04-22 13:16                       ` Frederic Weisbecker
2016-04-25 20:36                         ` Chris Metcalf
2016-04-25 20:36                           ` Chris Metcalf
2016-05-26  1:07                       ` Frederic Weisbecker
2016-06-03 19:32                         ` Chris Metcalf
2016-06-03 19:32                           ` Chris Metcalf
2016-06-29 15:18                           ` Frederic Weisbecker
2016-07-01 20:59                             ` Chris Metcalf
2016-07-01 20:59                               ` Chris Metcalf
2016-07-05 14:41                               ` Frederic Weisbecker
2016-07-05 14:41                                 ` Frederic Weisbecker
2016-07-05 17:47                                 ` Christoph Lameter
2016-01-04 19:34 ` [PATCH v9 05/13] task_isolation: support PR_TASK_ISOLATION_STRICT mode Chris Metcalf
2016-01-04 19:34   ` Chris Metcalf
2016-01-04 19:34 ` [PATCH v9 06/13] task_isolation: add debug boot flag Chris Metcalf
2016-01-04 22:52   ` Steven Rostedt
2016-01-04 23:42     ` Chris Metcalf
2016-01-05 13:42       ` Steven Rostedt
2016-01-04 19:34 ` [PATCH v9 07/13] arch/x86: enable task isolation functionality Chris Metcalf
2016-01-04 21:02   ` [PATCH v9bis " Chris Metcalf
2016-01-04 19:34 ` [PATCH v9 08/13] arch/arm64: adopt prepare_exit_to_usermode() model from x86 Chris Metcalf
2016-01-04 19:34   ` Chris Metcalf
2016-01-04 20:33   ` Mark Rutland
2016-01-04 20:33     ` Mark Rutland
2016-01-04 21:01     ` Chris Metcalf
2016-01-04 21:01       ` Chris Metcalf
2016-01-05 17:21       ` Mark Rutland
2016-01-05 17:21         ` Mark Rutland
2016-01-05 17:33         ` [PATCH 1/2] arm64: entry: remove pointless SPSR mode check Mark Rutland
2016-01-05 17:33           ` Mark Rutland
2016-01-06 12:15           ` Catalin Marinas
2016-01-06 12:15             ` Catalin Marinas
2016-01-05 17:33         ` [PATCH 2/2] arm64: factor work_pending state machine to C Mark Rutland
2016-01-05 17:33           ` Mark Rutland
2016-01-05 18:53           ` Chris Metcalf
2016-01-05 18:53             ` Chris Metcalf
2016-01-06 12:30           ` Catalin Marinas
2016-01-06 12:30             ` Catalin Marinas
2016-01-06 12:47             ` Mark Rutland
2016-01-06 12:47               ` Mark Rutland
2016-01-06 13:43           ` Mark Rutland
2016-01-06 13:43             ` Mark Rutland
2016-01-06 14:17             ` Catalin Marinas
2016-01-06 14:17               ` Catalin Marinas
2016-01-04 22:31     ` [PATCH v9 08/13] arch/arm64: adopt prepare_exit_to_usermode() model from x86 Andy Lutomirski
2016-01-04 22:31       ` Andy Lutomirski
2016-01-05 18:01       ` Mark Rutland
2016-01-05 18:01         ` Mark Rutland
2016-01-04 19:34 ` [PATCH v9 09/13] arch/arm64: enable task isolation functionality Chris Metcalf
2016-01-04 19:34   ` Chris Metcalf
2016-01-04 19:34 ` [PATCH v9 10/13] arch/tile: adopt prepare_exit_to_usermode() model from x86 Chris Metcalf
2016-01-04 19:34 ` [PATCH v9 11/13] arch/tile: move user_exit() to early kernel entry sequence Chris Metcalf
2016-01-04 19:34 ` [PATCH v9 12/13] arch/tile: enable task isolation functionality Chris Metcalf
2016-01-04 19:34 ` [PATCH v9 13/13] arm, tile: turn off timer tick for oneshot_stopped state Chris Metcalf
2016-01-11 21:15 ` [PATCH v9 00/13] support "task_isolation" mode for nohz_full Chris Metcalf
2016-01-11 21:15   ` Chris Metcalf
2016-01-12 10:07   ` Will Deacon
2016-01-12 17:49     ` Chris Metcalf
2016-01-12 17:49       ` Chris Metcalf
2016-01-13 10:44       ` Ingo Molnar
2016-01-13 10:44         ` Ingo Molnar
2016-01-13 21:19         ` Chris Metcalf
2016-01-13 21:19           ` Chris Metcalf
2016-01-20 13:27           ` Mark Rutland
2016-01-12 10:53   ` Ingo Molnar
2016-01-12 10:53     ` Ingo Molnar

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=56ABACDD.5090500@ezchip.com \
    --to=cmetcalf@ezchip.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=luto@amacapital.net \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.