PCI Section mismatch error in linux-next.

PCI Section mismatch error in linux-next.

[David Daney]

For MIPS, Thierry Reding's patch in linux-next (PCI: Keep 
pci_fixup_irqs() around after init) causes:

WARNING: vmlinux.o(.text+0x22c784): Section mismatch in reference from 
the function pci_fixup_irqs() to the function 
.init.text:pcibios_update_irq()

The MIPS implementation of pcibios_update_irq() is __init, so there is 
conflict with the removal of __init from pci_fixup_irqs() and 
pdev_fixup_irq().

Can you guys either remove the patch from linux-next, or improve it to 
also fix up any architecture implementations of pdev_update_irq()?


Thanks,
David Daney

Re: PCI Section mismatch error in linux-next.

[Bjorn Helgaas]

On Fri, Aug 17, 2012 at 11:36 AM, David Daney <ddaney.cavm@gmail.com> wrote:
> For MIPS, Thierry Reding's patch in linux-next (PCI: Keep pci_fixup_irqs()
> around after init) causes:
>
> WARNING: vmlinux.o(.text+0x22c784): Section mismatch in reference from the
> function pci_fixup_irqs() to the function .init.text:pcibios_update_irq()
>
> The MIPS implementation of pcibios_update_irq() is __init, so there is
> conflict with the removal of __init from pci_fixup_irqs() and
> pdev_fixup_irq().
>
> Can you guys either remove the patch from linux-next, or improve it to also
> fix up any architecture implementations of pdev_update_irq()?

Crap, there are lots of arches with this issue.  I'll fix it up.
Thanks for pointing it out!

Bjorn

Re: PCI Section mismatch error in linux-next.

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1045 bytes --]

On Fri, Aug 17, 2012 at 11:44:31AM -0600, Bjorn Helgaas wrote:
> On Fri, Aug 17, 2012 at 11:36 AM, David Daney <ddaney.cavm@gmail.com> wrote:
> > For MIPS, Thierry Reding's patch in linux-next (PCI: Keep pci_fixup_irqs()
> > around after init) causes:
> >
> > WARNING: vmlinux.o(.text+0x22c784): Section mismatch in reference from the
> > function pci_fixup_irqs() to the function .init.text:pcibios_update_irq()
> >
> > The MIPS implementation of pcibios_update_irq() is __init, so there is
> > conflict with the removal of __init from pci_fixup_irqs() and
> > pdev_fixup_irq().
> >
> > Can you guys either remove the patch from linux-next, or improve it to also
> > fix up any architecture implementations of pdev_update_irq()?
> 
> Crap, there are lots of arches with this issue.  I'll fix it up.
> Thanks for pointing it out!

Oh wow... looks like I've opened a can of worms there. This requires
quite a lot of other functions to have their annotations removed as
well. Bjorn, how do you want to handle this?

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: PCI Section mismatch error in linux-next.

[Bjorn Helgaas]

On Fri, Aug 17, 2012 at 12:29 PM, Thierry Reding
<thierry.reding@avionic-design.de> wrote:
> On Fri, Aug 17, 2012 at 11:44:31AM -0600, Bjorn Helgaas wrote:
>> On Fri, Aug 17, 2012 at 11:36 AM, David Daney <ddaney.cavm@gmail.com> wrote:
>> > For MIPS, Thierry Reding's patch in linux-next (PCI: Keep pci_fixup_irqs()
>> > around after init) causes:
>> >
>> > WARNING: vmlinux.o(.text+0x22c784): Section mismatch in reference from the
>> > function pci_fixup_irqs() to the function .init.text:pcibios_update_irq()
>> >
>> > The MIPS implementation of pcibios_update_irq() is __init, so there is
>> > conflict with the removal of __init from pci_fixup_irqs() and
>> > pdev_fixup_irq().
>> >
>> > Can you guys either remove the patch from linux-next, or improve it to also
>> > fix up any architecture implementations of pdev_update_irq()?
>>
>> Crap, there are lots of arches with this issue.  I'll fix it up.
>> Thanks for pointing it out!
>
> Oh wow... looks like I've opened a can of worms there. This requires
> quite a lot of other functions to have their annotations removed as
> well. Bjorn, how do you want to handle this?

David said "pdev_update_irq()," but I think he meant "pcibios_update_irq()."

Almost all the pcibios_update_irq() implementations are identical, so
I think I'll just supply a weak implementation and remove the
redundant arch versions.

This is just about the only thing in my "next" branch, so I'll clear
it out for now, until we get this resolved.

Bjorn

Re: PCI Section mismatch error in linux-next.

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2370 bytes --]

On Fri, Aug 17, 2012 at 01:32:45PM -0600, Bjorn Helgaas wrote:
> On Fri, Aug 17, 2012 at 12:29 PM, Thierry Reding
> <thierry.reding@avionic-design.de> wrote:
> > On Fri, Aug 17, 2012 at 11:44:31AM -0600, Bjorn Helgaas wrote:
> >> On Fri, Aug 17, 2012 at 11:36 AM, David Daney <ddaney.cavm@gmail.com> wrote:
> >> > For MIPS, Thierry Reding's patch in linux-next (PCI: Keep pci_fixup_irqs()
> >> > around after init) causes:
> >> >
> >> > WARNING: vmlinux.o(.text+0x22c784): Section mismatch in reference from the
> >> > function pci_fixup_irqs() to the function .init.text:pcibios_update_irq()
> >> >
> >> > The MIPS implementation of pcibios_update_irq() is __init, so there is
> >> > conflict with the removal of __init from pci_fixup_irqs() and
> >> > pdev_fixup_irq().
> >> >
> >> > Can you guys either remove the patch from linux-next, or improve it to also
> >> > fix up any architecture implementations of pdev_update_irq()?
> >>
> >> Crap, there are lots of arches with this issue.  I'll fix it up.
> >> Thanks for pointing it out!
> >
> > Oh wow... looks like I've opened a can of worms there. This requires
> > quite a lot of other functions to have their annotations removed as
> > well. Bjorn, how do you want to handle this?
> 
> David said "pdev_update_irq()," but I think he meant "pcibios_update_irq()."
> 
> Almost all the pcibios_update_irq() implementations are identical, so
> I think I'll just supply a weak implementation and remove the
> redundant arch versions.

