On Tue, Jan 18, 2011 at 01:52:06PM -0700, Bjorn Helgaas wrote: > On Friday, January 14, 2011 11:19:15 am Ram Pai wrote: > > PCI: reserve additional resources for hotplug bridges > > only after all essential resources are allocated. > > > > Linux tries to reserve minimal resources for hotplug > > bridges. This works fine as long as there are enough > > resources to satisfy all other essential resource > > requirements. However if enough resources are not > > available to satisfy both the essential resources > > and the reservation for hotplug resources, the > > resource-allocator reports error and returns failure. > > > > This patch distinguishes between essential resources and > > nice-to-have resources. Any failure to allocate > > nice-to-have resources are ignored. > > > > This behavior can be particularly useful to trigger > > automatic reallocation, if the OS discovers genuine > > resource allocation conflicts or genuine unallocated > > requests caused by not-so-smart allocation behavior of > > the native BIOS. > > > > https://bugzilla.kernel.org/show_bug.cgi?id=15960 > > captures the details of the issue and the motivation > > behind this patch. > > Please don't indent and right-justify the changelog. Ooops. I did not see your comments earlier. I just out a patch without looking at your comments. > > This is a PCI-specific issue, so I'm not comfortable adding add_size > to struct resource. That penalizes all resource users while only > helping PCI. Also, the struct resource lives forever, and the > add_size information is only useful while we're configuring the > bridge. Any suggestion on how to do this without adding a field to struct resource? RP > > Bjorn > > > Signed-off-by: Ram Pai <linuxram@xxxxxxxxxx> > > > > drivers/pci/setup-bus.c | 142 +++++++++++++++++++++++++++++++++++------------- > > include/linux/ioport.h | 1 > > 2 files changed, 105 insertions(+), 38 deletions(-) > > > > diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c > > index 66cb8f4..76a7fb7 100644 > > --- a/drivers/pci/setup-bus.c > > +++ b/drivers/pci/setup-bus.c > > @@ -91,7 +91,26 @@ static void __dev_sort_resources(struct pci_dev *dev, > > pdev_sort_resources(dev, head); > > } > > > > -static void __assign_resources_sorted(struct resource_list *head, > > + > > +static void adjust_resources_sorted(struct resource_list *head) > > +{ > > + struct resource *res; > > + struct resource_list *list, *tmp; > > + int idx; > > + > > + for (list = head->next; list;) { > > + res = list->res; > > + idx = res - &list->dev->resource[0]; > > + if (res->add_size) > > + adjust_resource(res, res->start, > > + resource_size(res) + res->add_size); > > + tmp = list; > > + list = list->next; > > + kfree(tmp); > > + } > > +} > > + > > +static void assign_requested_resources_sorted(struct resource_list *head, > > struct resource_list_x *fail_head) > > { > > struct resource *res; > > @@ -114,14 +133,25 @@ static void __assign_resources_sorted(struct resource_list *head, > > } > > res->start = 0; > > res->end = 0; > > + res->add_size = 0; > > res->flags = 0; > > } > > tmp = list; > > list = list->next; > > - kfree(tmp); > > } > > } > > > > +static void __assign_resources_sorted(struct resource_list *head, > > + struct resource_list_x *fail_head) > > +{ > > + /* Satisfy the must-have resource requests */ > > + assign_requested_resources_sorted(head, fail_head); > > + > > + /* Try to satisfy any additional nice-to-have resource > > + requests */ > > + adjust_resources_sorted(head); > > +} > > + > > static void pdev_assign_resources_sorted(struct pci_dev *dev, > > struct resource_list_x *fail_head) > > { > > @@ -404,15 +434,37 @@ static struct resource *find_free_bus_resource(struct pci_bus *bus, unsigned lon > > return NULL; > > } > > > > +static resource_size_t calculate_iosize(resource_size_t size, > > + resource_size_t min_size, > > + resource_size_t size1, > > + resource_size_t old_size, > > + resource_size_t align) > > +{ > > + if (size < min_size) > > + size = min_size; > > + if (old_size == 1 ) > > + old_size = 0; > > + /* To be fixed in 2.5: we should have sort of HAVE_ISA > > + flag in the struct pci_bus. */ > > +#if defined(CONFIG_ISA) || defined(CONFIG_EISA) > > + size = (size & 0xff) + ((size & ~0xffUL) << 2); > > +#endif > > + size = ALIGN(size + size1, align); > > + if (size < old_size) > > + size = old_size; > > + return size; > > +} > > + > > /* Sizing the IO windows of the PCI-PCI bridge is trivial, > > since these windows have 4K granularity and the IO ranges > > of non-bridge PCI devices are limited to 256 bytes. > > We must be careful with the ISA aliasing though. */ > > -static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size) > > +static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size, > > + resource_size_t add_size) > > { > > struct pci_dev *dev; > > struct resource *b_res = find_free_bus_resource(bus, IORESOURCE_IO); > > - unsigned long size = 0, size1 = 0, old_size; > > + unsigned long size = 0, size1 = 0; > > > > if (!b_res) > > return; > > @@ -435,20 +487,16 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size) > > size1 += r_size; > > } > > } > > - if (size < min_size) > > - size = min_size; > > - old_size = resource_size(b_res); > > - if (old_size == 1) > > - old_size = 0; > > -/* To be fixed in 2.5: we should have sort of HAVE_ISA > > - flag in the struct pci_bus. */ > > -#if defined(CONFIG_ISA) || defined(CONFIG_EISA) > > - size = (size & 0xff) + ((size & ~0xffUL) << 2); > > -#endif > > - size = ALIGN(size + size1, 4096); > > - if (size < old_size) > > - size = old_size; > > - if (!size) { > > + > > + size = calculate_iosize(size, min_size, size1, > > + resource_size(b_res), 4096); > > + if (add_size) > > + size1 = calculate_iosize(size, min_size+add_size, size1, > > + resource_size(b_res), 4096); > > + else > > + size1 = size; > > + > > + if (!size && !size1) { > > if (b_res->start || b_res->end) > > dev_info(&bus->self->dev, "disabling bridge window " > > "%pR to [bus %02x-%02x] (unused)\n", b_res, > > @@ -459,16 +507,35 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size) > > /* Alignment of the IO window is always 4K */ > > b_res->start = 4096; > > b_res->end = b_res->start + size - 1; > > + b_res->add_size = size1-size; > > b_res->flags |= IORESOURCE_STARTALIGN; > > } > > > > + > > +static resource_size_t calculate_memsize(resource_size_t size, > > + resource_size_t min_size, > > + resource_size_t size1, > > + resource_size_t old_size, > > + resource_size_t align) > > +{ > > + if (size < min_size) > > + size = min_size; > > + if (old_size == 1 ) > > + old_size = 0; > > + if (size < old_size) > > + size = old_size; > > + size = ALIGN(size + size1, align); > > + return size; > > +} > > + > > /* Calculate the size of the bus and minimal alignment which > > guarantees that all child resources fit in this size. */ > > static int pbus_size_mem(struct pci_bus *bus, unsigned long mask, > > - unsigned long type, resource_size_t min_size) > > + unsigned long type, resource_size_t min_size, > > + resource_size_t add_size) > > { > > struct pci_dev *dev; > > - resource_size_t min_align, align, size, old_size; > > + resource_size_t min_align, align, size, size1; > > resource_size_t aligns[12]; /* Alignments from 1Mb to 2Gb */ > > int order, max_order; > > struct resource *b_res = find_free_bus_resource(bus, type); > > @@ -516,13 +583,6 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask, > > mem64_mask &= r->flags & IORESOURCE_MEM_64; > > } > > } > > - if (size < min_size) > > - size = min_size; > > - old_size = resource_size(b_res); > > - if (old_size == 1) > > - old_size = 0; > > - if (size < old_size) > > - size = old_size; > > > > align = 0; > > min_align = 0; > > @@ -537,8 +597,14 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask, > > min_align = align1 >> 1; > > align += aligns[order]; > > } > > - size = ALIGN(size, min_align); > > - if (!size) { > > + > > + size = calculate_memsize(size, min_size, 0, resource_size(b_res), align); > > + if (add_size) > > + size1 = calculate_memsize(size, min_size+add_size, 0, resource_size(b_res), align); > > + else > > + size1 = size; > > + > > + if (!size && !size1) { > > if (b_res->start || b_res->end) > > dev_info(&bus->self->dev, "disabling bridge window " > > "%pR to [bus %02x-%02x] (unused)\n", b_res, > > @@ -548,8 +614,8 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask, > > } > > b_res->start = min_align; > > b_res->end = size + min_align - 1; > > - b_res->flags |= IORESOURCE_STARTALIGN; > > - b_res->flags |= mem64_mask; > > + b_res->add_size = size1 - size; > > + b_res->flags |= IORESOURCE_STARTALIGN | mem64_mask; > > return 1; > > } > > > > @@ -606,7 +672,7 @@ void __ref pci_bus_size_bridges(struct pci_bus *bus) > > { > > struct pci_dev *dev; > > unsigned long mask, prefmask; > > - resource_size_t min_mem_size = 0, min_io_size = 0; > > + resource_size_t additional_mem_size = 0, additional_io_size = 0; > > > > list_for_each_entry(dev, &bus->devices, bus_list) { > > struct pci_bus *b = dev->subordinate; > > @@ -637,11 +703,11 @@ void __ref pci_bus_size_bridges(struct pci_bus *bus) > > case PCI_CLASS_BRIDGE_PCI: > > pci_bridge_check_ranges(bus); > > if (bus->self->is_hotplug_bridge) { > > - min_io_size = pci_hotplug_io_size; > > - min_mem_size = pci_hotplug_mem_size; > > + additional_io_size = pci_hotplug_io_size; > > + additional_mem_size = pci_hotplug_mem_size; > > } > > default: > > - pbus_size_io(bus, min_io_size); > > + pbus_size_io(bus, 0, additional_io_size); > > /* If the bridge supports prefetchable range, size it > > separately. If it doesn't, or its prefetchable window > > has already been allocated by arch code, try > > @@ -649,11 +715,11 @@ void __ref pci_bus_size_bridges(struct pci_bus *bus) > > resources. */ > > mask = IORESOURCE_MEM; > > prefmask = IORESOURCE_MEM | IORESOURCE_PREFETCH; > > - if (pbus_size_mem(bus, prefmask, prefmask, min_mem_size)) > > + if (pbus_size_mem(bus, prefmask, prefmask, 0, additional_mem_size)) > > mask = prefmask; /* Success, size non-prefetch only. */ > > else > > - min_mem_size += min_mem_size; > > - pbus_size_mem(bus, mask, IORESOURCE_MEM, min_mem_size); > > + additional_mem_size += additional_mem_size; > > + pbus_size_mem(bus, mask, IORESOURCE_MEM, 0, additional_mem_size); > > break; > > } > > } > > diff --git a/include/linux/ioport.h b/include/linux/ioport.h > > index e9bb22c..d00c61c 100644 > > --- a/include/linux/ioport.h > > +++ b/include/linux/ioport.h > > @@ -18,6 +18,7 @@ > > struct resource { > > resource_size_t start; > > resource_size_t end; > > + resource_size_t add_size; > > const char *name; > > unsigned long flags; > > struct resource *parent, *sibling, *child; > > > -- > 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 -- Ram Pai System X Device-Driver Enablement Lead Linux Technology Center Beaverton OR-97006 503-5783752 t/l 7753752 linuxram@xxxxxxxxxx -- 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