Bug 78251 - x86/kernel/apic/apic_numachip.c: build failure with recent version of gcc
Summary: x86/kernel/apic/apic_numachip.c: build failure with recent version of gcc
Status: RESOLVED CODE_FIX
Alias: None
Product: Platform Specific/Hardware
Classification: Unclassified
Component: x86-64 (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: platform_x86_64@kernel-bugs.osdl.org
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-06-18 08:06 UTC by David Binderman
Modified: 2014-11-02 16:48 UTC (History)
3 users (show)

See Also:
Kernel Version: 3.16-rc1
Subsystem:
Regression: Yes
Bisected commit-id:


Attachments

Description David Binderman 2014-06-18 08:06:18 UTC
linux kernel 3.16-rc1 builds fine with stock gcc version 4.8.2 20131212.

I tried out today's (20140618) gcc trunk on the same build and got

arch/x86/kernel/apic/apic_numachip.c:33:12: error: numachip_system causes a section type conflict with apic_numachip
 static int numachip_system __read_mostly;
            ^
arch/x86/kernel/apic/apic_numachip.c:205:26: note: ‘apic_numachip’ was declared here
 static const struct apic apic_numachip __refconst = {
Comment 1 David Binderman 2014-08-01 10:42:30 UTC
Code still broken in 3.16-rc7 ;-|

I did some digging. In file arch/x86/kernel/apic/apic_numachip.c,
variable apic_numachip is put into two different sections, which
new gcc doesn't seem to like.

[dcb@zippy4 linux-3.16-rc7]$ fgrep apic_numachip !$
fgrep apic_numachip arch/x86/kernel/apic/apic_numachip.c.sav
static const struct apic apic_numachip __read_mostly;
	return apic == &apic_numachip;
static const struct apic apic_numachip __refconst = {
apic_driver(apic_numachip);
[dcb@zippy4 linux-3.16-rc7]$ 

Section _read_mostly doesn't seem to be the same as __refconst.

I bodged it by changing the forward reference

[dcb@zippy4 linux-3.16-rc7]$ diff -up arch/x86/kernel/apic/apic_numachip.c.sav arch/x86/kernel/apic/apic_numachip.c
--- arch/x86/kernel/apic/apic_numachip.c.sav	2014-08-01 11:33:25.924513607 +0100
+++ arch/x86/kernel/apic/apic_numachip.c	2014-08-01 11:20:44.826145444 +0100
@@ -32,7 +32,8 @@
 
 static int numachip_system __read_mostly;
 
-static const struct apic apic_numachip __read_mostly;
+/* static const struct apic apic_numachip __read_mostly; */
+static const struct apic apic_numachip __refconst;
 
 static unsigned int get_apic_id(unsigned long x)
 {

I've taken the liberty of adding a couple of keen folks,
who may be able to turn this suggestion into a proper patch.
Comment 2 Andrey Utkin 2014-08-01 17:21:05 UTC
Emailed to the developer who added those lines in commit 44b111b519160e33fdc41eadb39af86a24707edf
Comment 3 Daniel J Blueman 2014-09-10 16:35:19 UTC
I can't reproduce the build failure with either GCC 4.8.2 or GCC 4.9.1, building either linux v3.16-rc7 or v3.17-rc4 with CONFIG_X86_NUMACHIP.

CONFIG_DEBUG_SECTION_MISMATCH is set, so I was expecting a warning if I don't get build failure, but didn't see anything.

Can attach your kernel .config please? Thanks!
Comment 4 David Binderman 2014-09-10 18:15:20 UTC
>I can't reproduce the build failure with either GCC 4.8.2 or GCC 4.9.1

Of course. You need an even later version of gcc, like development trunk,
like I said originally.

>Can attach your kernel .config please? Thanks!

I could do that, but it won't help. 
My apologies for apparently being less than clear.
 
Let's have another go at explaining the one line fix I am suggesting.
Please have another look at the call to fgrep I originally gave.
Here it is again:

$ fgrep apic_numachip arch/x86/kernel/apic/apic_numachip.c.sav
static const struct apic apic_numachip __read_mostly;
	return apic == &apic_numachip;
static const struct apic apic_numachip __refconst = {
apic_driver(apic_numachip);
$ 

So const struct apic_numachip is declared to be in section
__read_mostly, then it is referenced, then it is 
defined to be in section __refconst.

Section __read_mostly isn't the same as __refconst,
so the declaration and the definition don't match up.
Newer compilers grumble, older ones don't.

The fix is to make sure the section names *do* match up.

The bodge I gave looks like a proper fix to me.
Comment 5 Daniel J Blueman 2014-11-02 07:57:45 UTC
Can you check if this has been fixed by Andi's patch in 3.18-rc2?

commit 2dee5c43da3a981489a4f18972827139afcbee82
Author: Andi Kleen <andi@firstfloor.org>
Date:   Wed Sep 24 06:32:19 2014 +0200

    x86: Fix section conflict for numachip
    
    A variable cannot be both __read_mostly and const. This
    is a meaningless combination.
    
    Just make it only const.
    
    This fixes the LTO build with numachip enabled.
    
    Signed-off-by: Andi Kleen <ak@linux.intel.com>
    Link: http://lkml.kernel.org/r/1411533139-25708-1-git-send-email-andi@firstfloor.org
    Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

diff --git a/arch/x86/kernel/apic/apic_numachip.c b/arch/x86/kernel/apic/apic_numachip.c
index ae91539..4128b5f 100644
--- a/arch/x86/kernel/apic/apic_numachip.c
+++ b/arch/x86/kernel/apic/apic_numachip.c
@@ -32,7 +32,7 @@
 
 static int numachip_system __read_mostly;
 
-static const struct apic apic_numachip __read_mostly;
+static const struct apic apic_numachip;
 
 static unsigned int get_apic_id(unsigned long x)
 {
Comment 6 David Binderman 2014-11-02 16:48:29 UTC
>Can you check if this has been fixed by Andi's patch in 3.18-rc2?

3.18-rc2 builds fine, so Andi's patch looks good to me.

Note You need to log in before you can comment on or make changes to this bug.