That makes sense. However I've just tested a build with section mismatch
debugging enabled on ARM and there are a few more that need __init or
__devinit removed to get rid of the warnings:

	pci_common_init()
	pcibios_init_hw()
	pcibios_init_resources()
	pcibios_swizzle()
	pcibios_update_irq()

pci_scan_root_bus() also needs __devinit removed. I haven't checked the
other architectures because I'll have to build cross-compilers for them
first, but I suspect most of them will have a similar list. I'm not sure
how well this kind of change is going to go down with the respective
architecture maintainers, though.

> This is just about the only thing in my "next" branch, so I'll clear
> it out for now, until we get this resolved.

Agreed. Do you want me to take a look at this or would you rather tackle
it yourself?

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: PCI Section mismatch error in linux-next.

[Bjorn Helgaas]

On Fri, Aug 17, 2012 at 2:07 PM, Thierry Reding
<thierry.reding@avionic-design.de> wrote:
> On Fri, Aug 17, 2012 at 01:32:45PM -0600, Bjorn Helgaas wrote:
>> On Fri, Aug 17, 2012 at 12:29 PM, Thierry Reding
>> <thierry.reding@avionic-design.de> wrote:
>> > On Fri, Aug 17, 2012 at 11:44:31AM -0600, Bjorn Helgaas wrote:
>> >> On Fri, Aug 17, 2012 at 11:36 AM, David Daney <ddaney.cavm@gmail.com> wrote:
>> >> > For MIPS, Thierry Reding's patch in linux-next (PCI: Keep pci_fixup_irqs()
>> >> > around after init) causes:
>> >> >
>> >> > WARNING: vmlinux.o(.text+0x22c784): Section mismatch in reference from the
>> >> > function pci_fixup_irqs() to the function .init.text:pcibios_update_irq()
>> >> >
>> >> > The MIPS implementation of pcibios_update_irq() is __init, so there is
>> >> > conflict with the removal of __init from pci_fixup_irqs() and
>> >> > pdev_fixup_irq().
>> >> >
>> >> > Can you guys either remove the patch from linux-next, or improve it to also
>> >> > fix up any architecture implementations of pdev_update_irq()?
>> >>
>> >> Crap, there are lots of arches with this issue.  I'll fix it up.
>> >> Thanks for pointing it out!
>> >
>> > Oh wow... looks like I've opened a can of worms there. This requires
>> > quite a lot of other functions to have their annotations removed as
>> > well. Bjorn, how do you want to handle this?
>>
>> David said "pdev_update_irq()," but I think he meant "pcibios_update_irq()."
>>
>> Almost all the pcibios_update_irq() implementations are identical, so
>> I think I'll just supply a weak implementation and remove the
>> redundant arch versions.
>
> That makes sense. However I've just tested a build with section mismatch
> debugging enabled on ARM and there are a few more that need __init or
> __devinit removed to get rid of the warnings:
>
>         pci_common_init()
>         pcibios_init_hw()
>         pcibios_init_resources()
>         pcibios_swizzle()
>         pcibios_update_irq()
>
> pci_scan_root_bus() also needs __devinit removed. I haven't checked the
> other architectures because I'll have to build cross-compilers for them
> first, but I suspect most of them will have a similar list. I'm not sure
> how well this kind of change is going to go down with the respective
> architecture maintainers, though.

Hmm, yeah, this is a mess, isn't it?  Just about everything in PCI
will need __devinit removed.  We've been assuming that the only way
for things to show up after init is via hotplug.  But you're breaking
that assumption by doing *all* enumeration after init.  There are
approximately a bajillion __init and __devinit annotations just in
drivers/pci, not to mention those in the architectures.

Well, maybe you just need to turn on CONFIG_HOTPLUG.  How would that
affect you?  I think we would still have to change some __inits to
__devinit, including pcibios_update_irq(), but it might be more
manageable.

I started working on this, but it sounds like you're in a better
position to find problems and test fixes, so how about if I just let
you handle it? :)

Bjorn

Re: PCI Section mismatch error in linux-next.

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 3941 bytes --]

On Fri, Aug 17, 2012 at 02:39:34PM -0600, Bjorn Helgaas wrote:
> On Fri, Aug 17, 2012 at 2:07 PM, Thierry Reding
> <thierry.reding@avionic-design.de> wrote:
> > On Fri, Aug 17, 2012 at 01:32:45PM -0600, Bjorn Helgaas wrote:
> >> On Fri, Aug 17, 2012 at 12:29 PM, Thierry Reding
> >> <thierry.reding@avionic-design.de> wrote:
> >> > On Fri, Aug 17, 2012 at 11:44:31AM -0600, Bjorn Helgaas wrote:
> >> >> On Fri, Aug 17, 2012 at 11:36 AM, David Daney <ddaney.cavm@gmail.com> wrote:
> >> >> > For MIPS, Thierry Reding's patch in linux-next (PCI: Keep pci_fixup_irqs()
> >> >> > around after init) causes:
> >> >> >
> >> >> > WARNING: vmlinux.o(.text+0x22c784): Section mismatch in reference from the
> >> >> > function pci_fixup_irqs() to the function .init.text:pcibios_update_irq()
> >> >> >
> >> >> > The MIPS implementation of pcibios_update_irq() is __init, so there is
> >> >> > conflict with the removal of __init from pci_fixup_irqs() and
> >> >> > pdev_fixup_irq().
> >> >> >
> >> >> > Can you guys either remove the patch from linux-next, or improve it to also
> >> >> > fix up any architecture implementations of pdev_update_irq()?
> >> >>
> >> >> Crap, there are lots of arches with this issue.  I'll fix it up.
> >> >> Thanks for pointing it out!
> >> >
> >> > Oh wow... looks like I've opened a can of worms there. This requires
> >> > quite a lot of other functions to have their annotations removed as
> >> > well. Bjorn, how do you want to handle this?
> >>
> >> David said "pdev_update_irq()," but I think he meant "pcibios_update_irq()."
> >>
> >> Almost all the pcibios_update_irq() implementations are identical, so
> >> I think I'll just supply a weak implementation and remove the
> >> redundant arch versions.
> >
> > That makes sense. However I've just tested a build with section mismatch
> > debugging enabled on ARM and there are a few more that need __init or
> > __devinit removed to get rid of the warnings:
> >
> >         pci_common_init()
> >         pcibios_init_hw()
> >         pcibios_init_resources()
> >         pcibios_swizzle()
> >         pcibios_update_irq()
> >
> > pci_scan_root_bus() also needs __devinit removed. I haven't checked the
> > other architectures because I'll have to build cross-compilers for them
> > first, but I suspect most of them will have a similar list. I'm not sure
> > how well this kind of change is going to go down with the respective
> > architecture maintainers, though.
> 
> Hmm, yeah, this is a mess, isn't it?  Just about everything in PCI
> will need __devinit removed.  We've been assuming that the only way
> for things to show up after init is via hotplug.  But you're breaking
> that assumption by doing *all* enumeration after init.  There are
> approximately a bajillion __init and __devinit annotations just in
> drivers/pci, not to mention those in the architectures.
> 
> Well, maybe you just need to turn on CONFIG_HOTPLUG.  How would that
> affect you?  I think we would still have to change some __inits to
> __devinit, including pcibios_update_irq(), but it might be more
> manageable.

