so later we can do sth with those failed one
-v2: store start, end, flags aside. so could keep res cleared when assign
failed. and make following assignment of its children do not go wild
Signed-off-by: Yinghai Lu <[email protected]>
---
drivers/pci/setup-bus.c | 66 +++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 62 insertions(+), 4 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
@@ -27,7 +27,52 @@
#include <linux/slab.h>
#include "pci.h"
-static void pbus_assign_resources_sorted(const struct pci_bus *bus)
+struct resource_list_x {
+ struct resource_list_x *next;
+ struct resource *res;
+ struct pci_dev *dev;
+ resource_size_t start;
+ resource_size_t end;
+ unsigned long flags;
+};
+
+static void add_to_failed_list(struct resource_list_x *head,
+ struct pci_dev *dev, struct resource *res)
+{
+ struct resource_list_x *list = head;
+ struct resource_list_x *ln = list->next;
+ struct resource_list_x *tmp;
+
+ tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
+ if (!tmp) {
+ pr_warning("add_to_failed_list: kmalloc() failed!\n");
+ return;
+ }
+
+ tmp->next = ln;
+ tmp->res = res;
+ tmp->dev = dev;
+ tmp->start = res->start;
+ tmp->end = res->end;
+ tmp->flags = res->flags;
+ list->next = tmp;
+}
+
+static void free_failed_list(struct resource_list_x *head)
+{
+ struct resource_list_x *list, *tmp;
+
+ for (list = head->next; list;) {
+ tmp = list;
+ list = list->next;
+ kfree(tmp);
+ }
+
+ head->next = NULL;
+}
+
+static void pbus_assign_resources_sorted(const struct pci_bus *bus,
+ struct resource_list_x *fail_head)
{
struct pci_dev *dev;
struct resource *res;
@@ -58,6 +103,13 @@ static void pbus_assign_resources_sorted
res = list->res;
idx = res - &list->dev->resource[0];
if (pci_assign_resource(list->dev, idx)) {
+ if (fail_head && !pci_is_root_bus(list->dev->bus)) {
+ /*
+ * device need to keep flags and size
+ * for next try
+ */
+ add_to_failed_list(fail_head, list->dev, res);
+ }
res->start = 0;
res->end = 0;
res->flags = 0;
@@ -575,19 +627,20 @@ void __ref pci_bus_size_bridges(struct p
}
EXPORT_SYMBOL(pci_bus_size_bridges);
-void __ref pci_bus_assign_resources(const struct pci_bus *bus)
+static void __ref __pci_bus_assign_resources(const struct pci_bus *bus,
+ struct resource_list_x *fail_head)
{
struct pci_bus *b;
struct pci_dev *dev;
- pbus_assign_resources_sorted(bus);
+ pbus_assign_resources_sorted(bus, fail_head);
list_for_each_entry(dev, &bus->devices, bus_list) {
b = dev->subordinate;
if (!b)
continue;
- pci_bus_assign_resources(b);
+ __pci_bus_assign_resources(b, fail_head);
switch (dev->class >> 8) {
case PCI_CLASS_BRIDGE_PCI:
@@ -605,6 +658,11 @@ void __ref pci_bus_assign_resources(cons
}
}
}
+
+void __ref pci_bus_assign_resources(const struct pci_bus *bus)
+{
+ __pci_bus_assign_resources(bus, NULL);
+}
EXPORT_SYMBOL(pci_bus_assign_resources);
static void release_child_resources(struct resource *r)
On Fri, 2009-12-18 at 12:55 -0800, Yinghai Lu wrote:
>
> so later we can do sth with those failed one
>
> -v2: store start, end, flags aside. so could keep res cleared when assign
> failed. and make following assignment of its children do not go wild
>
> Signed-off-by: Yinghai Lu <[email protected]>
>
> ---
> drivers/pci/setup-bus.c | 66 +++++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 62 insertions(+), 4 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
> @@ -27,7 +27,52 @@
> #include <linux/slab.h>
> #include "pci.h"
>
> -static void pbus_assign_resources_sorted(const struct pci_bus *bus)
> +struct resource_list_x {
> + struct resource_list_x *next;
> + struct resource *res;
> + struct pci_dev *dev;
> + resource_size_t start;
> + resource_size_t end;
> + unsigned long flags;
> +};
> +
> +static void add_to_failed_list(struct resource_list_x *head,
> + struct pci_dev *dev, struct resource *res)
> +{
> + struct resource_list_x *list = head;
> + struct resource_list_x *ln = list->next;
> + struct resource_list_x *tmp;
> +
> + tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
> + if (!tmp) {
> + pr_warning("add_to_failed_list: kmalloc() failed!\n");
> + return;
> + }
> +
> + tmp->next = ln;
> + tmp->res = res;
> + tmp->dev = dev;
> + tmp->start = res->start;
> + tmp->end = res->end;
> + tmp->flags = res->flags;
> + list->next = tmp;
> +}
> +
> +static void free_failed_list(struct resource_list_x *head)
> +{
> + struct resource_list_x *list, *tmp;
> +
> + for (list = head->next; list;) {
> + tmp = list;
> + list = list->next;
> + kfree(tmp);
> + }
> +
> + head->next = NULL;
> +}
This patch adds a call to add_to_failed_list(), but no call to
free_failed_list(), so at first glance, this patch appears to introduce
a leak. I see that it actually doesn't because you pass around a NULL
'fail_head', so you never actually call add_to_failed_list(), but it
would make more sense if you added the alloc and matching free in a
single patch.
> +static void pbus_assign_resources_sorted(const struct pci_bus *bus,
> + struct resource_list_x *fail_head)
> {
> struct pci_dev *dev;
> struct resource *res;
> @@ -58,6 +103,13 @@ static void pbus_assign_resources_sorted
> res = list->res;
> idx = res - &list->dev->resource[0];
> if (pci_assign_resource(list->dev, idx)) {
> + if (fail_head && !pci_is_root_bus(list->dev->bus)) {
> + /*
> + * device need to keep flags and size
> + * for next try
> + */
> + add_to_failed_list(fail_head, list->dev, res);
> + }
> res->start = 0;
> res->end = 0;
> res->flags = 0;
> @@ -575,19 +627,20 @@ void __ref pci_bus_size_bridges(struct p
> }
> EXPORT_SYMBOL(pci_bus_size_bridges);
>
> -void __ref pci_bus_assign_resources(const struct pci_bus *bus)
> +static void __ref __pci_bus_assign_resources(const struct pci_bus *bus,
> + struct resource_list_x *fail_head)
> {
> struct pci_bus *b;
> struct pci_dev *dev;
>
> - pbus_assign_resources_sorted(bus);
> + pbus_assign_resources_sorted(bus, fail_head);
>
> list_for_each_entry(dev, &bus->devices, bus_list) {
> b = dev->subordinate;
> if (!b)
> continue;
>
> - pci_bus_assign_resources(b);
> + __pci_bus_assign_resources(b, fail_head);
>
> switch (dev->class >> 8) {
> case PCI_CLASS_BRIDGE_PCI:
> @@ -605,6 +658,11 @@ void __ref pci_bus_assign_resources(cons
> }
> }
> }
> +
> +void __ref pci_bus_assign_resources(const struct pci_bus *bus)
> +{
> + __pci_bus_assign_resources(bus, NULL);
> +}
> EXPORT_SYMBOL(pci_bus_assign_resources);
>
> static void release_child_resources(struct resource *r)
>
>
Bjorn Helgaas wrote:
> On Fri, 2009-12-18 at 12:55 -0800, Yinghai Lu wrote:
>> so later we can do sth with those failed one
>>
>> -v2: store start, end, flags aside. so could keep res cleared when assign
>> failed. and make following assignment of its children do not go wild
>>
>> Signed-off-by: Yinghai Lu <[email protected]>
>>
>> ---
>> drivers/pci/setup-bus.c | 66 +++++++++++++++++++++++++++++++++++++++++++++---
>> 1 file changed, 62 insertions(+), 4 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
>> @@ -27,7 +27,52 @@
>> #include <linux/slab.h>
>> #include "pci.h"
>>
>> -static void pbus_assign_resources_sorted(const struct pci_bus *bus)
>> +struct resource_list_x {
>> + struct resource_list_x *next;
>> + struct resource *res;
>> + struct pci_dev *dev;
>> + resource_size_t start;
>> + resource_size_t end;
>> + unsigned long flags;
>> +};
>> +
>> +static void add_to_failed_list(struct resource_list_x *head,
>> + struct pci_dev *dev, struct resource *res)
>> +{
>> + struct resource_list_x *list = head;
>> + struct resource_list_x *ln = list->next;
>> + struct resource_list_x *tmp;
>> +
>> + tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
>> + if (!tmp) {
>> + pr_warning("add_to_failed_list: kmalloc() failed!\n");
>> + return;
>> + }
>> +
>> + tmp->next = ln;
>> + tmp->res = res;
>> + tmp->dev = dev;
>> + tmp->start = res->start;
>> + tmp->end = res->end;
>> + tmp->flags = res->flags;
>> + list->next = tmp;
>> +}
>> +
>> +static void free_failed_list(struct resource_list_x *head)
>> +{
>> + struct resource_list_x *list, *tmp;
>> +
>> + for (list = head->next; list;) {
>> + tmp = list;
>> + list = list->next;
>> + kfree(tmp);
>> + }
>> +
>> + head->next = NULL;
>> +}
>
> This patch adds a call to add_to_failed_list(), but no call to
> free_failed_list(), so at first glance, this patch appears to introduce
> a leak. I see that it actually doesn't because you pass around a NULL
> 'fail_head', so you never actually call add_to_failed_list(), but it
> would make more sense if you added the alloc and matching free in a
> single patch.
free_failed_list will free them all.
YH