2.6.25 and DEBUG_SECTION_MISMATCH

2.6.25 and DEBUG_SECTION_MISMATCH

[Adrian Bunk]

Hi Sam,

although DEBUG_SECTION_MISMATCH allows to also find exotig things like 
finding bugs only present when using gcc < 3.4 and CONFIG_HOTPLUG=n at 
the same time, the problem is that we print
    WARNING: modpost: Found 13 section mismatch(es).
    To see full details build your kernel with:
    'make CONFIG_DEBUG_SECTION_MISMATCH=y'
but CONFIG_DEBUG_SECTION_MISMATCH is not offered in 2.6.25.

The options seem to be:
- going back to the worse but for most practical purposes usable
  warnings in 2.6.24
- don't show these warnings during modpost for 2.6.25

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: 2.6.25 and DEBUG_SECTION_MISMATCH

[Jacek Luczak]

Hi Adrian,

> Hi Sam,
> 
> although DEBUG_SECTION_MISMATCH allows to also find exotig things like 
> finding bugs only present when using gcc < 3.4 and CONFIG_HOTPLUG=n at 
> the same time, the problem is that we print
>     WARNING: modpost: Found 13 section mismatch(es).
>     To see full details build your kernel with:
>     'make CONFIG_DEBUG_SECTION_MISMATCH=y'
> but CONFIG_DEBUG_SECTION_MISMATCH is not offered in 2.6.25.

Most of those ,,old mismatches'' where already fixed, so ...

> The options seem to be:
> - going back to the worse but for most practical purposes usable
>   warnings in 2.6.24
> - don't show these warnings during modpost for 2.6.25

... IMO it would be better to not show modpost warnings. It will result in great
number of reports from users. Sam is here a boss.

There's also second part of the story around inline functions which may be
reported while running with CONFIG_DEBUG_SECTION_MISMATCH.

Greats,
	Jacek

Re: 2.6.25 and DEBUG_SECTION_MISMATCH

[Sam Ravnborg]

Hi Adrian.

On Wed, Apr 16, 2008 at 08:51:35PM +0300, Adrian Bunk wrote:
> Hi Sam,
> 
> although DEBUG_SECTION_MISMATCH allows to also find exotig things like 
> finding bugs only present when using gcc < 3.4 and CONFIG_HOTPLUG=n at 
> the same time, the problem is that we print
>     WARNING: modpost: Found 13 section mismatch(es).
>     To see full details build your kernel with:
>     'make CONFIG_DEBUG_SECTION_MISMATCH=y'

So we print exact instructions how to get more info.
And several people has posted list of section mismatch warnings recently,
so the instructions are understood.
I fail to see any problem with that.

Obviously it would be better if we bailed out with a big fat warning
if someone _adds_ a new section mismatch warning.

> but CONFIG_DEBUG_SECTION_MISMATCH is not offered in 2.6.25.
If you by "not offered" try to say that the CONFIG option
is not available in kconfig then you are correct.
But you are in the instructions above asked to supply it
on the command line to make - not to enable it in the
kconfig configuration.

I would prefer we were down to less than 5 warnings so we could
enable it in kconfig but maybe after next merge window.

	Sam

Re: 2.6.25 and DEBUG_SECTION_MISMATCH

[Ingo Molnar]

* Sam Ravnborg <sam@ravnborg.org> wrote:

> I would prefer we were down to less than 5 warnings so we could enable 
> it in kconfig but maybe after next merge window.

they get regenerated. More than 1 out of every 100 new patches. Quite 
expensive feature (in terms of maintenance overhead) for a rather 
theoretical memory saving benefit.

	Ingo

Re: 2.6.25 and DEBUG_SECTION_MISMATCH

[Jacek Luczak]

Ingo Molnar pisze:
> * Sam Ravnborg <sam@ravnborg.org> wrote:
> 
>> I would prefer we were down to less than 5 warnings so we could enable 
>> it in kconfig but maybe after next merge window.
> 
> they get regenerated. More than 1 out of every 100 new patches. Quite 
> expensive feature (in terms of maintenance overhead) for a rather 
> theoretical memory saving benefit.

