Re: [3.0-git16] Oops at driver_uevent_store().

[Arnaud Lacombe]

Hi,

On Thu, Aug 4, 2011 at 4:40 AM, Adrian Bunk <bunk@xxxxxxxxxx> wrote:
> On Thu, Aug 04, 2011 at 01:47:11AM -0400, Arnaud Lacombe wrote:
>> Hi,
>>
>> [Added Adrian Bunk to the Cc: list]
>
> Thanks.
>
>> 2011/8/3 Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx>:
>>...
>> commit 74b9a297866d0416edd0be5014cb0810de049c6a
>> Author: Adrian Bunk <bunk@xxxxxxxxx>
>> Date:   Mon Mar 26 21:32:27 2007 -0800
>>
>>     [PATCH] drivers/eisa/pci_eisa.c:pci_eisa_init() should be init
>>
>>       WARNING: drivers/built-in.o - Section mismatch: reference to
>> .init.text:eisa_root_register from .text between 'pci_eisa_init' (at
>> offset 0xabf670) and 'virtual_eisa_release'
>>
>>     AFAIK a PCI to EISA bridge isn't anything hotpluggable, so
>>     pci_eisa_init() can become __init.
>>
>>     Signed-off-by: Adrian Bunk <bunk@xxxxxxxxx>
>>     Cc: Greg KH <greg@xxxxxxxxx>
>>     Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
>>     Signed-off-by: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
>>
>>
>> To gain a few bytes, 'pci_eisa_init' is freed, but its reference is
>> kept in `pci_eisa_driver' which triggers the warning. The commit log
>> points out that PCI to EISA should not be hotpluggable, so it is not
>> expected to be ever executed again (which would trigger a crash).
>
> This was not for gaining a few bytes, and I'm not exactly seeing why I'm
> to blame here, since my patch is invariant to what your patch does:
>
> Calling pci_eisa_init outside of __init:
> - might have crashed before my patch and
> - after my patch and
> - after your patch
> since eisa_root_register() is __init.
>
>> Marking `pci_eisa_driver' as __refdata fix the warning and leave the
>> struct untouched:
>>
>> diff --git a/drivers/eisa/pci_eisa.c b/drivers/eisa/pci_eisa.c
>> index 30da70d..cdae207 100644
>> --- a/drivers/eisa/pci_eisa.c
>> +++ b/drivers/eisa/pci_eisa.c
>> @@ -45,13 +45,13 @@ static int __init pci_eisa_init(struct pci_dev *pdev,
>>         return 0;
>>  }
>>
>> -static struct pci_device_id __initdata pci_eisa_pci_tbl[] = {
>> +static struct pci_device_id pci_eisa_pci_tbl[] = {
>>         { PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
>>           PCI_CLASS_BRIDGE_EISA << 8, 0xffff00, 0 },
>>         { 0, }
>>  };
>>
>> -static struct pci_driver __initdata pci_eisa_driver = {
>> +static struct pci_driver __refdata pci_eisa_driver = {
>>         .name           = "pci_eisa",
>>         .id_table       = pci_eisa_pci_tbl,
>>         .probe          = pci_eisa_init,
>
> So the commit that broke it was not mine, but
>
> commit 005bdad7b80ac017ca21d795639d4214b9844a84
> Author: Arnaud Lacombe <lacombar@xxxxxxxxx>
> Date:   Mon Jul 25 17:13:04 2011 -0700
>
>    eisa/pci_eisa.c: fix section mismatch
>
>    Fixes
>
>      WARNING: vmlinux.o(.data+0x15d3ac): Section mismatch in reference from the
>      The variable pci_eisa_driver references the function __init pci_eisa_init(
>      If the reference is valid then annotate the variable with __init* or __ref
>      *_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console
>
>    Signed-off-by: Arnaud Lacombe <lacombar@xxxxxxxxx>
>    Cc: Greg KH <greg@xxxxxxxxx>
>    Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
>    Signed-off-by: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
>
>
>
> Please don't blame me for bugs you introduced.
>
I was merely pointing out the modpost warning, but yes, I broke the stuff.

 - Arnaud

>
>>  - Arnaud
>
> cu
> Adrian
>
> --
>
>       "Is there not promise of rain?" Ling Tan asked suddenly out
>        of the darkness. There had been need of rain for many days.
>       "Only a promise," Lao Er said.
>                                       Pearl S. Buck - Dragon Seed
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/