mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Andrew Patterson <andrew.patterson@hp.com>
Cc: linux-pci@vger.kernel.org,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	jbarnes@virtuousgeek.org,
	Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Subject: Re: [PATCH 0/1] Recurse when searching for empty slots in resources trees
Date: Tue, 16 Jun 2009 16:56:12 -0700 (PDT)	[thread overview]
Message-ID: <alpine.LFD.2.01.0906161640250.16802@localhost.localdomain> (raw)
In-Reply-To: <1245195498.8234.236.camel@bluto.andrew>



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

  reply	other threads:[~2009-06-16 23:56 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-06-16 22:04 Andrew Patterson
2009-06-16 22:04 ` [PATCH] " Andrew Patterson
2009-06-16 22:19 ` [PATCH 0/1] " Linus Torvalds
2009-06-16 22:51   ` Andrew Patterson
2009-06-16 23:05     ` Linus Torvalds
2009-06-16 23:32       ` Linus Torvalds
2009-06-17 14:45         ` Ivan Kokshaysky
2009-06-17 16:28           ` Linus Torvalds
2009-06-16 23:38       ` Andrew Patterson
2009-06-16 23:56         ` Linus Torvalds [this message]
2009-06-17  0:19           ` Linus Torvalds
2009-06-17  1:04             ` Linus Torvalds
2009-06-17  3:19               ` Andrew Patterson
2009-06-17  4:19                 ` Linus Torvalds
2009-06-17  0:28           ` Jesse Barnes
2009-06-17 16:03             ` Alex Chiang
2009-06-17  9:13 ` Kenji Kaneshige
2009-06-17 13:43   ` Matthew Wilcox
2009-06-17 16:23     ` Linus Torvalds
2009-06-17 17:42       ` Andrew Patterson
2009-06-17 18:12         ` Linus Torvalds
2009-06-17 20:08           ` Andrew Patterson
2009-06-17 20:12             ` Linus Torvalds
2009-06-17 20:17               ` Matthew Wilcox

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.LFD.2.01.0906161640250.16802@localhost.localdomain \
    --to=torvalds@linux-foundation.org \
    --cc=andrew.patterson@hp.com \
    --cc=ink@jurassic.park.msu.ru \
    --cc=jbarnes@virtuousgeek.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®