Ok, that's why this should be done in places where we are sure that given
function is called once and none should call more. So in general this should
result in removing few(?) annotations not in adding it everywhere where mismatch
appears.

-Jacek

Re: 2.6.25 and DEBUG_SECTION_MISMATCH

[Ingo Molnar]

* Ingo Molnar <mingo@elte.hu> wrote:

> * Sam Ravnborg <sam@ravnborg.org> wrote:
> 
> > I would prefer we were down to less than 5 warnings so we could 
> > enable it in kconfig but maybe after next merge window.
> 
> they get regenerated. More than 1 out of every 100 new patches. Quite 
> expensive feature (in terms of maintenance overhead) for a rather 
> theoretical memory saving benefit.

in v2.6.24 we had at least 200 section fixes:

  $ git-log v2.6.24.. | egrep -c 'WARNING:.*Section mismatch'
  198

for 12671 commits - that's 1 section type fix for every 60 commits. Even 
considering that in v2.6.24 you did a titanic effort of fixing tons of 
section matches - chances are that we'll still see about 1 section fix 
for every 100 commits in the future too. That's more than 100 section 
mismatch fixes for v2.6.26. That's _much_ more maintenance overhead than 
a lot of _big_ features - and there's no end in sight. x86.git has more 
than 10 section mismatch fixes for about 1000 new patches - that matches 
the 1% cost approximately.

and 90% of the problems could be reduced via the very simple patch below 
...

so ... why shouldnt we say that if someone wants to have this feature, 
it should be done all automatically, in the link space. There's no 
reason why the dependencies couldnt be figured out all automatically.

	Ingo

------------------->
Subject: x86: force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
From: Andi Kleen <ak@suse.de>

This avoids the requirement to mark a lot of initialization functions not
__cpuinit just for resume from RAM.

More functions could be converted now, didn't do all.

Cc: rjw@sisk.pl
Cc: pavel@suse.cz

Signed-off-by: Andi Kleen <ak@suse.de>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/Kconfig                |    5 +++++
 arch/x86/kernel/cpu/mtrr/main.c |    2 +-
 arch/x86/kernel/trampoline_64.S |    2 +-
 include/linux/init.h            |    2 +-
 4 files changed, 8 insertions(+), 3 deletions(-)

Index: linux/arch/x86/Kconfig
===================================================================
--- linux.orig/arch/x86/Kconfig
+++ linux/arch/x86/Kconfig
@@ -130,6 +130,11 @@ config GENERIC_PENDING_IRQ
 	depends on GENERIC_HARDIRQS && SMP
 	default y
 
+config PM_CPUINIT
+	bool
+	depends on PM
+	default y
+
 config X86_SMP
 	bool
 	depends on SMP && ((X86_32 && !X86_VOYAGER) || X86_64)
Index: linux/arch/x86/kernel/cpu/mtrr/main.c
===================================================================
--- linux.orig/arch/x86/kernel/cpu/mtrr/main.c
+++ linux/arch/x86/kernel/cpu/mtrr/main.c
@@ -712,7 +712,7 @@ void __init mtrr_bp_init(void)
 	}
 }
 
