sparc boot failure due to perf init

sparc boot failure due to perf init

[Sam Ravnborg]

I have upgraded my Sun Blade 100 to latest mainline,
and when it booted I hit an oops.

Relevant bits from the oops:
Mount-cache hash table entries: 512
Performance events: perf (null)    <<<<<<<<<<<<<<<<<<<
Unable to handle kernel NULL pointer dereference
tsk->{mm,active_mm}->context = 0000000000000000
tsk->{mm,active_mm}->pgd = fffff800003e4634
              \|/ ____ \|/
              "@'/ .. \`@"
              /_| \__/ |_\
                 \__U_/
swapper(1): Oops [#1]
TSTATE: 0000004480001606 TPC: 000000000055c180 TNPC: 000000000055c184 Y: 00000000    Not tainted
TPC: <strcmp+0x0/0x40>
g0: fffff8000f03f800 g1: 00000000007df800 g2: 000000000078f918 g3: 0000000000799168
g4: fffff8000f0358c0 g5: fffff80000c22000 g6: fffff8000f038000 g7: 0000000000000001
o0: 0000000000000000 o1: 000000000073b050 o2: 0000000000000001 o3: 0000000000000001
o4: fffff8000f0356c8 o5: 0000000000000000 sp: fffff8000f03b531 ret_pc: 00000000007b9e30
RPC: <init_hw_perf_events+0x30/0x168>
l0: 0000000000000000 l1: 00000000007e0a80 l2: 00000000007e2140 l3: 0000000000000001
l4: 0000000000000000 l5: 0000000000000000 l6: 0000000000000000 l7: 0000000000777b78
i0: fffff80001402a80 i1: 0000000000000000 i2: 0000000000000001 i3: 0000000000000001
i4: 0000000000000000 i5: 0000000000000005 i6: fffff8000f03b5e1 i7: 00000000007b41c8
I7: <do_one_initcall+0x68/0x140>

The line mared "<<<<<<<<<<<<<" is added by my printf debugging.
It shows that sparc_pmu_type is NULL.

sparc_pmu_type is set in cpu_type_probe(void) which is:

   arch_initcall(cpu_type_probe);

But init_hw_perf_events() is:

   early_initcall(init_hw_perf_events);

So we end up calling init_hw_perf_events() before we call cpu_type_probe(),
and sparc_pmu_type is not set.

This was introduced by: 
"perf, arch: Cleanup perf-pmu init vs lockup-detector" - 004417a6d468e24399e383645c068b498eed84ad

I did not see any obvious fix.
(To avoid it is simple - I can disable it in my config).

	Sam

Re: sparc boot failure due to perf init

[Sam Ravnborg]

On Fri, Jan 07, 2011 at 09:18:05PM +0100, Sam Ravnborg wrote:
> I have upgraded my Sun Blade 100 to latest mainline,
> and when it booted I hit an oops.
> 
> Relevant bits from the oops:
> Mount-cache hash table entries: 512
> Performance events: perf (null)    <<<<<<<<<<<<<<<<<<<
> Unable to handle kernel NULL pointer dereference
> tsk->{mm,active_mm}->context = 0000000000000000
> tsk->{mm,active_mm}->pgd = fffff800003e4634
>               \|/ ____ \|/
>               "@'/ .. \`@"
>               /_| \__/ |_\
>                  \__U_/
> swapper(1): Oops [#1]
> TSTATE: 0000004480001606 TPC: 000000000055c180 TNPC: 000000000055c184 Y: 00000000    Not tainted
> TPC: <strcmp+0x0/0x40>
> g0: fffff8000f03f800 g1: 00000000007df800 g2: 000000000078f918 g3: 0000000000799168
> g4: fffff8000f0358c0 g5: fffff80000c22000 g6: fffff8000f038000 g7: 0000000000000001
> o0: 0000000000000000 o1: 000000000073b050 o2: 0000000000000001 o3: 0000000000000001
> o4: fffff8000f0356c8 o5: 0000000000000000 sp: fffff8000f03b531 ret_pc: 00000000007b9e30
> RPC: <init_hw_perf_events+0x30/0x168>
> l0: 0000000000000000 l1: 00000000007e0a80 l2: 00000000007e2140 l3: 0000000000000001
> l4: 0000000000000000 l5: 0000000000000000 l6: 0000000000000000 l7: 0000000000777b78
> i0: fffff80001402a80 i1: 0000000000000000 i2: 0000000000000001 i3: 0000000000000001
> i4: 0000000000000000 i5: 0000000000000005 i6: fffff8000f03b5e1 i7: 00000000007b41c8
> I7: <do_one_initcall+0x68/0x140>
> 
> The line mared "<<<<<<<<<<<<<" is added by my printf debugging.
> It shows that sparc_pmu_type is NULL.
> 
> sparc_pmu_type is set in cpu_type_probe(void) which is:
> 
>    arch_initcall(cpu_type_probe);
> 
> But init_hw_perf_events() is:
> 
>    early_initcall(init_hw_perf_events);
> 
> So we end up calling init_hw_perf_events() before we call cpu_type_probe(),
> and sparc_pmu_type is not set.
> 
> This was introduced by: 
> "perf, arch: Cleanup perf-pmu init vs lockup-detector" - 004417a6d468e24399e383645c068b498eed84ad
Note: I did not try a revert or anything.
I just looked at the patch and concluded this was the culprint.

	Sam

Re: sparc boot failure due to perf init

[Peter Zijlstra]

On Fri, 2011-01-07 at 21:18 +0100, Sam Ravnborg wrote:
> This was introduced by: 
> "perf, arch: Cleanup perf-pmu init vs lockup-detector" - 004417a6d468e24399e383645c068b498eed84ad
> 
> I did not see any obvious fix.
> (To avoid it is simple - I can disable it in my config).

Or you can switch back to arch_initcall() for now, it'll cause the
generic watchdog stuff not to work, but that's a lot less important.

I'll have a chat with Dave on how to best sort all this.

Re: sparc boot failure due to perf init

[Peter Zijlstra]

On Fri, 2011-01-07 at 21:23 +0100, Peter Zijlstra wrote:
> On Fri, 2011-01-07 at 21:18 +0100, Sam Ravnborg wrote:
> > This was introduced by: 
> > "perf, arch: Cleanup perf-pmu init vs lockup-detector" - 004417a6d468e24399e383645c068b498eed84ad
> > 
> > I did not see any obvious fix.
> > (To avoid it is simple - I can disable it in my config).
> 
> Or you can switch back to arch_initcall() for now, it'll cause the
> generic watchdog stuff not to work, but that's a lot less important.
> 
> I'll have a chat with Dave on how to best sort all this.

That is, simply revert the sparc bit for now.

Re: sparc boot failure due to perf init

[Sam Ravnborg]

On Fri, Jan 07, 2011 at 09:27:35PM +0100, Peter Zijlstra wrote:
> On Fri, 2011-01-07 at 21:23 +0100, Peter Zijlstra wrote:
> > On Fri, 2011-01-07 at 21:18 +0100, Sam Ravnborg wrote:
> > > This was introduced by: 
> > > "perf, arch: Cleanup perf-pmu init vs lockup-detector" - 004417a6d468e24399e383645c068b498eed84ad
> > > 
> > > I did not see any obvious fix.
> > > (To avoid it is simple - I can disable it in my config).
> > 
> > Or you can switch back to arch_initcall() for now, it'll cause the
> > generic watchdog stuff not to work, but that's a lot less important.
> > 
> > I'll have a chat with Dave on how to best sort all this.
> 
> That is, simply revert the sparc bit for now.

For now I just disabled the option as I am trying to
debug a console related problem and do not need the
perf_event support (or at least I do not know I need it :-) ).

Then you guys can sort it out and I can test any patches
you come up with.

	Sam

Re: sparc boot failure due to perf init

[David Miller]

From: Peter Zijlstra <a.p.zijlstra@chello.nl>
Date: Fri, 07 Jan 2011 21:23:52 +0100

> On Fri, 2011-01-07 at 21:18 +0100, Sam Ravnborg wrote:
>> This was introduced by: 
>> "perf, arch: Cleanup perf-pmu init vs lockup-detector" - 004417a6d468e24399e383645c068b498eed84ad
>> 
>> I did not see any obvious fix.
>> (To avoid it is simple - I can disable it in my config).
> 
> Or you can switch back to arch_initcall() for now, it'll cause the
> generic watchdog stuff not to work, but that's a lot less important.
> 
> I'll have a chat with Dave on how to best sort all this.

I'll take a look at this.

Thanks for bisecting this down Sam, I was seeing the same problem
on my machines and was about to debug it myself.

Re: sparc boot failure due to perf init

[David Miller]

From: Sam Ravnborg <sam@ravnborg.org>
Date: Fri, 7 Jan 2011 21:31:19 +0100

> Then you guys can sort it out and I can test any patches
> you come up with.

Here is the fix I will be pushing to Linus to fix this:

--------------------
sparc64: Fix bootup regression due to perf init ordering.

Commit 004417a6d468e24399e383645c068b498eed84ad
("perf, arch: Cleanup perf-pmu init vs lockup-detector")
move the perf events init to be an early_initcall.

But this won't work properly unless the dependencies for
this code initialize beforehand.

Fix it by making cpu_type_probe and pcr_arch_init be
an early_initcall as well.

Reported-by: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
 arch/sparc/kernel/cpu.c |    2 +-
 arch/sparc/kernel/pcr.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/sparc/kernel/cpu.c b/arch/sparc/kernel/cpu.c
index e447938..0dc714f 100644
--- a/arch/sparc/kernel/cpu.c
+++ b/arch/sparc/kernel/cpu.c
@@ -375,5 +375,5 @@ static int __init cpu_type_probe(void)
 	return 0;
 }
 
