sections mismatch detection

DISCUSSION Artem Bityutskiy 4 participants 9 messages Jan 6, 2010 – Jan 8, 2010 Kbuild

Share & tools

Artem Bityutskiy

author

2010-01-06 10:13 UTC #0 lore raw

Hi,

I wonder is it good time to enable DEBUG_SECTION_MISMATCH by default now? I mean, remove the following from lib/Kconfig.debug:

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)

Just spotted this, and decided to remind.

Signature · 2 lines

Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Sam Ravnborg

2010-01-06 10:48 UTC #1 lore raw

Quoted 5 lines · Artem Bityutskiy

Hi,
I wonder is it good time to enable DEBUG_SECTION_MISMATCH by default now? I mean, remove the following from lib/Kconfig.debug:

Quoted 8 lines

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)
Just spotted this, and decided to remind.

My original criteria for enabling this was that we had a warnign free build for all*config on x86 and a few other architectures.
While focusing on this I never reached this goal as I usually ended up with some problems related to CPU hotplug.
I dunno how close we are today at the "zero warning" target.

When people implemented CPU hotplug they started to use __cpuinit not only for code used during init time but also for code used solely for CPU hotplugging. That made it difficult to solve all the corner cases.
Because __cpu* annotation was used outside the original scope I suggested to remove them from the checks but alas got no response. Running out of time I gave up.

But I can see a steady flow of section mismatch fixes so I think the situation looks better these days.

As the kbuild maintainer traditionally has been a central person in this respect (due to the functionality in modpost),
I would like Michal to support this if we decide to enable the option.

Sam

Michal Marek

2010-01-06 11:38 UTC #2 lore raw

Quoted 21 lines · Sam Ravnborg

Quoted 5 lines · Artem Bityutskiy
Hi,
I wonder is it good time to enable DEBUG_SECTION_MISMATCH by default now? I mean, remove the following from lib/Kconfig.debug:
Quoted 8 lines
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)
Just spotted this, and decided to remind.
My original criteria for enabling this was that we had a warnign free build for all*config on x86 and a few other architectures.
While focusing on this I never reached this goal as I usually ended up with some problems related to CPU hotplug.
I dunno how close we are today at the "zero warning" target.

...

Quoted 3 lines

But I can see a steady flow of section mismatch fixes so I think the situation looks better these days.

There is also steady flow of new code ;-). FWIW, a 2.6.32.2 x86 distro build shows 29 section mismatch warnings, 12 of these come from drivers/staging, the remaining are (duplicates removed):

WARNING: drivers/pci/built-in.o(.text+0x10a12): Section mismatch in reference from the function dmar_ir_support() to the variable .init.data:dmar_tbl
WARNING: vmlinux.o(.devinit.text+0x3aa2): Section mismatch in reference from the function ezx_pcap_probe() to the function .init.text:set_irq_noprobe()
WARNING: drivers/char/hw_random/virtio-rng.o(.data+0x44): Section mismatch in reference from the variable virtio_rng to the function .devexit.text:virtrng_remove()
WARNING: drivers/scsi/hpsa.o(.text+0x192c): Section mismatch in reference from the function hpsa_pci_init() to the function .devinit.text:hpsa_interrupt_mode()
WARNING: drivers/virtio/virtio_balloon.o(.data+0x44): Section mismatch in reference from the variable virtio_balloon to the function .devexit.text:virtballoon_remove()

I'll try a current mainline build with DEBUG_SECTION_MISMATCH=y to see if/how this changed.

Michal

Artem Bityutskiy

author

2010-01-06 12:07 UTC #3 lore raw

Quoted 47 lines · Michal Marek

