public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Kevin Hilman <khilman@deeprootsystems.com>
To: Frederic Weisbecker <fweisbec@gmail.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Alessio Igor Bogani <abogani@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Chris Metcalf <cmetcalf@tilera.com>,
	Christoph Lameter <cl@linux.com>,
	Geoff Levand <geoff@infradead.org>,
	Gilad Ben Yossef <gilad@benyossef.com>,
	Hakan Akkan <hakanakkan@gmail.com>,
	Ingo Molnar <mingo@kernel.org>,
	Li Zhong <zhong@linux.vnet.ibm.com>,
	Namhyung Kim <namhyung.kim@lge.com>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
	Paul Gortmaker <paul.gortmaker@windriver.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [PATCH 30/33] sched: Debug nohz rq clock
Date: Wed, 20 Mar 2013 16:23:34 -0700	[thread overview]
Message-ID: <514A44F6.8010203@deeprootsystems.com> (raw)
In-Reply-To: <1357610913-1080-31-git-send-email-fweisbec@gmail.com>

Hi Frederic,

On 01/07/2013 06:08 PM, Frederic Weisbecker wrote:
> The runqueue clock is supposed to be periodically updated by the
> tick. On full dynticks CPU we call update_nohz_rq_clock() before
> reading it. Now the scheduler code is complicated enough that we
> may miss some update_nohz_rq_clock() calls before reading the
> runqueue clock.
> 
> This therefore introduce a new debugging feature that detects
> when the rq clock is stale due to missing updates on full
> dynticks CPUs.
> 
> This can be later expanded to debug stale clocks on dynticks-idle
> CPUs.

[...]

> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index e1bac76..0fef0b3 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -502,16 +502,39 @@ DECLARE_PER_CPU(struct rq, runqueues);
>  #define cpu_curr(cpu)		(cpu_rq(cpu)->curr)
>  #define raw_rq()		(&__raw_get_cpu_var(runqueues))
>  
> +static inline void rq_clock_check(struct rq *rq)
> +{
> +#if defined(CONFIG_SCHED_DEBUG) && defined(CONFIG_NO_HZ_FULL)
> +	unsigned long long clock;
> +	unsigned long flags;
> +	int cpu;
> +
> +	cpu = cpu_of(rq);
> +	if (!tick_nohz_full_cpu(cpu) || rq->curr == rq->idle)
> +		return;
> +
> +	local_irq_save(flags);
> +	clock = sched_clock_cpu(cpu_of(rq));
> +	local_irq_restore(flags);
> +
> +	if (abs(clock - rq->clock) > (TICK_NSEC * 3))
> +		WARN_ON_ONCE(1);
> +#endif
> +}

In working on the ARM port for full nohz, I'm hitting this
warning early in the kernel boot, well before userspace starts
(dump below[2].)

I've seen a few different variations of this, but the common
thing for all of them is the use of wait_for_completion().

During boot, only swapper is running so it seems
any waiting of sufficient length during boot will always trigger
this warning.  The hack below[1] avoids checking for the init task,
but I'm not sure if it's the right fix.

Kevin

[1]
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index f96329b..56e74df 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -512,7 +512,8 @@ static inline void rq_clock_check(struct rq *rq)
 	int cpu;

 	cpu = cpu_of(rq);
-	if (!tick_nohz_full_cpu(cpu) || rq->curr == rq->idle)
+	if (!tick_nohz_full_cpu(cpu) || rq->curr == rq->idle ||
+	    is_global_init(current))
 		return;


[2] Example warning dump
[    1.101013] ------------[ cut here ]------------
[    1.105865] WARNING: at
/work/kernel/linaro/dev/kernel/sched/sched.h:534 __schedule+0x510/0x790()
[    1.115051] Modules linked in:
[    1.118316] [<c00159d0>] (unwind_backtrace+0x0/0x104) from
[<c054dc98>] (dump_stack+0x20/0x24)
[    1.127227] [<c054dc98>] (dump_stack+0x20/0x24) from [<c0037f58>]
(warn_slowpath_common+0x5c/0x78)
[    1.136535] [<c0037f58>] (warn_slowpath_common+0x5c/0x78) from
[<c0037fa0>] (warn_slowpath_null+0x2c/0x34)
[    1.146545] [<c0037fa0>] (warn_slowpath_null+0x2c/0x34) from
[<c0560834>] (__schedule+0x510/0x790)
[    1.155822] [<c0560834>] (__schedule+0x510/0x790) from [<c0560ba8>]
(schedule+0x40/0x80)
[    1.164215] [<c0560ba8>] (schedule+0x40/0x80) from [<c055e288>]
(schedule_timeout+0x180/0x250)
[    1.173156] [<c055e288>] (schedule_timeout+0x180/0x250) from
[<c05601a4>] (wait_for_common+0xb8/0x15c)
[    1.182800] [<c05601a4>] (wait_for_common+0xb8/0x15c) from
[<c0560268>] (wait_for_completion+0x20/0x24)
[    1.192535] [<c0560268>] (wait_for_completion+0x20/0x24) from
[<c005ce4c>] (kthread_create_on_node+0x84/0xe8)
[    1.202819] [<c005ce4c>] (kthread_create_on_node+0x84/0xe8) from
[<c005732c>] (__alloc_workqueue_key+0x36c/0x440)
[    1.213470] [<c005732c>] (__alloc_workqueue_key+0x36c/0x440) from
[<c052d380>] (rpc_init_mempool+0x44/0x118)
[    1.223663] [<c052d380>] (rpc_init_mempool+0x44/0x118) from
[<c07f1714>] (init_sunrpc+0x10/0x6c)
[    1.232757] [<c07f1714>] (init_sunrpc+0x10/0x6c) from [<c00087f0>]
(do_one_initcall+0x48/0x190)
[    1.241790] [<c00087f0>] (do_one_initcall+0x48/0x190) from
[<c07c3988>] (do_basic_setup+0x9c/0xd0)
[    1.251068] [<c07c3988>] (do_basic_setup+0x9c/0xd0) from [<c07c3a88>]
(kernel_init_freeable+0xcc/0x16c)
[    1.260833] [<c07c3a88>] (kernel_init_freeable+0xcc/0x16c) from
[<c0548840>] (kernel_init+0x1c/0xf4)
[    1.270294] [<c0548840>] (kernel_init+0x1c/0xf4) from [<c000e390>]
(ret_from_fork+0x14/0x20)
[
> +
>  static inline u64 rq_clock(struct rq *rq)
>  {
> +	rq_clock_check(rq);
>  	return rq->clock;
>  }
>  
>  static inline u64 rq_clock_task(struct rq *rq)
>  {
> +	rq_clock_check(rq);
>  	return rq->clock_task;
>  }
>  
> +
>  #ifdef CONFIG_SMP
>  
>  #define rcu_dereference_check_sched_domain(p) \
> 

  reply	other threads:[~2013-03-20 23:23 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-08  2:08 [ANNOUNCE] 3.8-rc2-nohz2 Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 01/33] context_tracking: Add comments on interface and internals Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 02/33] context_tracking: Export context state for generic vtime Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 03/33] cputime: Generic on-demand virtual cputime accounting Frederic Weisbecker