You said that depending on HOTPLUG wouldn't be enough because it would
exclude reenumeration at runtime if HOTPLUG wasn't defined. Also it is
theoretically possible to build a kernel without HOTPLUG but have the
enumeration start after init because of deferred probing. Those cases
won't work if we keep __init or __devinit respectively, right?

> I started working on this, but it sounds like you're in a better
> position to find problems and test fixes, so how about if I just let
> you handle it? :)

I won't be able to test anything beyond Tegra because I'm lacking the
hardware. But with the section mismatch debugging enabled all issues
should be detected at compile time anyway, so it's just a problem of
getting cross-compilers for all architectures that support PCI.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: PCI Section mismatch error in linux-next.

[Bjorn Helgaas]

On Fri, Aug 17, 2012 at 2:48 PM, Thierry Reding
<thierry.reding@avionic-design.de> wrote:
> On Fri, Aug 17, 2012 at 02:39:34PM -0600, Bjorn Helgaas wrote:
>> On Fri, Aug 17, 2012 at 2:07 PM, Thierry Reding
>> <thierry.reding@avionic-design.de> wrote:
>> > On Fri, Aug 17, 2012 at 01:32:45PM -0600, Bjorn Helgaas wrote:
>> >> On Fri, Aug 17, 2012 at 12:29 PM, Thierry Reding
>> >> <thierry.reding@avionic-design.de> wrote:
>> >> > On Fri, Aug 17, 2012 at 11:44:31AM -0600, Bjorn Helgaas wrote:
>> >> >> On Fri, Aug 17, 2012 at 11:36 AM, David Daney <ddaney.cavm@gmail.com> wrote:
>> >> >> > For MIPS, Thierry Reding's patch in linux-next (PCI: Keep pci_fixup_irqs()
>> >> >> > around after init) causes:
>> >> >> >
>> >> >> > WARNING: vmlinux.o(.text+0x22c784): Section mismatch in reference from the
>> >> >> > function pci_fixup_irqs() to the function .init.text:pcibios_update_irq()
>> >> >> >
>> >> >> > The MIPS implementation of pcibios_update_irq() is __init, so there is
>> >> >> > conflict with the removal of __init from pci_fixup_irqs() and
>> >> >> > pdev_fixup_irq().
>> >> >> >
>> >> >> > Can you guys either remove the patch from linux-next, or improve it to also
>> >> >> > fix up any architecture implementations of pdev_update_irq()?
>> >> >>
>> >> >> Crap, there are lots of arches with this issue.  I'll fix it up.
>> >> >> Thanks for pointing it out!
>> >> >
>> >> > Oh wow... looks like I've opened a can of worms there. This requires
>> >> > quite a lot of other functions to have their annotations removed as
>> >> > well. Bjorn, how do you want to handle this?
>> >>
>> >> David said "pdev_update_irq()," but I think he meant "pcibios_update_irq()."
>> >>
>> >> Almost all the pcibios_update_irq() implementations are identical, so
>> >> I think I'll just supply a weak implementation and remove the
>> >> redundant arch versions.
>> >
>> > That makes sense. However I've just tested a build with section mismatch
>> > debugging enabled on ARM and there are a few more that need __init or
>> > __devinit removed to get rid of the warnings:
>> >
>> >         pci_common_init()
>> >         pcibios_init_hw()
>> >         pcibios_init_resources()
>> >         pcibios_swizzle()
>> >         pcibios_update_irq()
>> >
>> > pci_scan_root_bus() also needs __devinit removed. I haven't checked the
>> > other architectures because I'll have to build cross-compilers for them
>> > first, but I suspect most of them will have a similar list. I'm not sure
>> > how well this kind of change is going to go down with the respective
>> > architecture maintainers, though.
>>
>> Hmm, yeah, this is a mess, isn't it?  Just about everything in PCI
>> will need __devinit removed.  We've been assuming that the only way
>> for things to show up after init is via hotplug.  But you're breaking
>> that assumption by doing *all* enumeration after init.  There are
>> approximately a bajillion __init and __devinit annotations just in
>> drivers/pci, not to mention those in the architectures.
>>
>> Well, maybe you just need to turn on CONFIG_HOTPLUG.  How would that
>> affect you?  I think we would still have to change some __inits to
>> __devinit, including pcibios_update_irq(), but it might be more
>> manageable.
>
> You said that depending on HOTPLUG wouldn't be enough because it would
> exclude reenumeration at runtime if HOTPLUG wasn't defined.

I'm suggesting that maybe we shouldn't support enumeration at runtime
unless HOTPLUG is defined.  So if you want to enumerate after init,
set CONFIG_HOTPLUG=y.

