| Submitter | Yinghai Lu |
|---|---|
| Date | 2009-10-29 09:52:00 |
| Message ID | <4AE965C0.30206@kernel.org> |
| Download | mbox | patch |
| Permalink | /patch/56437/ |
| State | Superseded |
| Headers | show |
Comments
On Thu, 29 Oct 2009 02:52:00 -0700 Yinghai Lu <yinghai@kernel.org> wrote: > > only for index < 3 > > v2: Jesse doesn't like it is in find_free_bus_resource... > try to move out of pci_bus_size_bridges loop. > need to apply after: > [PATCH] pci: pciehp update the slot bridge res to get big > range for pcie devices - v4 v3: add pci_setup_bridge calling after > pci_bridge_release_not_used_res. only clear release those res for x86. > v4: Bjorn want to release use dev instead of bus. > v5: Kenji pointed out it will have problem with several level bridge. > so let only handle leaf bridge. > v6: address Kenji's request (new pci_bus_release...). and change > applying order move back release to pci_assign_unassigned_resource > > Signed-off-by: Yinghai Lu <yinghai@kernel.org> Starting to look better... > > --- > drivers/pci/setup-bus.c | 65 > +++++++++++++++++++++++++++++++++++++++++++++++- > include/linux/pci.h | 1 2 files changed, 65 insertions(+), 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 > @@ -573,13 +573,68 @@ void __ref pci_bus_assign_resources(cons > } > EXPORT_SYMBOL(pci_bus_assign_resources); > > +static void pci_bridge_release_not_used_res(struct pci_bus *bus) Can we call these functions something else? Maybe pci_bridge_release_unused() with a kdoc comment describing exactly what it does and why? > +{ > + int idx; > + bool changed = false; > + struct pci_dev *dev; > + struct resource *r; > + unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM | > + IORESOURCE_PREFETCH; > + > + /* for pci bridges res only */ > + dev = bus->self; > + for (idx = PCI_BRIDGE_RESOURCES; idx < PCI_BRIDGE_RESOURCES > + 3; > + idx++) { > + r = &dev->resource[idx]; > + if (!(r->flags & type_mask)) > + continue; > + if (!r->parent) > + continue; > + /* if there is no child under that, we should > release it */ > + if (r->child) > + continue; > + if (!release_resource(r)) { > + dev_info(&dev->dev, "resource %d %pRt > released\n", > + idx, r); > + r->flags = 0; > + changed = true; > + } > + } > + > + if (changed) > + pci_setup_bridge(bus); > +} > + > +void __ref pci_bus_release_bridges_not_used_res(struct pci_bus *bus) Likewise, maybe pci_bus_release_unused_bridge_res or something, with comments again. > +{ > + struct pci_bus *b; > + > + /* If the bus is cardbus, do nothing */ > + if (bus->self && (bus->self->class >> 8) == > PCI_CLASS_BRIDGE_CARDBUS) > + return; > + > + /* We only release the resources under the leaf bridge */ > + if (list_empty(&bus->children)) { > + if (!pci_is_root_bus(bus)) > + pci_bridge_release_not_used_res(bus); > + return; > + } > + > + /* Scan child buses if the bus is not leaf */ > + list_for_each_entry(b, &bus->children, node) > + pci_bus_release_bridges_not_used_res(b); > +} > +EXPORT_SYMBOL(pci_bus_release_bridges_not_used_res); > + > static void pci_bus_dump_res(struct pci_bus *bus) > { > int i; > > for (i = 0; i < PCI_BUS_NUM_RESOURCES; i++) { > struct resource *res = bus->resource[i]; > - if (!res || !res->end) > + > + if (!res || !res->end || !res->flags) > continue; > > dev_printk(KERN_DEBUG, &bus->dev, "resource %d > %pRt\n", i, res); @@ -608,6 +663,14 @@ > pci_assign_unassigned_resources(void) { > struct pci_bus *bus; > > + /* > + * Try to release leaf bridge's resources that there is not > child > + * under it > + */ > + list_for_each_entry(bus, &pci_root_buses, node) { > + pci_bus_release_bridges_not_used_res(bus); > + } > + Also I wonder if we need to make this a command line option that isn't run by default? Really, as Eric says, we need a real dynamic allocation system with the ability to move devices around at some point. Any volunteers? :)
Jesse Barnes wrote: > On Thu, 29 Oct 2009 02:52:00 -0700 > Yinghai Lu <yinghai@kernel.org> wrote: > >> only for index < 3 >> >> v2: Jesse doesn't like it is in find_free_bus_resource... >> try to move out of pci_bus_size_bridges loop. >> need to apply after: >> [PATCH] pci: pciehp update the slot bridge res to get big >> range for pcie devices - v4 v3: add pci_setup_bridge calling after >> pci_bridge_release_not_used_res. only clear release those res for x86. >> v4: Bjorn want to release use dev instead of bus. >> v5: Kenji pointed out it will have problem with several level bridge. >> so let only handle leaf bridge. >> v6: address Kenji's request (new pci_bus_release...). and change >> applying order move back release to pci_assign_unassigned_resource >> >> Signed-off-by: Yinghai Lu <yinghai@kernel.org> > > Starting to look better... > >> --- >> drivers/pci/setup-bus.c | 65 >> +++++++++++++++++++++++++++++++++++++++++++++++- >> include/linux/pci.h | 1 2 files changed, 65 insertions(+), 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 >> @@ -573,13 +573,68 @@ void __ref pci_bus_assign_resources(cons >> } >> EXPORT_SYMBOL(pci_bus_assign_resources); >> >> +static void pci_bridge_release_not_used_res(struct pci_bus *bus) > > Can we call these functions something else? Maybe > pci_bridge_release_unused() with a kdoc comment describing exactly what > it does and why? ok. > >> +{ >> + int idx; >> + bool changed = false; >> + struct pci_dev *dev; >> + struct resource *r; >> + unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM | >> + IORESOURCE_PREFETCH; >> + >> + /* for pci bridges res only */ >> + dev = bus->self; >> + for (idx = PCI_BRIDGE_RESOURCES; idx < PCI_BRIDGE_RESOURCES >> + 3; >> + idx++) { >> + r = &dev->resource[idx]; >> + if (!(r->flags & type_mask)) >> + continue; >> + if (!r->parent) >> + continue; >> + /* if there is no child under that, we should >> release it */ >> + if (r->child) >> + continue; >> + if (!release_resource(r)) { >> + dev_info(&dev->dev, "resource %d %pRt >> released\n", >> + idx, r); >> + r->flags = 0; >> + changed = true; >> + } >> + } >> + >> + if (changed) >> + pci_setup_bridge(bus); >> +} >> + >> +void __ref pci_bus_release_bridges_not_used_res(struct pci_bus *bus) > > Likewise, maybe pci_bus_release_unused_bridge_res or something, with > comments again. ok > >> +{ >> + struct pci_bus *b; >> + >> + /* If the bus is cardbus, do nothing */ >> + if (bus->self && (bus->self->class >> 8) == >> PCI_CLASS_BRIDGE_CARDBUS) >> + return; >> + >> + /* We only release the resources under the leaf bridge */ >> + if (list_empty(&bus->children)) { >> + if (!pci_is_root_bus(bus)) >> + pci_bridge_release_not_used_res(bus); >> + return; >> + } >> + >> + /* Scan child buses if the bus is not leaf */ >> + list_for_each_entry(b, &bus->children, node) >> + pci_bus_release_bridges_not_used_res(b); >> +} >> +EXPORT_SYMBOL(pci_bus_release_bridges_not_used_res); >> + >> static void pci_bus_dump_res(struct pci_bus *bus) >> { >> int i; >> >> for (i = 0; i < PCI_BUS_NUM_RESOURCES; i++) { >> struct resource *res = bus->resource[i]; >> - if (!res || !res->end) >> + >> + if (!res || !res->end || !res->flags) >> continue; >> >> dev_printk(KERN_DEBUG, &bus->dev, "resource %d >> %pRt\n", i, res); @@ -608,6 +663,14 @@ >> pci_assign_unassigned_resources(void) { >> struct pci_bus *bus; >> >> + /* >> + * Try to release leaf bridge's resources that there is not >> child >> + * under it >> + */ >> + list_for_each_entry(bus, &pci_root_buses, node) { >> + pci_bus_release_bridges_not_used_res(bus); >> + } >> + > > Also I wonder if we need to make this a command line option that isn't > run by default? thinking: 1. try not to touch bridge resource and do the allocation and record the fail device 2. release leaf bridge for those failed device 3. do second around bus_size and allocation. > > Really, as Eric says, we need a real dynamic allocation system with the > ability to move devices around at some point. Any volunteers? :) that will need suspend driver and rescan allocation and wakeup driver... YH -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 29 Oct 2009 10:10:22 -0700 Yinghai Lu <yinghai@kernel.org> wrote: > > Also I wonder if we need to make this a command line option that > > isn't run by default? > > thinking: > 1. try not to touch bridge resource and do the allocation and record > the fail device 2. release leaf bridge for those failed device > 3. do second around bus_size and allocation. Yeah, that would make it more automatic at least, which would be nice. > > Really, as Eric says, we need a real dynamic allocation system with > > the ability to move devices around at some point. Any > > volunteers? :) > > that will need suspend driver and rescan allocation and wakeup > driver... Right, it's fairly invasive. But it's the right direction to go long term.
Patch
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 @@ -573,13 +573,68 @@ void __ref pci_bus_assign_resources(cons } EXPORT_SYMBOL(pci_bus_assign_resources); +static void pci_bridge_release_not_used_res(struct pci_bus *bus) +{ + int idx; + bool changed = false; + struct pci_dev *dev; + struct resource *r; + unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM | + IORESOURCE_PREFETCH; + + /* for pci bridges res only */ + dev = bus->self; + for (idx = PCI_BRIDGE_RESOURCES; idx < PCI_BRIDGE_RESOURCES + 3; + idx++) { + r = &dev->resource[idx]; + if (!(r->flags & type_mask)) + continue; + if (!r->parent) + continue; + /* if there is no child under that, we should release it */ + if (r->child) + continue; + if (!release_resource(r)) { + dev_info(&dev->dev, "resource %d %pRt released\n", + idx, r); + r->flags = 0; + changed = true; + } + } + + if (changed) + pci_setup_bridge(bus); +} + +void __ref pci_bus_release_bridges_not_used_res(struct pci_bus *bus) +{ + struct pci_bus *b; + + /* If the bus is cardbus, do nothing */ + if (bus->self && (bus->self->class >> 8) == PCI_CLASS_BRIDGE_CARDBUS) + return; + + /* We only release the resources under the leaf bridge */ + if (list_empty(&bus->children)) { + if (!pci_is_root_bus(bus)) + pci_bridge_release_not_used_res(bus); + return; + } + + /* Scan child buses if the bus is not leaf */ + list_for_each_entry(b, &bus->children, node) + pci_bus_release_bridges_not_used_res(b); +} +EXPORT_SYMBOL(pci_bus_release_bridges_not_used_res); + static void pci_bus_dump_res(struct pci_bus *bus) { int i; for (i = 0; i < PCI_BUS_NUM_RESOURCES; i++) { struct resource *res = bus->resource[i]; - if (!res || !res->end) + + if (!res || !res->end || !res->flags) continue; dev_printk(KERN_DEBUG, &bus->dev, "resource %d %pRt\n", i, res); @@ -608,6 +663,14 @@ pci_assign_unassigned_resources(void) { struct pci_bus *bus; + /* + * Try to release leaf bridge's resources that there is not child + * under it + */ + list_for_each_entry(bus, &pci_root_buses, node) { + pci_bus_release_bridges_not_used_res(bus); + } + /* Depth first, calculate sizes and alignments of all subordinate buses. */ list_for_each_entry(bus, &pci_root_buses, node) { Index: linux-2.6/include/linux/pci.h =================================================================== --- linux-2.6.orig/include/linux/pci.h +++ linux-2.6/include/linux/pci.h @@ -758,6 +758,7 @@ int pci_vpd_truncate(struct pci_dev *dev /* Helper functions for low-level code (drivers/pci/setup-[bus,res].c) */ void pci_bus_assign_resources(const struct pci_bus *bus); void pci_bus_size_bridges(struct pci_bus *bus); +void pci_bus_release_bridges_not_used_res(struct pci_bus *bus); int pci_claim_resource(struct pci_dev *, int); void pci_assign_unassigned_resources(void); void pdev_enable_device(struct pci_dev *);
only for index < 3 v2: Jesse doesn't like it is in find_free_bus_resource... try to move out of pci_bus_size_bridges loop. need to apply after: [PATCH] pci: pciehp update the slot bridge res to get big range for pcie devices - v4 v3: add pci_setup_bridge calling after pci_bridge_release_not_used_res. only clear release those res for x86. v4: Bjorn want to release use dev instead of bus. v5: Kenji pointed out it will have problem with several level bridge. so let only handle leaf bridge. v6: address Kenji's request (new pci_bus_release...). and change applying order move back release to pci_assign_unassigned_resource Signed-off-by: Yinghai Lu <yinghai@kernel.org> --- drivers/pci/setup-bus.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++- include/linux/pci.h | 1 2 files changed, 65 insertions(+), 1 deletion(-) -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html