[PATCH] resources: when allocate_resource() fails, leave resource untouched

[PATCH] resources: when allocate_resource() fails, leave resource untouched

[Bjorn Helgaas]

When "allocate_resource(root, new, size, ...)" fails, we currently
clobber "new".  This is inconvenient for the caller, who might care
about the original contents of the resource.

For example, when pci_bus_alloc_resource() fails, the "can't allocate
mem resource %pR" message from pci_assign_resources() currently contains
junk for the resource start/end.

This patch delays the "new" update until we're about to return success.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
---
 kernel/resource.c |   26 ++++++++++++++------------
 1 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/kernel/resource.c b/kernel/resource.c
index fb11a58..dc15686 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -308,35 +308,37 @@ static int find_resource(struct resource *root, struct resource *new,
 			 void *alignf_data)
 {
 	struct resource *this = root->child;
+	resource_size_t start, end;
 
-	new->start = root->start;
+	start = root->start;
 	/*
 	 * Skip past an allocated resource that starts at 0, since the assignment
 	 * of this->start - 1 to new->end below would cause an underflow.
 	 */
 	if (this && this->start == 0) {
-		new->start = this->end + 1;
+		start = this->end + 1;
 		this = this->sibling;
 	}
 	for(;;) {
 		if (this)
-			new->end = this->start - 1;
+			end = this->start - 1;
 		else
-			new->end = root->end;
-		if (new->start < min)
-			new->start = min;
-		if (new->end > max)
-			new->end = max;
-		new->start = ALIGN(new->start, align);
+			end = root->end;
+		if (start < min)
+			start = min;
+		if (end > max)
+			end = max;
+		start = ALIGN(start, align);
 		if (alignf)
 			alignf(alignf_data, new, size, align);
-		if (new->start < new->end && new->end - new->start >= size - 1) {
-			new->end = new->start + size - 1;
+		if (start < end && end - start >= size - 1) {
+			new->start = start;
+			new->end = start + size - 1;
 			return 0;
 		}
 		if (!this)
 			break;
-		new->start = this->end + 1;
+		start = this->end + 1;
 		this = this->sibling;
 	}
 	return -EBUSY;

Re: [PATCH] resources: when allocate_resource() fails, leave resource untouched

[Jesse Barnes]

On Mon, 02 Nov 2009 10:45:36 -0700
Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:

> When "allocate_resource(root, new, size, ...)" fails, we currently
> clobber "new".  This is inconvenient for the caller, who might care
> about the original contents of the resource.
> 
> For example, when pci_bus_alloc_resource() fails, the "can't allocate
> mem resource %pR" message from pci_assign_resources() currently
> contains junk for the resource start/end.
> 
> This patch delays the "new" update until we're about to return
> success.
> 
> Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
> ---
>  kernel/resource.c |   26 ++++++++++++++------------
>  1 files changed, 14 insertions(+), 12 deletions(-)
> 
> diff --git a/kernel/resource.c b/kernel/resource.c
> index fb11a58..dc15686 100644
> --- a/kernel/resource.c
> +++ b/kernel/resource.c
> @@ -308,35 +308,37 @@ static int find_resource(struct resource *root,
> struct resource *new, void *alignf_data)
>  {
>  	struct resource *this = root->child;
> +	resource_size_t start, end;
>  
> -	new->start = root->start;
> +	start = root->start;
>  	/*
>  	 * Skip past an allocated resource that starts at 0, since
> the assignment
>  	 * of this->start - 1 to new->end below would cause an
> underflow. */
>  	if (this && this->start == 0) {
> -		new->start = this->end + 1;
> +		start = this->end + 1;
>  		this = this->sibling;
>  	}
>  	for(;;) {
>  		if (this)
> -			new->end = this->start - 1;
> +			end = this->start - 1;
>  		else
> -			new->end = root->end;
> -		if (new->start < min)
> -			new->start = min;
> -		if (new->end > max)
> -			new->end = max;
> -		new->start = ALIGN(new->start, align);
> +			end = root->end;
> +		if (start < min)
> +			start = min;
> +		if (end > max)
> +			end = max;
> +		start = ALIGN(start, align);
>  		if (alignf)
>  			alignf(alignf_data, new, size, align);
> -		if (new->start < new->end && new->end - new->start
> >= size - 1) {
> -			new->end = new->start + size - 1;
> +		if (start < end && end - start >= size - 1) {
> +			new->start = start;
> +			new->end = start + size - 1;
>  			return 0;
>  		}
>  		if (!this)
>  			break;
> -		new->start = this->end + 1;
> +		start = this->end + 1;
>  		this = this->sibling;
>  	}
>  	return -EBUSY;

Seems like a reasonable change to me.  Linus is usually the gatekeeper
for resource.c.

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH] resources: when allocate_resource() fails, leave resource untouched

[Linus Torvalds]

On Wed, 4 Nov 2009, Jesse Barnes wrote:
> 
> Seems like a reasonable change to me.  Linus is usually the gatekeeper
> for resource.c.

I'm certainly ok with this one. I wouldn't be surprised if it even allows 
for better code generation, apart from leaving resources untouched when 
allocation fails. So:

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

It was apparently sent to Andrew, but I can certainly take it. Or you can 
take it into the PCI tree. Or we can leave it in -mm. Anything goes. Just 
tell me.

			Linus

Re: [PATCH] resources: when allocate_resource() fails, leave resource untouched

[Yinghai Lu]

On Wed, Nov 4, 2009 at 10:29 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
>
> On Wed, 4 Nov 2009, Jesse Barnes wrote:
>>
>> Seems like a reasonable change to me.  Linus is usually the gatekeeper
>> for resource.c.
>
> I'm certainly ok with this one. I wouldn't be surprised if it even allows
> for better code generation, apart from leaving resources untouched when
> allocation fails. So:
>
>        Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
>
> It was apparently sent to Andrew, but I can certainly take it. Or you can
> take it into the PCI tree. Or we can leave it in -mm. Anything goes. Just
> tell me.
>

looks should be put into pci tree.

two of patches that is releasing not big enough range from leaf bridge
could be user.

don't need the "save and restore" trick anymore.

YH

@@ -57,10 +100,23 @@ static void pbus_assign_resources_sorted
        for (list = head.next; list;) {
                res = list->res;
                idx = res - &list->dev->resource[0];
+               /* save the size at first */
+               size = resource_size(res);
                if (pci_assign_resource(list->dev, idx)) {
-                       res->start = 0;
-                       res->end = 0;
-                       res->flags = 0;
+                       if (fail_head && !list->dev->subordinate &&
+                           !pci_is_root_bus(list->dev->bus)) {
+                               /*
+                                * device need to keep flags and size
+                                * for second try
+                                */
+                               res->start = 0;
+                               res->end   = size - 1;
+                               add_to_failed_list(fail_head, list->dev, res);
+                       } else {
+                               res->start = 0;
+                               res->end = 0;
+                               res->flags = 0;
+                       }
                }
                tmp = list;
                list = list->next;

Re: [PATCH] resources: when allocate_resource() fails, leave resource untouched

[Jesse Barnes]

On Wed, 4 Nov 2009 10:29:54 -0800 (PST)
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> 
> 
> On Wed, 4 Nov 2009, Jesse Barnes wrote:
> > 
> > Seems like a reasonable change to me.  Linus is usually the
> > gatekeeper for resource.c.
> 
> I'm certainly ok with this one. I wouldn't be surprised if it even
> allows for better code generation, apart from leaving resources
> untouched when allocation fails. So:
> 
> 	Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
> 
> It was apparently sent to Andrew, but I can certainly take it. Or you
> can take it into the PCI tree. Or we can leave it in -mm. Anything
> goes. Just tell me.

Ok, I'll pick it up.  Thanks.

-- 
Jesse Barnes, Intel Open Source Technology Center