Patchwork sparc boot failure due to perf init

login
register
mail settings
Submitter David Miller
Date Jan. 9, 2011, 11:37 p.m.
Message ID <20110109.153745.35032992.davem@davemloft.net>
Download mbox | patch
Permalink /patch/78061/
State Accepted
Delegated to: David Miller
Headers show

Comments

David Miller - Jan. 9, 2011, 11:37 p.m.
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(-)
Peter Zijlstra - Jan. 10, 2011, 3:21 p.m.
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.
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Peter Zijlstra - Jan. 13, 2011, 10:55 a.m.
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()?
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sam Ravnborg - Jan. 13, 2011, 12:25 p.m.
> >  }
> >  
> > -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
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Peter Zijlstra - Jan. 13, 2011, 12:33 p.m.
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?
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - Jan. 13, 2011, 8:25 p.m.
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.
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

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);