From: "Rafael J. Wysocki" <rjw@sisk.pl>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Matthew Wilcox <willy@linux.intel.com>,
LKML <linux-kernel@vger.kernel.org>,
Linux PCI <linux-pci@vger.kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
Andrew Patterson <andrew.patterson@hp.com>
Subject: Re: [Regression] PCI resources allocation problem on HP nx6325
Date: Sun, 2 Aug 2009 22:16:10 +0200 [thread overview]
Message-ID: <200908022216.10681.rjw@sisk.pl> (raw)
In-Reply-To: <alpine.LFD.2.01.0908020927410.3352@localhost.localdomain>
On Sunday 02 August 2009, Linus Torvalds wrote:
>
> On Sun, 2 Aug 2009, Rafael J. Wysocki wrote:
> >
> > Hi Matthew,
> >
> > As reported at
> >
> > http://bugzilla.kernel.org/show_bug.cgi?id=13891
> >
> > there is a problem with allocating PCI resources on HP nx6325 introduced by
> > your commit a76117dfd687ec4be0a9a05214f3009cc5f73a42
> > (x86: Use pci_claim_resource).
>
> Ooh, interesting. I thought that patch was a functionally equivalent
> cleanup of
>
> pr = pci_find_parent_resource(dev, r);
> if (!pr || request_resource(pr, r) < 0) {
>
> to
>
> if (pci_claim_resource(dev, idx) < 0) {
>
> but yeah, it's not exactly the same. pci_claim_resource() uses
> 'insert_resource()' rather than 'request_resource()'.
>
> We could certainly revert the commit, but I also wonder whether we should
> just change 'pci_claim_resource()' to use request_resource() instead.
>
> I _think_ the use of "insert_resource()" is purely historical, and is
> because that broken function _used_ to not look up the parent, but instead
> do that crazy "pcibios_select_root()" thing, and then it really does need
> to recurse down and "insert" the resource in the right place.
>
> We should no longer _need_ to do the "insert_resource()" thing, since we
> are inserting it into the exact parent that we want (as of commit
> cebd78a8c: "Fix pci_claim_resource").
>
> And if that "insert_resource()" in pci_claim_resource() ever does anything
> fancier than the raw "request_resource()", then that's a problem anyway.
>
> Willy, comments? x86 historically has never used pci_claim_resource() at
> all (it always open-coded the above) except for some quirk handling. So
> I'm pretty sure that a patch like the below should be safe and correct.
> But it's parisc machines that always seem to break.
>
> Added Andrew Patterson to the Cc, because his report was what caused us to
> originally look at pci_claim_resource() and make it use
> "pci_find_parent_resource()". We just never went whole hog, and we left
> that broken "insert_resource()" around.
>
> So Rafael and AndrewP, does this work for you? (I also moved the "dtype"
> thing around, it bothered me).
It works, ie. with this patch applied the PCI resources on the nx6325 look
exactly like with commit a76117dfd687ec4be0a9a05214f3009cc5f73a42 reverted.
Thanks,
Rafael
> ---
> drivers/pci/setup-res.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> index b711fb7..1898c7b 100644
> --- a/drivers/pci/setup-res.c
> +++ b/drivers/pci/setup-res.c
> @@ -100,16 +100,16 @@ int pci_claim_resource(struct pci_dev *dev, int resource)
> {
> struct resource *res = &dev->resource[resource];
> struct resource *root;
> - char *dtype = resource < PCI_BRIDGE_RESOURCES ? "device" : "bridge";
> int err;
>
> root = pci_find_parent_resource(dev, res);
>
> err = -EINVAL;
> if (root != NULL)
> - err = insert_resource(root, res);
> + err = request_resource(root, res);
>
> if (err) {
> + const char *dtype = resource < PCI_BRIDGE_RESOURCES ? "device" : "bridge";
> dev_err(&dev->dev, "BAR %d: %s of %s %pR\n",
> resource,
> root ? "address space collision on" :
>
>
next prev parent reply other threads:[~2009-08-02 20:15 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-08-02 14:19 [Regression] PCI resources allocation problem on HP nx6325 Rafael J. Wysocki
2009-08-02 16:39 ` Linus Torvalds
2009-08-02 17:15 ` Matthew Wilcox
2009-08-02 17:19 ` Linus Torvalds
2009-08-02 17:25 ` Matthew Wilcox
2009-08-02 20:16 ` Rafael J. Wysocki [this message]
2009-08-02 21:14 ` Linus Torvalds
2009-08-03 3:10 ` Andrew Patterson
2009-08-03 21:14 ` Andrew Patterson
2009-08-03 16:59 ` Manuel Lauss
2009-08-04 23:04 ` Linus Torvalds
2009-08-05 15:51 ` Manuel Lauss
2009-08-05 16:25 ` Linus Torvalds
2009-08-05 16:38 ` Linus Torvalds
2009-08-05 17:09 ` Manuel Lauss
2009-08-07 18:15 ` Linus Torvalds
2009-08-07 18:40 ` Linus Torvalds
2009-08-11 16:47 ` Manuel Lauss
2009-08-13 18:16 ` Linus Torvalds
2009-08-13 19:28 ` Frans Pop
2009-08-13 19:46 ` Linus Torvalds
2009-08-13 20:35 ` Frans Pop
2009-08-14 1:40 ` PCI resources allocation problem on Toshiba Satellite A40 Frans Pop
2009-08-14 1:47 ` Linus Torvalds
2009-08-14 16:50 ` Frans Pop
2009-08-14 17:04 ` Linus Torvalds
2009-08-14 17:35 ` Frans Pop
2009-08-02 16:59 ` [Regression] PCI resources allocation problem on HP nx6325 Matthew Wilcox
2009-08-02 20:18 ` Rafael J. Wysocki
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=200908022216.10681.rjw@sisk.pl \
--to=rjw@sisk.pl \
--cc=akpm@linux-foundation.org \
--cc=andrew.patterson@hp.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=torvalds@linux-foundation.org \
--cc=willy@linux.intel.com \
/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;
as well as URLs for NNTP newsgroup(s).