linux-kernel - Re: [PATCH 0/1] Recurse when searching for empty slots in resources trees

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20090616172811.7f6374f4@jbarnes-g45>
Date:	Tue, 16 Jun 2009 17:28:11 -0700
From:	Jesse Barnes <jbarnes@...tuousgeek.org>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Andrew Patterson <andrew.patterson@...com>,
	linux-pci@...r.kernel.org,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Ivan Kokshaysky <ink@...assic.park.msu.ru>,
	Alex Chiang <achiang@...com>
Subject: Re: [PATCH 0/1] Recurse when searching for empty slots in resources
 trees

On Tue, 16 Jun 2009 16:56:12 -0700 (PDT)
Linus Torvalds <torvalds@...ux-foundation.org> 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-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/