Re: linux-next: Tree for September 3

From: Yinghai Lu
Date: Thu Sep 04 2008 - 03:14:30 EST


On Wed, Sep 3, 2008 at 10:33 PM, Andrew Morton
<akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
> On Wed, 3 Sep 2008 22:21:54 -0700 (PDT) Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
>>
>>
>> On Wed, 3 Sep 2008, Andrew Morton wrote:
>> >
>> > The Vaio says:
>> >
>> > calling init_acpi_pm_clocksource+0x0/0x14a
>> > initcall init_acpi_pm_clocksource+0x0/0x14a returned 0 after 32 msecs
>> > calling pcibios_assign_resources+0x0/0x70
>> > pci 0000:06:05.0: BAR 9 too large: 0x00000000000000-0x00000003ffffff
>>
>> Hmm. This should not be a new warning, afaik.
>>
>> But it looks totally bogus.
>>
>> The code does:
>>
>> r_size = r->end - r->start + 1;
>> /* For bridges size != alignment */
>> align = (i < PCI_BRIDGE_RESOURCES) ? r_size : r->start;
>> order = __ffs(align) - 20;
>> if (order > 11) {
>> dev_warn(&dev->dev, "BAR %d too large: "
>> "%#016llx-%#016llx\n", i,
>> (unsigned long long)r->start,
>> (unsigned long long)r->end);
>>
>> and the thing is, that's a bridge resource, so it chooses r_start as the
>> alignment. Which is zero. so now __ffs(align) returns a bogus value, and
>> you get the bogus warning.
>>
>> But CARDBUS bridges are _different_ from normal PCI bridges, and the
>> alignment value isn't r->start. Strictly speaking it's not r_size either,
>> it's always a fixed alignment of 4096 for MMIO bridging, i think. Maybe.
>> Whatever. But our resource handling code can't handle that, and always
>> wants either size alignment or start alignment.
>>
>> But for cardbus bridges, we should be doing size alignment, and the
>> problem is that that code doesn't do the proper "resource_alignment()"
>> use.
>>
>> Can you apply this patch, just to see if it fixes things.
>
> Below..
>
>> And do you know when this started happening? It shouldn't have been all
>> that recent. Maybe you haven't tried your Vaio in a while?
>
> Yes, the Vaio had a vacation in New York. Last kernel it booted was
> 2.6.26-rc8-mm1.
>
> --- x 2008-09-03 21:38:24.000000000 -0700
> +++ y 2008-09-03 22:29:04.000000000 -0700
> ...
> @@ -503,15 +503,15 @@
> calling init_acpi_pm_clocksource+0x0/0x14a
> initcall init_acpi_pm_clocksource+0x0/0x14a returned 0 after 32 msecs
> calling pcibios_assign_resources+0x0/0x70
> -pci 0000:06:05.0: BAR 9 too large: 0x00000000000000-0x00000003ffffff
> pci 0000:06:05.0: CardBus bridge, secondary bus 0000:07
> pci 0000:06:05.0: IO window: 0x002400-0x0024ff
> pci 0000:06:05.0: IO window: 0x002800-0x0028ff
> -pci 0000:06:05.0: MEM window: 0x54000000-0x57ffffff
> +pci 0000:06:05.0: PREFETCH window: 0x50000000-0x53ffffff
> +pci 0000:06:05.0: MEM window: 0x58000000-0x5bffffff
> pci 0000:00:1e.0: PCI bridge, secondary bus 0000:06
> pci 0000:00:1e.0: IO window: 0x2000-0x2fff
> pci 0000:00:1e.0: MEM window: 0xb0100000-0xb01fffff
> -pci 0000:00:1e.0: PREFETCH window: disabled
> +pci 0000:00:1e.0: PREFETCH window: 0x00000050000000-0x00000053ffffff
> pci 0000:00:1e.0: setting latency timer to 64
> pci 0000:06:05.0: power state changed by ACPI to D0

06:05.0 is under 00:1e.0

wonder if some pci code change cause that.... doesn't get pref mem assigned.

can you apply attached patches to get more dump?

YH
From: Yinghai Lu <yhlu.kernel@xxxxxxxxx>
Subject: [PATCH] pci: fix merging left out for BAR print out

print out for Device BAR address before kernel try to update them.

also change it to KERN_DEBUG instead...

Signed-off-by: Yinghai Lu <yhlu.kernel@xxxxxxxxx>

---
drivers/pci/probe.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)

