[+cc Yinghai]
On Mon, Jun 09, 2014 at 10:45:30PM +0200, Andreas Noever wrote:
> pci_assign_unassigned_bridge_resources is used to assign resources below
> a hotplug bridge. If the first attempt fails it will release some
> resources and try again. If a resource allocation on the hotplug bridge
> itself fails then pci_assign_unassigned_bridge_resources will invoke
> pci_bus_release_bridge_resources on the parent bus of the hotplug
> bridge. This in turn will release resources assigned to siblings of the
> hotplug bridge which may already be in use.
>
> This patch checks for this case and prevents
> pci_bus_release_bridge_resources to be invoked on the parent bus.
>
> The problem can be reproduced by having two sibling hotplug bridges A
> and B. The problem will occour if the parent of A and B does not have
> enough resources to satisfy window allocations for B during a hotplug
> event.
>
> Signed-off-by: Andreas Noever <andreas.noever@xxxxxxxxx>
> ---
>
> I must admit that I do not fully understand how
> pci_assign_unassigned_bridge_resources works. Under which scenario would the
> second allocation attempt be successful?
I don't understand how all this works either. Yinghai?
We definitely don't want to release resources that are already in use. Can
you review and ack or nack this?
> drivers/pci/setup-bus.c | 21 +++++++++++++++++++--
> 1 file changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index 138bdd6..2e418d6 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -1560,6 +1560,12 @@ void __init pci_assign_unassigned_resources(void)
> pci_assign_unassigned_root_bus_resources(root_bus);
> }
>
> +/**
> + * pci_assign_unassigned_bridge_resources - greenfield resource allocation
> + *
> + * Try to assign io and memory resources on and below @bridge. The caller must
> + * ensure that no device below @bridge is active.
> + */
> void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge)
> {
> struct pci_bus *parent = bridge->subordinate;
> @@ -1567,7 +1573,7 @@ void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge)
> want additional resources */
> int tried_times = 0;
> LIST_HEAD(fail_head);
> - struct pci_dev_resource *fail_res;
> + struct pci_dev_resource *fail_res, *tmp;
> int retval;
> unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM |
> IORESOURCE_PREFETCH;
> @@ -1594,10 +1600,21 @@ again:
> * Try to release leaf bridge's resources that doesn't fit resource of
> * child device under that bridge
> */
> - list_for_each_entry(fail_res, &fail_head, list)
> + list_for_each_entry_safe(fail_res, tmp, &fail_head, list) {
> + /*
> + * The allocation of the mem/io window of the top level bridge
> + * can fail. Without the following check we would release our
> + * siblings' resources.
> + */
> + if (fail_res->dev == bridge) {
> + list_del(&fail_res->list);
> + kfree(fail_res);
> + continue;
> + }
> pci_bus_release_bridge_resources(fail_res->dev->bus,
> fail_res->flags & type_mask,
> whole_subtree);
> + }
>
> /* restore size and flags */
> list_for_each_entry(fail_res, &fail_head, list) {
> --
> 2.0.0
>
--
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
