Re: [git pull] x86 fixes

From: Ingo Molnar
Date: Sat Feb 21 2009 - 03:32:35 EST



* Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:

> On Thu, 19 Feb 2009, Ingo Molnar wrote:
> >
> > Andi Kleen (3):
> > x86, mce: reinitialize per cpu features on resume
>
> This one causes
>
> WARNING: arch/x86/kernel/cpu/mcheck/built-in.o(.text+0xf72): Section mismatch in reference from the function mce_resume() to the function .cpuinit.text:mce_intel_feature_init()
> The function mce_resume() references
> the function __cpuinit mce_intel_feature_init().
> This is often because mce_resume lacks a __cpuinit annotation or the annotation of mce_intel_feature_init is wrong.
>
> which looks like a real bug.
>
> On UP - withot CPU hotplug - I think __cpuinit becomes __init.
> No? So now mce_resume() will call some function that was long
> since free'd.

Indeed. hpa pushed the fix for it - it can be found below.

> I didn't look closer, but I wish somebody else would. And I
> wish people looked at warnings that are introduced by their
> new code before sending said changes to me.

Hm, i build and boot every tree i send to you, but this warning
didnt pop up and i specifically watch for build warnings and
section warnings in particular. (50% of all section warnings are
real.)

And i dont see it in an allyesconfig build either. Weird.

After half an hour of head scratching (including two
allyesconfig builds) i found these two gems commits that were
merged in the last week or two:

| commit fa2144ba9a31d1d0dc9607508576c3850e0d95b1
| Author: Sam Ravnborg <sam@xxxxxxxxxxxx>
| Date: Fri Feb 15 13:53:11 2008 +0100
|
| kbuild: explain why DEBUG_SECTION_MISMATCH is UNDEFINED
|
| We started to see patches enabling this - so explain why
| it is disabled and the condition to enable it again.
|
| Signed-off-by: Sam Ravnborg <sam@xxxxxxxxxxxx>
|
| diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
| index a370fe8..ab408aa 100644
| --- a/lib/Kconfig.debug
| +++ b/lib/Kconfig.debug
| @@ -82,6 +82,9 @@ config HEADERS_CHECK
| config DEBUG_SECTION_MISMATCH
| bool "Enable full Section mismatch analysis"
| depends on UNDEFINED
| + # This option is on purpose disabled for now.
| + # It will be enabled when we are down to a resonable number
| + # of section mismatch warnings (< 10 for an allyesconfig build)

| commit e5f95c8b7700a7bf1c2d24eedc677954d9aa0285
| Author: Sam Ravnborg <sam@xxxxxxxxxxxx>
| Date: Sat Feb 2 18:57:18 2008 +0100
|
| kbuild: print only total number of section mismatces found
|
| We have too many section mismatches detected at the moment.
| So silence modpost and prevent the option from being
| set in a typical allyesconfig build.
|
| Tell the user how to see all the deteils in the summary
| message from modpost.
|
| Signed-off-by: Sam Ravnborg <sam@xxxxxxxxxxxx>

These commits are an utter joke.

Sam, i virtually _begged_ you a few months ago to actually turn
section warnings into hard build failures, because 50% of the
time the warnings show real bugs and we want to fix real bugs
not hide them. Having them as build failures and not as warnings
is the only way to realistically fix them, in the current
upstream kernel climate.

Not only didnt what i suggest happen, but apparently this useful
debug feature got turned _off_ in rc5, silently :-( This broke
my push-to-Linus qa.

And the thing is, this commit log title:

kbuild: print only total number of section mismatces found

is utterly deceiving as well. It should have been:

kbuild: disable CONFIG_DEBUG_SECTION_MISMATCH

(i'd probably have noticed that patch - i check every patch
title that goes upstream.)

I've now worked it around in tip:master but that's not a good
solution because when i send a tree to Linus i build _that
specific tree_, with _zero_ additional patches, to make sure
that what i send to Linus is sane. (to make sure that none of
our other fixes/changes in tip:master interact, etc.)

So these patches need to be reverted in -rc6.

Ingo

------------------------->