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

[Arnaud Lacombe]

Hi,

[Added Adrian Bunk to the Cc: list]

2011/8/3 Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx>:
> Arnaud Lacombe wrote:
>> Looking more closely, I'd say I have been too eager in the marking.
>> The warning concern `pci_eisa_driver', not `pci_eisa_pci_tbl' which is
>> also referenced by MODULE_DEVICE_TABLE().
>>
>> Does removing the annotation from `pci_eisa_pci_tbl' fix the BUG ?
> No.
>
ok, I had to enable DEBUG_PAGEALLOC to trigger the crash.

> -static struct pci_device_id __initdata pci_eisa_pci_tbl[] = {
> +static struct pci_device_id pci_eisa_pci_tbl[] = {
>
> did not help.
>
> Printing dmesg in verbose showed below three lines.
>
> [    0.262828] EISA bus registered
> [    6.291117] EISA: Probing bus 0 at eisa.0
> [    6.295445] EISA: Cannot allocate resource for mainboard
>
> Manual printk() tracing showed below callchain.
>
> virtual_eisa_root_init() calls eisa_root_register().
>  eisa_root_register() calls eisa_probe().
>    eisa_probe() returns -EBUSY since root->force_probe == 0.
>  eisa_root_register() calls release_resource() and returns -EBUSY.
> virtual_eisa_root_init() calls platform_device_unregister() and returns -1.
>
> Also, eisa_root_register() in pci_eisa_init() is not called.
>
I'd guess you just have no PCI-EISA bridge in your VM. The trace you
point out should only indicate that you have no EISA device either, as
the comment a bit before point out:

        /* First try to get hold of slot 0. If there is no device
         * here, simply fail, unless root->force_probe is set. */


> So, it seems that something is wrong with cleanup.
> pci_unregister_driver() is missing?
>
The crash happens when sysfs tries to access the device_driver inside
'pci_eisa_driver'. I'd argue that the root of the problem is commit
74b9a297866d which created the inconsistency at the first place:

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

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,


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