FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Ubuntu > Ubuntu Kernel Team

 
 
LinkBack Thread Tools
 
Old 05-23-2011, 09:53 PM
Brad Figg
 
Default PCI: allow matching of prefetchable resources to non-prefetchable windows

On 05/23/2011 02:36 PM, Seth Forshee wrote:

From: Linus Torvalds<torvalds@linux-foundation.org>

BugLink: http://bugs.launchpad.net/bugs/424142

I'm not entirely sure it needs to go into 32, but it's probably the right
thing to do. Another way of explaining the patch is:

- we currently pick the _first_ exactly matching bus resource entry, but
the _last_ inexactly matching one. Normally first/last shouldn't
matter, but bus resource entries aren't actually all created equal: in
a transparent bus, the last resources will be the parent resources,
which we should generally try to avoid unless we have no choice. So
"first matching" is the thing we should always aim for.

- the patch is a bit bigger than it needs to be, because I simplified the
logic at the same time. It used to be a fairly incomprehensible

if ((res->flags& IORESOURCE_PREFETCH)&& !(r->flags& IORESOURCE_PREFETCH))
best = r; /* Approximating prefetchable by non-prefetchable */

and technically, all the patch did was to make that complex choice be
even more complex (it basically added a "&& !best" to say that if we
already gound a non-prefetchable window for the prefetchable resource,
then we won't override an earlier one with that later one: remember
"first matching").

- So instead of that complex one with three separate conditionals in one,
I split it up a bit, and am taking advantage of the fact that we
already handled the exact case, so if 'res->flags' has the PREFETCH
bit, then we already know that 'r->flags' will _not_ have it. So the
simplified code drops the redundant test, and does the new '!best' test
separately. It also uses 'continue' as a way to ignore the bus
resource we know doesn't work (ie a prefetchable bus resource is _not_
acceptable for anything but an exact match), so it turns into:

/* We can't insert a non-prefetch resource inside a prefetchable parent .. */
if (r->flags& IORESOURCE_PREFETCH)
continue;
/* .. but we can put a prefetchable resource inside a non-prefetchable one */
if (!best)
best = r;

instead. With the comments, it's now six lines instead of two, but it's
conceptually simpler, and I _could_ have written it as two lines:

if ((res->flags& IORESOURCE_PREFETCH)&& !best)
best = r; /* Approximating prefetchable by non-prefetchable */

but I thought that was too damn subtle.

Signed-off-by: Linus Torvalds<torvalds@linux-foundation.org>
Signed-off-by: Jesse Barnes<jbarnes@virtuousgeek.org>
(cherry picked from commit 8c8def26bfaa704db67d515da3eb92cf26067548)

Signed-off-by: Seth Forshee<seth.forshee@canonical.com>
---
drivers/pci/pci.c | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 812d4ac..0d3326d 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -373,8 +373,12 @@ pci_find_parent_resource(const struct pci_dev *dev, struct resource *res)
continue; /* Wrong type */
if (!((res->flags ^ r->flags)& IORESOURCE_PREFETCH))
return r; /* Exact match */
- if ((res->flags& IORESOURCE_PREFETCH)&& !(r->flags& IORESOURCE_PREFETCH))
- best = r; /* Approximating prefetchable by non-prefetchable */
+ /* We can't insert a non-prefetch resource inside a prefetchable parent .. */
+ if (r->flags& IORESOURCE_PREFETCH)
+ continue;
+ /* .. but we can put a prefetchable resource inside a non-prefetchable one */
+ if (!best)
+ best = r;
}
return best;
}


For Lucid:

Acked-by: Brad Figg <brad.figg@canonical.com>

--
Brad Figg brad.figg@canonical.com http://www.canonical.com

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team
 
Old 05-23-2011, 10:02 PM
John Johansen
 
Default PCI: allow matching of prefetchable resources to non-prefetchable windows