Quoted 21 lines · Sam Ravnborg
Quoted 5 lines · Artem Bityutskiy
Hi,
I wonder is it good time to enable DEBUG_SECTION_MISMATCH by default now? I mean, remove the following from lib/Kconfig.debug:
Quoted 8 lines
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)
Just spotted this, and decided to remind.
My original criteria for enabling this was that we had a warnign free build for all*config on x86 and a few other architectures.
While focusing on this I never reached this goal as I usually ended up with some problems related to CPU hotplug.
I dunno how close we are today at the "zero warning" target.
...
Quoted 3 lines
But I can see a steady flow of section mismatch fixes so I think the situation looks better these days.
There is also steady flow of new code ;-). FWIW, a 2.6.32.2 x86 distro build shows 29 section mismatch warnings, 12 of these come from drivers/staging, the remaining are (duplicates removed):
WARNING: drivers/pci/built-in.o(.text+0x10a12): Section mismatch in reference from the function dmar_ir_support() to the variable .init.data:dmar_tbl
WARNING: vmlinux.o(.devinit.text+0x3aa2): Section mismatch in reference from the function ezx_pcap_probe() to the function .init.text:set_irq_noprobe()
WARNING: drivers/char/hw_random/virtio-rng.o(.data+0x44): Section mismatch in reference from the variable virtio_rng to the function .devexit.text:virtrng_remove()
WARNING: drivers/scsi/hpsa.o(.text+0x192c): Section mismatch in reference from the function hpsa_pci_init() to the function .devinit.text:hpsa_interrupt_mode()
WARNING: drivers/virtio/virtio_balloon.o(.data+0x44): Section mismatch in reference from the variable virtio_balloon to the function .devexit.text:virtballoon_remove()

Quoted 2 lines

I'll try a current mainline build with DEBUG_SECTION_MISMATCH=y to see if/how this changed.

I thought this option was there disabled by default for enough time. And if we want to fix section mismatches some day, we should better enable it by default. Otherwise high flow of new section mismatches will never end.

Signature · 2 lines

Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Randy Dunlap

2010-01-07 23:03 UTC #4 lore raw

In reply to Michal Marek

Quoted 47 lines · On Wed, 06 Jan 2010 12:38:20 +0100 Michal Marek wrote:

Quoted 21 lines · Sam Ravnborg
Quoted 5 lines · Artem Bityutskiy
Hi,
I wonder is it good time to enable DEBUG_SECTION_MISMATCH by default now? I mean, remove the following from lib/Kconfig.debug:
Quoted 8 lines
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)
Just spotted this, and decided to remind.
My original criteria for enabling this was that we had a warnign free build for all*config on x86 and a few other architectures.
While focusing on this I never reached this goal as I usually ended up with some problems related to CPU hotplug.
I dunno how close we are today at the "zero warning" target.
...
Quoted 3 lines
But I can see a steady flow of section mismatch fixes so I think the situation looks better these days.
There is also steady flow of new code ;-). FWIW, a 2.6.32.2 x86 distro build shows 29 section mismatch warnings, 12 of these come from drivers/staging, the remaining are (duplicates removed):
WARNING: drivers/pci/built-in.o(.text+0x10a12): Section mismatch in reference from the function dmar_ir_support() to the variable .init.data:dmar_tbl
WARNING: vmlinux.o(.devinit.text+0x3aa2): Section mismatch in reference from the function ezx_pcap_probe() to the function .init.text:set_irq_noprobe()
WARNING: drivers/char/hw_random/virtio-rng.o(.data+0x44): Section mismatch in reference from the variable virtio_rng to the function .devexit.text:virtrng_remove()
WARNING: drivers/scsi/hpsa.o(.text+0x192c): Section mismatch in reference from the function hpsa_pci_init() to the function .devinit.text:hpsa_interrupt_mode()
WARNING: drivers/virtio/virtio_balloon.o(.data+0x44): Section mismatch in reference from the variable virtio_balloon to the function .devexit.text:virtballoon_remove()

Quoted 2 lines

I'll try a current mainline build with DEBUG_SECTION_MISMATCH=y to see if/how this changed.

I would prefer to see (logically) the same section mismatch not be reported multiple times, even though they are in different binary files. This would reduce the noise level quite a bit IMO.

E.g., I see this:

WARNING: drivers/scsi/hpsa.o(.text+0x1bd1): Section mismatch in reference from the function hpsa_pci_init() to the function .devinit.text:hpsa_interrupt_mode() The function hpsa_pci_init() references
the function __devinit hpsa_interrupt_mode().
This is often because hpsa_pci_init lacks a __devinit
annotation or the annotation of hpsa_interrupt_mode is wrong.

... followed later by ...

WARNING: drivers/built-in.o(.text+0x28e734): Section mismatch in reference from the function hpsa_pci_init() to the function .devinit.text:hpsa_interrupt_mode() The function hpsa_pci_init() references
the function __devinit hpsa_interrupt_mode().
This is often because hpsa_pci_init lacks a __devinit
annotation or the annotation of hpsa_interrupt_mode is wrong.

... followed later by ...

WARNING: vmlinux.o(.text+0x5b5cc4): Section mismatch in reference from the function hpsa_pci_init() to the function .devinit.text:hpsa_interrupt_mode() The function hpsa_pci_init() references
the function __devinit hpsa_interrupt_mode().
This is often because hpsa_pci_init lacks a __devinit
annotation or the annotation of hpsa_interrupt_mode is wrong.