> Also it is
> theoretically possible to build a kernel without HOTPLUG but have the
> enumeration start after init because of deferred probing. Those cases
> won't work if we keep __init or __devinit respectively, right?

This is the situation (deferred probing with CONFIG_HOTPLUG=n) that
I'm suggesting might not need to work.  After all, hotplug essentially
means "adding devices after init."

> I won't be able to test anything beyond Tegra because I'm lacking the
> hardware. But with the section mismatch debugging enabled all issues
> should be detected at compile time anyway, so it's just a problem of
> getting cross-compilers for all architectures that support PCI.

I have cross-compilers for many of the architectures (relatively
painless to build with
http://git.infradead.org/users/segher/buildall.git), but this is
starting to look like it will take more time than I have right now.

Re: PCI Section mismatch error in linux-next.

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 5377 bytes --]

On Fri, Aug 17, 2012 at 03:06:42PM -0600, Bjorn Helgaas wrote:
> On Fri, Aug 17, 2012 at 2:48 PM, Thierry Reding
> <thierry.reding@avionic-design.de> wrote:
> > On Fri, Aug 17, 2012 at 02:39:34PM -0600, Bjorn Helgaas wrote:
> >> On Fri, Aug 17, 2012 at 2:07 PM, Thierry Reding
> >> <thierry.reding@avionic-design.de> wrote:
> >> > On Fri, Aug 17, 2012 at 01:32:45PM -0600, Bjorn Helgaas wrote:
> >> >> On Fri, Aug 17, 2012 at 12:29 PM, Thierry Reding
> >> >> <thierry.reding@avionic-design.de> wrote:
> >> >> > On Fri, Aug 17, 2012 at 11:44:31AM -0600, Bjorn Helgaas wrote:
> >> >> >> On Fri, Aug 17, 2012 at 11:36 AM, David Daney <ddaney.cavm@gmail.com> wrote:
> >> >> >> > For MIPS, Thierry Reding's patch in linux-next (PCI: Keep pci_fixup_irqs()
> >> >> >> > around after init) causes:
> >> >> >> >
> >> >> >> > WARNING: vmlinux.o(.text+0x22c784): Section mismatch in reference from the
> >> >> >> > function pci_fixup_irqs() to the function .init.text:pcibios_update_irq()
> >> >> >> >
> >> >> >> > The MIPS implementation of pcibios_update_irq() is __init, so there is
> >> >> >> > conflict with the removal of __init from pci_fixup_irqs() and
> >> >> >> > pdev_fixup_irq().
> >> >> >> >
> >> >> >> > Can you guys either remove the patch from linux-next, or improve it to also
> >> >> >> > fix up any architecture implementations of pdev_update_irq()?
> >> >> >>
> >> >> >> Crap, there are lots of arches with this issue.  I'll fix it up.
> >> >> >> Thanks for pointing it out!
> >> >> >
> >> >> > Oh wow... looks like I've opened a can of worms there. This requires
> >> >> > quite a lot of other functions to have their annotations removed as
> >> >> > well. Bjorn, how do you want to handle this?
> >> >>
> >> >> David said "pdev_update_irq()," but I think he meant "pcibios_update_irq()."
> >> >>
> >> >> Almost all the pcibios_update_irq() implementations are identical, so
> >> >> I think I'll just supply a weak implementation and remove the
> >> >> redundant arch versions.
> >> >
> >> > That makes sense. However I've just tested a build with section mismatch
> >> > debugging enabled on ARM and there are a few more that need __init or
> >> > __devinit removed to get rid of the warnings:
> >> >
> >> >         pci_common_init()
> >> >         pcibios_init_hw()
> >> >         pcibios_init_resources()
> >> >         pcibios_swizzle()
> >> >         pcibios_update_irq()
> >> >
> >> > pci_scan_root_bus() also needs __devinit removed. I haven't checked the
> >> > other architectures because I'll have to build cross-compilers for them
> >> > first, but I suspect most of them will have a similar list. I'm not sure
> >> > how well this kind of change is going to go down with the respective
> >> > architecture maintainers, though.
> >>
> >> Hmm, yeah, this is a mess, isn't it?  Just about everything in PCI
> >> will need __devinit removed.  We've been assuming that the only way
> >> for things to show up after init is via hotplug.  But you're breaking
> >> that assumption by doing *all* enumeration after init.  There are
> >> approximately a bajillion __init and __devinit annotations just in
> >> drivers/pci, not to mention those in the architectures.
> >>
> >> Well, maybe you just need to turn on CONFIG_HOTPLUG.  How would that
> >> affect you?  I think we would still have to change some __inits to
> >> __devinit, including pcibios_update_irq(), but it might be more
> >> manageable.
> >
> > You said that depending on HOTPLUG wouldn't be enough because it would
> > exclude reenumeration at runtime if HOTPLUG wasn't defined.
> 
> I'm suggesting that maybe we shouldn't support enumeration at runtime
> unless HOTPLUG is defined.  So if you want to enumerate after init,
> set CONFIG_HOTPLUG=y.
> 
> > Also it is
> > theoretically possible to build a kernel without HOTPLUG but have the
> > enumeration start after init because of deferred probing. Those cases
> > won't work if we keep __init or __devinit respectively, right?
> 
> This is the situation (deferred probing with CONFIG_HOTPLUG=n) that
> I'm suggesting might not need to work.  After all, hotplug essentially
> means "adding devices after init."

Yes, I guess that would be appropriate. However I don't see how this
could be expressed in Kconfig unless the deferred probing itself is
conditionalized on HOTPLUG. Even in that case it would still be possible
to build a PCIe controller driver as a module and load it at runtime
after init.

> > I won't be able to test anything beyond Tegra because I'm lacking the
> > hardware. But with the section mismatch debugging enabled all issues
> > should be detected at compile time anyway, so it's just a problem of
> > getting cross-compilers for all architectures that support PCI.
> 
> I have cross-compilers for many of the architectures (relatively
> painless to build with
> http://git.infradead.org/users/segher/buildall.git), but this is
> starting to look like it will take more time than I have right now.

I have my own set of scripts and I already have toolchains for ARM,
MIPS, x86 and PowerPC. I've started a build for SPARC and will look
at some of the more exotic ones after that.

This also looks a little daunting to me, but I'll give it a shot. I
don't have plenty of time either so it may take a while.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: PCI Section mismatch error in linux-next.

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 991 bytes --]