-arch_initcall(cpu_type_probe);
+early_initcall(cpu_type_probe);
 #endif
diff --git a/arch/sparc/kernel/pcr.c b/arch/sparc/kernel/pcr.c
index b87873c..ae96cf5 100644
--- a/arch/sparc/kernel/pcr.c
+++ b/arch/sparc/kernel/pcr.c
@@ -168,4 +168,4 @@ out_unregister:
 	return err;
 }
 
-arch_initcall(pcr_arch_init);
+early_initcall(pcr_arch_init);
-- 
1.7.3.4

Re: sparc boot failure due to perf init

[Peter Zijlstra]

On Sun, 2011-01-09 at 15:37 -0800, David Miller wrote:
> sparc64: Fix bootup regression due to perf init ordering.
> 
> Commit 004417a6d468e24399e383645c068b498eed84ad
> ("perf, arch: Cleanup perf-pmu init vs lockup-detector")
> move the perf events init to be an early_initcall.
> 
> But this won't work properly unless the dependencies for
> this code initialize beforehand.
> 
> Fix it by making cpu_type_probe and pcr_arch_init be
> an early_initcall as well.
> 
> Reported-by: Sam Ravnborg <sam@ravnborg.org>
> Signed-off-by: David S. Miller <davem@davemloft.net> 

