diff mbox

PCI: Reset failed bridge resources in PCI realloc

Message ID CAE9FiQVkGcqFQ7e22Ekg7t-gWrQ9FdtadbVCASTGqU20FPE50g@mail.gmail.com
State Rejected
Headers show

Commit Message

Yinghai Lu July 30, 2014, 7:59 p.m. UTC
On Wed, Jul 30, 2014 at 1:00 AM, Guo Chao <yan@linux.vnet.ibm.com> wrote:
>         * However, in pbus_size_mem() we call calculate_memsize() to
>           calculate the final size. calculate_memsize() never shrinks
>           window, i.e. if old size is larger than newly sized result,
>           old size will be honored. Unfortunately, the old size at this
>           time is the sum of both normal BARs and SR-IOV BARs when doing
>           realloc.
>

that checking about old_size is added by:

| commit d65245c3297ac63abc51a976d92f45f2195d2854
| Author: Yinghai Lu <yinghai@kernel.org>
| Date:   Fri Jan 22 01:02:23 2010 -0800
|
|    PCI: don't shrink bridge resources
|
|    When clearing leaf bridge resources, trying to get a big enough one, we
|    could shrink the bridge if there is no resource under it.  Confirm
|    against the old resource side to make sure we're increasing the
|    allocation.

so to make both cases ok, may need to check if there is any children devices
on the dev->subordinate. Please check attached.

Thanks

Yinghai


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

Comments

Guo Chao July 31, 2014, 8:48 a.m. UTC | #1
On Wed, Jul 30, 2014 at 12:59:35PM -0700, Yinghai Lu wrote:
> On Wed, Jul 30, 2014 at 1:00 AM, Guo Chao <yan@linux.vnet.ibm.com> wrote:
> >         * However, in pbus_size_mem() we call calculate_memsize() to
> >           calculate the final size. calculate_memsize() never shrinks
> >           window, i.e. if old size is larger than newly sized result,
> >           old size will be honored. Unfortunately, the old size at this
> >           time is the sum of both normal BARs and SR-IOV BARs when doing
> >           realloc.
> >
> 
> that checking about old_size is added by:
> 
> | commit d65245c3297ac63abc51a976d92f45f2195d2854
> | Author: Yinghai Lu <yinghai@kernel.org>
> | Date:   Fri Jan 22 01:02:23 2010 -0800
> |
> |    PCI: don't shrink bridge resources
> |
> |    When clearing leaf bridge resources, trying to get a big enough one, we
> |    could shrink the bridge if there is no resource under it.  Confirm
> |    against the old resource side to make sure we're increasing the
> |    allocation.
> 
> so to make both cases ok, may need to check if there is any children devices
> on the dev->subordinate. Please check attached.
> 

Works well on one of the machines. Thanks.

Guo Chao

> Thanks
> 
> Yinghai
> 
> 
> ---
>  drivers/pci/setup-bus.c |   10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> Index: linux-2.6/drivers/pci/setup-bus.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/setup-bus.c
> +++ linux-2.6/drivers/pci/setup-bus.c
> @@ -1622,8 +1622,11 @@ again:
>          res->start = fail_res->start;
>          res->end = fail_res->end;
>          res->flags = fail_res->flags;
> -        if (fail_res->dev->subordinate)
> +        if (fail_res->dev->subordinate) {
>              res->flags = 0;
> +            if (!list_empty(&fail_res->dev->subordinate->devices))
> +                res->start = res->end = 0;
> +        }
>      }
>      free_list(&fail_head);
> 
> @@ -1688,8 +1691,11 @@ again:
>          res->start = fail_res->start;
>          res->end = fail_res->end;
>          res->flags = fail_res->flags;
> -        if (fail_res->dev->subordinate)
> +        if (fail_res->dev->subordinate) {
>              res->flags = 0;
> +            if (!list_empty(&fail_res->dev->subordinate->devices))
> +                res->start = res->end = 0;
> +        }
>      }
>      free_list(&fail_head);