On Fri, Aug 17, 2012 at 10:48:39PM +0200, Thierry Reding wrote:
> On Fri, Aug 17, 2012 at 02:39:34PM -0600, Bjorn Helgaas wrote:
[...]
> > Well, maybe you just need to turn on CONFIG_HOTPLUG.  How would that
> > affect you?  I think we would still have to change some __inits to
> > __devinit, including pcibios_update_irq(), but it might be more
> > manageable.
> 
> You said that depending on HOTPLUG wouldn't be enough because it would
> exclude reenumeration at runtime if HOTPLUG wasn't defined. Also it is
> theoretically possible to build a kernel without HOTPLUG but have the
> enumeration start after init because of deferred probing. Those cases
> won't work if we keep __init or __devinit respectively, right?

Another possibility would be to make PCI select HOTPLUG or depend on it.
That way it would be made sure that __devinit wouldn't cause all the
functions to be discarded after init.

Also, using PCI without HOTPLUG is sort of contradictory.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: PCI Section mismatch error in linux-next.

[Bjorn Helgaas]

On Fri, Aug 17, 2012 at 3:07 PM, Thierry Reding
<thierry.reding@avionic-design.de> wrote:
> On Fri, Aug 17, 2012 at 10:48:39PM +0200, Thierry Reding wrote:
>> On Fri, Aug 17, 2012 at 02:39:34PM -0600, Bjorn Helgaas wrote:
> [...]
>> > Well, maybe you just need to turn on CONFIG_HOTPLUG.  How would that
>> > affect you?  I think we would still have to change some __inits to
>> > __devinit, including pcibios_update_irq(), but it might be more
>> > manageable.
>>
>> You said that depending on HOTPLUG wouldn't be enough because it would
>> exclude reenumeration at runtime if HOTPLUG wasn't defined. Also it is
>> theoretically possible to build a kernel without HOTPLUG but have the
>> enumeration start after init because of deferred probing. Those cases
>> won't work if we keep __init or __devinit respectively, right?
>
> Another possibility would be to make PCI select HOTPLUG or depend on it.
> That way it would be made sure that __devinit wouldn't cause all the
> functions to be discarded after init.

There's been some discussion recently about whether CONFIG_HOTPLUG is
worth keeping any more, but nothing's been resolved yet.  If we did
decide to remove CONFIG_HOTPLUG, or require it for PCI, I would rather
just remove all the __devinit annotations because they'd be
superfluous.

> Also, using PCI without HOTPLUG is sort of contradictory.

I'm not sure I follow this one.  I can easily imagine embedded systems
that use PCI internally but have no slots or connectors, so there's no
possibility of anything changing.

Re: PCI Section mismatch error in linux-next.

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2005 bytes --]

On Fri, Aug 17, 2012 at 03:25:22PM -0600, Bjorn Helgaas wrote:
> On Fri, Aug 17, 2012 at 3:07 PM, Thierry Reding
> <thierry.reding@avionic-design.de> wrote:
> > On Fri, Aug 17, 2012 at 10:48:39PM +0200, Thierry Reding wrote:
> >> On Fri, Aug 17, 2012 at 02:39:34PM -0600, Bjorn Helgaas wrote:
> > [...]
> >> > Well, maybe you just need to turn on CONFIG_HOTPLUG.  How would that
> >> > affect you?  I think we would still have to change some __inits to
> >> > __devinit, including pcibios_update_irq(), but it might be more
> >> > manageable.
> >>
> >> You said that depending on HOTPLUG wouldn't be enough because it would
> >> exclude reenumeration at runtime if HOTPLUG wasn't defined. Also it is
> >> theoretically possible to build a kernel without HOTPLUG but have the
> >> enumeration start after init because of deferred probing. Those cases
> >> won't work if we keep __init or __devinit respectively, right?
> >
> > Another possibility would be to make PCI select HOTPLUG or depend on it.
> > That way it would be made sure that __devinit wouldn't cause all the
> > functions to be discarded after init.
> 
> There's been some discussion recently about whether CONFIG_HOTPLUG is
> worth keeping any more, but nothing's been resolved yet.  If we did
> decide to remove CONFIG_HOTPLUG, or require it for PCI, I would rather
> just remove all the __devinit annotations because they'd be
> superfluous.

I've missed that discussion. Can you point me to it?

> > Also, using PCI without HOTPLUG is sort of contradictory.
> 
> I'm not sure I follow this one.  I can easily imagine embedded systems
> that use PCI internally but have no slots or connectors, so there's no
> possibility of anything changing.

Contradictory was probably the wrong word. What I meant to say was that
for systems that use PCI it probably wouldn't hurt to enable HOTPLUG as
well. That would sort of go with what you said above, regarding the
removal of HOTPLUG altogether.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: PCI Section mismatch error in linux-next.

[Bjorn Helgaas]

On Fri, Aug 17, 2012 at 3:32 PM, Thierry Reding
<thierry.reding@avionic-design.de> wrote:
> On Fri, Aug 17, 2012 at 03:25:22PM -0600, Bjorn Helgaas wrote:
>> On Fri, Aug 17, 2012 at 3:07 PM, Thierry Reding
>> <thierry.reding@avionic-design.de> wrote:
>> > On Fri, Aug 17, 2012 at 10:48:39PM +0200, Thierry Reding wrote:
>> >> On Fri, Aug 17, 2012 at 02:39:34PM -0600, Bjorn Helgaas wrote:
>> > [...]
>> >> > Well, maybe you just need to turn on CONFIG_HOTPLUG.  How would that
>> >> > affect you?  I think we would still have to change some __inits to
>> >> > __devinit, including pcibios_update_irq(), but it might be more
>> >> > manageable.
>> >>
>> >> You said that depending on HOTPLUG wouldn't be enough because it would
>> >> exclude reenumeration at runtime if HOTPLUG wasn't defined. Also it is
>> >> theoretically possible to build a kernel without HOTPLUG but have the
>> >> enumeration start after init because of deferred probing. Those cases
>> >> won't work if we keep __init or __devinit respectively, right?
>> >
>> > Another possibility would be to make PCI select HOTPLUG or depend on it.
>> > That way it would be made sure that __devinit wouldn't cause all the
>> > functions to be discarded after init.
>>
>> There's been some discussion recently about whether CONFIG_HOTPLUG is
>> worth keeping any more, but nothing's been resolved yet.  If we did
>> decide to remove CONFIG_HOTPLUG, or require it for PCI, I would rather
>> just remove all the __devinit annotations because they'd be
>> superfluous.
>
> I've missed that discussion. Can you point me to it?

