On Tue, 16 Jun 2009 16:56:12 -0700 (PDT) Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
