linux-kernel - Re: [PATCH] ACPI / PCI: Initialize PCI root drivers after PCI bus enumeration

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAE9FiQXk1VhA2+qAcp8jL=TG28acePyX4SOYALiAQ0bGnLaFOA@mail.gmail.com>
Date:	Tue, 12 Feb 2013 17:54:08 -0800
From:	Yinghai Lu <yinghai@...nel.org>
To:	"Rafael J. Wysocki" <rjw@...k.pl>
Cc:	ACPI Devel Maling List <linux-acpi@...r.kernel.org>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	LKML <linux-kernel@...r.kernel.org>,
	Jiang Liu <liuj97@...il.com>, Toshi Kani <toshi.kani@...com>,
	Yasuaki Ishimatsu <isimatu.yasuaki@...fujitsu.com>,
	Myron Stowe <mstowe@...hat.com>, linux-pci@...r.kernel.org
Subject: Re: [PATCH] ACPI / PCI: Initialize PCI root drivers after PCI bus enumeration

On Tue, Feb 12, 2013 at 4:16 PM, Rafael J. Wysocki <rjw@...k.pl> wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
>
> After commit 1aeae82 (ACPI / PCI: avoid building pci_slot as module)
> the pci_slot driver's .add() callback routine, acpi_pci_slot_add(),
> is executed from within acpi_pci_root_add() before the PCI devices on
> the bus are enumerated and that triggers the WARN_ON() in
> kobject_get():
>
> WARNING: at /scratch/rafael/work/test/include/linux/kref.h:42 kobject_get+0x33/0x40()
> Hardware name: PORTEGE R500
> Modules linked in:
> Pid: 1, comm: swapper/0 Not tainted 3.8.0-rc7+ #160
> Call Trace:
>  [<ffffffff8103d38a>] warn_slowpath_common+0x7a/0xb0
>  [<ffffffff8103d3d5>] warn_slowpath_null+0x15/0x20
>  [<ffffffff812f99b3>] kobject_get+0x33/0x40
>  [<ffffffff81452b82>] get_device+0x12/0x30
>  [<ffffffff8138df7b>] register_slot+0x243/0x295
>  [<ffffffff810a7b6d>] ? trace_hardirqs_on+0xd/0x10
>  [<ffffffff8137b7b9>] acpi_ns_walk_namespace+0x10f/0x24a
>  [<ffffffff8138dd38>] ? walk_p2p_bridge+0x13b/0x13b
>  [<ffffffff8138dd38>] ? walk_p2p_bridge+0x13b/0x13b
>  [<ffffffff8137bdc3>] acpi_walk_namespace+0xe9/0x132
>  [<ffffffff8138dd38>] ? walk_p2p_bridge+0x13b/0x13b
>  [<ffffffff8138dcf5>] walk_p2p_bridge+0xf8/0x13b
>  [<ffffffff8134d606>] ? acpi_os_signal_semaphore+0x76/0x87
>  [<ffffffff8138dd38>] ? walk_p2p_bridge+0x13b/0x13b
>  [<ffffffff8137b7b9>] acpi_ns_walk_namespace+0x10f/0x24a
>  [<ffffffff8138dbfd>] ? do_sta_before_sun+0x2b/0x2b
>  [<ffffffff8138dbfd>] ? do_sta_before_sun+0x2b/0x2b
>  [<ffffffff8137bdc3>] acpi_walk_namespace+0xe9/0x132
>  [<ffffffff81356071>] ? acpi_pci_root_add+0x3e7/0x49a
>  [<ffffffff8138e07f>] acpi_pci_slot_add+0xb2/0x103
>  [<ffffffff8138dd38>] ? walk_p2p_bridge+0x13b/0x13b
>  [<ffffffff81356089>] acpi_pci_root_add+0x3ff/0x49a
>
> which means that pci_bus->dev used in register_slot() has not been
> registered yet at that point.
>
> To fix this use the observation that before commit 1aeae82
> acpi_pci_slot_add() was always run after pci_bus_add_devices()
> and that happened to the acpiphp's .add() callback routine too.
> Thus it is safe to reorder acpi_pci_root_add() to make the PCI root
> drivers' .add() callbacks be run after pci_bus_add_devices(), so do
> that.
>
> This approach was previously proposed by Myron Stowe.
>
> References: https://patchwork.kernel.org/patch/1848781/
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> ---
>
> The commit mentioned in the changelog above is in linux-pm.git/linux-next.
>
> Thanks,
> Rafael
>
> ---
>  drivers/acpi/pci_root.c |   13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
>
> Index: test/drivers/acpi/pci_root.c
> ===================================================================
> --- test.orig/drivers/acpi/pci_root.c
> +++ test/drivers/acpi/pci_root.c
> @@ -600,17 +600,20 @@ static int acpi_pci_root_add(struct acpi
>         if (system_state != SYSTEM_BOOTING)
>                 pci_assign_unassigned_bus_resources(root->bus);
>
> +       /* need to after hot-added ioapic is registered */
> +       if (system_state != SYSTEM_BOOTING)
> +               pci_enable_bridges(root->bus);
> +
> +       pci_bus_add_devices(root->bus);
> +
>         mutex_lock(&acpi_pci_root_lock);
> +
>         list_for_each_entry(driver, &acpi_pci_drivers, node)
>                 if (driver->add)
>                         driver->add(root);
> -       mutex_unlock(&acpi_pci_root_lock);
>
> -       /* need to after hot-added ioapic is registered */
> -       if (system_state != SYSTEM_BOOTING)
> -               pci_enable_bridges(root->bus);
> +       mutex_unlock(&acpi_pci_root_lock);
>
> -       pci_bus_add_devices(root->bus);
>         return 1;
>
>  out_del_root:
>

No, we don't need this after

| commit 4f535093cf8f6da8cfda7c36c2c1ecd2e9586ee4
|     PCI: Put pci_dev in device tree as early as possible

in pci/next.

So we can move that Jiang's patch from your tree to Bjorn's tree?

Thanks

Yinghai
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/