I recently ran into a resource collision problem where PCI hot-plug
operations are failing for certain PCI topologies. One case
illustrating the problem is using a QLogic PCIe HBA in a slot with a
PCIe root port as its parent bus. Here is an abbreviated lspci output
for this topology:
-+-[0000:c2]---00.0-[0000:c3-fb]--+-00.0 QLogic Corp. 8Gb Fibre Channel HBA
| \-00.1 QLogic Corp. 8Gb Fibre Channel HBA
c2:00.0 PCI bridge: PCIe Root Port (prog-if 00 [Normal decode])
Bus: primary=c2, secondary=c3, subordinate=fb, sec-latency=0
I/O behind bridge: 00001000-0000ffff
Memory behind bridge: f0000000-fdffffff
Prefetchable memory behind bridge: 0000080780000000-00000807ffffffff
c3:00.0 Fibre Channel: QLogic Corp. 8Gb Fibre Channel HBA
Region 0: I/O ports at 8001100 [size=256]
Region 1: Memory at f0284000 (64-bit, non-prefetchable) [size=16K]
Region 3: Memory at f0100000 (64-bit, non-prefetchable) [size=1M]
Expansion ROM at f0240000 [disabled] [size=256K]
c3:00.1 Fibre Channel: QLogic Corp. 8Gb Fibre Channel HBA
Region 0: I/O ports at 8001000 [size=256]
Region 1: Memory at f0280000 (64-bit, non-prefetchable) [size=16K]
Region 3: Memory at f0000000 (64-bit, non-prefetchable) [size=1M]
Expansion ROM at f0200000 [disabled] [size=256K]
After boot, the resource tree looks like:
f0000000-fdffffff : PCI Bus 0000:c3
f0000000-fdffffff : PCI Bus 0000:c2
f0000000-f00fffff : 0000:c3:00.1
f0000000-f00fffff : qla2xxx
f0100000-f01fffff : 0000:c3:00.0
f0100000-f01fffff : qla2xxx
f0200000-f023ffff : 0000:c3:00.1
f0240000-f027ffff : 0000:c3:00.0
f0280000-f0283fff : 0000:c3:00.1
f0280000-f0283fff : qla2xxx
f0284000-f0287fff : 0000:c3:00.0
f0284000-f0287fff : qla2xxx
Note that PCI Bus 0000:c2 is a child of PCI Bus 0000:c3 and has an
identical address range.
When performing a PCI physical hot add and replace, logical hot add
and replace, or PCI error recovery of one the QLogic card functions in
the above topology, we get the following error messages (PCI debug in on):
GSI 85 (level, low) -> CPU 6 (0x0600) vector 87 unregistered
PCI: Scanning bus 0000:c2
pcieport-driver 0000:c2:00.0: scanning behind bridge, config fbc3c2, pass 0
PCI: Scanning bus 0000:c3
pci 0000:c3:00.0: found [1077:2532] class 000c04 header type 00
pci 0000:c3:00.0: reg 10 io port: [0x1100-0x11ff]
pci 0000:c3:00.0: reg 14 64bit mmio: [0xf0284000-0xf0287fff]
pci 0000:c3:00.0: reg 1c 64bit mmio: [0xf0100000-0xf01fffff]
pci 0000:c3:00.0: reg 30 32bit mmio: [0xf0240000-0xf027ffff]
pci 0000:c3:00.0: calling quirk_resource_alignment+0x0/0x3a0
pci 0000:c3:00.0: calling pci_fixup_video+0x0/0x280
PCI: Bus scan for 0000:c3 returning with max=c3
pcieport-driver 0000:c2:00.0: scanning behind bridge, config fbc3c2, pass 1
PCI: Bus scan for 0000:c2 returning with max=fb
pci 0000:c3:00.0: BAR 3: can't allocate mem resource [0xfe000000-0xfdffffff]
pci 0000:c3:00.0: BAR 6: got res [0x80780000000-0x8078003ffff] bus [0x80780000000-0x8078003ffff] flags 0x27200
pci 0000:c3:00.0: BAR 1: can't allocate mem resource [0xfe000000-0xfdffffff]
pci 0000:c3:00.0: BAR 0: got res [0x8001100-0x80011ff] bus [0x1100-0x11ff] flags 0x20101
pci 0000:c3:00.0: BAR 0: moved to bus [0x1100-0x11ff] flags 0x20101
GSI 85 (level, low) -> CPU 0 (0x0000) vector 87
qla2xxx 0000:c3:00.0: PCI INT A -> GSI 85 (level, low) -> IRQ 87
qla2xxx 0000:c3:00.0: region #1 not an MMIO resource (0000:c3:00.0), aborting
qla2xxx 0000:c3:00.0: PCI INT A disabled
GSI 85 (level, low) -> CPU 0 (0x0000) vector 87 unregistered
qla2xxx: probe of 0000:c3:00.0 failed with error -12
And the hot add operation fails. This failure is due to how PCI BAR
address resources are assigned in the parent buses.
BAR resources for PCI devices are allocated during hot add operations
using pci_allocate_resource() which calls find_resource() to find
empty resource slots and allocate_resource() to insert the resource in
the tree. Both find_resource() and allocate_resource() only search the
immediate child and its siblings of the root resource passed to it,
(f0000000-fdffffff : PCI Bus 0000:c3 in this example). The child
(f0000000-fdffffff : PCI Bus 0000:c2) has the same exact address range
resulting in a conflict and eventually returning -EBUSY. This
patchset changes find_resource() and allocate_resource() to
recursively search the resource tree below the root so the appropriate
entry is then located.
A similar problem was found in:
http://thread.gmane.org/gmane.linux.kernel/768526/
This patch does not address the possibly incorrect parenting of
identical resource ranges found in the above discussion, but it does
"fix" the problem when this condition occurs for the hot plug case.
Diff stats:
kernel/resource.c | 59 +++++++++++++++++++++++++++++++++++++++++++----------
1 files changed, 48 insertions(+), 11 deletions(-)
--
Andrew Patterson
Recurse when searching for empty slots in resources trees
The function pci_assign_resource() is called to allocate address
ranges for PCI device BARs in the parent bridges resource tree during
hot add operations (such as during physical hot-plug, logical
hot-plug, or PCI error recovery) of a PCI device. This function in
turn calls find_resource() to see if resources are available, and then
allocate_resource() to insert the resource into the tree. Both of
these functions only check the immediate children and its siblings of
the root resource passed to them. This algorithm can fail in certain
topologies where a resource is only available further down the
resource tree.
This patch changes find_resources() and allocate_resources() so that
they recursively descend the resource tree instead of just checking the
immediate child and its siblings.
Descendents are checked after the immediate child and its siblings to
maintain as much as possible the existing tree resource layout.
Signed-off-by: Andrew Patterson <[email protected]>
---
diff --git a/kernel/resource.c b/kernel/resource.c
index ac5f3a3..636c3b0 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -301,19 +301,28 @@ static int find_resource(struct resource *root, struct resource *new,
struct resource *this = root->child;
new->start = root->start;
+
/*
- * Skip past an allocated resource that starts at 0, since the assignment
- * of this->start - 1 to new->end below would cause an underflow.
+ * See if there is room before, after, or in between child
+ * and/or its siblings.
*/
- if (this && this->start == 0) {
- new->start = this->end + 1;
- this = this->sibling;
- }
for(;;) {
- if (this)
- new->end = this->start - 1;
- else
+ if (this) {
new->end = root->end;
+ if (new->start + size - 1 < this->start)
+ new->end = new->start + size - 1;
+ else {
+ if (this->sibling) {
+ if (this->sibling->start - this->end >= size - 1) {
+ new->start = this->end + 1;
+ new->end = this->sibling->start - 1;
+ } else
+ goto next;
+ } else
+ new->start = this->end + 1;
+ }
+ }
+
if (new->start < min)
new->start = min;
if (new->end > max)
@@ -327,9 +336,22 @@ static int find_resource(struct resource *root, struct resource *new,
}
if (!this)
break;
+next:
new->start = this->end + 1;
this = this->sibling;
}
+
+ /* See if there is room inside the child or its siblings. */
+ if (root->child) {
+ for (this = root->child; this; this = this->sibling) {
+ if (find_resource(this, new,
+ size, min, max, align,
+ alignf, alignf_data) == 0) {
+ return 0;
+ }
+ }
+ }
+
return -EBUSY;
}
@@ -352,11 +374,21 @@ int allocate_resource(struct resource *root, struct resource *new,
void *alignf_data)
{
int err;
+ struct resource *parent, *first;
write_lock(&resource_lock);
err = find_resource(root, new, size, min, max, align, alignf, alignf_data);
- if (err >= 0 && __request_resource(root, new))
- err = -EBUSY;
+ if (err >= 0) {
+ for (parent = root;; parent = first) {
+ first = __request_resource(parent, new);
+ if (!first)
+ break;
+ if (first == parent) {
+ err = -EBUSY;
+ break;
+ }
+ }
+ }
write_unlock(&resource_lock);
return err;
}
@@ -627,6 +659,11 @@ struct resource * __request_region(struct resource *parent,
parent = conflict;
if (!(conflict->flags & IORESOURCE_BUSY))
continue;
+ /* May have a duplicate that is not busy */
+ if (conflict->child) {
+ parent = conflict->child;
+ continue;
+ }
}
/* Uhhuh, that didn't work out.. */
On Tue, 16 Jun 2009, Andrew Patterson wrote:
>
> I recently ran into a resource collision problem where PCI hot-plug
> operations are failing for certain PCI topologies. One case
> illustrating the problem is using a QLogic PCIe HBA in a slot with a
> PCIe root port as its parent bus. Here is an abbreviated lspci output
> for this topology:
I think the problem is real, but the fix is wrong.
> After boot, the resource tree looks like:
>
> f0000000-fdffffff : PCI Bus 0000:c3
> f0000000-fdffffff : PCI Bus 0000:c2
So the problem is 9if I get this correctly) that c3 should be _inside_ c2.
No?
But you fix it by making find_resource always go as deep as it can (if I
read the code correctly). Which is not necessarily always what we want
either - we've had this kind of confusion before, and the problem is that
different users of find_resource simply want different things.
The deeper problem (I think) is that the whole "find c3 resource" thing
should not have started from the root IN THE FIRST PLACE. I think it
should have started from its parent resources (and then, as a special
case, if the parent is transparent, look into the parent of the parent,
etc - in which case it can easily end up in the root in the end, but only
if it couldn't fit things in a parent window).
And I'm almost certain that I've seen that patch from Ivan at some point,
but it was after the merge window closed so it didn't get merged.
Ivan? Or anybody else who remembers the patch?
Linus
On Tue, 2009-06-16 at 15:19 -0700, Linus Torvalds wrote:
>
> On Tue, 16 Jun 2009, Andrew Patterson wrote:
> >
> > I recently ran into a resource collision problem where PCI hot-plug
> > operations are failing for certain PCI topologies. One case
> > illustrating the problem is using a QLogic PCIe HBA in a slot with a
> > PCIe root port as its parent bus. Here is an abbreviated lspci output
> > for this topology:
>
> I think the problem is real, but the fix is wrong.
>
> > After boot, the resource tree looks like:
> >
> > f0000000-fdffffff : PCI Bus 0000:c3
> > f0000000-fdffffff : PCI Bus 0000:c2
>
> So the problem is 9if I get this correctly) that c3 should be _inside_ c2.
That is at least one problem. I initially tried reparenting this stuff.
That is what got backed out in
http://thread.gmane.org/gmane.linux.kernel/768526/
> No?
>
> But you fix it by making find_resource always go as deep as it can (if I
> read the code correctly).
Well, just deep enough.
> Which is not necessarily always what we want
> either - we've had this kind of confusion before, and the problem is that
> different users of find_resource simply want different thing
find_resources is only called by allocate_resources, which in turn is
called in about 15 places. I think this change won't affect callers
other than pci_bus_alloc_resource as I am careful to maintain current
behavior as much as possible.
Is there a reason that find_resources should stop at the roots immediate
child/sibling. It seems like a bug to me. Hence this patch.
>
> The deeper problem (I think) is that the whole "find c3 resource" thing
> should not have started from the root IN THE FIRST PLACE. I think it
> should have started from its parent resources (and then, as a special
> case, if the parent is transparent, look into the parent of the parent,
> etc - in which case it can easily end up in the root in the end, but only
> if it couldn't fit things in a parent window).
>
> And I'm almost certain that I've seen that patch from Ivan at some point,
> but it was after the merge window closed so it didn't get merged.
>
> Ivan? Or anybody else who remembers the patch?
>
> Linus
>
--
Andrew Patterson
Hewlett-Packard
On Tue, 16 Jun 2009, Andrew Patterson wrote:
>
> That is at least one problem. I initially tried reparenting this stuff.
> That is what got backed out in
> http://thread.gmane.org/gmane.linux.kernel/768526/
Well, aren't we in the exact same situation still? Ie the problem (as
Matthew claims) is:
'Basically it was that we came across a machine with the opposite
problem -- that we found a parent after we found a child (and claimed
the child's resources), and had no way to insert the parent's region
above the child's region. Alex's machine finds the child after the
parent and needs to insert the child's resource inside the parent's
resource.'
and the problem is that anything that isn't explicitly aware of the
topology is always going to be potentially confused about things like
this, since it's not clear at which level you want to find or add a
resource.
> > But you fix it by making find_resource always go as deep as it can (if I
> > read the code correctly).
>
> Well, just deep enough.
Ok, color me confused now. When is "as deep as it can" different from your
"just deep enough"?
> Is there a reason that find_resources should stop at the roots immediate
> child/sibling. It seems like a bug to me. Hence this patch.
Well, find_resource() found room for a resource. So it returns it. The
point is, your patch returns another - equally valid one.
Now, I'm not saying that your patch is wrong, but I _am_ worried that it
(once more) changes some random heuristic when we have two choices, and it
just makes it choose the other choice.
We've had those kinds of situations before. The thread you point to is an
exact case of this. My point is that I'd rather try to _avoid_ any
ambiguous cases, and try to solve it properly at a higher PCI level, where
the ambiguity doesn't exist any more (because we'd explicitly take the
actual bus topology into account).
So your patch may fix a bug, but I'm pretty sure I've seen a patch from
Ivan that should _also_ fix it, and that I would expect to do it not by
just tweaking a fundamentally ambiguous case.
Linus
On Tue, 16 Jun 2009, Linus Torvalds wrote:
>
> So your patch may fix a bug, but I'm pretty sure I've seen a patch from
> Ivan that should _also_ fix it, and that I would expect to do it not by
> just tweaking a fundamentally ambiguous case.
Hmm. For the life of me, I can't seem to find this patch. Maybe it wasn't
Ivan who wrote it after all. Or maybe my google-fu is weak. Or maybe I'm
just delusional, and the patch never existed.
However, regardless of that, I'm now confused about your patch too. So you
have this layout:
-+-[0000:c2]---00.0-[0000:c3-fb]--+-00.0 QLogic Corp. 8Gb Fibre Channel HBA
| \-00.1 QLogic Corp. 8Gb Fibre Channel HBA
where bus c3 is inside bus c2. Fine. And we clearly get that wrong in the
resource tree:
f0000000-fdffffff : PCI Bus 0000:c3
f0000000-fdffffff : PCI Bus 0000:c2
f0000000-f00fffff : 0000:c3:00.1
since that one end up having the c3 bus resource _outside_ of the c2 one.
That is, I think, the real bug. However, your patch doesn't try to fix
that bad nesting, but instead seems to try to work around it some odd way.
But looking at things, I don't even see how this happens in the first
place. Afaik, we use pci_assign_resource() to assign bus resources, and
that one _should_ nest properly. So now I'm really confused about how you
got that /proc/iomem in the first place.
Is this perhaps some hotplug-pci specific bug? How did that bus resource
for "PCI Bus 0000:c3" get allocated?
Linus
On Tue, 2009-06-16 at 16:05 -0700, Linus Torvalds wrote:
>
> On Tue, 16 Jun 2009, Andrew Patterson wrote:
> >
> > That is at least one problem. I initially tried reparenting this stuff.
> > That is what got backed out in
> > http://thread.gmane.org/gmane.linux.kernel/768526/
>
> Well, aren't we in the exact same situation still? Ie the problem (as
> Matthew claims) is:
>
> 'Basically it was that we came across a machine with the opposite
> problem -- that we found a parent after we found a child (and claimed
> the child's resources), and had no way to insert the parent's region
> above the child's region. Alex's machine finds the child after the
> parent and needs to insert the child's resource inside the parent's
> resource.'
>
> and the problem is that anything that isn't explicitly aware of the
> topology is always going to be potentially confused about things like
> this, since it's not clear at which level you want to find or add a
> resource.
>
> > > But you fix it by making find_resource always go as deep as it can (if I
> > > read the code correctly).
> >
> > Well, just deep enough.
>
> Ok, color me confused now. When is "as deep as it can" different from your
> "just deep enough"?
Maybe confusion on what is meant by 'as deep as'. My patch continues
until it doesn't find a conflict including checking sub-children and
stops as soon as an appropriate resource is found that does not
conflict. Perhaps we mean the same thing.
>
> > Is there a reason that find_resources should stop at the roots immediate
> > child/sibling. It seems like a bug to me. Hence this patch.
>
> Well, find_resource() found room for a resource. So it returns it. The
> point is, your patch returns another - equally valid one.
I am confused. The existing code will return a conflict and bomb out.
>
> Now, I'm not saying that your patch is wrong, but I _am_ worried that it
> (once more) changes some random heuristic when we have two choices, and it
> just makes it choose the other choice.
Agreed.
>
> We've had those kinds of situations before. The thread you point to is an
> exact case of this. My point is that I'd rather try to _avoid_ any
> ambiguous cases, and try to solve it properly at a higher PCI level, where
> the ambiguity doesn't exist any more (because we'd explicitly take the
> actual bus topology into account).
> So your patch may fix a bug, but I'm pretty sure I've seen a patch from
> Ivan that should _also_ fix it, and that I would expect to do it not by
> just tweaking a fundamentally ambiguous case.
>
OK. I would be happy to test Ivan's patch.
> Linus
>
--
Andrew Patterson
Hewlett-Packard
On Tue, 16 Jun 2009, Andrew Patterson wrote:
> >
> > Well, find_resource() found room for a resource. So it returns it. The
> > point is, your patch returns another - equally valid one.
>
> I am confused. The existing code will return a conflict and bomb out.
My point is, there are two answers - returning the conflicting resource,
or trying to recurse into it. Both are "valid", in the sense that both
have real semantics.
But the reason I don't like your patch is that I think the _deeper_
problem is the fact that the resource tree isn't set up right in the first
place.
It looks to me like your patch works around the bug (by trying to find
that "other possible case"), while my disagreement with that patch is that
we should never need to care about these kinds of "you can try to find
ambiguous cases" in the first place.
> > We've had those kinds of situations before. The thread you point to is an
> > exact case of this. My point is that I'd rather try to _avoid_ any
> > ambiguous cases, and try to solve it properly at a higher PCI level, where
> > the ambiguity doesn't exist any more (because we'd explicitly take the
> > actual bus topology into account).
> > So your patch may fix a bug, but I'm pretty sure I've seen a patch from
> > Ivan that should _also_ fix it, and that I would expect to do it not by
> > just tweaking a fundamentally ambiguous case.
>
> OK. I would be happy to test Ivan's patch.
I just looked at our PCI bus resource allocation code, and afaik it does
everything right even as-is. Which is why I now wonder if that incorrectly
nested bus resource was perhaps set up by the PCI hotplug code (which I do
not know, and didn't look at).
I may also be confused, and not have found the right place that actually
inserts the resource. Afaik, bus resources get allocated through
pbus_assign_resources_sorted ->
pci_assign_resource ->
pci_bus_alloc_resource ->
allocate_resource
and that "pci_bus_alloc_resource()" thing only allocates within the parent
bus, so it _should_ nest correctly.
It would be interesting to see where that resource actually gets
allocated. I'm clearly missing something, since clearly your resources do
_not_ nest correctly.
Linus
On Tue, 16 Jun 2009, Linus Torvalds wrote:
>
> I may also be confused, and not have found the right place that actually
> inserts the resource.
Can you try this patch? It's simple and stupid and will be very noisy, but
it should hopefully get a call chain for all the cases where we set
parenthood, and then we can hopefully see exactly where that incorrect
nesting gets done.
No guarantees, I didn't test this. But I suspect you see where I'm trying
to go.
Linus
On Tue, 16 Jun 2009 16:56:12 -0700 (PDT)
Linus Torvalds <[email protected]> wrote:
>
>
> On Tue, 16 Jun 2009, Andrew Patterson wrote:
> > >
> > > Well, find_resource() found room for a resource. So it returns
> > > it. The point is, your patch returns another - equally valid one.
> >
> > I am confused. The existing code will return a conflict and bomb
> > out.
>
> My point is, there are two answers - returning the conflicting
> resource, or trying to recurse into it. Both are "valid", in the
> sense that both have real semantics.
>
> But the reason I don't like your patch is that I think the _deeper_
> problem is the fact that the resource tree isn't set up right in the
> first place.
>
> It looks to me like your patch works around the bug (by trying to
> find that "other possible case"), while my disagreement with that
> patch is that we should never need to care about these kinds of "you
> can try to find ambiguous cases" in the first place.
>
> > > We've had those kinds of situations before. The thread you point
> > > to is an exact case of this. My point is that I'd rather try to
> > > _avoid_ any ambiguous cases, and try to solve it properly at a
> > > higher PCI level, where the ambiguity doesn't exist any more
> > > (because we'd explicitly take the actual bus topology into
> > > account). So your patch may fix a bug, but I'm pretty sure I've
> > > seen a patch from Ivan that should _also_ fix it, and that I
> > > would expect to do it not by just tweaking a fundamentally
> > > ambiguous case.
> >
> > OK. I would be happy to test Ivan's patch.
>
> I just looked at our PCI bus resource allocation code, and afaik it
> does everything right even as-is. Which is why I now wonder if that
> incorrectly nested bus resource was perhaps set up by the PCI hotplug
> code (which I do not know, and didn't look at).
>
> I may also be confused, and not have found the right place that
> actually inserts the resource. Afaik, bus resources get allocated
> through
>
> pbus_assign_resources_sorted ->
> pci_assign_resource ->
> pci_bus_alloc_resource ->
> allocate_resource
>
> and that "pci_bus_alloc_resource()" thing only allocates within the
> parent bus, so it _should_ nest correctly.
>
> It would be interesting to see where that resource actually gets
> allocated. I'm clearly missing something, since clearly your
> resources do _not_ nest correctly.
Alex has been fixing up hotplug related code recently too; Alex? Also
you didn't actually include a patch in your last mail that I could
see...
--
Jesse Barnes, Intel Open Source Technology Center
On Tue, 16 Jun 2009, Linus Torvalds wrote:
>
> Can you try this patch?
Oops. As Jesse pointed out, there was no patch.
_This_ time.
Linus
---
kernel/resource.c | 13 ++++++++++---
1 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/kernel/resource.c b/kernel/resource.c
index ac5f3a3..d9d7ede 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -140,6 +140,13 @@ __initcall(ioresources_init);
#endif /* CONFIG_PROC_FS */
+#define set_parent(x,p) __set_parent(__FUNCTION__, x, p)
+static void __set_parent(const char *fn, struct resource *x, struct resource *parent)
+{
+ WARN("%s: parent of '%s' is '%s'\n", fn, x->name, parent ? parent->name : "none");
+ x->parent = parent;
+}
+
/* Return the conflict entry if you can't request it */
static struct resource * __request_resource(struct resource *root, struct resource *new)
{
@@ -159,7 +166,7 @@ static struct resource * __request_resource(struct resource *root, struct resour
if (!tmp || tmp->start > end) {
new->sibling = tmp;
*p = new;
- new->parent = root;
+ set_parent(new, root);
return NULL;
}
p = &tmp->sibling;
@@ -395,13 +402,13 @@ static struct resource * __insert_resource(struct resource *parent, struct resou
break;
}
- new->parent = parent;
+ set_parent(new, parent);
new->sibling = next->sibling;
new->child = first;
next->sibling = NULL;
for (next = first; next; next = next->sibling)
- next->parent = new;
+ set_parent(next, new);
if (parent->child == first) {
parent->child = new;
On Tue, 2009-06-16 at 18:04 -0700, Linus Torvalds wrote:
>
> On Tue, 16 Jun 2009, Linus Torvalds wrote:
> >
> > Can you try this patch?
>
This looks functionally equivalent to the current code. I get way too
much output to be useful. I can log the serial port if we need it all,
or I can change it to just look at the buses in question if desired.
My hardware got changed around on me, so I will have to try this
tomorrow with the original hardware config.
Andrew
> Oops. As Jesse pointed out, there was no patch.
>
> _This_ time.
>
> Linus
>
> ---
> kernel/resource.c | 13 ++++++++++---
> 1 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/resource.c b/kernel/resource.c
> index ac5f3a3..d9d7ede 100644
> --- a/kernel/resource.c
> +++ b/kernel/resource.c
> @@ -140,6 +140,13 @@ __initcall(ioresources_init);
>
> #endif /* CONFIG_PROC_FS */
>
> +#define set_parent(x,p) __set_parent(__FUNCTION__, x, p)
> +static void __set_parent(const char *fn, struct resource *x, struct resource *parent)
> +{
> + WARN("%s: parent of '%s' is '%s'\n", fn, x->name, parent ? parent->name : "none");
> + x->parent = parent;
> +}
> +
> /* Return the conflict entry if you can't request it */
> static struct resource * __request_resource(struct resource *root, struct resource *new)
> {
> @@ -159,7 +166,7 @@ static struct resource * __request_resource(struct resource *root, struct resour
> if (!tmp || tmp->start > end) {
> new->sibling = tmp;
> *p = new;
> - new->parent = root;
> + set_parent(new, root);
> return NULL;
> }
> p = &tmp->sibling;
> @@ -395,13 +402,13 @@ static struct resource * __insert_resource(struct resource *parent, struct resou
> break;
> }
>
> - new->parent = parent;
> + set_parent(new, parent);
> new->sibling = next->sibling;
> new->child = first;
>
> next->sibling = NULL;
> for (next = first; next; next = next->sibling)
> - next->parent = new;
> + set_parent(next, new);
>
> if (parent->child == first) {
> parent->child = new;
>
--
Andrew Patterson
Hewlett-Packard
On Wed, 17 Jun 2009, Andrew Patterson wrote:
>
> This looks functionally equivalent to the current code.
Absolutely. It's only meant to add the call chains for all the resource
additions, so that we could see how that buggy 'c2' resource gets added as
a parent to 'c3' (when it should be the other way around).
> I get way too much output to be useful. I can log the serial port if we
> need it all, or I can change it to just look at the buses in question if
> desired.
Sure. Or increase CONFIG_LOG_BUF_SHIFT to 22 or somethng insane like that
(so that it logs up to 4MB of messages).
But yeah, if you just make it smarter, and only do that WARN() thing for
resources that match "PCI Bus" at the beginning of the name, that should
be plenty good enough.
Linus
Andrew Patterson wrote:
> I recently ran into a resource collision problem where PCI hot-plug
> operations are failing for certain PCI topologies. One case
> illustrating the problem is using a QLogic PCIe HBA in a slot with a
> PCIe root port as its parent bus. Here is an abbreviated lspci output
> for this topology:
>
> -+-[0000:c2]---00.0-[0000:c3-fb]--+-00.0 QLogic Corp. 8Gb Fibre Channel HBA
> | \-00.1 QLogic Corp. 8Gb Fibre Channel HBA
>
>
>
> c2:00.0 PCI bridge: PCIe Root Port (prog-if 00 [Normal decode])
> Bus: primary=c2, secondary=c3, subordinate=fb, sec-latency=0
> I/O behind bridge: 00001000-0000ffff
> Memory behind bridge: f0000000-fdffffff
> Prefetchable memory behind bridge: 0000080780000000-00000807ffffffff
>
> c3:00.0 Fibre Channel: QLogic Corp. 8Gb Fibre Channel HBA
> Region 0: I/O ports at 8001100 [size=256]
> Region 1: Memory at f0284000 (64-bit, non-prefetchable) [size=16K]
> Region 3: Memory at f0100000 (64-bit, non-prefetchable) [size=1M]
> Expansion ROM at f0240000 [disabled] [size=256K]
>
> c3:00.1 Fibre Channel: QLogic Corp. 8Gb Fibre Channel HBA
> Region 0: I/O ports at 8001000 [size=256]
> Region 1: Memory at f0280000 (64-bit, non-prefetchable) [size=16K]
> Region 3: Memory at f0000000 (64-bit, non-prefetchable) [size=1M]
> Expansion ROM at f0200000 [disabled] [size=256K]
>
> After boot, the resource tree looks like:
>
> f0000000-fdffffff : PCI Bus 0000:c3
> f0000000-fdffffff : PCI Bus 0000:c2
> f0000000-f00fffff : 0000:c3:00.1
> f0000000-f00fffff : qla2xxx
> f0100000-f01fffff : 0000:c3:00.0
> f0100000-f01fffff : qla2xxx
> f0200000-f023ffff : 0000:c3:00.1
> f0240000-f027ffff : 0000:c3:00.0
> f0280000-f0283fff : 0000:c3:00.1
> f0280000-f0283fff : qla2xxx
> f0284000-f0287fff : 0000:c3:00.0
> f0284000-f0287fff : qla2xxx
>
> Note that PCI Bus 0000:c2 is a child of PCI Bus 0000:c3 and has an
> identical address range.
According to the lspci output, PCI Bus 0000:c2 is a parent of PCI
Bus 0000:c3. But they are upside down in resource tree. I guess
this is the root cause of the problem.
I think the reason why it happen is ia64 (I believe you are using
ia64 environment) uses pci_claim_resource(), which calls
insert_resource(), to insert resources. In my understanding, if
two resources having an identical address range are inserted using
insert_resource(), the latter one becomes parent of the first one.
I made a sample patch, though I don't know if it fixes the problem.
I hope this would help you. But please note that it is NOT well
tested.
Thanks,
Kenji Kaneshige
arch/ia64/pci/pci.c | 32 +++++++++++++++++++++++---------
1 file changed, 23 insertions(+), 9 deletions(-)
Index: 20090612/arch/ia64/pci/pci.c
===================================================================
--- 20090612.orig/arch/ia64/pci/pci.c
+++ 20090612/arch/ia64/pci/pci.c
@@ -301,9 +301,10 @@ static __devinit acpi_status add_window(
}
static void __devinit
-pcibios_setup_root_windows(struct pci_bus *bus, struct pci_controller *ctrl)
+pcibios_setup_root_windows(struct pci_bus *bus)
{
int i, j;
+ struct pci_controller *ctrl = bus->sysdata;
j = 0;
for (i = 0; i < ctrl->windows; i++) {
@@ -371,9 +372,6 @@ pci_acpi_scan_root(struct acpi_device *d
* such quirk. So we just ignore the case now.
*/
pbus = pci_scan_bus_parented(NULL, bus, &pci_root_ops, controller);
- if (pbus)
- pcibios_setup_root_windows(pbus, controller);
-
return pbus;
out3:
@@ -456,15 +454,29 @@ pcibios_fixup_resources(struct pci_dev *
{
struct pci_bus_region region;
int i;
+ struct resource *devr, *busr;
for (i = start; i < limit; i++) {
- if (!dev->resource[i].flags)
+ char *dtype = i < PCI_BRIDGE_RESOURCES ? "device" : "bridge";
+
+ devr = &dev->resource[i];
+ if (!devr->flags)
continue;
+
region.start = dev->resource[i].start;
region.end = dev->resource[i].end;
- pcibios_bus_to_resource(dev, &dev->resource[i], ®ion);
- if ((is_valid_resource(dev, i)))
- pci_claim_resource(dev, i);
+ pcibios_bus_to_resource(dev, devr, ®ion);
+ if (!is_valid_resource(dev, i))
+ continue;
+
+ busr = pci_find_parent_resource(dev, devr);
+ if (!busr || request_resource(busr, devr)) {
+ dev_err(&dev->dev, "BAR %d: %s of %s %pR\n", i,
+ busr ? "address space collision on" :
+ "no parent found for",
+ dtype, devr);
+ devr->flags = 0;
+ }
}
}
@@ -487,7 +499,9 @@ pcibios_fixup_bus (struct pci_bus *b)
{
struct pci_dev *dev;
- if (b->self) {
+ if (pci_is_root_bus(b))
+ pcibios_setup_root_windows(b);
+ else {
pci_read_bridge_bases(b);
pcibios_fixup_bridge_resources(b->self);
}
On Wed, Jun 17, 2009 at 06:13:24PM +0900, Kenji Kaneshige wrote:
> I think the reason why it happen is ia64 (I believe you are using
> ia64 environment) uses pci_claim_resource(), which calls
> insert_resource(), to insert resources. In my understanding, if
> two resources having an identical address range are inserted using
> insert_resource(), the latter one becomes parent of the first one.
>
> I made a sample patch, though I don't know if it fixes the problem.
> I hope this would help you. But please note that it is NOT well
> tested.
This patch seems to be saying that it's ia64's fault for using
pci_claim_resource. Surely pci_claim_resource() is buggy and is the
right place to fix the problem (other users include alpha, mn10300,
powerpc and parisc).
This all kind of begs the question why x86 isn't using
pci_claim_resource(); if it were, bugs like this would be found and
fixed earlier.
It further begs the question why architectures are doing this in the
first place. All PCI device resources should be in /proc/iomem, no
matter what architecture.
Anyway, this should fix pci_claim_resource (untested):
Signed-off-by: Matthew Wilcox <[email protected]>
diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index 3039fcb..1240351 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -99,11 +99,11 @@ void pci_update_resource(struct pci_dev *dev, int resno)
int pci_claim_resource(struct pci_dev *dev, int resource)
{
struct resource *res = &dev->resource[resource];
- struct resource *root = NULL;
+ struct resource *root;
char *dtype = resource < PCI_BRIDGE_RESOURCES ? "device" : "bridge";
int err;
- root = pcibios_select_root(dev, res);
+ root = pci_find_parent_resource(dev, res);
err = -EINVAL;
if (root != NULL)
--
Matthew Wilcox Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."
On Tue, Jun 16, 2009 at 04:32:18PM -0700, Linus Torvalds wrote:
> > So your patch may fix a bug, but I'm pretty sure I've seen a patch from
> > Ivan that should _also_ fix it, and that I would expect to do it not by
> > just tweaking a fundamentally ambiguous case.
>
> Hmm. For the life of me, I can't seem to find this patch. Maybe it wasn't
> Ivan who wrote it after all. Or maybe my google-fu is weak. Or maybe I'm
> just delusional, and the patch never existed.
No, it wasn't me.
Anyway, pci_claim_resource() fix suggested by Matthew seems to be
correct, if the problematic system was indeed ia64 and not x86.
Ivan.
* Jesse Barnes <[email protected]>:
>
> Alex has been fixing up hotplug related code recently too; Alex? Also
> you didn't actually include a patch in your last mail that I could
> see...
Oh, I'm well aware that Andrew has been working on this, and am
happy to let him keep driving. :)
I may drag out my old machine that I use to test cpqphp and see
if we have the same issues there, as it's x86, and I did notice
some BAR collisions last time I checked.
Thanks.
/ac
On Wed, 17 Jun 2009, Matthew Wilcox wrote:
>
> Anyway, this should fix pci_claim_resource (untested):
Ack. This looks very sane. I'll happily commit this, but would be even
happier to hear that it got some testing too, so I'll hold off for a
while.
Linus
On Wed, 17 Jun 2009, Ivan Kokshaysky wrote:
> On Tue, Jun 16, 2009 at 04:32:18PM -0700, Linus Torvalds wrote:
> > > So your patch may fix a bug, but I'm pretty sure I've seen a patch from
> > > Ivan that should _also_ fix it, and that I would expect to do it not by
> > > just tweaking a fundamentally ambiguous case.
> >
> > Hmm. For the life of me, I can't seem to find this patch. Maybe it wasn't
> > Ivan who wrote it after all. Or maybe my google-fu is weak. Or maybe I'm
> > just delusional, and the patch never existed.
>
> No, it wasn't me.
Ingo pointed out that it was probably Yinghai. And now that I'm googling
for the right author, I found it on the first try. It's this patch
http://lkml.org/lkml/2009/4/22/433
I was thinking of.
But that was before I realized that pci_assign_resource() _already_ always
chose the right parent bus (and it was just that it can't handle
transparent buses at all). I then ended up not understanding how the
incorrect nesting could possibly happen at all. So Yinghai's patch is
irrelevant for this particular problem.
And:
> Anyway, pci_claim_resource() fix suggested by Matthew seems to be
> correct, if the problematic system was indeed ia64 and not x86.
I agree. That one explains why the nesting is wrong, and also why I
couldn't figure out how it happened.
Linus
On Wed, 2009-06-17 at 09:23 -0700, Linus Torvalds wrote:
>
> On Wed, 17 Jun 2009, Matthew Wilcox wrote:
> >
> > Anyway, this should fix pci_claim_resource (untested):
>
> Ack. This looks very sane. I'll happily commit this, but would be even
> happier to hear that it got some testing too, so I'll hold off for a
> while.
>
I tried Mathew's patch. It did not help. Here are the resources with
the applied patch:
0000000-fdffffff : PCI Bus 0000:c3
f0000000-fdffffff : PCI Bus 0000:c2
f0000000-f00fffff : 0000:c3:00.1
f0000000-f00fffff : qla2xxx
f0100000-f01fffff : 0000:c3:00.0
f0100000-f01fffff : qla2xxx
f0200000-f023ffff : 0000:c3:00.1
f0240000-f027ffff : 0000:c3:00.0
f0280000-f0283fff : 0000:c3:00.1
f0280000-f0283fff : qla2xxx
f0284000-f0287fff : 0000:c3:00.0
f0284000-f0287fff : qla2xxx
Note we still have the incorrect parenting problem.
At Mathew's suggestion, I added some trace code using the following
patch:
diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index 3039fcb..e2d2814 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -99,11 +99,12 @@ void pci_update_resource(struct pci_dev *dev, int resno)
int pci_claim_resource(struct pci_dev *dev, int resource)
{
struct resource *res = &dev->resource[resource];
- struct resource *root = NULL;
+ struct resource *root;
char *dtype = resource < PCI_BRIDGE_RESOURCES ? "device" : "bridge";
int err;
- root = pcibios_select_root(dev, res);
+ root = pci_find_parent_resource(dev, res);
+ dev_printk(KERN_EMERG, &dev->dev, "%s: root = %pR, res = %pR\n", __func__, root, res);
err = -EINVAL;
if (root != NULL)
And here is the output:
PCI: Scanning bus 0000:c2
pci 0000:c2:00.0: found [103c:403b] class 000604 header type 01
pci 0000:c2:00.0: calling quirk_resource_alignment+0x0/0x3a0
pci 0000:c2:00.0: calling pci_fixup_video+0x0/0x280
pci 0000:c2:00.0: PME# supported from D0 D3hot D3cold
pci 0000:c2:00.0: PME# disabled
PCI: Fixups for bus 0000:c2
pci 0000:c2:00.0: scanning behind bridge, config fbc3c2, pass 0
PCI: Scanning bus 0000:c3
pci 0000:c3:00.0: found [1077:2532] class 000c04 header type 00
pci 0000:c3:00.0: reg 10 io port: [0x1100-0x11ff]
pci 0000:c3:00.0: reg 14 64bit mmio: [0xf0284000-0xf0287fff]
pci 0000:c3:00.0: reg 1c 64bit mmio: [0xf0100000-0xf01fffff]
pci 0000:c3:00.0: reg 30 32bit mmio: [0xf0240000-0xf027ffff]
pci 0000:c3:00.0: calling quirk_resource_alignment+0x0/0x3a0
pci 0000:c3:00.0: calling pci_fixup_video+0x0/0x280
pci 0000:c3:00.1: found [1077:2532] class 000c04 header type 00
pci 0000:c3:00.1: reg 10 io port: [0x1000-0x10ff]
pci 0000:c3:00.1: reg 14 64bit mmio: [0xf0280000-0xf0283fff]
pci 0000:c3:00.1: reg 1c 64bit mmio: [0xf0000000-0xf00fffff]
pci 0000:c3:00.1: reg 30 32bit mmio: [0xf0200000-0xf023ffff]
pci 0000:c3:00.1: calling quirk_resource_alignment+0x0/0x3a0
pci 0000:c3:00.1: calling pci_fixup_video+0x0/0x280
PCI: Fixups for bus 0000:c3
pci 0000:c2:00.0: bridge io port: [0x1000-0xffff]
pci 0000:c2:00.0: bridge 32bit mmio: [0xf0000000-0xfdffffff]
pci 0000:c2:00.0: bridge 64bit mmio pref: [0x80780000000-0x807ffffffff]
pci 0000:c2:00.0: pci_claim_resource: root = [0x00-0xffffffffffffffff], res = [0x8
001000-0x800ffff]
pci 0000:c2:00.0: pci_claim_resource: root = [0x000000-0xffffffffffffffff], res =
[0xf0000000-0xfdffffff]
pci 0000:c2:00.0: pci_claim_resource: root = [0x000000-0xffffffffffffffff], res =
[0x80780000000-0x807ffffffff]
pci 0000:c3:00.0: pci_claim_resource: root = [0x8001000-0x800ffff], res = [0x80011
00-0x80011ff]
pci 0000:c3:00.0: pci_claim_resource: root = [0xf0000000-0xfdffffff], res = [0xf02
84000-0xf0287fff]
pci 0000:c3:00.0: pci_claim_resource: root = [0xf0000000-0xfdffffff], res = [0xf01
00000-0xf01fffff]
pci 0000:c3:00.0: pci_claim_resource: root = [0xf0000000-0xfdffffff], res = [0xf02
40000-0xf027ffff]
pci 0000:c3:00.1: pci_claim_resource: root = [0x8001000-0x800ffff], res = [0x80010
00-0x80010ff]
pci 0000:c3:00.1: pci_claim_resource: root = [0xf0000000-0xfdffffff], res = [0xf02
80000-0xf0283fff]
pci 0000:c3:00.1: pci_claim_resource: root = [0xf0000000-0xfdffffff], res = [0xf00
00000-0xf00fffff]
pci 0000:c3:00.1: pci_claim_resource: root = [0xf0000000-0xfdffffff], res = [0xf02
00000-0xf023ffff]
PCI: Bus scan for 0000:c3 returning with max=c3
pci 0000:c2:00.0: scanning behind bridge, config fbc3c2, pass 1
PCI: Bus scan for 0000:c2 returning with max=fb
> Linus
>
--
Andrew Patterson
Hewlett-Packard
On Wed, 17 Jun 2009, Andrew Patterson wrote:
>
> I tried Mathew's patch. It did not help. Here are the resources with
> the applied patch:
>
> 0000000-fdffffff : PCI Bus 0000:c3
^^^^
I think you missed the initial 'f' in your cut-and-paste.
> f0000000-fdffffff : PCI Bus 0000:c2
> f0000000-f00fffff : 0000:c3:00.1
> f0000000-f00fffff : qla2xxx
> f0100000-f01fffff : 0000:c3:00.0
> f0100000-f01fffff : qla2xxx
> f0200000-f023ffff : 0000:c3:00.1
> f0240000-f027ffff : 0000:c3:00.0
> f0280000-f0283fff : 0000:c3:00.1
> f0280000-f0283fff : qla2xxx
> f0284000-f0287fff : 0000:c3:00.0
> f0284000-f0287fff : qla2xxx
>
> Note we still have the incorrect parenting problem.
Hmm.
> At Mathew's suggestion, I added some trace code using the following
> patch:
>
> diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> index 3039fcb..e2d2814 100644
> --- a/drivers/pci/setup-res.c
> +++ b/drivers/pci/setup-res.c
> @@ -99,11 +99,12 @@ void pci_update_resource(struct pci_dev *dev, int resno)
> int pci_claim_resource(struct pci_dev *dev, int resource)
> {
> struct resource *res = &dev->resource[resource];
> - struct resource *root = NULL;
> + struct resource *root;
> char *dtype = resource < PCI_BRIDGE_RESOURCES ? "device" : "bridge";
> int err;
>
> - root = pcibios_select_root(dev, res);
> + root = pci_find_parent_resource(dev, res);
> + dev_printk(KERN_EMERG, &dev->dev, "%s: root = %pR, res = %pR\n", __func__, root, res);
I'd really like to have seen the name of the resource too.
Just showing the resource ranges is kind of pointless when they match.
But I'm starting to have a suspicion here:
> PCI: Scanning bus 0000:c2
> pci 0000:c2:00.0: found [103c:403b] class 000604 header type 01
> pci 0000:c2:00.0: calling quirk_resource_alignment+0x0/0x3a0
> pci 0000:c2:00.0: calling pci_fixup_video+0x0/0x280
> pci 0000:c2:00.0: PME# supported from D0 D3hot D3cold
> pci 0000:c2:00.0: PME# disabled
> PCI: Fixups for bus 0000:c2
> pci 0000:c2:00.0: scanning behind bridge, config fbc3c2, pass 0
> PCI: Scanning bus 0000:c3
> pci 0000:c3:00.0: found [1077:2532] class 000c04 header type 00
> pci 0000:c3:00.0: reg 10 io port: [0x1100-0x11ff]
> pci 0000:c3:00.0: reg 14 64bit mmio: [0xf0284000-0xf0287fff]
> pci 0000:c3:00.0: reg 1c 64bit mmio: [0xf0100000-0xf01fffff]
> pci 0000:c3:00.0: reg 30 32bit mmio: [0xf0240000-0xf027ffff]
> pci 0000:c3:00.0: calling quirk_resource_alignment+0x0/0x3a0
> pci 0000:c3:00.0: calling pci_fixup_video+0x0/0x280
> pci 0000:c3:00.1: found [1077:2532] class 000c04 header type 00
> pci 0000:c3:00.1: reg 10 io port: [0x1000-0x10ff]
> pci 0000:c3:00.1: reg 14 64bit mmio: [0xf0280000-0xf0283fff]
> pci 0000:c3:00.1: reg 1c 64bit mmio: [0xf0000000-0xf00fffff]
> pci 0000:c3:00.1: reg 30 32bit mmio: [0xf0200000-0xf023ffff]
> pci 0000:c3:00.1: calling quirk_resource_alignment+0x0/0x3a0
> pci 0000:c3:00.1: calling pci_fixup_video+0x0/0x280
> PCI: Fixups for bus 0000:c3
> pci 0000:c2:00.0: bridge io port: [0x1000-0xffff]
> pci 0000:c2:00.0: bridge 32bit mmio: [0xf0000000-0xfdffffff]
> pci 0000:c2:00.0: bridge 64bit mmio pref: [0x80780000000-0x807ffffffff]
> pci 0000:c2:00.0: pci_claim_resource: root = [0x00-0xffffffffffffffff], res = [0x8001000-0x800ffff]
> pci 0000:c2:00.0: pci_claim_resource: root = [0x000000-0xffffffffffffffff], res = [0xf0000000-0xfdffffff]
> pci 0000:c2:00.0: pci_claim_resource: root = [0x000000-0xffffffffffffffff], res = [0x80780000000-0x807ffffffff]
> pci 0000:c3:00.0: pci_claim_resource: root = [0x8001000-0x800ffff], res = [0x8001100-0x80011ff]
> pci 0000:c3:00.0: pci_claim_resource: root = [0xf0000000-0xfdffffff], res = [0xf0284000-0xf0287fff]
> pci 0000:c3:00.0: pci_claim_resource: root = [0xf0000000-0xfdffffff], res = [0xf0100000-0xf01fffff]
> pci 0000:c3:00.0: pci_claim_resource: root = [0xf0000000-0xfdffffff], res = [0xf0240000-0xf027ffff]
> pci 0000:c3:00.1: pci_claim_resource: root = [0x8001000-0x800ffff], res = [0x8001000-0x80010ff]
> pci 0000:c3:00.1: pci_claim_resource: root = [0xf0000000-0xfdffffff], res = [0xf0280000-0xf0283fff]
> pci 0000:c3:00.1: pci_claim_resource: root = [0xf0000000-0xfdffffff], res = [0xf0000000-0xf00fffff]
> pci 0000:c3:00.1: pci_claim_resource: root = [0xf0000000-0xfdffffff], res = [0xf0200000-0xf023ffff]
> PCI: Bus scan for 0000:c3 returning with max=c3
> pci 0000:c2:00.0: scanning behind bridge, config fbc3c2, pass 1
> PCI: Bus scan for 0000:c2 returning with max=fb
You're not actually showing the case where you have that error case of
"0xf0000000-0xfdffffff" inside another "0xf0000000-0xfdffffff"
IOW, that one is done in some totally different place, not in
'pci_claim_resource()' at all.
So no wonder it makes no difference when pci_claim_resource() is fixed.
This is why I'd really like to see the output of my test-patch. It would
show exactly _where_ that resource is inserted, and the whole call-chain.
I'm appending a version that only does it for resources that have names
starting with "PCI Bus", so it should be less noisy. But again, it's
totally untested.
Linus
---
kernel/resource.c | 14 +++++++++++---
1 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/kernel/resource.c b/kernel/resource.c
index ac5f3a3..023ba7a 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -140,6 +140,14 @@ __initcall(ioresources_init);
#endif /* CONFIG_PROC_FS */
+#define set_parent(x,p) __set_parent(__FUNCTION__, x, p)
+static void __set_parent(const char *fn, struct resource *x, struct resource *parent)
+{
+ WARN(!strncmp(x->name, "PCI Bus", 7),
+ "%s: parent of '%s' is '%s'\n", fn, x->name, parent ? parent->name : "none");
+ x->parent = parent;
+}
+
/* Return the conflict entry if you can't request it */
static struct resource * __request_resource(struct resource *root, struct resource *new)
{
@@ -159,7 +167,7 @@ static struct resource * __request_resource(struct resource *root, struct resour
if (!tmp || tmp->start > end) {
new->sibling = tmp;
*p = new;
- new->parent = root;
+ set_parent(new, root);
return NULL;
}
p = &tmp->sibling;
@@ -395,13 +403,13 @@ static struct resource * __insert_resource(struct resource *parent, struct resou
break;
}
- new->parent = parent;
+ set_parent(new, parent);
new->sibling = next->sibling;
new->child = first;
next->sibling = NULL;
for (next = first; next; next = next->sibling)
- next->parent = new;
+ set_parent(next, new);
if (parent->child == first) {
parent->child = new;
On Wed, 2009-06-17 at 11:12 -0700, Linus Torvalds wrote:
>
> On Wed, 17 Jun 2009, Andrew Patterson wrote:
> >
> > I tried Mathew's patch. It did not help. Here are the resources with
> > the applied patch:
> >
> > 0000000-fdffffff : PCI Bus 0000:c3
>
> ^^^^
> I think you missed the initial 'f' in your cut-and-paste.
>
> > f0000000-fdffffff : PCI Bus 0000:c2
> > f0000000-f00fffff : 0000:c3:00.1
> > f0000000-f00fffff : qla2xxx
> > f0100000-f01fffff : 0000:c3:00.0
> > f0100000-f01fffff : qla2xxx
> > f0200000-f023ffff : 0000:c3:00.1
> > f0240000-f027ffff : 0000:c3:00.0
> > f0280000-f0283fff : 0000:c3:00.1
> > f0280000-f0283fff : qla2xxx
> > f0284000-f0287fff : 0000:c3:00.0
> > f0284000-f0287fff : qla2xxx
> >
> > Note we still have the incorrect parenting problem.
>
> Hmm.
>
> > At Mathew's suggestion, I added some trace code using the following
> > patch:
> >
> > diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> > index 3039fcb..e2d2814 100644
> > --- a/drivers/pci/setup-res.c
> > +++ b/drivers/pci/setup-res.c
> > @@ -99,11 +99,12 @@ void pci_update_resource(struct pci_dev *dev, int resno)
> > int pci_claim_resource(struct pci_dev *dev, int resource)
> > {
> > struct resource *res = &dev->resource[resource];
> > - struct resource *root = NULL;
> > + struct resource *root;
> > char *dtype = resource < PCI_BRIDGE_RESOURCES ? "device" : "bridge";
> > int err;
> >
> > - root = pcibios_select_root(dev, res);
> > + root = pci_find_parent_resource(dev, res);
> > + dev_printk(KERN_EMERG, &dev->dev, "%s: root = %pR, res = %pR\n", __func__, root, res);
>
> I'd really like to have seen the name of the resource too.
>
I can do that.
> Just showing the resource ranges is kind of pointless when they match.
>
> But I'm starting to have a suspicion here:
>
> > PCI: Scanning bus 0000:c2
> > pci 0000:c2:00.0: found [103c:403b] class 000604 header type 01
> > pci 0000:c2:00.0: calling quirk_resource_alignment+0x0/0x3a0
> > pci 0000:c2:00.0: calling pci_fixup_video+0x0/0x280
> > pci 0000:c2:00.0: PME# supported from D0 D3hot D3cold
> > pci 0000:c2:00.0: PME# disabled
> > PCI: Fixups for bus 0000:c2
> > pci 0000:c2:00.0: scanning behind bridge, config fbc3c2, pass 0
> > PCI: Scanning bus 0000:c3
> > pci 0000:c3:00.0: found [1077:2532] class 000c04 header type 00
> > pci 0000:c3:00.0: reg 10 io port: [0x1100-0x11ff]
> > pci 0000:c3:00.0: reg 14 64bit mmio: [0xf0284000-0xf0287fff]
> > pci 0000:c3:00.0: reg 1c 64bit mmio: [0xf0100000-0xf01fffff]
> > pci 0000:c3:00.0: reg 30 32bit mmio: [0xf0240000-0xf027ffff]
> > pci 0000:c3:00.0: calling quirk_resource_alignment+0x0/0x3a0
> > pci 0000:c3:00.0: calling pci_fixup_video+0x0/0x280
> > pci 0000:c3:00.1: found [1077:2532] class 000c04 header type 00
> > pci 0000:c3:00.1: reg 10 io port: [0x1000-0x10ff]
> > pci 0000:c3:00.1: reg 14 64bit mmio: [0xf0280000-0xf0283fff]
> > pci 0000:c3:00.1: reg 1c 64bit mmio: [0xf0000000-0xf00fffff]
> > pci 0000:c3:00.1: reg 30 32bit mmio: [0xf0200000-0xf023ffff]
> > pci 0000:c3:00.1: calling quirk_resource_alignment+0x0/0x3a0
> > pci 0000:c3:00.1: calling pci_fixup_video+0x0/0x280
> > PCI: Fixups for bus 0000:c3
> > pci 0000:c2:00.0: bridge io port: [0x1000-0xffff]
> > pci 0000:c2:00.0: bridge 32bit mmio: [0xf0000000-0xfdffffff]
> > pci 0000:c2:00.0: bridge 64bit mmio pref: [0x80780000000-0x807ffffffff]
> > pci 0000:c2:00.0: pci_claim_resource: root = [0x00-0xffffffffffffffff], res = [0x8001000-0x800ffff]
> > pci 0000:c2:00.0: pci_claim_resource: root = [0x000000-0xffffffffffffffff], res = [0xf0000000-0xfdffffff]
> > pci 0000:c2:00.0: pci_claim_resource: root = [0x000000-0xffffffffffffffff], res = [0x80780000000-0x807ffffffff]
> > pci 0000:c3:00.0: pci_claim_resource: root = [0x8001000-0x800ffff], res = [0x8001100-0x80011ff]
> > pci 0000:c3:00.0: pci_claim_resource: root = [0xf0000000-0xfdffffff], res = [0xf0284000-0xf0287fff]
> > pci 0000:c3:00.0: pci_claim_resource: root = [0xf0000000-0xfdffffff], res = [0xf0100000-0xf01fffff]
> > pci 0000:c3:00.0: pci_claim_resource: root = [0xf0000000-0xfdffffff], res = [0xf0240000-0xf027ffff]
> > pci 0000:c3:00.1: pci_claim_resource: root = [0x8001000-0x800ffff], res = [0x8001000-0x80010ff]
> > pci 0000:c3:00.1: pci_claim_resource: root = [0xf0000000-0xfdffffff], res = [0xf0280000-0xf0283fff]
> > pci 0000:c3:00.1: pci_claim_resource: root = [0xf0000000-0xfdffffff], res = [0xf0000000-0xf00fffff]
> > pci 0000:c3:00.1: pci_claim_resource: root = [0xf0000000-0xfdffffff], res = [0xf0200000-0xf023ffff]
> > PCI: Bus scan for 0000:c3 returning with max=c3
> > pci 0000:c2:00.0: scanning behind bridge, config fbc3c2, pass 1
> > PCI: Bus scan for 0000:c2 returning with max=fb
>
> You're not actually showing the case where you have that error case of
> "0xf0000000-0xfdffffff" inside another "0xf0000000-0xfdffffff"
>
> IOW, that one is done in some totally different place, not in
> 'pci_claim_resource()' at all.
>
> So no wonder it makes no difference when pci_claim_resource() is fixed.
>
> This is why I'd really like to see the output of my test-patch. It would
> show exactly _where_ that resource is inserted, and the whole call-chain.
>
> I'm appending a version that only does it for resources that have names
> starting with "PCI Bus", so it should be less noisy. But again, it's
> totally untested.
>
Here you go (I can provide the full boot log if needed). Note, there is
nothing for c3 here:
.
.
.
ACPI: PCI Root Bridge [L007] (0000:c2)
------------[ cut here ]------------
WARNING: at kernel/resource.c:147 __set_parent+0xc0/0xe0()
Hardware name: server rx6600
__request_resource: parent of 'PCI Bus 0000:c2 I/O Ports
08000000-0800ffff' is 'PCI mem'
Modules linked in:
Call Trace:
[<a000000100015b70>] show_stack+0x50/0xa0
sp=e0000100603dfb30 bsp=e0000100603d9188
[<a000000100015bf0>] dump_stack+0x30/0x60
sp=e0000100603dfd00 bsp=e0000100603d9170
[<a00000010006fa00>] warn_slowpath_common+0xc0/0x100
sp=e0000100603dfd00 bsp=e0000100603d9138
[<a00000010006fb30>] warn_slowpath_fmt+0x90/0xc0
sp=e0000100603dfd00 bsp=e0000100603d90d8
[<a00000010007ef40>] __set_parent+0xc0/0xe0
sp=e0000100603dfd40 bsp=e0000100603d90a0
[<a00000010007f060>] __request_resource+0x100/0x160
sp=e0000100603dfd40 bsp=e0000100603d9078
[<a00000010007f3e0>] __insert_resource+0x20/0x2a0
sp=e0000100603dfd40 bsp=e0000100603d9038
[<a00000010007f990>] insert_resource+0x30/0x80
sp=e0000100603dfd40 bsp=e0000100603d9010
[<a000000100a008d0>] add_window+0x490/0x6a0
sp=e0000100603dfd40 bsp=e0000100603d8fa0
[<a0000001005b2250>] acpi_walk_resources+0x1f0/0x340
sp=e0000100603dfd90 bsp=e0000100603d8f60
[<a000000100a00110>] pci_acpi_scan_root+0x210/0x3e0
sp=e0000100603dfda0 bsp=e0000100603d8f18
[<a0000001009e95c0>] acpi_pci_root_add+0x5c0/0x8c0
sp=e0000100603dfdc0 bsp=e0000100603d8ed8
[<a000000100555420>] acpi_device_probe+0xa0/0x3c0
sp=e0000100603dfde0 bsp=e0000100603d8e98
[<a0000001006395a0>] driver_probe_device+0x180/0x2a0
sp=e0000100603dfde0 bsp=e0000100603d8e68
[<a0000001006397a0>] __driver_attach+0xe0/0x140
sp=e0000100603dfde0 bsp=e0000100603d8e38
[<a000000100637aa0>] bus_for_each_dev+0xa0/0x120
sp=e0000100603dfde0 bsp=e0000100603d8e00
[<a0000001006391e0>] driver_attach+0x40/0x60
sp=e0000100603dfdf0 bsp=e0000100603d8dd8
[<a000000100638840>] bus_add_driver+0x1a0/0x540
sp=e0000100603dfdf0 bsp=e0000100603d8d90
[<a00000010063a0b0>] driver_register+0x210/0x3a0
sp=e0000100603dfdf0 bsp=e0000100603d8d48
[<a000000100558d30>] acpi_bus_register_driver+0x50/0x80
sp=e0000100603dfdf0 bsp=e0000100603d8d28
[<a000000100cd1740>] acpi_pci_root_init+0x20/0x60
sp=e0000100603dfdf0 bsp=e0000100603d8d10
[<a00000010000a630>] do_one_initcall+0xb0/0x2a0
sp=e0000100603dfdf0 bsp=e0000100603d8cd0
[<a000000100c9c630>] kernel_init+0x310/0x3c0
sp=e0000100603dfe30 bsp=e0000100603d8c88
[<a000000100014390>] kernel_thread_helper+0x30/0x60
sp=e0000100603dfe30 bsp=e0000100603d8c60
[<a00000010000a0c0>] start_kernel_thread+0x20/0x40
sp=e0000100603dfe30 bsp=e0000100603d8c60
---[ end trace 4eaa2a86a8e2da42 ]---
------------[ cut here ]------------
WARNING: at kernel/resource.c:147 __set_parent+0xc0/0xe0()
Hardware name: server rx6600
__request_resource: parent of 'PCI Bus 0000:c2' is 'PCI IO'
Modules linked in:
Call Trace:
[<a000000100015b70>] show_stack+0x50/0xa0
sp=e0000100603dfb30 bsp=e0000100603d9188
[<a000000100015bf0>] dump_stack+0x30/0x60
sp=e0000100603dfd00 bsp=e0000100603d9170
[<a00000010006fa00>] warn_slowpath_common+0xc0/0x100
sp=e0000100603dfd00 bsp=e0000100603d9138
[<a00000010006fb30>] warn_slowpath_fmt+0x90/0xc0
sp=e0000100603dfd00 bsp=e0000100603d90d8
[<a00000010007ef40>] __set_parent+0xc0/0xe0
sp=e0000100603dfd40 bsp=e0000100603d90a0
[<a00000010007f060>] __request_resource+0x100/0x160
sp=e0000100603dfd40 bsp=e0000100603d9078
[<a00000010007f3e0>] __insert_resource+0x20/0x2a0
sp=e0000100603dfd40 bsp=e0000100603d9038
[<a00000010007f990>] insert_resource+0x30/0x80
sp=e0000100603dfd40 bsp=e0000100603d9010
[<a000000100a00a20>] add_window+0x5e0/0x6a0
sp=e0000100603dfd40 bsp=e0000100603d8fa0
[<a0000001005b2250>] acpi_walk_resources+0x1f0/0x340
sp=e0000100603dfd90 bsp=e0000100603d8f60
[<a000000100a00110>] pci_acpi_scan_root+0x210/0x3e0
sp=e0000100603dfda0 bsp=e0000100603d8f18
[<a0000001009e95c0>] acpi_pci_root_add+0x5c0/0x8c0
sp=e0000100603dfdc0 bsp=e0000100603d8ed8
[<a000000100555420>] acpi_device_probe+0xa0/0x3c0
sp=e0000100603dfde0 bsp=e0000100603d8e98
[<a0000001006395a0>] driver_probe_device+0x180/0x2a0
sp=e0000100603dfde0 bsp=e0000100603d8e68
[<a0000001006397a0>] __driver_attach+0xe0/0x140
sp=e0000100603dfde0 bsp=e0000100603d8e38
[<a000000100637aa0>] bus_for_each_dev+0xa0/0x120
sp=e0000100603dfde0 bsp=e0000100603d8e00
[<a0000001006391e0>] driver_attach+0x40/0x60
sp=e0000100603dfdf0 bsp=e0000100603d8dd8
[<a000000100638840>] bus_add_driver+0x1a0/0x540
sp=e0000100603dfdf0 bsp=e0000100603d8d90
[<a00000010063a0b0>] driver_register+0x210/0x3a0
sp=e0000100603dfdf0 bsp=e0000100603d8d48
[<a000000100558d30>] acpi_bus_register_driver+0x50/0x80
sp=e0000100603dfdf0 bsp=e0000100603d8d28
[<a000000100cd1740>] acpi_pci_root_init+0x20/0x60
sp=e0000100603dfdf0 bsp=e0000100603d8d10
[<a00000010000a630>] do_one_initcall+0xb0/0x2a0
sp=e0000100603dfdf0 bsp=e0000100603d8cd0
[<a000000100c9c630>] kernel_init+0x310/0x3c0
sp=e0000100603dfe30 bsp=e0000100603d8c88
[<a000000100014390>] kernel_thread_helper+0x30/0x60
sp=e0000100603dfe30 bsp=e0000100603d8c60
[<a00000010000a0c0>] start_kernel_thread+0x20/0x40
sp=e0000100603dfe30 bsp=e0000100603d8c60
---[ end trace 4eaa2a86a8e2da43 ]---
------------[ cut here ]------------
WARNING: at kernel/resource.c:147 __set_parent+0xc0/0xe0()
Hardware name: server rx6600
__request_resource: parent of 'PCI Bus 0000:c2' is 'PCI mem'
Modules linked in:
Call Trace:
[<a000000100015b70>] show_stack+0x50/0xa0
sp=e0000100603dfb30 bsp=e0000100603d9188
[<a000000100015bf0>] dump_stack+0x30/0x60
sp=e0000100603dfd00 bsp=e0000100603d9170
[<a00000010006fa00>] warn_slowpath_common+0xc0/0x100
sp=e0000100603dfd00 bsp=e0000100603d9138
[<a00000010006fb30>] warn_slowpath_fmt+0x90/0xc0
sp=e0000100603dfd00 bsp=e0000100603d90d8
[<a00000010007ef40>] __set_parent+0xc0/0xe0
sp=e0000100603dfd40 bsp=e0000100603d90a0
[<a00000010007f060>] __request_resource+0x100/0x160
sp=e0000100603dfd40 bsp=e0000100603d9078
[<a00000010007f3e0>] __insert_resource+0x20/0x2a0
sp=e0000100603dfd40 bsp=e0000100603d9038
[<a00000010007f990>] insert_resource+0x30/0x80
sp=e0000100603dfd40 bsp=e0000100603d9010
[<a000000100a00a20>] add_window+0x5e0/0x6a0
sp=e0000100603dfd40 bsp=e0000100603d8fa0
[<a0000001005b2250>] acpi_walk_resources+0x1f0/0x340
sp=e0000100603dfd90 bsp=e0000100603d8f60
[<a000000100a00110>] pci_acpi_scan_root+0x210/0x3e0
sp=e0000100603dfda0 bsp=e0000100603d8f18
[<a0000001009e95c0>] acpi_pci_root_add+0x5c0/0x8c0
sp=e0000100603dfdc0 bsp=e0000100603d8ed8
[<a000000100555420>] acpi_device_probe+0xa0/0x3c0
sp=e0000100603dfde0 bsp=e0000100603d8e98
[<a0000001006395a0>] driver_probe_device+0x180/0x2a0
sp=e0000100603dfde0 bsp=e0000100603d8e68
[<a0000001006397a0>] __driver_attach+0xe0/0x140
sp=e0000100603dfde0 bsp=e0000100603d8e38
[<a000000100637aa0>] bus_for_each_dev+0xa0/0x120
sp=e0000100603dfde0 bsp=e0000100603d8e00
[<a0000001006391e0>] driver_attach+0x40/0x60
sp=e0000100603dfdf0 bsp=e0000100603d8dd8
[<a000000100638840>] bus_add_driver+0x1a0/0x540
sp=e0000100603dfdf0 bsp=e0000100603d8d90
[<a00000010063a0b0>] driver_register+0x210/0x3a0
sp=e0000100603dfdf0 bsp=e0000100603d8d48
[<a000000100558d30>] acpi_bus_register_driver+0x50/0x80
sp=e0000100603dfdf0 bsp=e0000100603d8d28
[<a000000100cd1740>] acpi_pci_root_init+0x20/0x60
sp=e0000100603dfdf0 bsp=e0000100603d8d10
[<a00000010000a630>] do_one_initcall+0xb0/0x2a0
sp=e0000100603dfdf0 bsp=e0000100603d8cd0
[<a000000100c9c630>] kernel_init+0x310/0x3c0
sp=e0000100603dfe30 bsp=e0000100603d8c88
[<a000000100014390>] kernel_thread_helper+0x30/0x60
sp=e0000100603dfe30 bsp=e0000100603d8c60
[<a00000010000a0c0>] start_kernel_thread+0x20/0x40
sp=e0000100603dfe30 bsp=e0000100603d8c60
---[ end trace 4eaa2a86a8e2da44 ]---
------------[ cut here ]------------
WARNING: at kernel/resource.c:147 __set_parent+0xc0/0xe0()
Hardware name: server rx6600
__request_resource: parent of 'PCI Bus 0000:c2' is 'PCI mem'
Modules linked in:
Call Trace:
[<a000000100015b70>] show_stack+0x50/0xa0
sp=e0000100603dfb30 bsp=e0000100603d9188
[<a000000100015bf0>] dump_stack+0x30/0x60
sp=e0000100603dfd00 bsp=e0000100603d9170
[<a00000010006fa00>] warn_slowpath_common+0xc0/0x100
sp=e0000100603dfd00 bsp=e0000100603d9138
[<a00000010006fb30>] warn_slowpath_fmt+0x90/0xc0
sp=e0000100603dfd00 bsp=e0000100603d90d8
[<a00000010007ef40>] __set_parent+0xc0/0xe0
sp=e0000100603dfd40 bsp=e0000100603d90a0
[<a00000010007f060>] __request_resource+0x100/0x160
sp=e0000100603dfd40 bsp=e0000100603d9078
[<a00000010007f3e0>] __insert_resource+0x20/0x2a0
sp=e0000100603dfd40 bsp=e0000100603d9038
[<a00000010007f990>] insert_resource+0x30/0x80
sp=e0000100603dfd40 bsp=e0000100603d9010
[<a000000100a00a20>] add_window+0x5e0/0x6a0
sp=e0000100603dfd40 bsp=e0000100603d8fa0
[<a0000001005b2250>] acpi_walk_resources+0x1f0/0x340
sp=e0000100603dfd90 bsp=e0000100603d8f60
[<a000000100a00110>] pci_acpi_scan_root+0x210/0x3e0
sp=e0000100603dfda0 bsp=e0000100603d8f18
[<a0000001009e95c0>] acpi_pci_root_add+0x5c0/0x8c0
sp=e0000100603dfdc0 bsp=e0000100603d8ed8
[<a000000100555420>] acpi_device_probe+0xa0/0x3c0
sp=e0000100603dfde0 bsp=e0000100603d8e98
[<a0000001006395a0>] driver_probe_device+0x180/0x2a0
sp=e0000100603dfde0 bsp=e0000100603d8e68
[<a0000001006397a0>] __driver_attach+0xe0/0x140
sp=e0000100603dfde0 bsp=e0000100603d8e38
[<a000000100637aa0>] bus_for_each_dev+0xa0/0x120
sp=e0000100603dfde0 bsp=e0000100603d8e00
[<a0000001006391e0>] driver_attach+0x40/0x60
sp=e0000100603dfdf0 bsp=e0000100603d8dd8
[<a000000100638840>] bus_add_driver+0x1a0/0x540
sp=e0000100603dfdf0 bsp=e0000100603d8d90
[<a00000010063a0b0>] driver_register+0x210/0x3a0
sp=e0000100603dfdf0 bsp=e0000100603d8d48
[<a000000100558d30>] acpi_bus_register_driver+0x50/0x80
sp=e0000100603dfdf0 bsp=e0000100603d8d28
[<a000000100cd1740>] acpi_pci_root_init+0x20/0x60
sp=e0000100603dfdf0 bsp=e0000100603d8d10
[<a00000010000a630>] do_one_initcall+0xb0/0x2a0
sp=e0000100603dfdf0 bsp=e0000100603d8cd0
[<a000000100c9c630>] kernel_init+0x310/0x3c0
sp=e0000100603dfe30 bsp=e0000100603d8c88
[<a000000100014390>] kernel_thread_helper+0x30/0x60
sp=e0000100603dfe30 bsp=e0000100603d8c60
[<a00000010000a0c0>] start_kernel_thread+0x20/0x40
sp=e0000100603dfe30 bsp=e0000100603d8c60
---[ end trace 4eaa2a86a8e2da45 ]---
pci 0000:c2:00.0: PME# supported from D0 D3hot D3cold
pci 0000:c2:00.0: PME# disabled
------------[ cut here ]------------
WARNING: at kernel/resource.c:147 __set_parent+0xc0/0xe0()
Hardware name: server rx6600
__insert_resource: parent of 'PCI Bus 0000:c2' is ''
Modules linked in:
Call Trace:
[<a000000100015b70>] show_stack+0x50/0xa0
sp=e0000100603dfb70 bsp=e0000100603d92b0
[<a000000100015bf0>] dump_stack+0x30/0x60
sp=e0000100603dfd40 bsp=e0000100603d9298
[<a00000010006fa00>] warn_slowpath_common+0xc0/0x100
sp=e0000100603dfd40 bsp=e0000100603d9260
[<a00000010006fb30>] warn_slowpath_fmt+0x90/0xc0
sp=e0000100603dfd40 bsp=e0000100603d9200
[<a00000010007ef40>] __set_parent+0xc0/0xe0
sp=e0000100603dfd80 bsp=e0000100603d91c0
[<a00000010007f590>] __insert_resource+0x1d0/0x2a0
sp=e0000100603dfd80 bsp=e0000100603d9180
[<a00000010007f990>] insert_resource+0x30/0x80
sp=e0000100603dfd80 bsp=e0000100603d9158
[<a00000010052c560>] pci_claim_resource+0xc0/0x160
sp=e0000100603dfd80 bsp=e0000100603d9110
[<a0000001009ffd20>] pcibios_fixup_resources+0x200/0x240
sp=e0000100603dfd80 bsp=e0000100603d90b8
[<a0000001009ffe20>] pcibios_fixup_bus+0x60/0x140
sp=e0000100603dfd90 bsp=e0000100603d9088
[<a0000001009e4610>] pci_scan_child_bus+0x130/0x2c0
sp=e0000100603dfd90 bsp=e0000100603d9048
[<a0000001009e3eb0>] pci_scan_bridge+0x350/0x980
sp=e0000100603dfd90 bsp=e0000100603d8fd8
[<a0000001009e46b0>] pci_scan_child_bus+0x1d0/0x2c0
sp=e0000100603dfda0 bsp=e0000100603d8f98
[<a0000001009e47f0>] pci_scan_bus_parented+0x50/0xa0
sp=e0000100603dfda0 bsp=e0000100603d8f60
[<a000000100a00150>] pci_acpi_scan_root+0x250/0x3e0
sp=e0000100603dfda0 bsp=e0000100603d8f18
[<a0000001009e95c0>] acpi_pci_root_add+0x5c0/0x8c0
sp=e0000100603dfdc0 bsp=e0000100603d8ed8
[<a000000100555420>] acpi_device_probe+0xa0/0x3c0
sp=e0000100603dfde0 bsp=e0000100603d8e98
[<a0000001006395a0>] driver_probe_device+0x180/0x2a0
sp=e0000100603dfde0 bsp=e0000100603d8e68
[<a0000001006397a0>] __driver_attach+0xe0/0x140
sp=e0000100603dfde0 bsp=e0000100603d8e38
[<a000000100637aa0>] bus_for_each_dev+0xa0/0x120
sp=e0000100603dfde0 bsp=e0000100603d8e00
[<a0000001006391e0>] driver_attach+0x40/0x60
sp=e0000100603dfdf0 bsp=e0000100603d8dd8
[<a000000100638840>] bus_add_driver+0x1a0/0x540
sp=e0000100603dfdf0 bsp=e0000100603d8d90
[<a00000010063a0b0>] driver_register+0x210/0x3a0
sp=e0000100603dfdf0 bsp=e0000100603d8d48
[<a000000100558d30>] acpi_bus_register_driver+0x50/0x80
sp=e0000100603dfdf0 bsp=e0000100603d8d28
[<a000000100cd1740>] acpi_pci_root_init+0x20/0x60
sp=e0000100603dfdf0 bsp=e0000100603d8d10
[<a00000010000a630>] do_one_initcall+0xb0/0x2a0
sp=e0000100603dfdf0 bsp=e0000100603d8cd0
[<a000000100c9c630>] kernel_init+0x310/0x3c0
sp=e0000100603dfe30 bsp=e0000100603d8c88
[<a000000100014390>] kernel_thread_helper+0x30/0x60
sp=e0000100603dfe30 bsp=e0000100603d8c60
[<a00000010000a0c0>] start_kernel_thread+0x20/0x40
sp=e0000100603dfe30 bsp=e0000100603d8c60
---[ end trace 4eaa2a86a8e2da46 ]---
.
.
.
GSI 85 (level, low) -> CPU 4 (0x0400) vector 77
qla2xxx 0000:c3:00.0: PCI INT A -> GSI 85 (level, low) -> IRQ 77
qla2xxx 0000:c3:00.0: MSI-X vector count: 31
qla2xxx 0000:c3:00.0: Found an ISP2532, irq 77, iobase
0xc0000000f0284000
qla2xxx 0000:c3:00.0: Configuring PCI space...
qla2xxx 0000:c3:00.0: Configure NVRAM parameters...
qla2xxx 0000:c3:00.0: Verifying loaded RISC code...
qla2xxx 0000:c3:00.0: firmware: requesting ql2500_fw.bin
qla2xxx 0000:c3:00.0: FW: Loading via request-firmware...
qla2xxx 0000:c3:00.0: Allocated (64 KB) for FCE...
qla2xxx 0000:c3:00.0: Allocated (64 KB) for EFT...
qla2xxx 0000:c3:00.0: Allocated (1350 KB) for firmware dump...
scsi13 : qla2xxx
qla2xxx 0000:c3:00.0:
QLogic Fibre Channel HBA Driver: 8.03.01-k3
QLogic HPAJ764A - HP 8Gb Dual Channel PCI-e 2.0 FC HBA
ISP2532: PCIe (2.5GT/s x4) @ 0000:c3:00.0 hdma+, host#=13, fw=4.04.05
(85)
GSI 86 (level, low) -> CPU 5 (0x0500) vector 80
qla2xxx 0000:c3:00.1: PCI INT B -> GSI 86 (level, low) -> IRQ 80
qla2xxx 0000:c3:00.1: MSI-X vector count: 31
qla2xxx 0000:c3:00.1: Found an ISP2532, irq 80, iobase
0xc0000000f0280000
qla2xxx 0000:c3:00.1: Configuring PCI space...
qla2xxx 0000:c3:00.1: Configure NVRAM parameters...
qla2xxx 0000:c3:00.1: Verifying loaded RISC code...
qla2xxx 0000:c3:00.1: FW: Loading via request-firmware...
qla2xxx 0000:c3:00.1: Allocated (64 KB) for FCE...
qla2xxx 0000:c3:00.1: Allocated (64 KB) for EFT...
qla2xxx 0000:c3:00.1: Allocated (1350 KB) for firmware dump...
scsi14 : qla2xxx
qla2xxx 0000:c3:00.1:
QLogic Fibre Channel HBA Driver: 8.03.01-k3
QLogic HPAJ764A - HP 8Gb Dual Channel PCI-e 2.0 FC HBA
ISP2532: PCIe (2.5GT/s x4) @ 0000:c3:00.1 hdma+, host#=14, fw=4.04.05
(85)
.
.
.
> Linus
>
> ---
> kernel/resource.c | 14 +++++++++++---
> 1 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/resource.c b/kernel/resource.c
> index ac5f3a3..023ba7a 100644
> --- a/kernel/resource.c
> +++ b/kernel/resource.c
> @@ -140,6 +140,14 @@ __initcall(ioresources_init);
>
> #endif /* CONFIG_PROC_FS */
>
> +#define set_parent(x,p) __set_parent(__FUNCTION__, x, p)
> +static void __set_parent(const char *fn, struct resource *x, struct resource *parent)
> +{
> + WARN(!strncmp(x->name, "PCI Bus", 7),
> + "%s: parent of '%s' is '%s'\n", fn, x->name, parent ? parent->name : "none");
> + x->parent = parent;
> +}
> +
> /* Return the conflict entry if you can't request it */
> static struct resource * __request_resource(struct resource *root, struct resource *new)
> {
> @@ -159,7 +167,7 @@ static struct resource * __request_resource(struct resource *root, struct resour
> if (!tmp || tmp->start > end) {
> new->sibling = tmp;
> *p = new;
> - new->parent = root;
> + set_parent(new, root);
> return NULL;
> }
> p = &tmp->sibling;
> @@ -395,13 +403,13 @@ static struct resource * __insert_resource(struct resource *parent, struct resou
> break;
> }
>
> - new->parent = parent;
> + set_parent(new, parent);
> new->sibling = next->sibling;
> new->child = first;
>
> next->sibling = NULL;
> for (next = first; next; next = next->sibling)
> - next->parent = new;
> + set_parent(next, new);
>
> if (parent->child == first) {
> parent->child = new;
>
--
Andrew Patterson
Hewlett-Packard
On Wed, 17 Jun 2009, Andrew Patterson wrote:
> >
> > This is why I'd really like to see the output of my test-patch. It would
> > show exactly _where_ that resource is inserted, and the whole call-chain.
> >
> > I'm appending a version that only does it for resources that have names
> > starting with "PCI Bus", so it should be less noisy. But again, it's
> > totally untested.
> >
>
> Here you go (I can provide the full boot log if needed). Note, there is
> nothing for c3 here:
Ok, so that means it got inserted into the resource tree some other way
entirely. Or maybe the name got changed after-the-fact. Both of which
imply that something is really really wrong.
The ones your trace _does_ show are the ones that got inserted correctly
and aren't buggy. Can anybody see how that buggy resource got inserted?
Linus
On Wed, Jun 17, 2009 at 01:12:42PM -0700, Linus Torvalds wrote:
> On Wed, 17 Jun 2009, Andrew Patterson wrote:
> > >
> > > This is why I'd really like to see the output of my test-patch. It would
> > > show exactly _where_ that resource is inserted, and the whole call-chain.
> > >
> > > I'm appending a version that only does it for resources that have names
> > > starting with "PCI Bus", so it should be less noisy. But again, it's
> > > totally untested.
> > >
> >
> > Here you go (I can provide the full boot log if needed). Note, there is
> > nothing for c3 here:
>
> Ok, so that means it got inserted into the resource tree some other way
> entirely. Or maybe the name got changed after-the-fact. Both of which
> imply that something is really really wrong.
Yes.
> The ones your trace _does_ show are the ones that got inserted correctly
> and aren't buggy. Can anybody see how that buggy resource got inserted?
Carefully studying the results of git grep insert_resource doesn't show
too many options. Most are in code which doesn't get executed on ia64.
add_window is the only one which looks even remotely plausible, and
that's only supposed to be called for root bridges.
I did notice that pcibios_setup_root_windows() is being called too late.
It's currently called after pci_scan_bus_parented() when it needs to be
called from pcibios_fixup_bus() [ie during the scan_bus, before child
busses are scanned].
Andrew's currently testing this patch:
diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
index 61f1af5..ae5ee8a 100644
--- a/arch/ia64/pci/pci.c
+++ b/arch/ia64/pci/pci.c
@@ -371,8 +371,6 @@ pci_acpi_scan_root(struct acpi_device *device, int domain, int bus)
* such quirk. So we just ignore the case now.
*/
pbus = pci_scan_bus_parented(NULL, bus, &pci_root_ops, controller);
- if (pbus)
- pcibios_setup_root_windows(pbus, controller);
return pbus;
@@ -490,6 +488,8 @@ pcibios_fixup_bus (struct pci_bus *b)
if (b->self) {
pci_read_bridge_bases(b);
pcibios_fixup_bridge_resources(b->self);
+ } else {
+ pcibios_setup_root_windows(b, b->sysdata);
}
list_for_each_entry(dev, &b->devices, bus_list)
pcibios_fixup_device_resources(dev);
--
Matthew Wilcox Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."