One of these (preferably the first one) is enough for me.

---
~Randy

Sam Ravnborg

2010-01-08 05:43 UTC #5 lore raw

Quoted 4 lines

I would prefer to see (logically) the same section mismatch not be reported multiple times, even though they are in different binary files. This would reduce the noise level quite a bit IMO.

That would be nice but is not trivially doable. We need to discover when there is a section mismatch resulting from for example a function in kernel/* calling a function in arch/$ARCH/kernel so we need to execute the check on multiple levels i order to catch the bugs as early as possible.

The easy 'fix' would be to execute the check only on vmlinux.o (default behaviour) but the added checks with the option enabled allows us to be much more precise in reporting where
the section mismatch originate from.

Sam

Michal Marek

2010-01-08 13:18 UTC #6 lore raw

Sam Ravnborg napsal(a):

Quoted 11 lines

Quoted 3 lines
I would prefer to see (logically) the same section mismatch not be reported multiple times, even though they are in different binary files. This would reduce the noise level quite a bit IMO.
That would be nice but is not trivially doable. We need to discover when there is a section mismatch resulting from for example a function in kernel/* calling a function in arch/$ARCH/kernel so we need to execute the check on multiple levels i order to catch the bugs as early as possible.

Quoted 5 lines

The easy 'fix' would be to execute the check only on vmlinux.o (default behaviour) but the added checks with the option enabled allows us to be much more precise in reporting where
the section mismatch originate from.

But we could remember the (caller,callee) pairs and only emit the first warning. Or am I missing something?

Michal

Sam Ravnborg

2010-01-08 16:28 UTC #7 lore raw

Quoted 12 lines · Michal Marek

Sam Ravnborg napsal(a):
Quoted 11 lines
Quoted 3 lines
I would prefer to see (logically) the same section mismatch not be reported multiple times, even though they are in different binary files. This would reduce the noise level quite a bit IMO.
That would be nice but is not trivially doable. We need to discover when there is a section mismatch resulting from for example a function in kernel/* calling a function in arch/$ARCH/kernel so we need to execute the check on multiple levels i order to catch the bugs as early as possible.

Quoted 8 lines

Quoted 5 lines
The easy 'fix' would be to execute the check only on vmlinux.o (default behaviour) but the added checks with the option enabled allows us to be much more precise in reporting where
the section mismatch originate from.
But we could remember the (caller,callee) pairs and only emit the first warning. Or am I missing something?

The warnings comes from individual calls to mdpost. When we enable CONFIG_DEBUG_SECTION.. then we will
use modpost to analyse for section mismatches each time we link.

This allows us to emit warnings when we have much better chance to tell in what .o file we saw the mismatch.
But the downside as Randy also highlight is that the same warning will be displayed several times as we see it for each linking step.

The nomal way to do section mismatch checks is to only do it on vmlinux.o and thus we get only one warning per mismatch but we are much less precise in where it comes from.

Now we could save (caller, callee) from each warning and read in these and then only emit new warnings.
But it suddenly became non-trivial to do.

And we really want to do the check on several levels. Think of for example ext4.
Within fs/ext4/ there may be no problems.
But when linkend with the rest of the fs/ subsystem we may see issues, which is why we need to do the checks on both level (when we have the config flag enabled).

Sam

Michal Marek

2010-01-08 16:32 UTC #8 lore raw

Sam Ravnborg napsal(a):

Quoted 17 lines · Michal Marek

Quoted 16 lines
Sam Ravnborg napsal(a):
Quoted 15 lines
Quoted 3 lines
I would prefer to see (logically) the same section mismatch not be reported multiple times, even though they are in different binary files. This would reduce the noise level quite a bit IMO.
That would be nice but is not trivially doable. We need to discover when there is a section mismatch resulting from for example a function in kernel/* calling a function in arch/$ARCH/kernel so we need to execute the check on multiple levels i order to catch the bugs as early as possible.
The easy 'fix' would be to execute the check only on vmlinux.o (default behaviour) but the added checks with the option enabled allows us to be much more precise in reporting where
the section mismatch originate from.

Quoted 4 lines

Quoted 2 lines
But we could remember the (caller,callee) pairs and only emit the first warning. Or am I missing something?
The warnings comes from individual calls to mdpost.

Ah, that was the piece I was missing :).

Thanks,
Michal