-void mtrr_ap_init(void)
+void __cpuinit mtrr_ap_init(void)
 {
 	unsigned long flags;
 
Index: linux/arch/x86/kernel/trampoline_64.S
===================================================================
--- linux.orig/arch/x86/kernel/trampoline_64.S
+++ linux/arch/x86/kernel/trampoline_64.S
@@ -34,7 +34,7 @@
 #include <asm/segment.h>
 
 /* We can free up trampoline after bootup if cpu hotplug is not supported. */
-#ifndef CONFIG_HOTPLUG_CPU
+#if !defined(CONFIG_HOTPLUG_CPU) && !defined(CONFIG_PM_CPUINIT)
 .section .init.data, "aw", @progbits
 #else
 .section .rodata, "a", @progbits
Index: linux/include/linux/init.h
===================================================================
--- linux.orig/include/linux/init.h
+++ linux/include/linux/init.h
@@ -266,7 +266,7 @@ void __init parse_early_param(void);
 #define __devexitdata __exitdata
 #endif
 
-#ifdef CONFIG_HOTPLUG_CPU
+#if defined(CONFIG_HOTPLUG_CPU) || defined(CONFIG_PM_CPUINIT)
 #define __cpuinit
 #define __cpuinitdata
 #define __cpuexit

Re: 2.6.25 and DEBUG_SECTION_MISMATCH

[Sam Ravnborg]

On Wed, Apr 16, 2008 at 09:55:04PM +0200, Ingo Molnar wrote:
> 
> * Ingo Molnar <mingo@elte.hu> wrote:
> 
> > * Sam Ravnborg <sam@ravnborg.org> wrote:
> > 
> > > I would prefer we were down to less than 5 warnings so we could 
> > > enable it in kconfig but maybe after next merge window.
> > 
> > they get regenerated. More than 1 out of every 100 new patches. Quite 
> > expensive feature (in terms of maintenance overhead) for a rather 
> > theoretical memory saving benefit.
> 
> in v2.6.24 we had at least 200 section fixes:
> 
>   $ git-log v2.6.24.. | egrep -c 'WARNING:.*Section mismatch'
>   198
> 
> for 12671 commits - that's 1 section type fix for every 60 commits.
Please get your numbers right. What you count is the number of
Section mismatch warnings that has been fixed.
Sometime fixing up one line of code fixes >5 section mismatch warnings.

> and 90% of the problems could be reduced via the very simple patch below 
> ...
> 
> so ... why shouldnt we say that if someone wants to have this feature, 
> it should be done all automatically, in the link space. There's no 
> reason why the dependencies couldnt be figured out all automatically.

Or we could fix the blatant misuse of __cpuinit. If has
replaced a proper separation of code that is only relevant for
CPU hotplug and code that is used with and without CPUHOTPLUG.

I question that we have 580 functions that is used duing init in
normal cases and during CPU hotplug.

	Sam

Re: 2.6.25 and DEBUG_SECTION_MISMATCH

[Ingo Molnar]

* Sam Ravnborg <sam@ravnborg.org> wrote:

> On Wed, Apr 16, 2008 at 09:55:04PM +0200, Ingo Molnar wrote:
> > 
> > * Ingo Molnar <mingo@elte.hu> wrote:
> > 
> > > * Sam Ravnborg <sam@ravnborg.org> wrote:
> > > 
> > > > I would prefer we were down to less than 5 warnings so we could 
> > > > enable it in kconfig but maybe after next merge window.
> > > 
> > > they get regenerated. More than 1 out of every 100 new patches. Quite 
> > > expensive feature (in terms of maintenance overhead) for a rather 
> > > theoretical memory saving benefit.
> > 
> > in v2.6.24 we had at least 200 section fixes:
> > 
> >   $ git-log v2.6.24.. | egrep -c 'WARNING:.*Section mismatch'
> >   198
> > 
> > for 12671 commits - that's 1 section type fix for every 60 commits.
> Please get your numbers right. What you count is the number of Section 
> mismatch warnings that has been fixed. Sometime fixing up one line of 
> code fixes >5 section mismatch warnings.

the revised number is: at least 84 commits that fixed section bugs. 
That's still rather large.

> > and 90% of the problems could be reduced via the very simple patch 
> > below ...
> > 
> > so ... why shouldnt we say that if someone wants to have this 
> > feature, it should be done all automatically, in the link space. 
> > There's no reason why the dependencies couldnt be figured out all 
> > automatically.
> 
> Or we could fix the blatant misuse of __cpuinit. If has replaced a 
> proper separation of code that is only relevant for CPU hotplug and 
> code that is used with and without CPUHOTPLUG.
> 
> I question that we have 580 functions that is used duing init in 
> normal cases and during CPU hotplug.

sure, if you can solve it, please do - but the stream of new section 
warnings and seems somewhat worrying for the long run.

	Ingo