On Fri, Sep 21, 2012 at 1:14 AM, Taku Izumi <izumi.taku@xxxxxxxxxxxxxx> wrote:
> On Wed, 19 Sep 2012 16:03:27 -0600
> Bjorn Helgaas <bhelgaas@xxxxxxxxxx> wrote:
>
>> On Tue, Sep 18, 2012 at 12:23 AM, Taku Izumi <izumi.taku@xxxxxxxxxxxxxx> wrote:
>> >
>> > This patch changes the implementation of acpi_pci_find_root().
>> >
>> > We can access acpi_pci_root without scanning acpi_pci_roots list.
>> > If hostbridge hotplug is supported, acpi_pci_roots list will be
>> > protected by mutex. We should not access acpi_pci_roots list
>> > if preventable to lessen deadlock risk.
>> >
>> > Signed-off-by: Taku Izumi <izumi.taku@xxxxxxxxxxxxxx>
>> > ---
>> > drivers/acpi/pci_root.c | 13 ++++++++-----
>> > 1 file changed, 8 insertions(+), 5 deletions(-)
>> >
>> > Index: Bjorn-next-0903/drivers/acpi/pci_root.c
>> > ===================================================================
>> > --- Bjorn-next-0903.orig/drivers/acpi/pci_root.c
>> > +++ Bjorn-next-0903/drivers/acpi/pci_root.c
>> > @@ -265,12 +265,15 @@ static acpi_status acpi_pci_osc_support(
>> > struct acpi_pci_root *acpi_pci_find_root(acpi_handle handle)
>> > {
>> > struct acpi_pci_root *root;
>> > + struct acpi_device *device;
>> >
>> > - list_for_each_entry(root, &acpi_pci_roots, node) {
>> > - if (root->device->handle == handle)
>> > - return root;
>> > - }
>> > - return NULL;
>> > + if (acpi_bus_get_device(handle, &device) ||
>> > + acpi_match_device_ids(device, root_device_ids))
>>
>> What's the purpose of the acpi_match_device_ids() check? It's not
>> obvious, so worth calling it out in the changelog, and maybe even a
>> comment in the code.
>
> My intention is to reject acpi_handle which doesn't represent
> hostbrige. I think this is reasonable...
Yes, I understand that part. I was just trying to figure out why that
check is needed. Is there a path where we can call
acpi_pci_find_root() with a handle that is *not* a PNP0A03 device? I
looked at all the callers, and as far as I can tell, the supplied
handle is always for a host bridge device.
But I guess I don't object to the check. This is just another case of
a lookup that in many cases is unnecessary because we should have the
struct acpi_pci_root * available already.
>> Nice to get rid of the list traversal.
>>
>> > + return NULL;
>> > +
>> > + root = acpi_driver_data(device);
>> > +
>> > + return root;
>> > }
>> > EXPORT_SYMBOL_GPL(acpi_pci_find_root);
>> >
>> >
>>
>
>
> --
> Taku Izumi <izumi.taku@xxxxxxxxxxxxxx>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