On 05/23/2011 02:36 PM, Seth Forshee wrote:
> From: Linus Torvalds <torvalds@linux-foundation.org>
>
> BugLink: http://bugs.launchpad.net/bugs/424142
>
> I'm not entirely sure it needs to go into 32, but it's probably the right
> thing to do. Another way of explaining the patch is:
>
> - we currently pick the _first_ exactly matching bus resource entry, but
> the _last_ inexactly matching one. Normally first/last shouldn't
> matter, but bus resource entries aren't actually all created equal: in
> a transparent bus, the last resources will be the parent resources,
> which we should generally try to avoid unless we have no choice. So
> "first matching" is the thing we should always aim for.
>
> - the patch is a bit bigger than it needs to be, because I simplified the
> logic at the same time. It used to be a fairly incomprehensible
>
> if ((res->flags & IORESOURCE_PREFETCH) && !(r->flags & IORESOURCE_PREFETCH))
> best = r; /* Approximating prefetchable by non-prefetchable */
>
> and technically, all the patch did was to make that complex choice be
> even more complex (it basically added a "&& !best" to say that if we
> already gound a non-prefetchable window for the prefetchable resource,
> then we won't override an earlier one with that later one: remember
> "first matching").
>
> - So instead of that complex one with three separate conditionals in one,
> I split it up a bit, and am taking advantage of the fact that we
> already handled the exact case, so if 'res->flags' has the PREFETCH
> bit, then we already know that 'r->flags' will _not_ have it. So the
> simplified code drops the redundant test, and does the new '!best' test
> separately. It also uses 'continue' as a way to ignore the bus
> resource we know doesn't work (ie a prefetchable bus resource is _not_
> acceptable for anything but an exact match), so it turns into:
>
> /* We can't insert a non-prefetch resource inside a prefetchable parent .. */
> if (r->flags & IORESOURCE_PREFETCH)
> continue;
> /* .. but we can put a prefetchable resource inside a non-prefetchable one */
> if (!best)
> best = r;
>
> instead. With the comments, it's now six lines instead of two, but it's
> conceptually simpler, and I _could_ have written it as two lines:
>
> if ((res->flags & IORESOURCE_PREFETCH) && !best)
> best = r; /* Approximating prefetchable by non-prefetchable */
>
> but I thought that was too damn subtle.
>
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> (cherry picked from commit 8c8def26bfaa704db67d515da3eb92cf26067548)
>
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>

As has been already pointed out, for Lucid

Acked-by: John Johansen <john.johansen@canonical.com>

> ---
> drivers/pci/pci.c | 8 ++++++--
> 1 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 812d4ac..0d3326d 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -373,8 +373,12 @@ pci_find_parent_resource(const struct pci_dev *dev, struct resource *res)
> continue; /* Wrong type */
> if (!((res->flags ^ r->flags) & IORESOURCE_PREFETCH))
> return r; /* Exact match */
> - if ((res->flags & IORESOURCE_PREFETCH) && !(r->flags & IORESOURCE_PREFETCH))
> - best = r; /* Approximating prefetchable by non-prefetchable */
> + /* We can't insert a non-prefetch resource inside a prefetchable parent .. */
> + if (r->flags & IORESOURCE_PREFETCH)
> + continue;
> + /* .. but we can put a prefetchable resource inside a non-prefetchable one */
> + if (!best)
> + best = r;
> }
> return best;
> }


--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team
 
Old 05-25-2011, 04:14 PM
Tim Gardner
 
Default PCI: allow matching of prefetchable resources to non-prefetchable windows

On 05/23/2011 03:40 PM, Seth Forshee wrote:

D'oh. This is for lucid, not natty.

On Mon, May 23, 2011 at 04:36:36PM -0500, Seth Forshee wrote:

SRU Justification:

Impact: PCI devices may fail to work due to a resource allocation
failure. When this happens the kernel prints a "address space collision"
message.

Fix: Upstream commit 8c8def26bfaa704db67d515da3eb92cf26067548

Test case: Boot a kernel on an affected machine and observe that the
"address space collision" messages are no longer present. Resources for
the affected devices will appear in /proc/iomem and /proc/ioports with
the patch applied. Verified on LP #424142.


--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team





Have you proposed it for 2.6.32 stable ?

--
Tim Gardner tim.gardner@canonical.com

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team
 

Thread Tools




All times are GMT. The time now is 01:40 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org