Thanks for sorting that David! and sorry for causing the breakage to
begin with.

Re: sparc boot failure due to perf init

[Peter Zijlstra]

On Sun, 2011-01-09 at 15:37 -0800, David Miller wrote:
> sparc64: Fix bootup regression due to perf init ordering.
> 
> Commit 004417a6d468e24399e383645c068b498eed84ad
> ("perf, arch: Cleanup perf-pmu init vs lockup-detector")
> move the perf events init to be an early_initcall.
> 
> But this won't work properly unless the dependencies for
> this code initialize beforehand.
> 
> Fix it by making cpu_type_probe and pcr_arch_init be
> an early_initcall as well.
> 
> Reported-by: Sam Ravnborg <sam@ravnborg.org>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> ---
>  arch/sparc/kernel/cpu.c |    2 +-
>  arch/sparc/kernel/pcr.c |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/sparc/kernel/cpu.c b/arch/sparc/kernel/cpu.c
> index e447938..0dc714f 100644
> --- a/arch/sparc/kernel/cpu.c
> +++ b/arch/sparc/kernel/cpu.c
> @@ -375,5 +375,5 @@ static int __init cpu_type_probe(void)
>         return 0;
>  }
>  
> -arch_initcall(cpu_type_probe);
> +early_initcall(cpu_type_probe);
>  #endif
> diff --git a/arch/sparc/kernel/pcr.c b/arch/sparc/kernel/pcr.c
> index b87873c..ae96cf5 100644
> --- a/arch/sparc/kernel/pcr.c
> +++ b/arch/sparc/kernel/pcr.c
> @@ -168,4 +168,4 @@ out_unregister:
>         return err;
>  }
>  
> -arch_initcall(pcr_arch_init);
> +early_initcall(pcr_arch_init);


I just realized, doesn't this make bootability depend on link order?
Since both the pmu init and its dependencies are now early_initcall()
how do we guarantee pcr_arch_init() and cpu_type_probe() are in fact
called _before_ init_hw_perf_events()?

Re: sparc boot failure due to perf init

[Sam Ravnborg]

> >  }
> >  
> > -arch_initcall(pcr_arch_init);
> > +early_initcall(pcr_arch_init);
> 
> 
> I just realized, doesn't this make bootability depend on link order?
Yes.

> Since both the pmu init and its dependencies are now early_initcall()
> how do we guarantee pcr_arch_init() and cpu_type_probe() are in fact
> called _before_ init_hw_perf_events()?

Today this is achieved by perf_event.o being located last
in the Makefile in arch/sparc/kernel/

It deserves a comment..

	Sam

Re: sparc boot failure due to perf init

[Peter Zijlstra]

On Thu, 2011-01-13 at 13:25 +0100, Sam Ravnborg wrote:
> > >  }
> > >  
> > > -arch_initcall(pcr_arch_init);
> > > +early_initcall(pcr_arch_init);
> > 
> > 
> > I just realized, doesn't this make bootability depend on link order?
> Yes.
> 
> > Since both the pmu init and its dependencies are now early_initcall()
> > how do we guarantee pcr_arch_init() and cpu_type_probe() are in fact
> > called _before_ init_hw_perf_events()?
> 
> Today this is achieved by perf_event.o being located last
> in the Makefile in arch/sparc/kernel/
> 
> It deserves a comment..

Ah, right! and while make -j will compile concurrently, link order is
still maintained, and I guess until the linker decides to process its
input files in random order (or reverse order, or whatever) we're good.

Does something like GOLD which was supposed to be threaded or somesuch
still work as expected here?

Re: sparc boot failure due to perf init

[David Miller]

From: Peter Zijlstra <peterz@infradead.org>
Date: Thu, 13 Jan 2011 11:55:28 +0100

> I just realized, doesn't this make bootability depend on link order?
> Since both the pmu init and its dependencies are now early_initcall()
> how do we guarantee pcr_arch_init() and cpu_type_probe() are in fact
> called _before_ init_hw_perf_events()?

Yes, it does, and I made sure the ordering in the sparc Makefile
satisfied the constraints.