Index: linux-2.6/drivers/pci/probe.c
===================================================================
--- linux-2.6.orig/drivers/pci/probe.c
+++ linux-2.6/drivers/pci/probe.c
@@ -304,6 +304,8 @@ static int __pci_read_base(struct pci_de
} else {
res->start = l64;
res->end = l64 + sz64;
+ printk(KERN_DEBUG "PCI: %s reg %x 64bit mmio: [%llx, %llx]\n",
+ pci_name(dev), pos, res->start, res->end);
}
} else {
sz = pci_size(l, sz, mask);
@@ -313,6 +315,10 @@ static int __pci_read_base(struct pci_de

res->start = l;
res->end = l + sz;
+ printk(KERN_DEBUG "PCI: %s reg %x %s: [%llx, %llx]\n", pci_name(dev),
+ pos, (res->flags & IORESOURCE_IO) ? "io port":"32bit mmio",
+ res->start, res->end);
+
}

out:
@@ -383,7 +389,8 @@ void __devinit pci_read_bridge_bases(str
res->start = base;
if (!res->end)
res->end = limit + 0xfff;
- printk(KERN_INFO "PCI: bridge %s io port: [%llx, %llx]\n", pci_name(dev), res->start, res->end);
+ printk(KERN_DEBUG "PCI: bridge %s io port: [%llx, %llx]\n",
+ pci_name(dev), res->start, res->end);
}

res = child->resource[1];
@@ -395,7 +402,8 @@ void __devinit pci_read_bridge_bases(str
res->flags = (mem_base_lo & PCI_MEMORY_RANGE_TYPE_MASK) | IORESOURCE_MEM;
res->start = base;
res->end = limit + 0xfffff;
- printk(KERN_INFO "PCI: bridge %s 32bit mmio: [%llx, %llx]\n", pci_name(dev), res->start, res->end);
+ printk(KERN_DEBUG "PCI: bridge %s 32bit mmio: [%llx, %llx]\n",
+ pci_name(dev), res->start, res->end);
}

res = child->resource[2];
@@ -431,7 +439,9 @@ void __devinit pci_read_bridge_bases(str
res->flags = (mem_base_lo & PCI_MEMORY_RANGE_TYPE_MASK) | IORESOURCE_MEM | IORESOURCE_PREFETCH;
res->start = base;
res->end = limit + 0xfffff;
- printk(KERN_INFO "PCI: bridge %s %sbit mmio pref: [%llx, %llx]\n", pci_name(dev), (res->flags & PCI_PREF_RANGE_TYPE_64)?"64":"32",res->start, res->end);
+ printk(KERN_DEBUG "PCI: bridge %s %sbit mmio pref: [%llx, %llx]\n",
+ pci_name(dev), (res->flags & PCI_PREF_RANGE_TYPE_64)?"64":"32",
+ res->start, res->end);
}
}

From: Yinghai Lu <yhlu.kernel@xxxxxxxxx>
Subject: [PATCH] x86: split e820 reserved entries record to late v4

Linus said we should register some entries in e820 later,
so could let BAR res register at first, or even pnp?

this one replace
| commit a2bd7274b47124d2fc4dfdb8c0591f545ba749dd
| Author: Yinghai Lu <yhlu.kernel@xxxxxxxxx>
| Date: Mon Aug 25 00:56:08 2008 -0700
|
| x86: fix HPET regression in 2.6.26 versus 2.6.25, check hpet against BAR, v3

v2: insert e820 reserve resources before pnp_system_init
v3: fix merging problem in tip/x86/core
please drop the one in tip/x86/core use this one instead
v4: address Linus's review about comments and condition in _late()

Signed-off-by: Yinghai Lu <yhlu.kernel@xxxxxxxxx>

---
arch/x86/kernel/e820.c | 24 +++++++++++++++++++++++-
arch/x86/pci/i386.c | 3 +++
include/asm-x86/e820.h | 1 +
3 files changed, 27 insertions(+), 1 deletion(-)

