| Message ID | 20210713075726.1232938-1-kai.heng.feng@canonical.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | [1/1] PCI: Coalesce host bridge contiguous apertures without sorting | expand |
On 7/13/21 12:57 AM, Kai-Heng Feng wrote: > Commit 65db04053efe ("PCI: Coalesce host bridge contiguous apertures") > sorts the resources by address so the resources can be swapped. > > Before: > PCI host bridge to bus 0002:00 > pci_bus 0002:00: root bus resource [io 0x0000-0xffff] > pci_bus 0002:00: root bus resource [mem 0xd80000000-0xdffffffff] (bus address [0x80000000-0xffffffff]) > pci_bus 0002:00: root bus resource [mem 0xc0ee00000-0xc0eefffff] (bus address [0x00000000-0x000fffff]) > > And after: > PCI host bridge to bus 0002:00 > pci_bus 0002:00: root bus resource [io 0x0000-0xffff] > pci_bus 0002:00: root bus resource [mem 0xc0ee00000-0xc0eefffff] (bus address [0x00000000-0x000fffff]) > pci_bus 0002:00: root bus resource [mem 0xd80000000-0xdffffffff] (bus address [0x80000000-0xffffffff]) > > However, the sorted resources make NVMe stops working on QEMU ppc:sam460ex. > > Resources in the original issue are already in ascending order: > pci_bus 0000:00: root bus resource [mem 0x10020200000-0x100303fffff window] > pci_bus 0000:00: root bus resource [mem 0x10030400000-0x100401fffff window] > > So remove the sorting part to resolve the issue. > > Reported-by: Guenter Roeck <linux@roeck-us.net> > Fixes: 65db04053efe ("PCI: Coalesce host bridge contiguous apertures") > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> I think the original commit message would make more sense here. This patch doesn't fix 65db04053efe, it replaces it. The commit message now misses the point, and the patch coalesces continuous apertures without explaining the reason. Guenter > --- > drivers/pci/probe.c | 31 +++++++++++++++++++++++++++---- > 1 file changed, 27 insertions(+), 4 deletions(-) > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > index 79177ac37880..5de157600466 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c > @@ -877,11 +877,11 @@ static void pci_set_bus_msi_domain(struct pci_bus *bus) > static int pci_register_host_bridge(struct pci_host_bridge *bridge) > { > struct device *parent = bridge->dev.parent; > - struct resource_entry *window, *n; > + struct resource_entry *window, *next, *n; > struct pci_bus *bus, *b; > - resource_size_t offset; > + resource_size_t offset, next_offset; > LIST_HEAD(resources); > - struct resource *res; > + struct resource *res, *next_res; > char addr[64], *fmt; > const char *name; > int err; > @@ -961,11 +961,34 @@ static int pci_register_host_bridge(struct pci_host_bridge *bridge) > if (nr_node_ids > 1 && pcibus_to_node(bus) == NUMA_NO_NODE) > dev_warn(&bus->dev, "Unknown NUMA node; performance will be reduced\n"); > > + /* Coalesce contiguous windows */ > + resource_list_for_each_entry_safe(window, n, &resources) { > + if (list_is_last(&window->node, &resources)) > + break; > + > + next = list_next_entry(window, node); > + offset = window->offset; > + res = window->res; > + next_offset = next->offset; > + next_res = next->res; > + > + if (res->flags != next_res->flags || offset != next_offset) > + continue; > + > + if (res->end + 1 == next_res->start) { > + next_res->start = res->start; > + res->flags = res->start = res->end = 0; > + } > + } > + > /* Add initial resources to the bus */ > resource_list_for_each_entry_safe(window, n, &resources) { > - list_move_tail(&window->node, &bridge->windows); > offset = window->offset; > res = window->res; > + if (!res->end) > + continue; > + > + list_move_tail(&window->node, &bridge->windows); > > if (res->flags & IORESOURCE_BUS) > pci_bus_insert_busn_res(bus, bus->number, res->end); >
On Tue, Jul 13, 2021 at 4:45 PM Guenter Roeck <linux@roeck-us.net> wrote: > > On 7/13/21 12:57 AM, Kai-Heng Feng wrote: > > Commit 65db04053efe ("PCI: Coalesce host bridge contiguous apertures") > > sorts the resources by address so the resources can be swapped. > > > > Before: > > PCI host bridge to bus 0002:00 > > pci_bus 0002:00: root bus resource [io 0x0000-0xffff] > > pci_bus 0002:00: root bus resource [mem 0xd80000000-0xdffffffff] (bus address [0x80000000-0xffffffff]) > > pci_bus 0002:00: root bus resource [mem 0xc0ee00000-0xc0eefffff] (bus address [0x00000000-0x000fffff]) > > > > And after: > > PCI host bridge to bus 0002:00 > > pci_bus 0002:00: root bus resource [io 0x0000-0xffff] > > pci_bus 0002:00: root bus resource [mem 0xc0ee00000-0xc0eefffff] (bus address [0x00000000-0x000fffff]) > > pci_bus 0002:00: root bus resource [mem 0xd80000000-0xdffffffff] (bus address [0x80000000-0xffffffff]) > > > > However, the sorted resources make NVMe stops working on QEMU ppc:sam460ex. > > > > Resources in the original issue are already in ascending order: > > pci_bus 0000:00: root bus resource [mem 0x10020200000-0x100303fffff window] > > pci_bus 0000:00: root bus resource [mem 0x10030400000-0x100401fffff window] > > > > So remove the sorting part to resolve the issue. > > > > Reported-by: Guenter Roeck <linux@roeck-us.net> > > Fixes: 65db04053efe ("PCI: Coalesce host bridge contiguous apertures") > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > > I think the original commit message would make more sense here. This patch > doesn't fix 65db04053efe, it replaces it. The commit message now misses > the point, and the patch coalesces continuous apertures without explaining > the reason. Because the message is already in the git log so I didn't think that's necessary. Will send another one with the original message along with this one. Kai-Heng > > Guenter > > > --- > > drivers/pci/probe.c | 31 +++++++++++++++++++++++++++---- > > 1 file changed, 27 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > > index 79177ac37880..5de157600466 100644 > > --- a/drivers/pci/probe.c > > +++ b/drivers/pci/probe.c > > @@ -877,11 +877,11 @@ static void pci_set_bus_msi_domain(struct pci_bus *bus) > > static int pci_register_host_bridge(struct pci_host_bridge *bridge) > > { > > struct device *parent = bridge->dev.parent; > > - struct resource_entry *window, *n; > > + struct resource_entry *window, *next, *n; > > struct pci_bus *bus, *b; > > - resource_size_t offset; > > + resource_size_t offset, next_offset; > > LIST_HEAD(resources); > > - struct resource *res; > > + struct resource *res, *next_res; > > char addr[64], *fmt; > > const char *name; > > int err; > > @@ -961,11 +961,34 @@ static int pci_register_host_bridge(struct pci_host_bridge *bridge) > > if (nr_node_ids > 1 && pcibus_to_node(bus) == NUMA_NO_NODE) > > dev_warn(&bus->dev, "Unknown NUMA node; performance will be reduced\n"); > > > > + /* Coalesce contiguous windows */ > > + resource_list_for_each_entry_safe(window, n, &resources) { > > + if (list_is_last(&window->node, &resources)) > > + break; > > + > > + next = list_next_entry(window, node); > > + offset = window->offset; > > + res = window->res; > > + next_offset = next->offset; > > + next_res = next->res; > > + > > + if (res->flags != next_res->flags || offset != next_offset) > > + continue; > > + > > + if (res->end + 1 == next_res->start) { > > + next_res->start = res->start; > > + res->flags = res->start = res->end = 0; > > + } > > + } > > + > > /* Add initial resources to the bus */ > > resource_list_for_each_entry_safe(window, n, &resources) { > > - list_move_tail(&window->node, &bridge->windows); > > offset = window->offset; > > res = window->res; > > + if (!res->end) > > + continue; > > + > > + list_move_tail(&window->node, &bridge->windows); > > > > if (res->flags & IORESOURCE_BUS) > > pci_bus_insert_busn_res(bus, bus->number, res->end); > > >
On 7/13/21 1:49 AM, Kai-Heng Feng wrote: > On Tue, Jul 13, 2021 at 4:45 PM Guenter Roeck <linux@roeck-us.net> wrote: >> >> On 7/13/21 12:57 AM, Kai-Heng Feng wrote: >>> Commit 65db04053efe ("PCI: Coalesce host bridge contiguous apertures") >>> sorts the resources by address so the resources can be swapped. >>> >>> Before: >>> PCI host bridge to bus 0002:00 >>> pci_bus 0002:00: root bus resource [io 0x0000-0xffff] >>> pci_bus 0002:00: root bus resource [mem 0xd80000000-0xdffffffff] (bus address [0x80000000-0xffffffff]) >>> pci_bus 0002:00: root bus resource [mem 0xc0ee00000-0xc0eefffff] (bus address [0x00000000-0x000fffff]) >>> >>> And after: >>> PCI host bridge to bus 0002:00 >>> pci_bus 0002:00: root bus resource [io 0x0000-0xffff] >>> pci_bus 0002:00: root bus resource [mem 0xc0ee00000-0xc0eefffff] (bus address [0x00000000-0x000fffff]) >>> pci_bus 0002:00: root bus resource [mem 0xd80000000-0xdffffffff] (bus address [0x80000000-0xffffffff]) >>> >>> However, the sorted resources make NVMe stops working on QEMU ppc:sam460ex. >>> >>> Resources in the original issue are already in ascending order: >>> pci_bus 0000:00: root bus resource [mem 0x10020200000-0x100303fffff window] >>> pci_bus 0000:00: root bus resource [mem 0x10030400000-0x100401fffff window] >>> >>> So remove the sorting part to resolve the issue. >>> >>> Reported-by: Guenter Roeck <linux@roeck-us.net> >>> Fixes: 65db04053efe ("PCI: Coalesce host bridge contiguous apertures") >>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> >> >> I think the original commit message would make more sense here. This patch >> doesn't fix 65db04053efe, it replaces it. The commit message now misses >> the point, and the patch coalesces continuous apertures without explaining >> the reason. > > Because the message is already in the git log so I didn't think that's > necessary. Hmm, not my decision to make, but the original commit got reverted. The commit log associated with this patch should still reflect what the patch does. Guenter > Will send another one with the original message along with this one. > > Kai-Heng > >> >> Guenter >> >>> --- >>> drivers/pci/probe.c | 31 +++++++++++++++++++++++++++---- >>> 1 file changed, 27 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c >>> index 79177ac37880..5de157600466 100644 >>> --- a/drivers/pci/probe.c >>> +++ b/drivers/pci/probe.c >>> @@ -877,11 +877,11 @@ static void pci_set_bus_msi_domain(struct pci_bus *bus) >>> static int pci_register_host_bridge(struct pci_host_bridge *bridge) >>> { >>> struct device *parent = bridge->dev.parent; >>> - struct resource_entry *window, *n; >>> + struct resource_entry *window, *next, *n; >>> struct pci_bus *bus, *b; >>> - resource_size_t offset; >>> + resource_size_t offset, next_offset; >>> LIST_HEAD(resources); >>> - struct resource *res; >>> + struct resource *res, *next_res; >>> char addr[64], *fmt; >>> const char *name; >>> int err; >>> @@ -961,11 +961,34 @@ static int pci_register_host_bridge(struct pci_host_bridge *bridge) >>> if (nr_node_ids > 1 && pcibus_to_node(bus) == NUMA_NO_NODE) >>> dev_warn(&bus->dev, "Unknown NUMA node; performance will be reduced\n"); >>> >>> + /* Coalesce contiguous windows */ >>> + resource_list_for_each_entry_safe(window, n, &resources) { >>> + if (list_is_last(&window->node, &resources)) >>> + break; >>> + >>> + next = list_next_entry(window, node); >>> + offset = window->offset; >>> + res = window->res; >>> + next_offset = next->offset; >>> + next_res = next->res; >>> + >>> + if (res->flags != next_res->flags || offset != next_offset) >>> + continue; >>> + >>> + if (res->end + 1 == next_res->start) { >>> + next_res->start = res->start; >>> + res->flags = res->start = res->end = 0; >>> + } >>> + } >>> + >>> /* Add initial resources to the bus */ >>> resource_list_for_each_entry_safe(window, n, &resources) { >>> - list_move_tail(&window->node, &bridge->windows); >>> offset = window->offset; >>> res = window->res; >>> + if (!res->end) >>> + continue; >>> + >>> + list_move_tail(&window->node, &bridge->windows); >>> >>> if (res->flags & IORESOURCE_BUS) >>> pci_bus_insert_busn_res(bus, bus->number, res->end); >>> >>
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 79177ac37880..5de157600466 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -877,11 +877,11 @@ static void pci_set_bus_msi_domain(struct pci_bus *bus) static int pci_register_host_bridge(struct pci_host_bridge *bridge) { struct device *parent = bridge->dev.parent; - struct resource_entry *window, *n; + struct resource_entry *window, *next, *n; struct pci_bus *bus, *b; - resource_size_t offset; + resource_size_t offset, next_offset; LIST_HEAD(resources); - struct resource *res; + struct resource *res, *next_res; char addr[64], *fmt; const char *name; int err; @@ -961,11 +961,34 @@ static int pci_register_host_bridge(struct pci_host_bridge *bridge) if (nr_node_ids > 1 && pcibus_to_node(bus) == NUMA_NO_NODE) dev_warn(&bus->dev, "Unknown NUMA node; performance will be reduced\n"); + /* Coalesce contiguous windows */ + resource_list_for_each_entry_safe(window, n, &resources) { + if (list_is_last(&window->node, &resources)) + break; + + next = list_next_entry(window, node); + offset = window->offset; + res = window->res; + next_offset = next->offset; + next_res = next->res; + + if (res->flags != next_res->flags || offset != next_offset) + continue; + + if (res->end + 1 == next_res->start) { + next_res->start = res->start; + res->flags = res->start = res->end = 0; + } + } + /* Add initial resources to the bus */ resource_list_for_each_entry_safe(window, n, &resources) { - list_move_tail(&window->node, &bridge->windows); offset = window->offset; res = window->res; + if (!res->end) + continue; + + list_move_tail(&window->node, &bridge->windows); if (res->flags & IORESOURCE_BUS) pci_bus_insert_busn_res(bus, bus->number, res->end);
Commit 65db04053efe ("PCI: Coalesce host bridge contiguous apertures") sorts the resources by address so the resources can be swapped. Before: PCI host bridge to bus 0002:00 pci_bus 0002:00: root bus resource [io 0x0000-0xffff] pci_bus 0002:00: root bus resource [mem 0xd80000000-0xdffffffff] (bus address [0x80000000-0xffffffff]) pci_bus 0002:00: root bus resource [mem 0xc0ee00000-0xc0eefffff] (bus address [0x00000000-0x000fffff]) And after: PCI host bridge to bus 0002:00 pci_bus 0002:00: root bus resource [io 0x0000-0xffff] pci_bus 0002:00: root bus resource [mem 0xc0ee00000-0xc0eefffff] (bus address [0x00000000-0x000fffff]) pci_bus 0002:00: root bus resource [mem 0xd80000000-0xdffffffff] (bus address [0x80000000-0xffffffff]) However, the sorted resources make NVMe stops working on QEMU ppc:sam460ex. Resources in the original issue are already in ascending order: pci_bus 0000:00: root bus resource [mem 0x10020200000-0x100303fffff window] pci_bus 0000:00: root bus resource [mem 0x10030400000-0x100401fffff window] So remove the sorting part to resolve the issue. Reported-by: Guenter Roeck <linux@roeck-us.net> Fixes: 65db04053efe ("PCI: Coalesce host bridge contiguous apertures") Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> --- drivers/pci/probe.c | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-)