| Message ID | 20140307204021.GA9822@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 2014-03-07 at 13:40 -0700, Bjorn Helgaas wrote: > It seems quite possible that I broke pci_bus_alloc_resource(), which could > cause an allocation failure like this. > > If you have a chance to try it, here's a debug patch against v3.14-rc5. It > should apply cleanly to 96702be56037. If you can try it, please attach the > dmesg log to the bugzilla. That ThinkPad X41 is now building 96702be56037. Once that build is finished and tested I'll try your debug patch (on top of v3.14-rc5, see later). It might take some time to finish both builds and test boots. > diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c > index 5c85350f4c3d..0dbba6c7c001 100644 > --- a/drivers/char/agp/intel-gtt.c > +++ b/drivers/char/agp/intel-gtt.c > @@ -997,6 +997,7 @@ static int intel_alloc_chipset_flush_resource(void) > ret = pci_bus_alloc_resource(intel_private.bridge_dev->bus, &intel_private.ifp_resource, PAGE_SIZE, > PAGE_SIZE, PCIBIOS_MIN_MEM, 0, > pcibios_align_resource, intel_private.bridge_dev); > + dev_info(&intel_private.bridge_dev->dev, "pci_bus_alloc ret %d\n", ret); > > return ret; > } > @@ -1007,6 +1008,7 @@ static void intel_i915_setup_chipset_flush(void) > u32 temp; > > pci_read_config_dword(intel_private.bridge_dev, I915_IFPADDR, &temp); > + dev_info(&intel_private.bridge_dev->dev, "I915_IFPADDR %#010x\n", temp); > if (!(temp & 0x1)) { > intel_alloc_chipset_flush_resource(); > intel_private.resource_valid = 1; > @@ -1022,6 +1024,7 @@ static void intel_i915_setup_chipset_flush(void) > if (ret) > intel_private.resource_valid = 0; > } > + dev_info(&intel_private.bridge_dev->dev, "ifp_resource %pR\n", &intel_private.ifp_resource); > } > > static void intel_i965_g33_setup_chipset_flush(void) My v3.13 based builds don't have INTEL_GTT set! My v3.14-rcy based builds do. I have not yet investigated why that is. (Note that the .config on that ThinkPad X41 is - in short - rebranched from the kernel .config that is shipped for Fedora 20 every time a rc1 is released.) > diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c > index 00660cc502c5..1c6d75ae34d9 100644 > --- a/drivers/pci/bus.c > +++ b/drivers/pci/bus.c > @@ -146,24 +146,31 @@ static int pci_bus_alloc_from_region(struct pci_bus *bus, struct resource *res, > > type_mask |= IORESOURCE_IO | IORESOURCE_MEM; > > + dev_info(&bus->dev, "%s: alloc %pR size %#llx from bus region [%#010llx-%#010llx]\n", __func__, res, (long long) size, (long long) region->start, (long long) region->end); > pci_bus_for_each_resource(bus, r, i) { > if (!r) > continue; > > /* type_mask must match */ > - if ((res->flags ^ r->flags) & type_mask) > + if ((res->flags ^ r->flags) & type_mask) { > + dev_info(&bus->dev, "%s: %pR: wrong type (%#lx %#lx mask %#x)\n", __func__, r, res->flags, r->flags, type_mask); > continue; > + } > > /* We cannot allocate a non-prefetching resource > from a pre-fetching area */ > if ((r->flags & IORESOURCE_PREFETCH) && > - !(res->flags & IORESOURCE_PREFETCH)) > + !(res->flags & IORESOURCE_PREFETCH)) { > + dev_info(&bus->dev, "%s: %pR: wrong prefetchability\n", __func__, r); > continue; > + } > > avail = *r; > pci_clip_resource_to_region(bus, &avail, region); > - if (!resource_size(&avail)) > + if (!resource_size(&avail)) { > + dev_info(&bus->dev, "%s: %pR: no space (avail %pR)\n", __func__, r, &avail); > continue; > + } > > /* > * "min" is typically PCIBIOS_MIN_IO or PCIBIOS_MIN_MEM to > @@ -179,6 +186,7 @@ static int pci_bus_alloc_from_region(struct pci_bus *bus, struct resource *res, > /* Ok, try it out.. */ > ret = allocate_resource(r, res, size, min, max, > align, alignf, alignf_data); > + dev_info(&bus->dev, "%s: %pR: alloc from %#llx-%#llx, ret %d\n", __func__, r, min, max, ret); > if (ret == 0) > return 0; > } Too bad drivers/pci/bus.o is built in by definition. If only one could build a kernel without rebuilding all modules. Or is there some way to actually do that? Paul Bolle
On Fri, Mar 7, 2014 at 2:03 PM, Paul Bolle <pebolle@tiscali.nl> wrote: > On Fri, 2014-03-07 at 13:40 -0700, Bjorn Helgaas wrote: >> It seems quite possible that I broke pci_bus_alloc_resource(), which could >> cause an allocation failure like this. >> >> If you have a chance to try it, here's a debug patch against v3.14-rc5. It >> should apply cleanly to 96702be56037. If you can try it, please attach the >> dmesg log to the bugzilla. > > That ThinkPad X41 is now building 96702be56037. Once that build is > finished and tested I'll try your debug patch (on top of v3.14-rc5, see > later). It might take some time to finish both builds and test boots. > My v3.13 based builds don't have INTEL_GTT set! My v3.14-rcy based > builds do. I have not yet investigated why that is. I think that's OK. CONFIG_INTEL_GTT was added after v3.13 (00fe639a56b4 "drm/i915: Make AGP support optional"). It looks like in v3.13, intel-gtt.o was built if CONFIG_AGP_INTEL=y (or =m), which you probably do have (see drivers/char/agp/Makefile). > Too bad drivers/pci/bus.o is built in by definition. If only one could > build a kernel without rebuilding all modules. Or is there some way to > actually do that? You should be able to "make bzImage" and get just the kernel. But there might be module loading issues if the modules don't exactly match the kernel. I think it's possible to turn that checking off, but I don't do it often enough to remember details. Bjorn
On Fri, Mar 7, 2014 at 1:40 PM, Bjorn Helgaas <bhelgaas@google.com> wrote: > It seems quite possible that I broke pci_bus_alloc_resource(), which could > cause an allocation failure like this. > > If you have a chance to try it, here's a debug patch against v3.14-rc5. It > should apply cleanly to 96702be56037. If you can try it, please attach the > dmesg log to the bugzilla. Paul verified that I *did* break this. More details in the bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=71691 The problem is basically that I used resource_size() to figure out whether there's any available space. resource_size() is res->end - res->start + 1, so applying it to [mem 0x00000000-0xffffffff] returns zero in a kernel 32-bit resource addresses, i.e., with CONFIG_PHYS_ADDR_T_64BIT=n. Paul, I assume you have CONFIG_PHYS_ADDR_T_64BIT=n (which is perfectly legal); let me know if otherwise. pci_bus 0000:00: pci_bus_alloc_from_region: alloc [mem 0x00000000] size 0x1000 from bus region [0x00000000-0xffffffff] pci_bus 0000:00: pci_bus_alloc_from_region: [mem 0x00000000-0xffffffff]: no space (avail [mem 0x00000000-0xffffffff]) Bjorn
Bjorn Helgaas schreef op za 08-03-2014 om 07:12 [-0700]: > I assume you have CONFIG_PHYS_ADDR_T_64BIT=n (which is perfectly > legal); let me know if otherwise. $ grep CONFIG_PHYS_ADDR_T_64BIT /boot/config-3.14.0-0.rc5.1.local2.fc20.i686 # CONFIG_PHYS_ADDR_T_64BIT is not set So, yes, CONFIG_PHYS_ADDR_T_64BIT=n here. Paul Bolle
diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c index 5c85350f4c3d..0dbba6c7c001 100644 --- a/drivers/char/agp/intel-gtt.c +++ b/drivers/char/agp/intel-gtt.c @@ -997,6 +997,7 @@ static int intel_alloc_chipset_flush_resource(void) ret = pci_bus_alloc_resource(intel_private.bridge_dev->bus, &intel_private.ifp_resource, PAGE_SIZE, PAGE_SIZE, PCIBIOS_MIN_MEM, 0, pcibios_align_resource, intel_private.bridge_dev); + dev_info(&intel_private.bridge_dev->dev, "pci_bus_alloc ret %d\n", ret); return ret; } @@ -1007,6 +1008,7 @@ static void intel_i915_setup_chipset_flush(void) u32 temp; pci_read_config_dword(intel_private.bridge_dev, I915_IFPADDR, &temp); + dev_info(&intel_private.bridge_dev->dev, "I915_IFPADDR %#010x\n", temp); if (!(temp & 0x1)) { intel_alloc_chipset_flush_resource(); intel_private.resource_valid = 1; @@ -1022,6 +1024,7 @@ static void intel_i915_setup_chipset_flush(void) if (ret) intel_private.resource_valid = 0; } + dev_info(&intel_private.bridge_dev->dev, "ifp_resource %pR\n", &intel_private.ifp_resource); } static void intel_i965_g33_setup_chipset_flush(void) diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c index 00660cc502c5..1c6d75ae34d9 100644 --- a/drivers/pci/bus.c +++ b/drivers/pci/bus.c @@ -146,24 +146,31 @@ static int pci_bus_alloc_from_region(struct pci_bus *bus, struct resource *res, type_mask |= IORESOURCE_IO | IORESOURCE_MEM; + dev_info(&bus->dev, "%s: alloc %pR size %#llx from bus region [%#010llx-%#010llx]\n", __func__, res, (long long) size, (long long) region->start, (long long) region->end); pci_bus_for_each_resource(bus, r, i) { if (!r) continue; /* type_mask must match */ - if ((res->flags ^ r->flags) & type_mask) + if ((res->flags ^ r->flags) & type_mask) { + dev_info(&bus->dev, "%s: %pR: wrong type (%#lx %#lx mask %#x)\n", __func__, r, res->flags, r->flags, type_mask); continue; + } /* We cannot allocate a non-prefetching resource from a pre-fetching area */ if ((r->flags & IORESOURCE_PREFETCH) && - !(res->flags & IORESOURCE_PREFETCH)) + !(res->flags & IORESOURCE_PREFETCH)) { + dev_info(&bus->dev, "%s: %pR: wrong prefetchability\n", __func__, r); continue; + } avail = *r; pci_clip_resource_to_region(bus, &avail, region); - if (!resource_size(&avail)) + if (!resource_size(&avail)) { + dev_info(&bus->dev, "%s: %pR: no space (avail %pR)\n", __func__, r, &avail); continue; + } /* * "min" is typically PCIBIOS_MIN_IO or PCIBIOS_MIN_MEM to @@ -179,6 +186,7 @@ static int pci_bus_alloc_from_region(struct pci_bus *bus, struct resource *res, /* Ok, try it out.. */ ret = allocate_resource(r, res, size, min, max, align, alignf, alignf_data); + dev_info(&bus->dev, "%s: %pR: alloc from %#llx-%#llx, ret %d\n", __func__, r, min, max, ret); if (ret == 0) return 0; }