2013-01-08 20:23   ` Steven Rostedt
2013-01-08 20:26   ` Steven Rostedt
2013-01-08 21:00     ` Paul E. McKenney
2013-01-08 20:45   ` Steven Rostedt
2013-01-09 13:46   ` Steven Rostedt
2013-01-09 13:50     ` Steven Rostedt
2013-01-08  2:08 ` [PATCH 04/33] cputime: Allow dynamic switch between tick/virtual based " Frederic Weisbecker
2013-01-08 21:20   ` Steven Rostedt
2013-01-08 23:22     ` Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 05/33] cputime: Use accessors to read task cputime stats Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 06/33] cputime: Safely read cputime of full dynticks CPUs Frederic Weisbecker
2013-01-09 14:54   ` Steven Rostedt
2013-01-09 18:35     ` Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 07/33] nohz: Basic full dynticks interface Frederic Weisbecker
2013-02-11 14:35   ` Borislav Petkov
2013-02-20 16:32     ` Borislav Petkov
2013-03-07 23:41       ` Frederic Weisbecker
2013-03-07 23:35     ` Frederic Weisbecker
2013-03-08 10:17       ` Borislav Petkov
2013-03-08 13:45         ` Frederic Weisbecker
2013-03-08 14:32           ` Borislav Petkov
2013-03-08 16:55             ` Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 08/33] nohz: Assign timekeeping duty to a non-full-nohz CPU Frederic Weisbecker
2013-02-15 11:57   ` Borislav Petkov
2013-02-20 15:57     ` Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 09/33] nohz: Trace timekeeping update Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 10/33] nohz: Wake up full dynticks CPUs when a timer gets enqueued Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 11/33] rcu: Restart the tick on non-responding full dynticks CPUs Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 12/33] sched: Comment on rq->clock correctness in ttwu_do_wakeup() in nohz Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 13/33] sched: Update rq clock on nohz CPU before migrating tasks Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 14/33] sched: Update rq clock on nohz CPU before setting fair group shares Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 15/33] sched: Update rq clock on tickless CPUs before calling check_preempt_curr() Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 16/33] sched: Update rq clock earlier in unthrottle_cfs_rq Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 17/33] sched: Update clock of nohz busiest rq before balancing Frederic Weisbecker
2013-01-08 10:20   ` Li Zhong
2013-03-07 23:51     ` Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 18/33] sched: Update rq clock before idle balancing Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 19/33] sched: Update nohz rq clock before searching busiest group on load balancing Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 20/33] nohz: Move nohz load balancer selection into idle logic Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 21/33] nohz: Full dynticks mode Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 22/33] nohz: Only stop the tick on RCU nocb CPUs Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 23/33] nohz: Don't turn off the tick if rcu needs it Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 24/33] nohz: Don't stop the tick if posix cpu timers are running Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 25/33] nohz: Add some tracing Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 26/33] rcu: Don't keep the tick for RCU while in userspace Frederic Weisbecker
2013-01-08  4:06   ` Paul E. McKenney
2013-01-08  2:08 ` [PATCH 27/33] profiling: Remove unused timer hook Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 28/33] timer: Don't run non-pinned timer to full dynticks CPUs Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 29/33] sched: Use an accessor to read rq clock Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 30/33] sched: Debug nohz " Frederic Weisbecker
2013-03-20 23:23   ` Kevin Hilman [this message]
2013-04-11 16:47     ` Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 31/33] sched: Remove broken check for skip clock update Frederic Weisbecker
2013-01-08  2:11   ` Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 32/33] sched: Update rq clock before rt sched average scale Frederic Weisbecker
2013-01-08  2:08 ` [PATCH 33/33] sched: Disable lb_bias feature for full dynticks Frederic Weisbecker

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=514A44F6.8010203@deeprootsystems.com \
    --to=khilman@deeprootsystems.com \
    --cc=abogani@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=cl@linux.com \
    --cc=cmetcalf@tilera.com \
    --cc=fweisbec@gmail.com \
    --cc=geoff@infradead.org \
    --cc=gilad@benyossef.com \
    --cc=hakanakkan@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=namhyung.kim@lge.com \
    --cc=paul.gortmaker@windriver.com \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=zhong@linux.vnet.ibm.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