sections mismatch detection

sections mismatch detection

[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:

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.

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

Re: sections mismatch detection

[Sam Ravnborg]

On Wed, Jan 06, 2010 at 12:11:23PM +0200, Artem Bityutskiy wrote:
> 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.

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

Re: sections mismatch detection

[Michal Marek]

On 6.1.2010 11:48, Sam Ravnborg wrote:
> On Wed, Jan 06, 2010 at 12:11:23PM +0200, Artem Bityutskiy wrote:
>> 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.
> 
> 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.
>
...
> 
> 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

Re: sections mismatch detection

[Artem Bityutskiy]

On Wed, 2010-01-06 at 12:38 +0100, Michal Marek wrote:
> On 6.1.2010 11:48, Sam Ravnborg wrote:
> > On Wed, Jan 06, 2010 at 12:11:23PM +0200, Artem Bityutskiy wrote:
> >> 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.
> > 
> > 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.
> >
> ...
> > 
> > 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.

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.

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

Re: sections mismatch detection

[Randy Dunlap]

On Wed, 06 Jan 2010 12:38:20 +0100 Michal Marek wrote:

> On 6.1.2010 11:48, Sam Ravnborg wrote:
> > On Wed, Jan 06, 2010 at 12:11:23PM +0200, Artem Bityutskiy wrote:
> >> 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.
> > 
> > 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.
> >
> ...
> > 
> > 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.

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

Re: sections mismatch detection

[Sam Ravnborg]

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

Re: sections mismatch detection

[Michal Marek]

Sam Ravnborg napsal(a):
>> 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.

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

Michal

Re: sections mismatch detection

[Sam Ravnborg]

On Fri, Jan 08, 2010 at 02:18:56PM +0100, Michal Marek wrote:
> Sam Ravnborg napsal(a):
> >> 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.
> 
> 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

Re: sections mismatch detection

[Michal Marek]

Sam Ravnborg napsal(a):
> On Fri, Jan 08, 2010 at 02:18:56PM +0100, Michal Marek wrote:
>> Sam Ravnborg napsal(a):
>>>> 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.
>> 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