On Wed, Jun 18, 2014 at 09:41:02PM -0700, Yinghai Lu wrote: > On Wed, Jun 18, 2014 at 3:40 PM, Andreas Noever > <[email protected]> wrote: > > On Wed, Jun 18, 2014 at 1:39 AM, Yinghai Lu <[email protected]> wrote: > > It seems to fix the testcase (no unwanted resources are released). But > > why do you reassign bus and not just skip the top level bridge? If one > > of the allocations below bridge failed then a resource of that device > > will be in fail_res and bridge->subordinate will get released anyways? > > Also by not removing fail_res from the list you trigger the code in > > the next loop for the top level bridge (in particular the res->flags = > > 0 line looks dangerous). > > Should not be dangerous, just second try.
I still don't understand this. Why do we set "res->flags = 0"? That clears out the resource type. Where do we figure out the type of "res" again? > > Could you explain why this function attempts to assign resources two > > times? In which scenario will a second attempt be successful? > > For example, at first mmio is assigned (by firmware), but pref mmio fails, > then before second try, mmio get cleared, then we could separate > mmio and mmio pref. So need to try again for pref mmio. > > Also I missed one MEM_64 for hotplug path. > > So we need two patches. > > Thanks > > Yinghai > Subject: [PATCH] pci: Don't release sibiling bridge resources > > On hotplug case, we should not touch sibling bridges that is out > side of the slots. > > Signed-off-by: Yinghai Lu <[email protected]> > > --- > drivers/pci/setup-bus.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > Index: linux-2.6/drivers/pci/setup-bus.c > =================================================================== > --- linux-2.6.orig/drivers/pci/setup-bus.c > +++ linux-2.6/drivers/pci/setup-bus.c > @@ -1676,10 +1676,16 @@ 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) > - pci_bus_release_bridge_resources(fail_res->dev->bus, > + list_for_each_entry(fail_res, &fail_head, list) { > + struct pci_bus *bus = fail_res->dev->bus; > + > + if (fail_res->dev == bridge) > + bus = bridge->subordinate; > + > + pci_bus_release_bridge_resources(bus, > fail_res->flags & type_mask, > whole_subtree); > + } > > /* restore size and flags */ > list_for_each_entry(fail_res, &fail_head, list) { > Subject: [PATCH] pci: Add back missing MEM_64 check for hotplug path > > We miss that in > | commit 5b28541552ef5eeffc41d6936105f38c2508e566 > | PCI: Restrict 64-bit prefetchable bridge windows to 64-bit resources > for pci hotplug path. This changelog is useless. I don't have time to spend a few hours figuring out why we want this change. > Signed-off-by: Yinghai Lu <[email protected]> > > --- > drivers/pci/setup-bus.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > Index: linux-2.6/drivers/pci/setup-bus.c > =================================================================== > --- linux-2.6.orig/drivers/pci/setup-bus.c > +++ linux-2.6/drivers/pci/setup-bus.c > @@ -1652,7 +1652,7 @@ void pci_assign_unassigned_bridge_resour > struct pci_dev_resource *fail_res; > int retval; > unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM | > - IORESOURCE_PREFETCH; > + IORESOURCE_PREFETCH | IORESOURCE_MEM_64; > > again: > __pci_bus_size_bridges(parent, &add_list); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
