| 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
| ||
|
Message-Id: <200906031046.36158.bjorn.helgaas@hp.com>
Date: Wed, 3 Jun 2009 10:46:35 -0600
From: Bjorn Helgaas <bjorn.helgaas@...com>
To: Alex Chiang <achiang@...com>
Cc: lenb@...nel.org, linux-acpi@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org
Subject: Re: [PATCH 04/10] ACPI: Introduce acpi_get_pci_dev()
On Tuesday 02 June 2009 09:24:56 am Alex Chiang wrote:
> Convert an ACPI CA handle to a struct pci_dev.
>
> Performing this lookup dynamically allows us to get rid of the
> ACPI-PCI binding code, which:
>
> - eliminates struct acpi_device vs struct pci_dev lifetime issues
> - lays more groundwork for eliminating .start from acpi_device_ops
> and thus simplifying ACPI drivers
> - whacks out a lot of code
>
> This change also changes the time-space tradeoff ever so slightly.
>
> Looking up the ACPI-PCI binding is never in the performance path, and by
> eliminating this caching, we save 24 bytes for each _ADR device in the
> ACPI namespace.
Just for future changelog readers, I think this space savings actually
occurs in the "eviscerate pci_bind.c" patch where you remove the struct
acpi_pci_data.
I definitely agree that you're making the right time/space tradeoff.
> Signed-off-by: Alex Chiang <achiang@...com>
> ---
>
> drivers/acpi/pci_bind.c | 83 +++++++++++++++++++++++++++++++++++++++++++
> include/acpi/acpi_drivers.h | 1 +
> 2 files changed, 84 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/acpi/pci_bind.c b/drivers/acpi/pci_bind.c
> index 236765c..584fa30 100644
> --- a/drivers/acpi/pci_bind.c
> +++ b/drivers/acpi/pci_bind.c
> @@ -56,6 +56,89 @@ static void acpi_pci_data_handler(acpi_handle handle, u32 function,
> return;
> }
>
> +struct acpi_handle_node {
> + struct list_head node;
> + acpi_handle handle;
> +};
> +
> +/**
> + * acpi_get_pci_dev - convert ACPI CA handle to struct pci_dev
> + * @handle: the handle in question
> + *
> + * Given an ACPI CA handle, the desired PCI device is located in the
> + * list of PCI devices.
> + *
> + * If the device is found, its reference count is increased and this
> + * function returns a pointer to its data structure. The caller must
> + * decrement the reference count by calling pci_dev_put().
> + * If no device is found, %NULL is returned.
> + */
> +struct pci_dev *acpi_get_pci_dev(acpi_handle handle)
> +{
> + int dev, fn;
> + unsigned long long adr;
> + acpi_status status;
> + acpi_handle phandle;
> + struct pci_bus *pbus;
> + struct pci_dev *pdev = NULL;
> + struct acpi_handle_node *node, *tmp;
> + struct acpi_pci_root *root;
> + LIST_HEAD(device_list);
> +
> + /*
> + * Walk up the ACPI CA namespace until we reach a PCI root bridge.
> + */
> + phandle = handle;
> + while (!acpi_is_root_bridge(phandle)) {
> + node = kzalloc(sizeof(struct acpi_handle_node), GFP_KERNEL);
> + if (!node)
> + goto out;
Since there's no cleanup to do at "out", a simple "return NULL"
here is a bit more obvious. Whoops, I think there actually *is*
a little cleanup to do -- I think we leak the list if we take
these early error exits.
> +
> + INIT_LIST_HEAD(&node->node);
> + node->handle = phandle;
> + list_add(&node->node, &device_list);
> +
> + status = acpi_get_parent(phandle, &phandle);
> + if (ACPI_FAILURE(status))
> + goto out;
> + }
> +
> + root = acpi_pci_find_root(phandle);
> + if (!root)
> + goto out;
> +
> + pbus = pci_find_bus(root->id.segment, root->id.bus);
> + if (!pbus)
> + goto out;
Isn't this simply "root->bus"?
> + /*
> + * Now, walk back down the PCI device tree until we return to our
> + * original handle. Assumes that everything between the PCI root
> + * bridge and the device we're looking for must be a P2P bridge.
> + */
> + list_for_each_entry_safe(node, tmp, &device_list, node) {
> + acpi_handle hnd = node->handle;
> + status = acpi_evaluate_integer(hnd, "_ADR", NULL, &adr);
> + if (ACPI_FAILURE(status))
> + goto out;
> + dev = (adr >> 16) & 0xffff;
> + fn = adr & 0xffff;
> +
> + list_del(&node->node);
> + kfree(node);
> +
> + pdev = pci_get_slot(pbus, PCI_DEVFN(dev, fn));
> + if (hnd == handle)
> + break;
> +
> + pbus = pdev->subordinate;
> + pci_dev_put(pdev);
> + }
> +out:
> + return pdev;
> +}
> +EXPORT_SYMBOL_GPL(acpi_get_pci_dev);
> +
> /**
> * acpi_get_pci_id
> * ------------------
> diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
> index bbe9207..1ef529b 100644
> --- a/include/acpi/acpi_drivers.h
> +++ b/include/acpi/acpi_drivers.h
> @@ -97,6 +97,7 @@ void acpi_pci_irq_del_prt(int segment, int bus);
>
> struct pci_bus;
>
> +struct pci_dev *acpi_get_pci_dev(acpi_handle);
> acpi_status acpi_get_pci_id(acpi_handle handle, struct acpi_pci_id *id);
> int acpi_pci_bind_root(struct acpi_device *device, struct acpi_pci_id *id,
> struct pci_bus *bus);
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
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/
Powered by blists - more mailing lists