Index: linux-2.6/arch/x86/kernel/e820.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/e820.c
+++ linux-2.6/arch/x86/kernel/e820.c
@@ -1267,6 +1267,7 @@ static inline const char *e820_type_to_s
/*
* Mark e820 reserved areas as busy for the resource manager.
*/
+static struct resource __initdata *e820_res;
void __init e820_reserve_resources(void)
{
int i;
@@ -1274,6 +1275,7 @@ void __init e820_reserve_resources(void)
u64 end;

res = alloc_bootmem_low(sizeof(struct resource) * e820.nr_map);
+ e820_res = res;
for (i = 0; i < e820.nr_map; i++) {
end = e820.map[i].addr + e820.map[i].size - 1;
#ifndef CONFIG_RESOURCES_64BIT
@@ -1287,7 +1289,14 @@ void __init e820_reserve_resources(void)
res->end = end;

res->flags = IORESOURCE_MEM | IORESOURCE_BUSY;
- insert_resource(&iomem_resource, res);
+
+ /*
+ * don't register the region that could be conflicted with
+ * pci device BAR resource and insert them later in
+ * pcibios_resource_survey()
+ */
+ if (e820.map[i].type != E820_RESERVED || res->start < (1ULL<<20))
+ insert_resource(&iomem_resource, res);
res++;
}

@@ -1299,6 +1308,19 @@ void __init e820_reserve_resources(void)
}
}

