Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1441550 > unrolled thread

Re: [perf/x86] 8de4a00661: WARNING: CPU: 0 PID: 1 at kernel/locking/mutex-debug.c:80 debug_mutex_unlock+0x20c/0x2b3

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [perf/x86] 8de4a00661: WARNING: CPU: 0 PID: 1 at  kernel/locking/mutex-debug.c:80 debug_mutex_unlock+0x20c/0x2b3 Thomas Gleixner <tglx@linutronix.de> - 2016-07-12 18:50 +0200
    Re: [perf/x86] 8de4a00661: WARNING: CPU: 0 PID: 1 at  kernel/locking/mutex-debug.c:80 debug_mutex_unlock+0x20c/0x2b3 Thomas Gleixner <tglx@linutronix.de> - 2016-07-12 19:30 +0200
      Re: [perf/x86] 8de4a00661: WARNING: CPU: 0 PID: 1 at  kernel/locking/mutex-debug.c:80 debug_mutex_unlock+0x20c/0x2b3 Thomas Gleixner <tglx@linutronix.de> - 2016-07-12 22:10 +0200
        [tip:core/urgent] cpu/hotplug: Keep enough storage space if SMP=n  to avoid array out of bounds scribble tip-bot for Thomas Gleixner <tipbot@zytor.com> - 2016-07-13 10:10 +0200

#1441550 — Re: [perf/x86] 8de4a00661: WARNING: CPU: 0 PID: 1 at kernel/locking/mutex-debug.c:80 debug_mutex_unlock+0x20c/0x2b3

On Wed, 13 Jul 2016, kernel test robot wrote:
> [    1.863354] WARNING: CPU: 0 PID: 1 at kernel/locking/mutex-debug.c:80 debug_mutex_unlock+0x20c/0x2b3
> [    1.877193] DEBUG_LOCKS_WARN_ON(lock->owner != current)
> 
> [    1.979431]  [<ffffffff8167508c>] mutex_unlock+0x9/0xb
> [    1.979431]  [<ffffffff8167508c>] mutex_unlock+0x9/0xb
> [    1.990691]  [<ffffffff8105a67f>] cpuhp_store_callbacks+0x5a/0x63

I have a hard time to figure out how that can happen:

static void cpuhp_store_callbacks(enum cpuhp_state state,
                                  const char *name,
                                  int (*startup)(unsigned int cpu),
                                  int (*teardown)(unsigned int cpu))
{
        /* (Un)Install the callbacks for further cpu hotplug operations */
        struct cpuhp_step *sp;

        mutex_lock(&cpuhp_state_mutex);
 	sp = cpuhp_get_step(state);
	sp->startup = startup;
        sp->teardown = teardown;
        sp->name = name;
        mutex_unlock(&cpuhp_state_mutex);
}

Confused ....

[toc] | [next] | [standalone]

#1441612

On Tue, 12 Jul 2016, Thomas Gleixner wrote:
> On Wed, 13 Jul 2016, kernel test robot wrote:
> > [    1.863354] WARNING: CPU: 0 PID: 1 at kernel/locking/mutex-debug.c:80 debug_mutex_unlock+0x20c/0x2b3
> > [    1.877193] DEBUG_LOCKS_WARN_ON(lock->owner != current)
> > 
> > [    1.979431]  [<ffffffff8167508c>] mutex_unlock+0x9/0xb
> > [    1.979431]  [<ffffffff8167508c>] mutex_unlock+0x9/0xb
> > [    1.990691]  [<ffffffff8105a67f>] cpuhp_store_callbacks+0x5a/0x63
> 
> I have a hard time to figure out how that can happen:
> 
> static void cpuhp_store_callbacks(enum cpuhp_state state,
>                                   const char *name,
>                                   int (*startup)(unsigned int cpu),
>                                   int (*teardown)(unsigned int cpu))
> {
>         /* (Un)Install the callbacks for further cpu hotplug operations */
>         struct cpuhp_step *sp;
> 
>         mutex_lock(&cpuhp_state_mutex);
>  	sp = cpuhp_get_step(state);
> 	sp->startup = startup;
>         sp->teardown = teardown;
>         sp->name = name;
>         mutex_unlock(&cpuhp_state_mutex);
> }
> 
> Confused ....