Sure: http://lists.linux-foundation.org/pipermail/ksummit-2012-discuss/2012-June/000051.html

>From previous email:
>> This is the situation (deferred probing with CONFIG_HOTPLUG=n) that
>> I'm suggesting might not need to work.  After all, hotplug essentially
>> means "adding devices after init."

> Yes, I guess that would be appropriate. However I don't see how this
> could be expressed in Kconfig unless the deferred probing itself is
> conditionalized on HOTPLUG. Even in that case it would still be possible
> to build a PCIe controller driver as a module and load it at runtime
> after init.

That's exactly it -- the deferred probing and any loadable PCI host
bridge drivers would have to depend on HOTPLUG.  That seems like the
most straightforward, least-surprises, approach for now.

Bjorn

Re: PCI Section mismatch error in linux-next.

[Greg KH]

On Fri, Aug 17, 2012 at 11:32:47PM +0200, Thierry Reding wrote:
> On Fri, Aug 17, 2012 at 03:25:22PM -0600, Bjorn Helgaas wrote:
> > On Fri, Aug 17, 2012 at 3:07 PM, Thierry Reding
> > <thierry.reding@avionic-design.de> wrote:
> > > On Fri, Aug 17, 2012 at 10:48:39PM +0200, Thierry Reding wrote:
> > >> On Fri, Aug 17, 2012 at 02:39:34PM -0600, Bjorn Helgaas wrote:
> > > [...]
> > >> > Well, maybe you just need to turn on CONFIG_HOTPLUG.  How would that
> > >> > affect you?  I think we would still have to change some __inits to
> > >> > __devinit, including pcibios_update_irq(), but it might be more
> > >> > manageable.
> > >>
> > >> You said that depending on HOTPLUG wouldn't be enough because it would
> > >> exclude reenumeration at runtime if HOTPLUG wasn't defined. Also it is
> > >> theoretically possible to build a kernel without HOTPLUG but have the
> > >> enumeration start after init because of deferred probing. Those cases
> > >> won't work if we keep __init or __devinit respectively, right?
> > >
> > > Another possibility would be to make PCI select HOTPLUG or depend on it.
> > > That way it would be made sure that __devinit wouldn't cause all the
> > > functions to be discarded after init.
> > 
> > There's been some discussion recently about whether CONFIG_HOTPLUG is
> > worth keeping any more, but nothing's been resolved yet.  If we did
> > decide to remove CONFIG_HOTPLUG, or require it for PCI, I would rather
> > just remove all the __devinit annotations because they'd be
> > superfluous.
> 
> I've missed that discussion. Can you point me to it?

It's pretty much just me saying the whole thing is a mess, causes
problems, and really doesn't solve any memory usage issues.  Ideally we
should just:
	- remove CONFIG_HOTPLUG and assume it is enabled
	- because of that, we can delete the large majority of the
	  __dev* markings

The memory savings these days are so tiny, if at all, and everyone,
including me, gets it wrong all the time.

As we pretty much allow anything to be disabled/enabled at any point in
time after boot, we are all running systems that rely on CONFIG_HOTPLUG
anyway.  To find a static system that doesn't need it is quite rare from
what I have found.

greg k-h

Re: PCI Section mismatch error in linux-next.

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2554 bytes --]

On Sun, Aug 19, 2012 at 10:30:36PM -0700, Greg KH wrote:
> On Fri, Aug 17, 2012 at 11:32:47PM +0200, Thierry Reding wrote:
> > On Fri, Aug 17, 2012 at 03:25:22PM -0600, Bjorn Helgaas wrote:
> > > On Fri, Aug 17, 2012 at 3:07 PM, Thierry Reding
> > > <thierry.reding@avionic-design.de> wrote:
> > > > On Fri, Aug 17, 2012 at 10:48:39PM +0200, Thierry Reding wrote:
> > > >> On Fri, Aug 17, 2012 at 02:39:34PM -0600, Bjorn Helgaas wrote:
> > > > [...]
> > > >> > Well, maybe you just need to turn on CONFIG_HOTPLUG.  How would that
> > > >> > affect you?  I think we would still have to change some __inits to
> > > >> > __devinit, including pcibios_update_irq(), but it might be more
> > > >> > manageable.
> > > >>
> > > >> You said that depending on HOTPLUG wouldn't be enough because it would
> > > >> exclude reenumeration at runtime if HOTPLUG wasn't defined. Also it is
> > > >> theoretically possible to build a kernel without HOTPLUG but have the
> > > >> enumeration start after init because of deferred probing. Those cases
> > > >> won't work if we keep __init or __devinit respectively, right?
> > > >
> > > > Another possibility would be to make PCI select HOTPLUG or depend on it.
> > > > That way it would be made sure that __devinit wouldn't cause all the
> > > > functions to be discarded after init.
> > > 
> > > There's been some discussion recently about whether CONFIG_HOTPLUG is
> > > worth keeping any more, but nothing's been resolved yet.  If we did
> > > decide to remove CONFIG_HOTPLUG, or require it for PCI, I would rather
> > > just remove all the __devinit annotations because they'd be
> > > superfluous.
> > 
> > I've missed that discussion. Can you point me to it?
> 
> It's pretty much just me saying the whole thing is a mess, causes
> problems, and really doesn't solve any memory usage issues.  Ideally we
> should just:
> 	- remove CONFIG_HOTPLUG and assume it is enabled
> 	- because of that, we can delete the large majority of the
> 	  __dev* markings
> 
> The memory savings these days are so tiny, if at all, and everyone,
> including me, gets it wrong all the time.
> 
> As we pretty much allow anything to be disabled/enabled at any point in
> time after boot, we are all running systems that rely on CONFIG_HOTPLUG
> anyway.  To find a static system that doesn't need it is quite rare from
> what I have found.

