pci_find_parent_resource patch

pci_find_parent_resource patch

[Yinghai Lu]

it seems this patch from Linus get lost ?

-----------------------------------------------------



from Linus

---
 drivers/pci/pci.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Index: linux-2.6/drivers/pci/pci.c
===================================================================
--- linux-2.6.orig/drivers/pci/pci.c
+++ linux-2.6/drivers/pci/pci.c
@@ -382,8 +382,12 @@ pci_find_parent_resource(const struct pc
 			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;
 }

Re: pci_find_parent_resource patch

[Jesse Barnes]

I don't remember seeing it, is it for 2.6.32?

Jesse

On Mon, 09 Nov 2009 12:04:32 -0800
Yinghai Lu <yinghai@kernel.org> wrote:

> it seems this patch from Linus get lost ?
> 
> -----------------------------------------------------
> 
> 
> 
> from Linus
> 
> ---
>  drivers/pci/pci.c |    8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> Index: linux-2.6/drivers/pci/pci.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/pci.c
> +++ linux-2.6/drivers/pci/pci.c
> @@ -382,8 +382,12 @@ pci_find_parent_resource(const struct pc
>  			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;
>  }
> 

Re: pci_find_parent_resource patch

[Yinghai Lu]

On Tue, Nov 10, 2009 at 12:23 AM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> I don't remember seeing it, is it for 2.6.32?

http://lkml.org/lkml/2009/8/7/352

YH

Re: pci_find_parent_resource patch

[Linus Torvalds]

On Tue, 10 Nov 2009, Yinghai Lu wrote:

> On Tue, Nov 10, 2009 at 12:23 AM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > I don't remember seeing it, is it for 2.6.32?
> 
> http://lkml.org/lkml/2009/8/7/352

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.

Feel free to use this long explanation as a commit message if you want,

			Linus

Re: pci_find_parent_resource patch

[Jesse Barnes]

On Tue, 10 Nov 2009 07:57:47 -0800 (PST)
Linus Torvalds <torvalds@linux-foundation.org> wrote:
> 	/* 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.
> 
> Feel free to use this long explanation as a commit message if you
> want,

Ok, I applied it to my linux-next tree.

Thanks,
Jesse

Re: pci_find_parent_resource patch

[Yinghai Lu]

Jesse Barnes wrote:
> I don't remember seeing it, is it for 2.6.32?
I
http://lkml.org/lkml/2009/8/7/352

not sure. better to make it in pci-next for a while.

in my local tree for a while

YH

Re: pci_find_parent_resource patch

[Jesse Barnes]

On Tue, 24 Nov 2009 16:22:20 -0800
Yinghai Lu <yinghai@kernel.org> wrote:

> Jesse Barnes wrote:
> > I don't remember seeing it, is it for 2.6.32?
> I
> http://lkml.org/lkml/2009/8/7/352
> 
> not sure. better to make it in pci-next for a while.
> 
> in my local tree for a while

Wow, that's an old one.  Linus said a few times he didn't want that one
to go upstream though.

Linus?  You mentioned something about re-working it a bit...

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center

Re: pci_find_parent_resource patch

[Linus Torvalds]

On Fri, 4 Dec 2009, Jesse Barnes wrote:

> On Tue, 24 Nov 2009 16:22:20 -0800
> Yinghai Lu <yinghai@kernel.org> wrote:
> 
> > Jesse Barnes wrote:
> > > I don't remember seeing it, is it for 2.6.32?
> > I
> > http://lkml.org/lkml/2009/8/7/352
> > 
> > not sure. better to make it in pci-next for a while.
> > 
> > in my local tree for a while
> 
> Wow, that's an old one.  Linus said a few times he didn't want that one
> to go upstream though.
> 
> Linus?  You mentioned something about re-working it a bit...

Oh, no, I'm perfectly fine with it. I just think it needs testing. Merging 
it early in the merge window is fine.

		Linus

Re: pci_find_parent_resource patch

[Jesse Barnes]

On Fri, 4 Dec 2009 16:24:58 -0800 (PST)
Linus Torvalds <torvalds@linux-foundation.org> wrote:
> > On Tue, 24 Nov 2009 16:22:20 -0800
> > Yinghai Lu <yinghai@kernel.org> wrote:
> > 
> > > Jesse Barnes wrote:
> > > > I don't remember seeing it, is it for 2.6.32?
> > > I
> > > http://lkml.org/lkml/2009/8/7/352
> > > 
> > > not sure. better to make it in pci-next for a while.
> > > 
> > > in my local tree for a while
> > 
> > Wow, that's an old one.  Linus said a few times he didn't want that
> > one to go upstream though.
> > 
> > Linus?  You mentioned something about re-working it a bit...
> 
> Oh, no, I'm perfectly fine with it. I just think it needs testing.
> Merging it early in the merge window is fine.

Well I guess I didn't miss it and I knew we'd had this conversation
before.

I applied it back in early Nov but forgot to update my "todo"
mailbox. Anyway it'll go in with my first pull request.

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center