And printing cpuhp_state_mutex.owner does not reduce the confusion level.

[    0.186490] WTF 1           (null)
>         mutex_lock(&cpuhp_state_mutex);
[    0.186848] WTF 2 ffff8800002a4000
>  	sp = cpuhp_get_step(state);
[    0.187174] WTF 3 ffff8800002a4000

and current is:

[    0.205749] CPU: 0 PID: 1 Comm: swapper 
[    0.207410] task: ffff8800002a4000

/me goes digging deeper

 

[toc] | [prev] | [next] | [standalone]

#1441700

On Tue, 12 Jul 2016, Thomas Gleixner wrote:
> /me goes digging deeper

and dons a brown paperbag....

8<----------------

Subject: cpuhotplug: Keep enough storage space if SMP=n
From: Thomas Gleixner <tglx@linutronix.de>

The cpuhp_bp_states array is cut short when CONFIG_SMP=n, but the dynamically
registered callbacks are stored nevertheless and happily scribble outside of
the array bounds.

We need to store them in case that the state is unregistered so we can invoke
the teardown function. That's independent of CONFIG_SMP. Make sure the array
is large enough.

Fixes: cff7d378d3fd "cpu/hotplug: Convert to a state machine for the control processor"
Reported-by: kernel test robot <xiaolong.ye@intel.com>
Cc: stable@vger.kernel.org
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
diff --git a/kernel/cpu.c b/kernel/cpu.c
index d948e44c471e..7b61887f7ccd 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -1201,6 +1201,8 @@ static struct cpuhp_step cpuhp_bp_states[] = {
 		.teardown		= takedown_cpu,
 		.cant_stop		= true,
 	},
+#else
+	[CPUHP_BRINGUP_CPU] = { },
 #endif
 };
 

[toc] | [prev] | [next] | [standalone]

#1442145 — [tip:core/urgent] cpu/hotplug: Keep enough storage space if SMP=n to avoid array out of bounds scribble

Commit-ID:  a7c734140aa36413944eef0f8c660e0e2256357d
Gitweb:     http://git.kernel.org/tip/a7c734140aa36413944eef0f8c660e0e2256357d
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Tue, 12 Jul 2016 21:59:23 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 13 Jul 2016 09:29:39 +0200

cpu/hotplug: Keep enough storage space if SMP=n to avoid array out of bounds scribble

Xiaolong Ye reported lock debug warnings triggered by the following commit:

  8de4a0066106 ("perf/x86: Convert the core to the hotplug state machine")

The bug is the following: the cpuhp_bp_states[] array is cut short when
CONFIG_SMP=n, but the dynamically registered callbacks are stored nevertheless
and happily scribble outside of the array bounds...

We need to store them in case that the state is unregistered so we can invoke
the teardown function. That's independent of CONFIG_SMP. Make sure the array
is large enough.

Reported-by: kernel test robot <xiaolong.ye@intel.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Adam Borowski <kilobyte@angband.pl>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Anna-Maria Gleixner <anna-maria@linutronix.de>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Stephane Eranian <eranian@google.com>
Cc: Vince Weaver <vincent.weaver@maine.edu>
Cc: lkp@01.org
Cc: stable@vger.kernel.org
Cc: tipbuild@zytor.com
Fixes: cff7d378d3fd "cpu/hotplug: Convert to a state machine for the control processor"
Link: http://lkml.kernel.org/r/alpine.DEB.2.11.1607122144560.4083@nanos
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/cpu.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/cpu.c b/kernel/cpu.c
index d948e44..7b61887 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -1201,6 +1201,8 @@ static struct cpuhp_step cpuhp_bp_states[] = {
 		.teardown		= takedown_cpu,
 		.cant_stop		= true,
 	},
+#else
+	[CPUHP_BRINGUP_CPU] = { },
 #endif
 };
 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web