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

[Jesse Barnes <jbarnes@virtuousgeek.org>]

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