I've been reading through the thread it seems like the consensus is to
just drop it. Though there seems to be lacking a formal decision.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: PCI Section mismatch error in linux-next.

[Geert Uytterhoeven]

Hi Greg,

On Mon, Aug 20, 2012 at 7:30 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Fri, Aug 17, 2012 at 11:32:47PM +0200, Thierry Reding wrote:
>> On Fri, Aug 17, 2012 at 03:25:22PM -0600, Bjorn Helgaas wrote:
>> > On Fri, Aug 17, 2012 at 3:07 PM, Thierry Reding
>> > <thierry.reding@avionic-design.de> wrote:
>> > > On Fri, Aug 17, 2012 at 10:48:39PM +0200, Thierry Reding wrote:
>> > >> On Fri, Aug 17, 2012 at 02:39:34PM -0600, Bjorn Helgaas wrote:
>> > > [...]
>> > >> > Well, maybe you just need to turn on CONFIG_HOTPLUG.  How would that
>> > >> > affect you?  I think we would still have to change some __inits to
>> > >> > __devinit, including pcibios_update_irq(), but it might be more
>> > >> > manageable.
>> > >>
>> > >> You said that depending on HOTPLUG wouldn't be enough because it would
>> > >> exclude reenumeration at runtime if HOTPLUG wasn't defined. Also it is
>> > >> theoretically possible to build a kernel without HOTPLUG but have the
>> > >> enumeration start after init because of deferred probing. Those cases
>> > >> won't work if we keep __init or __devinit respectively, right?
>> > >
>> > > Another possibility would be to make PCI select HOTPLUG or depend on it.
>> > > That way it would be made sure that __devinit wouldn't cause all the
>> > > functions to be discarded after init.
>> >
>> > There's been some discussion recently about whether CONFIG_HOTPLUG is
>> > worth keeping any more, but nothing's been resolved yet.  If we did
>> > decide to remove CONFIG_HOTPLUG, or require it for PCI, I would rather
>> > just remove all the __devinit annotations because they'd be
>> > superfluous.
>>
>> I've missed that discussion. Can you point me to it?
>
> It's pretty much just me saying the whole thing is a mess, causes
> problems, and really doesn't solve any memory usage issues.  Ideally we
> should just:
>         - remove CONFIG_HOTPLUG and assume it is enabled
>         - because of that, we can delete the large majority of the
>           __dev* markings
>
> The memory savings these days are so tiny, if at all, and everyone,
> including me, gets it wrong all the time.
>
> As we pretty much allow anything to be disabled/enabled at any point in
> time after boot, we are all running systems that rely on CONFIG_HOTPLUG
> anyway.  To find a static system that doesn't need it is quite rare from
> what I have found.

Anyone who disables CONFIG_HOTPLUG in his defconfig files?

$ git grep CONFIG_HOTPLUG arch/*/*config
arch/frv/defconfig:# CONFIG_HOTPLUG is not set
arch/h8300/defconfig:# CONFIG_HOTPLUG is not set
arch/um/defconfig:CONFIG_HOTPLUG=y
$

Yep, (at least --- not all defconfigs are up-to-date) frv and h8300.

It seems to be hard, as it's enabled for all non-CONFIG_EXPERT (and
before that non-CONFIG_EMBEDDED) builds since Nov 2005
(full-history-linux commit 712f47cea7703a340406fde61e84eb86ce781988
 "[PATCH] HOTPLUG: always enable the .config option, unless EMBEDDED").

$ git grep CONFIG_EXPERT arch/*/*config
arch/frv/defconfig:CONFIG_EXPERT=y
arch/h8300/defconfig:CONFIG_EXPERT=y
arch/s390/defconfig:CONFIG_EXPERT=y
arch/um/defconfig:# CONFIG_EXPERT is not set
$

The same architectures, plus s390, who used to be an explicit negative
dependency before the commit above.

For a quick test, I enabled CONFIG_EXPERT and disabled CONFIG_HOTPLUG
in my Atari defconfig build, and it saved 168 bytes of code and 112 bytes of
data. Note that Atari doesn't have any hotpluggable hardware, hence no
real hotpluggable drivers.
Anyway, I'll sleep better now ;-)

FWIW, both tile and xtensa have their own duplicates of CONFIG_HOTPLUG:

$ git grep -w 'config HOTPLUG' -- "*Kconf*"
arch/tile/Kconfig:config HOTPLUG
arch/xtensa/Kconfig:config HOTPLUG
init/Kconfig:config HOTPLUG
$

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: PCI Section mismatch error in linux-next.

[Ralf Baechle]

On Tue, Aug 21, 2012 at 04:32:45PM +0200, Geert Uytterhoeven wrote:

> Anyone who disables CONFIG_HOTPLUG in his defconfig files?
> 
> $ git grep CONFIG_HOTPLUG arch/*/*config
> arch/frv/defconfig:# CONFIG_HOTPLUG is not set
> arch/h8300/defconfig:# CONFIG_HOTPLUG is not set
> arch/um/defconfig:CONFIG_HOTPLUG=y
> $
> 
> Yep, (at least --- not all defconfigs are up-to-date) frv and h8300.

Since we started stripping all the defconfigs down grepping through
arch/*/configs/ doesn't yield much useful information anymore :-(

There are currently 8 MIPS default configurations that dondo not enable
CONFIG_HOTPLUG.  I didn't check other architectures.

  Ralf

Re: PCI Section mismatch error in linux-next.

[Geert Uytterhoeven]

Hi Ralf,