> ---
>  drivers/pci/setup-bus.c |   10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> Index: linux-2.6/drivers/pci/setup-bus.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/setup-bus.c
> +++ linux-2.6/drivers/pci/setup-bus.c
> @@ -1622,8 +1622,11 @@ again:
>  		res->start = fail_res->start;
>  		res->end = fail_res->end;
>  		res->flags = fail_res->flags;
> -		if (fail_res->dev->subordinate)
> +		if (fail_res->dev->subordinate) {
>  			res->flags = 0;
> +			if (!list_empty(&fail_res->dev->subordinate->devices))
> +				res->start = res->end = 0;
> +		}
>  	}
>  	free_list(&fail_head);
>  
> @@ -1688,8 +1691,11 @@ again:
>  		res->start = fail_res->start;
>  		res->end = fail_res->end;
>  		res->flags = fail_res->flags;
> -		if (fail_res->dev->subordinate)
> +		if (fail_res->dev->subordinate) {
>  			res->flags = 0;
> +			if (!list_empty(&fail_res->dev->subordinate->devices))
> +				res->start = res->end = 0;
> +		}
>  	}
>  	free_list(&fail_head);
>  

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bjorn Helgaas Sept. 4, 2014, 4:23 a.m. UTC | #2
On Wed, Jul 30, 2014 at 12:59:35PM -0700, Yinghai Lu wrote:
> On Wed, Jul 30, 2014 at 1:00 AM, Guo Chao <yan@linux.vnet.ibm.com> wrote:
> >         * However, in pbus_size_mem() we call calculate_memsize() to
> >           calculate the final size. calculate_memsize() never shrinks
> >           window, i.e. if old size is larger than newly sized result,
> >           old size will be honored. Unfortunately, the old size at this
> >           time is the sum of both normal BARs and SR-IOV BARs when doing
> >           realloc.
> >
> 
> that checking about old_size is added by:
> 
> | commit d65245c3297ac63abc51a976d92f45f2195d2854
> | Author: Yinghai Lu <yinghai@kernel.org>
> | Date:   Fri Jan 22 01:02:23 2010 -0800
> |
> |    PCI: don't shrink bridge resources
> |
> |    When clearing leaf bridge resources, trying to get a big enough one, we
> |    could shrink the bridge if there is no resource under it.  Confirm
> |    against the old resource side to make sure we're increasing the
> |    allocation.
> 
> so to make both cases ok, may need to check if there is any children devices
> on the dev->subordinate. Please check attached.

I think we should postpone this until we figure out how we're going to
manage resource start/end/flags, i.e., the IORESOURCE_UNSET and
IORESOURCE_DISABLED question.

> Thanks
> 
> Yinghai
> 
> 
> ---
>  drivers/pci/setup-bus.c |   10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> Index: linux-2.6/drivers/pci/setup-bus.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/setup-bus.c
> +++ linux-2.6/drivers/pci/setup-bus.c
> @@ -1622,8 +1622,11 @@ again:
>          res->start = fail_res->start;
>          res->end = fail_res->end;
>          res->flags = fail_res->flags;
> -        if (fail_res->dev->subordinate)
> +        if (fail_res->dev->subordinate) {
>              res->flags = 0;
> +            if (!list_empty(&fail_res->dev->subordinate->devices))
> +                res->start = res->end = 0;
> +        }
>      }
>      free_list(&fail_head);
> 
> @@ -1688,8 +1691,11 @@ again:
>          res->start = fail_res->start;
>          res->end = fail_res->end;
>          res->flags = fail_res->flags;
> -        if (fail_res->dev->subordinate)
> +        if (fail_res->dev->subordinate) {
>              res->flags = 0;
> +            if (!list_empty(&fail_res->dev->subordinate->devices))
> +                res->start = res->end = 0;
> +        }
>      }
>      free_list(&fail_head);

> ---
>  drivers/pci/setup-bus.c |   10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> Index: linux-2.6/drivers/pci/setup-bus.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/setup-bus.c
> +++ linux-2.6/drivers/pci/setup-bus.c
> @@ -1622,8 +1622,11 @@ again:
>  		res->start = fail_res->start;
>  		res->end = fail_res->end;
>  		res->flags = fail_res->flags;
> -		if (fail_res->dev->subordinate)
> +		if (fail_res->dev->subordinate) {
>  			res->flags = 0;
> +			if (!list_empty(&fail_res->dev->subordinate->devices))
> +				res->start = res->end = 0;
> +		}
>  	}
>  	free_list(&fail_head);
>  
> @@ -1688,8 +1691,11 @@ again:
>  		res->start = fail_res->start;
>  		res->end = fail_res->end;
>  		res->flags = fail_res->flags;
> -		if (fail_res->dev->subordinate)
> +		if (fail_res->dev->subordinate) {
>  			res->flags = 0;
> +			if (!list_empty(&fail_res->dev->subordinate->devices))
> +				res->start = res->end = 0;
> +		}
>  	}
>  	free_list(&fail_head);
>  

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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

Index: linux-2.6/drivers/pci/setup-bus.c
===================================================================
--- linux-2.6.orig/drivers/pci/setup-bus.c
+++ linux-2.6/drivers/pci/setup-bus.c
@@ -1622,8 +1622,11 @@  again:
 		res->start = fail_res->start;
 		res->end = fail_res->end;
 		res->flags = fail_res->flags;
-		if (fail_res->dev->subordinate)
+		if (fail_res->dev->subordinate) {
 			res->flags = 0;
+			if (!list_empty(&fail_res->dev->subordinate->devices))
+				res->start = res->end = 0;
+		}
 	}
 	free_list(&fail_head);
 
@@ -1688,8 +1691,11 @@  again:
 		res->start = fail_res->start;
 		res->end = fail_res->end;
 		res->flags = fail_res->flags;
-		if (fail_res->dev->subordinate)
+		if (fail_res->dev->subordinate) {
 			res->flags = 0;
+			if (!list_empty(&fail_res->dev->subordinate->devices))
+				res->start = res->end = 0;
+		}
 	}
 	free_list(&fail_head);