+void __init e820_reserve_resources_late(void)
+{
+ int i;
+ struct resource *res;
+
+ res = e820_res;
+ for (i = 0; i < e820.nr_map; i++) {
+ if (!res->parent && res->end)
+ insert_resource(&iomem_resource, res);
+ res++;
+ }
+}
+
char *__init default_machine_specific_memory_setup(void)
{
char *who = "BIOS-e820";
Index: linux-2.6/arch/x86/pci/i386.c
===================================================================
--- linux-2.6.orig/arch/x86/pci/i386.c
+++ linux-2.6/arch/x86/pci/i386.c
@@ -33,6 +33,7 @@
#include <linux/bootmem.h>

#include <asm/pat.h>
+#include <asm/e820.h>

#include "pci.h"

@@ -230,6 +231,8 @@ void __init pcibios_resource_survey(void
pcibios_allocate_bus_resources(&pci_root_buses);
pcibios_allocate_resources(0);
pcibios_allocate_resources(1);
+
+ e820_reserve_resources_late();
}

/**
Index: linux-2.6/include/asm-x86/e820.h
===================================================================
--- linux-2.6.orig/include/asm-x86/e820.h
+++ linux-2.6/include/asm-x86/e820.h
@@ -120,6 +120,7 @@ extern void e820_register_active_regions
extern u64 e820_hole_size(u64 start, u64 end);
extern void finish_e820_parsing(void);
extern void e820_reserve_resources(void);
+extern void e820_reserve_resources_late(void);
extern void setup_memory_map(void);
extern char *default_machine_specific_memory_setup(void);
extern char *machine_specific_memory_setup(void);
From: Yinghai Lu <yhlu.kernel@xxxxxxxxx>
Subject: [PATCH] x86: split e820 reserved entries record to late v4 - fix v7

try to insert_resource second time, by expand the resource...

for case: e820 reserved entry is partially overlapped with bar res...

hope it will never happen

v2: according to Linus, add insert_resource_expand_to_fit, and change
__insert_resource to static without lock

v3: use reserve_region_with_split() instead to hand overlapping
with test case by extend 0xe0000000 - 0xeffffff to 0xdd800000 -
get
e0000000-efffffff : PCI MMCONFIG 0
e0000000-efffffff : reserved
in /proc/iomem
get
found conflict for reserved [dd800000, efffffff], try to reserve with split
__reserve_region_with_split: (PCI Bus #80) [dd000000, ddffffff], res: (reserved) [dd800000, efffffff]
__reserve_region_with_split: (PCI Bus #00) [de000000, dfffffff], res: (reserved) [de000000, efffffff]
initcall pci_subsys_init+0x0/0x121 returned 0 after 381 msecs
in dmesg

v4: take out __insert_resource and insert_resource_expand_to_fit : Linus already check in.
use reserve_region_with_split at the first point
use const char *name

v5: fix checking on overlapping

v6: only reserve common area in conflict
so got sth in /proc/iomem
d8000000-dfffffff : PCI Bus #00
dc000000-dfffffff : GART
dc000000-dfffffff : reserved
e0000000-efffffff : PCI MMCONFIG 0
e0000000-efffffff : reserved
when we have big range in e820
00000000dc000000-00000000f0000000 (reserved)

v7: remove wrong removing of write_unlock(&resource_lock) in adjust_resource()

Signed-off-by: Yinghai Lu <yhlu.kernel@xxxxxxxxx>

---
arch/x86/kernel/e820.c | 2 -
include/linux/ioport.h | 3 ++
kernel/resource.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 72 insertions(+), 1 deletion(-)

Index: linux-2.6/arch/x86/kernel/e820.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/e820.c
+++ linux-2.6/arch/x86/kernel/e820.c
@@ -1320,7 +1320,7 @@ void __init e820_reserve_resources_late(
res = e820_res;
for (i = 0; i < e820.nr_map; i++) {
if (!res->parent && res->end)
- insert_resource(&iomem_resource, res);
+ reserve_region_with_split(&iomem_resource, res->start, res->end, res->name);
res++;
}
}
Index: linux-2.6/include/linux/ioport.h
===================================================================
--- linux-2.6.orig/include/linux/ioport.h
+++ linux-2.6/include/linux/ioport.h
@@ -108,6 +108,9 @@ extern struct resource iomem_resource;

extern int request_resource(struct resource *root, struct resource *new);
extern int release_resource(struct resource *new);
+extern void reserve_region_with_split(struct resource *root,
+ resource_size_t start, resource_size_t end,
+ const char *name);
extern int insert_resource(struct resource *parent, struct resource *new);
extern void insert_resource_expand_to_fit(struct resource *root, struct resource *new);
extern int allocate_resource(struct resource *root, struct resource *new,
Index: linux-2.6/kernel/resource.c
===================================================================
--- linux-2.6.orig/kernel/resource.c
+++ linux-2.6/kernel/resource.c
@@ -516,6 +516,74 @@ int adjust_resource(struct resource *res
return result;
}

+static void __init __reserve_region_with_split(struct resource *root,
+ resource_size_t start, resource_size_t end,
+ const char *name)
+{
+ struct resource *parent = root;
+ struct resource *conflict;
+ struct resource *res = kzalloc(sizeof(*res), GFP_KERNEL);
+
+ if (!res)
+ return;
+
+ res->name = name;
+ res->start = start;
+ res->end = end;
+ res->flags = IORESOURCE_BUSY;
+
+ for (;;) {
+ conflict = __request_resource(parent, res);
+ if (!conflict)
+ break;
+ if (conflict != parent) {
+ parent = conflict;
+ if (!(conflict->flags & IORESOURCE_BUSY))
+ continue;
+ }
+
+ /* Uhhuh, that didn't work out.. */
+ kfree(res);
+ res = NULL;
+ break;
+ }
+
+ if (!res) {
+ printk(KERN_DEBUG " __reserve_region_with_split: (%s) [%llx, %llx], res: (%s) [%llx, %llx]\n",
+ conflict->name, conflict->start, conflict->end,
+ name, start, end);
+
+ /* failed, split and try again */
+
+ /* conflict coverred whole area */
+ if (conflict->start <= start && conflict->end >= end)
+ return;
+
+ if (conflict->start > start)
+ __reserve_region_with_split(root, start, conflict->start-1, name);
+ if (!(conflict->flags & IORESOURCE_BUSY)) {
+ resource_size_t common_start, common_end;
+
+ common_start = max(conflict->start, start);
+ common_end = min(conflict->end, end);
+ if (common_start < common_end)
+ __reserve_region_with_split(root, common_start, common_end, name);
+ }
+ if (conflict->end < end)
+ __reserve_region_with_split(root, conflict->end+1, end, name);
+ }
+
+}
+
+void reserve_region_with_split(struct resource *root,
+ resource_size_t start, resource_size_t end,
+ const char *name)
+{
+ write_lock(&resource_lock);
+ __reserve_region_with_split(root, start, end, name);
+ write_unlock(&resource_lock);
+}
+
EXPORT_SYMBOL(adjust_resource);