On Tue, Aug 21, 2012 at 7:04 PM, Ralf Baechle <ralf@linux-mips.org> wrote:
>> Anyone who disables CONFIG_HOTPLUG in his defconfig files?
>>
>> $ git grep CONFIG_HOTPLUG arch/*/*config
>> arch/frv/defconfig:# CONFIG_HOTPLUG is not set
>> arch/h8300/defconfig:# CONFIG_HOTPLUG is not set
>> arch/um/defconfig:CONFIG_HOTPLUG=y
>> $
>>
>> Yep, (at least --- not all defconfigs are up-to-date) frv and h8300.
>
> Since we started stripping all the defconfigs down grepping through
> arch/*/configs/ doesn't yield much useful information anymore :-(

Sure, but since CONFIG_HOTPLUG=y and CONFIG_EXPERT=n are the
defaults, disabling CONFIG_HOTPLUG should show up in the miniconfig.

> There are currently 8 MIPS default configurations that dondo not enable
> CONFIG_HOTPLUG.  I didn't check other architectures.

Strange, how come I didn't see those?
Ah, wrong file pattern. They're stored in the "configs" subdir, not in "config".
Better list:

$ git grep -w CONFIG_HOTPLUG arch/*/*config*
arch/arm/configs/at91x40_defconfig:# CONFIG_HOTPLUG is not set
arch/arm/configs/bcmring_defconfig:# CONFIG_HOTPLUG is not set
arch/arm/configs/edb7211_defconfig:# CONFIG_HOTPLUG is not set
arch/arm/configs/footbridge_defconfig:# CONFIG_HOTPLUG is not set
arch/arm/configs/fortunet_defconfig:# CONFIG_HOTPLUG is not set
arch/arm/configs/pleb_defconfig:# CONFIG_HOTPLUG is not set
arch/blackfin/configs/IP0X_defconfig:# CONFIG_HOTPLUG is not set
arch/cris/configs/artpec_3_defconfig:# CONFIG_HOTPLUG is not set
arch/cris/configs/etrax-100lx_v2_defconfig:# CONFIG_HOTPLUG is not set
arch/cris/configs/etraxfs_defconfig:# CONFIG_HOTPLUG is not set
arch/frv/defconfig:# CONFIG_HOTPLUG is not set
arch/h8300/defconfig:# CONFIG_HOTPLUG is not set
arch/m68k/configs/m5208evb_defconfig:# CONFIG_HOTPLUG is not set
arch/m68k/configs/m5249evb_defconfig:# CONFIG_HOTPLUG is not set
arch/m68k/configs/m5272c3_defconfig:# CONFIG_HOTPLUG is not set
arch/m68k/configs/m5275evb_defconfig:# CONFIG_HOTPLUG is not set
arch/m68k/configs/m5307c3_defconfig:# CONFIG_HOTPLUG is not set
arch/m68k/configs/m5407c3_defconfig:# CONFIG_HOTPLUG is not set
arch/m68k/configs/m5475evb_defconfig:# CONFIG_HOTPLUG is not set
arch/microblaze/configs/mmu_defconfig:# CONFIG_HOTPLUG is not set
arch/microblaze/configs/nommu_defconfig:# CONFIG_HOTPLUG is not set
arch/mips/configs/db1000_defconfig:CONFIG_HOTPLUG=y
arch/mips/configs/db1300_defconfig:CONFIG_HOTPLUG=y
arch/mips/configs/db1550_defconfig:CONFIG_HOTPLUG=y
arch/mips/configs/decstation_defconfig:# CONFIG_HOTPLUG is not set
arch/mips/configs/e55_defconfig:# CONFIG_HOTPLUG is not set
arch/mips/configs/ip22_defconfig:# CONFIG_HOTPLUG is not set
arch/mips/configs/ip28_defconfig:# CONFIG_HOTPLUG is not set
arch/mips/configs/jmr3927_defconfig:# CONFIG_HOTPLUG is not set
arch/mips/configs/lasat_defconfig:# CONFIG_HOTPLUG is not set
arch/mips/configs/pnx8550-stb810_defconfig:# CONFIG_HOTPLUG is not set
arch/mips/configs/rbtx49xx_defconfig:# CONFIG_HOTPLUG is not set
arch/mn10300/configs/asb2303_defconfig:# CONFIG_HOTPLUG is not set
arch/powerpc/configs/85xx/socrates_defconfig:# CONFIG_HOTPLUG is not set
arch/powerpc/configs/85xx/tqm8540_defconfig:# CONFIG_HOTPLUG is not set
arch/powerpc/configs/85xx/tqm8541_defconfig:# CONFIG_HOTPLUG is not set
arch/powerpc/configs/85xx/tqm8555_defconfig:# CONFIG_HOTPLUG is not set
arch/powerpc/configs/85xx/tqm8560_defconfig:# CONFIG_HOTPLUG is not set
arch/powerpc/configs/mpc866_ads_defconfig:# CONFIG_HOTPLUG is not set
arch/score/configs/spct6600_defconfig:# CONFIG_HOTPLUG is not set
arch/sh/configs/edosk7705_defconfig:# CONFIG_HOTPLUG is not set
arch/sh/configs/se7619_defconfig:# CONFIG_HOTPLUG is not set
arch/sh/configs/se7705_defconfig:# CONFIG_HOTPLUG is not set
arch/sh/configs/se7750_defconfig:# CONFIG_HOTPLUG is not set
arch/sh/configs/se7751_defconfig:# CONFIG_HOTPLUG is not set
arch/sh/configs/se7780_defconfig:# CONFIG_HOTPLUG is not set
arch/sh/configs/secureedge5410_defconfig:# CONFIG_HOTPLUG is not set
arch/sh/configs/shmin_defconfig:# CONFIG_HOTPLUG is not set
arch/um/defconfig:CONFIG_HOTPLUG=y
arch/unicore32/configs/unicore32_defconfig:CONFIG_HOTPLUG=y
arch/xtensa/configs/common_defconfig:# CONFIG_HOTPLUG is not set
arch/xtensa/configs/iss_defconfig:# CONFIG_HOTPLUG is not set
arch/xtensa/configs/s6105_defconfig:# CONFIG_HOTPLUG is not set
$

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds