All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chris Metcalf <cmetcalf@mellanox.com>
To: Frederic Weisbecker <fweisbec@gmail.com>,
	Christoph Lameter <cl@linux.com>, Ingo Molnar <mingo@kernel.org>,
	Luiz Capitulino <lcapitulino@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Rik van Riel <riel@redhat.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Viresh Kumar <viresh.kumar@linaro.org>,
	<linux-kernel@vger.kernel.org>
Cc: Chris Metcalf <cmetcalf@mellanox.com>
Subject: [PATCH] nohz_full: Make sched_should_stop_tick() more conservative
Date: Fri, 1 Apr 2016 15:42:51 -0400	[thread overview]
Message-ID: <1459539771-4251-1-git-send-email-cmetcalf@mellanox.com> (raw)

On arm64, when calling enqueue_task_fair() from migration_cpu_stop(),
we find the nr_running value updated by add_nr_running(), but the
cfs.nr_running value has not always yet been updated.  Accordingly,
the sched_can_stop_tick() false returns true when we are migrating a
second task onto a core.

Correct this by using rq->nr_running instead of rq->cfs.nr_running.
This should always be more conservative, and reverts the test to the
form it had before commit 76d92ac305f2 ("sched: Migrate sched to use
new tick dependency mask model").

Signed-off-by: Chris Metcalf <cmetcalf@mellanox.com>
---
I found this bug because I had a program running in nohz_full
on a core, and from a different core I called sched_setaffinity()
to force that task onto the nohz_full core, but I did not end up with
a kick to the nohz_full core, so tick-based scheduling did not start.
This is probably bad enough that we should fix it for 4.6.

Strangely, for some reason, the existing code worked correctly for me
for tilegx, but not for arm64.  I see that the enqueue_task_fair()
code calls enqueue_entity(), which calls account_entity_enqueue() to
adjust cfs.nr_running.  That seemed to happen on tilegx, but not arm64.
Perhaps there is some difference in how the sched_entity stuff is done,
but frankly that took me a little deeper into the CFS stuff than I was
willing to dive in this moment.

I could also argue that sched/core.c shouldn't have a lot of CFS
stuff in it anyway, and if we view the FIFO/RR stuff as handling the
real special cases in sched_can_stop_tick() anyway, then just checking
the core nr_running feels like the right thing to do regardless.

 kernel/sched/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 00649f7ad567..1737d63c65fa 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -599,7 +599,7 @@ bool sched_can_stop_tick(struct rq *rq)
 	}
 
 	/* Normal multitasking need periodic preemption checks */
-	if (rq->cfs.nr_running > 1)
+	if (rq->nr_running > 1)
 		return false;
 
 	return true;
-- 
2.7.2

             reply	other threads:[~2016-04-01 19:43 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-01 19:42 Chris Metcalf [this message]
2016-04-04 19:12 ` [PATCH] nohz_full: Make sched_should_stop_tick() more conservative Rik van Riel
2016-04-04 19:23   ` Peter Zijlstra
2016-04-18  2:00     ` Wanpeng Li
2016-04-21 14:42       ` Peter Zijlstra
2016-04-21 16:03         ` Peter Zijlstra
2016-04-25 21:30           ` Chris Metcalf
2016-04-28 10:24           ` [tip:sched/urgent] nohz/full, sched/rt: Fix missed tick-reenabling bug in sched_can_stop_tick() tip-bot for Peter Zijlstra
2016-04-28 13:30             ` Frederic Weisbecker
2016-04-04 19:31   ` [PATCH] nohz_full: Make sched_should_stop_tick() more conservative Chris Metcalf
2016-04-04 19:36     ` Rik van Riel
2016-04-05  0:27       ` 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=1459539771-4251-1-git-send-email-cmetcalf@mellanox.com \
    --to=cmetcalf@mellanox.com \
    --cc=cl@linux.com \
    --cc=fweisbec@gmail.com \
    --cc=lcapitulino@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=riel@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=viresh.kumar@linaro.org \
    /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.