/**
---
kernel/resource.c | 27 ++++++++++++++++++++++-----
1 file changed, 22 insertions(+), 5 deletions(-)

Index: linux-2.6/kernel/resource.c
===================================================================
--- linux-2.6.orig/kernel/resource.c
+++ linux-2.6/kernel/resource.c
@@ -201,6 +201,7 @@ int request_resource(struct resource *ro
write_lock(&resource_lock);
conflict = __request_resource(root, new);
write_unlock(&resource_lock);
+ printk(KERN_DEBUG "request_resource: root: (%s) [%llx, %llx], new: (%s) [%llx, %llx] conflict %d\n", root->name, root->start, root->end, new->name, new->start, new->end, !!conflict);
return conflict ? -EBUSY : 0;
}

@@ -372,12 +373,16 @@ static struct resource * __insert_resour

for (;; parent = first) {
first = __request_resource(parent, new);
- if (!first)
+ if (!first) {
+ printk(KERN_DEBUG " insert_resource: good with request direct parent: (%s) [%llx, %llx], new: (%s) [%llx, %llx]\n", parent->name, parent->start, parent->end, new->name, new->start, new->end);
+
return first;
+ }

if (first == parent)
return first;

+ printk(KERN_DEBUG " insert_resource: first: (%s) [%llx, %llx], new: (%s) [%llx, %llx]\n", first->name, first->start, first->end, new->name, new->start, new->end);
if ((first->start > new->start) || (first->end < new->end))
break;
if ((first->start == new->start) && (first->end == new->end))
@@ -397,10 +402,13 @@ static struct resource * __insert_resour
new->parent = parent;
new->sibling = next->sibling;
new->child = first;
+ printk(KERN_DEBUG " insert_resource: direct parent: (%s) [%llx, %llx], new: (%s) [%llx, %llx]\n", parent->name, parent->start, parent->end, new->name, new->start, new->end);

next->sibling = NULL;
- for (next = first; next; next = next->sibling)
+ for (next = first; next; next = next->sibling) {
next->parent = new;
+ printk(KERN_DEBUG " insert_resource: child: (%s) [%llx, %llx], new: (%s) [%llx, %llx]\n", next->name, next->start, next->end, new->name, new->start, new->end);
+ }

if (parent->child == first) {
parent->child = new;
@@ -533,8 +541,13 @@ static void __init __reserve_region_with

for (;;) {
conflict = __request_resource(parent, res);
- if (!conflict)
+ if (!conflict) {
+ printk(KERN_DEBUG " __reserve_region_with_split: good with request direct parent: (%s) [%llx, %llx], res: (%s) [%llx, %llx]\n",
+ parent->name, parent->start, parent->end,
+ name, start, end);
+
break;
+ }
if (conflict != parent) {
parent = conflict;
if (!(conflict->flags & IORESOURCE_BUSY))
@@ -548,7 +561,7 @@ static void __init __reserve_region_with
}

if (!res) {
- printk(KERN_DEBUG " __reserve_region_with_split: (%s) [%llx, %llx], res: (%s) [%llx, %llx]\n",
+ printk(KERN_DEBUG " __reserve_region_with_split: conflict: (%s) [%llx, %llx], res: (%s) [%llx, %llx]\n",
conflict->name, conflict->start, conflict->end,
name, start, end);

@@ -641,12 +654,16 @@ struct resource * __request_region(struc
struct resource *conflict;

conflict = __request_resource(parent, res);
- if (!conflict)
+ if (!conflict) {
+ printk(KERN_DEBUG " __request_region: good with request direct parent: (%s) [%llx, %llx], res: (%s) [%llx, %llx]\n", parent->name, parent->start, parent->end, res->name, res->start, res->end);
break;
+ }
if (conflict != parent) {
+ printk(KERN_DEBUG " __request_region: conflict: (%s) [%llx, %llx], res: (%s) [%llx, %llx]\n", conflict->name, conflict->start, conflict->end, res->name, res->start, res->end);
parent = conflict;
if (!(conflict->flags & IORESOURCE_BUSY))
continue;
+ printk(KERN_DEBUG "busy flag\n");
}

/* Uhhuh, that didn't work out.. */