Assign mem resource fail after remove and rescan

DISCUSSION Yijing Wang 3 participants 11 messages Mar 28, 2015 – Jun 25, 2015 PCI

Share & tools

Yijing Wang

author

2015-03-28 10:02 UTC #0 lore raw

Hi Yinghai, Bjorn
I found a memory resource assignment fail after I did remove and rescan a bridge device,
I don't know whether this is a kernel bug, I hope could get some advice from you, thanks!

My pci tree:
-[0000:00]-+-00.0 Intel Corporation 2nd Generation Core Processor Family DRAM Controller
...

           +-1c.0-[02-21]----00.0-[03-21]--+-01.0-[04-12]----00.0-[05-12]----19.0-[06-12]----00.0  PLX Technology, Inc. Device 1009
           |                               +-05.0-[13]--
           |                               +-07.0-[14-20]----00.0-[15-20]--+-08.0-[16]--+-00.0  NVIDIA Corporation GT218 [GeForce 210]
           |                               |                               |            \-00.1  NVIDIA Corporation High Definition Audio Controller
           |                               |                               +-14.0-[17]----00.0  Intel Corporation Device 0953
           |                               |                               \-19.0-[18-20]----00.0  PLX Technology, Inc. Device 1009
           |                               \-09.0-[21]--

Reproduce action:
1. echo 1 > /sys/bus/pci/devices/0000:05:19.0/remove
2. echo 1 > /sys/bus/pci/rescan

After above operations, I found the memory resource assigned fail.

Fail log:

[  105.905480] pci_bus 0000:06: busn_res: [bus 06-12] is released
[  125.771655] pci_bus 0000:01: busn_res: [bus 01] end is updated to 01
[  125.772519] pci 0000:05:19.0: [10b5:9797] type 01 class 0x060400
[  125.772846] pci 0000:05:19.0: PME# supported from D0 D3hot D3cold
[  125.778576] pci_bus 0000:06: busn_res: can not insert [bus 06-ff] under [bus 05-12] (conflicts with (null) [bus 05-12])
[  125.778638] pci 0000:06:00.0: [10b5:1009] type 00 class 0x088000
[  125.778693] pci 0000:06:00.0: reg 0x10: [mem 0xe8000000-0xe87fffff]
[  125.778761] pci 0000:06:00.0: reg 0x18: [mem 0xe4000000-0xe7ffffff 64bit]
[  125.782315] pci 0000:05:19.0: PCI bridge to [bus 06-ff]
[  125.782345] pci 0000:05:19.0:   bridge window [mem 0xe4000000-0xe87fffff]
[  125.782366] pci_bus 0000:06: busn_res: [bus 06-ff] end is updated to 12
[  125.782380] pci_bus 0000:05: busn_res: [bus 05-12] end is updated to 12
[  125.782393] pci_bus 0000:04: busn_res: [bus 04-12] end is updated to 12
[  125.782422] pci_bus 0000:13: busn_res: [bus 13] end is updated to 13
[  125.782674] pci_bus 0000:16: busn_res: [bus 16] end is updated to 16
[  125.782721] pci_bus 0000:17: busn_res: [bus 17] end is updated to 17
[  125.782751] pci_bus 0000:18: busn_res: [bus 18-20] end is updated to 20
[  125.782763] pci_bus 0000:15: busn_res: [bus 15-20] end is updated to 20
[  125.782775] pci_bus 0000:14: busn_res: [bus 14-20] end is updated to 20
[  125.782802] pci_bus 0000:21: busn_res: [bus 21] end is updated to 21
[  125.782812] pci_bus 0000:03: busn_res: [bus 03-21] end is updated to 21
[  125.782822] pci_bus 0000:02: busn_res: [bus 02-21] end is updated to 21
[  125.782845] pci_bus 0000:22: busn_res: [bus 22] end is updated to 22
[  125.786433] pci_bus 0000:24: busn_res: [bus 24] end is updated to 24
[  125.786450] pci_bus 0000:23: busn_res: [bus 23-24] end is updated to 24
[  125.786749] pci 0000:05:19.0: BAR 14: no space for [mem size 0x06000000]
[  125.786754] pci 0000:05:19.0: BAR 14: failed to assign [mem size 0x06000000]
[  125.786760] pci 0000:06:00.0: BAR 2: no space for [mem size 0x04000000 64bit]
[  125.786764] pci 0000:06:00.0: BAR 2: failed to assign [mem size 0x04000000 64bit]
[  125.786769] pci 0000:06:00.0: BAR 0: no space for [mem size 0x00800000]
[  125.786773] pci 0000:06:00.0: BAR 0: failed to assign [mem size 0x00800000]
[  125.786777] pci 0000:05:19.0: PCI bridge to [bus 06-12]
[  125.786819] pci 0000:23:00.0: PCI bridge to [bus 24]

System boot log:

...

[    0.122097] pci 0000:05:19.0: [10b5:9797] type 01 class 0x060400
[    0.122373] pci 0000:05:19.0: PME# supported from D0 D3hot D3cold
[    0.122481] pci 0000:04:00.0: PCI bridge to [bus 05-ff]
[    0.122501] pci 0000:04:00.0:   bridge window [mem 0xe4000000-0xe87fffff]
[    0.122708] pci 0000:06:00.0: [10b5:1009] type 00 class 0x088000
[    0.122749] pci 0000:06:00.0: reg 0x10: [mem 0xe8000000-0xe87fffff]
[    0.122816] pci 0000:06:00.0: reg 0x18: [mem 0xe4000000-0xe7ffffff 64bit]
[    0.123323] pci 0000:05:19.0: PCI bridge to [bus 06-ff]
[    0.123346] pci 0000:05:19.0:   bridge window [mem 0xe4000000-0xe87fffff]

...

I compared above log and found after we did remove and rescan, the bridge requested resource size extended to 0x06000000, and when system boot up, it requested only 0x4800000.

In hotplug(remove and rescan) path, we would call calculate_mem_align() function which would align the resource at 0x2000000.

I wonder is this a resource assignment bug ?

Thanks!
Yijing.

Signature · 2 lines

Thanks!
Yijing

Yinghai Lu

2015-03-29 06:18 UTC #1 lore raw

Quoted 6 lines · Yijing Wang

...
I compared above log and found after we did remove and rescan, the bridge requested resource size extended to 0x06000000, and when system boot up, it requested only 0x4800000.
In hotplug(remove and rescan) path, we would call calculate_mem_align() function which would align the resource at 0x2000000.

Looks like we align the size too early. Please check if attach patch could fix the problem.

Thanks

Yinghai

Subject: [RFT PATCH] PCI: Align resource size later in bus mem sizing

TEST only.

We only need to align the size on bridge up one level later.

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

drivers/pci/setup-bus.c +7 -8

Index: linux-2.6/drivers/pci/setup-bus.c
===================================================================
@@ -783,8 +783,7 @@ static resource_size_t calculate_iosize(
 static resource_size_t calculate_memsize(resource_size_t size,
 		resource_size_t min_size,
 		resource_size_t size1,
-		resource_size_t old_size,
-		resource_size_t align)
+		resource_size_t old_size)
 {
 	if (size < min_size)
 		size = min_size;

@@ -792,8 +791,8 @@ static resource_size_t calculate_memsize
 		old_size = 0;
 	if (size < old_size)
 		size = old_size;
-	size = ALIGN(size + size1, align);
-	return size;
+
+	return size + size1;
 }
 
 resource_size_t __weak pcibios_window_alignment(struct pci_bus *bus,

@@ -1008,10 +1007,10 @@ static int pbus_size_mem(struct pci_bus
 				r->flags = 0;
 				continue;
 			}
-			size += r_size;
+			size += ALIGN(r_size, align);
 			/* Exclude ranges with size > align from
 			   calculation of the alignment. */
-			if (r_size == align)
+			if (r_size <= align)
 				aligns[order] += align;
 			if (order > max_order)
 				max_order = order;

@@ -1023,12 +1022,12 @@ static int pbus_size_mem(struct pci_bus
 
 	min_align = calculate_mem_align(aligns, max_order);
 	min_align = max(min_align, window_alignment(bus, b_res->flags));
-	size0 = calculate_memsize(size, min_size, 0, resource_size(b_res), min_align);
+	size0 = calculate_memsize(size, min_size, 0, resource_size(b_res));
 	if (children_add_size > add_size)
 		add_size = children_add_size;
 	size1 = (!realloc_head || (realloc_head && !add_size)) ? size0 :
 		calculate_memsize(size, min_size, add_size,
-				resource_size(b_res), min_align);
+				resource_size(b_res));
 	if (!size0 && !size1) {
 		if (b_res->start || b_res->end)
 			dev_info(&bus->self->dev, "disabling bridge window %pR to %pR (unused)\n",

Yijing Wang

author

2015-03-30 04:06 UTC #2 lore raw

Quoted 10 lines · Yinghai Lu

Quoted 6 lines · Yijing Wang
...
I compared above log and found after we did remove and rescan, the bridge requested resource size extended to 0x06000000, and when system boot up, it requested only 0x4800000.
In hotplug(remove and rescan) path, we would call calculate_mem_align() function which would align the resource at 0x2000000.
Looks like we align the size too early. Please check if attach patch could fix the problem.

OK, I will test it today, thanks!

Quoted 5 lines

Thanks
Yinghai

Signature · 2 lines

Thanks!
Yijing

Yijing Wang

author

2015-03-31 06:38 UTC #3 lore raw

In reply to Yinghai Lu

Quoted 11 lines · Yinghai Lu

Quoted 6 lines · Yijing Wang
...
I compared above log and found after we did remove and rescan, the bridge requested resource size extended to 0x06000000, and when system boot up, it requested only 0x4800000.
In hotplug(remove and rescan) path, we would call calculate_mem_align() function which would align the resource at 0x2000000.
Looks like we align the size too early. Please check if attach patch could fix the problem.

Hi Yinghai,
I tested it, remove and rescan 05:19.0 device is ok now, but
if do the operations for the parent device of 05:19.0, the result is still fail.

-[0000:00]-+-00.0 Intel Corporation 2nd Generation Core Processor Family DRAM Controller

           +-01.0-[01]--
           +-16.0  Intel Corporation 6 Series/C200 Series Chipset Family MEI Controller #1
           +-19.0  Intel Corporation 82579LM Gigabit Network Connection
           +-1a.0  Intel Corporation 6 Series/C200 Series Chipset Family USB Enhanced Host Controller #2
           +-1c.0-[02-21]----00.0-[03-21]--+-01.0-[04-12]----00.0-[05-12]----19.0-[06-12]----00.0  PLX Technology, Inc. Device 1009
           |                               +-05.0-[13]--

Remove and rescan for 04:00.0

...

[  152.657657] pci_bus 0000:06: busn_res: [bus 06-12] is released
[  152.659086] pci_bus 0000:05: busn_res: [bus 05-12] is released
[  165.353539] pci_bus 0000:01: busn_res: [bus 01] end is updated to 01
[  165.353789] pci 0000:04:00.0: [10b5:9797] type 01 class 0x060400
[  165.354149] pci 0000:04:00.0: PME# supported from D0 D3hot D3cold
[  165.362337] pci_bus 0000:05: busn_res: can not insert [bus 05-ff] under [bus 04-12] (conflicts with (null) [bus 04-12])
[  165.362935] pci 0000:05:19.0: [10b5:9797] type 01 class 0x060400
[  165.363250] pci 0000:05:19.0: PME# supported from D0 D3hot D3cold
[  165.365996] pci 0000:04:00.0: PCI bridge to [bus 05-ff]
[  165.366026] pci 0000:04:00.0:   bridge window [mem 0xe4000000-0xe87fffff]
[  165.368389] pci 0000:06:00.0: [10b5:1009] type 00 class 0x088000
[  165.368458] pci 0000:06:00.0: reg 0x10: [mem 0xe8000000-0xe87fffff]
[  165.368524] pci 0000:06:00.0: reg 0x18: [mem 0xe4000000-0xe7ffffff 64bit]
[  165.369912] pci 0000:05:19.0: PCI bridge to [bus 06-ff]
[  165.369939] pci 0000:05:19.0:   bridge window [mem 0xe4000000-0xe87fffff]
[  165.369958] pci_bus 0000:06: busn_res: [bus 06-ff] end is updated to 12
[  165.369972] pci_bus 0000:05: busn_res: [bus 05-ff] end is updated to 12
[  165.369984] pci_bus 0000:04: busn_res: [bus 04-12] end is updated to 12
[  165.370014] pci_bus 0000:13: busn_res: [bus 13] end is updated to 13
[  165.370259] pci_bus 0000:16: busn_res: [bus 16] end is updated to 16
[  165.370304] pci_bus 0000:17: busn_res: [bus 17] end is updated to 17
[  165.370334] pci_bus 0000:18: busn_res: [bus 18-20] end is updated to 20
[  165.370346] pci_bus 0000:15: busn_res: [bus 15-20] end is updated to 20
[  165.370358] pci_bus 0000:14: busn_res: [bus 14-20] end is updated to 20
[  165.370384] pci_bus 0000:21: busn_res: [bus 21] end is updated to 21
[  165.370395] pci_bus 0000:03: busn_res: [bus 03-21] end is updated to 21
[  165.370404] pci_bus 0000:02: busn_res: [bus 02-21] end is updated to 21
[  165.374463] pci_bus 0000:22: busn_res: [bus 22] end is updated to 22
[  165.374657] pci_bus 0000:24: busn_res: [bus 24] end is updated to 24
[  165.374670] pci_bus 0000:23: busn_res: [bus 23-24] end is updated to 24
[  165.374956] pci 0000:04:00.0: BAR 14: no space for [mem size 0x06000000]
[  165.374960] pci 0000:04:00.0: BAR 14: failed to assign [mem size 0x06000000]
[  165.374965] pci 0000:05:19.0: BAR 14: no space for [mem size 0x04800000]
[  165.374968] pci 0000:05:19.0: BAR 14: failed to assign [mem size 0x04800000]
[  165.374973] pci 0000:06:00.0: BAR 2: no space for [mem size 0x04000000 64bit]
[  165.374977] pci 0000:06:00.0: BAR 2: failed to assign [mem size 0x04000000 64bit]
[  165.374981] pci 0000:06:00.0: BAR 0: no space for [mem size 0x00800000]
[  165.374984] pci 0000:06:00.0: BAR 0: failed to assign [mem size 0x00800000]
[  165.374989] pci 0000:05:19.0: PCI bridge to [bus 06-12]
[  165.375025] pci 0000:04:00.0: PCI bridge to [bus 05-12]
[  165.375064] pci 0000:23:00.0: PCI bridge to [bus 24]

Quoted 4 lines

Thanks
Yinghai

Signature · 2 lines

Thanks!
Yijing

Yinghai Lu

2015-04-01 22:21 UTC #4 lore raw

Quoted 10 lines · Yijing Wang

Quoted 6 lines · Yinghai Lu
Quoted 5 lines · Yijing Wang
...
I compared above log and found after we did remove and rescan, the bridge requested resource size extended to 0x06000000, and when system boot up, it requested only 0x4800000.
I tested it, remove and rescan 05:19.0 device is ok now, but
if do the operations for the parent device of 05:19.0, the result is still fail.

Found the problem, attached patch should fix the problem.

Thanks

Yinghai

Subject: [PATCH] PCI: Optimize bus mem sizing to small size

Current code try to get min_align as possible and use that to align final size.

That could cause generate wrong align/size or too big size in some case.

when we have align/size: 16M/64M
min_align/size0 will be 8M/64M, that is wrong, align must be 16M.

when we have align/size: 1M/1M, 64M/64M,
min_align/size0 will be 32M/96M, that is way too big for sum size 65M.

That will cuase allocation fails.

The patch introduce max_align/size0_max, and size0_max is just sum of all children resource.

Prefer small size instead of small align. Only use min_align when two size is the same.

The new size will only need to be aligned to bus window alignment.

Signed-off-by Yinghai Lu <[email protected]>

---
 drivers/pci/setup-bus.c |   50 +++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 47 insertions(+), 3 deletions(-)

drivers/pci/setup-bus.c +47 -3

Index: linux-2.6/drivers/pci/setup-bus.c
===================================================================
@@ -882,12 +882,14 @@ static void pbus_size_io(struct pci_bus
 	}
 
 	size0 = calculate_iosize(size, min_size, size1,
-			resource_size(b_res), min_align);
+				 resource_size(b_res),
+				 window_alignment(bus, IORESOURCE_IO));
 	if (children_add_size > add_size)
 		add_size = children_add_size;
 	size1 = (!realloc_head || (realloc_head && !add_size)) ? size0 :
 		calculate_iosize(size, min_size, add_size + size1,
-			resource_size(b_res), min_align);
+				 resource_size(b_res),
+				 window_alignment(bus, IORESOURCE_IO));
 	if (!size0 && !size1) {
 		if (b_res->start || b_res->end)
 			dev_info(&bus->self->dev, "disabling bridge window %pR to %pR (unused)\n",

@@ -962,6 +964,8 @@ static int pbus_size_mem(struct pci_bus
 	struct resource *b_res = find_free_bus_resource(bus,
 					mask | IORESOURCE_PREFETCH, type);
 	resource_size_t children_add_size = 0;
+	resource_size_t max_align = 0, size0_max;
+	int count = 0;
 
 	if (!b_res)
 		return -ENOSPC;

@@ -1016,19 +1020,59 @@ static int pbus_size_mem(struct pci_bus
 			if (order > max_order)
 				max_order = order;
 
+			count++;
+			if (align > max_align)
+				max_align = align;
+
 			if (realloc_head)
 				children_add_size += get_res_add_size(realloc_head, r);
 		}
 	}
 
+	/*
+	 * New rule: Prefer to small size instead of small align,
+	 * when we have align/size: 1M/1M, 2M/2M,
+	 *  min_align/size0: 1M/3M, max_align/size0_max: 2M/3M
+	 *  pick 1M/3M.
+	 * when we have align/size: 1M/1M, 64M/64M,
+	 *  min_align/size0: 32M/96M, max_align/size0_max: 64M/65M
+	 *  pick 64M/65M.
+	 * when we have align/size: 1M/1M, 16M/64M,
+	 *  min_align/size0: 8M/72M, max_align/size0_max: 16M/65M
+	 *  pick 16M/65M.
+	 * when we have align/size: 32M/64M, 128M/512M
+	 *  min_align/size0: 64M/576M, max_align/size0_max: 128M/576M
+	 *  pick 64M/576M.
+	 * when we have align/size: 16M/32M, 128M/512M
+	 *  min_align/size0: 64M/576M, max_align/size0_max: 128M/554M
+	 *  pick 128M/554M.
+	 * when we have align/size: 16M/64M
+	 *  min_align/size0: 8M/64M, max_align/size0_max: 16M/64M
+	 *  have to use 16M/64M.
+	 */
 	min_align = calculate_mem_align(aligns, max_order);
 	min_align = max(min_align, window_alignment(bus, b_res->flags));
+	max_align = max(max_align, window_alignment(bus, b_res->flags));
+	if (count == 1)
+		min_align = max_align;
+
 	size0 = calculate_memsize(size, min_size, 0, resource_size(b_res), min_align);
+	size0_max = calculate_memsize(size, min_size, 0, resource_size(b_res),
+					window_alignment(bus, b_res->flags));
+
+	if (size0_max < size0) {
+		size0 = size0_max;
+		min_align = max_align;
+		max_align--; /* to use small align for size1 calculation */
+	}
+
 	if (children_add_size > add_size)
 		add_size = children_add_size;
 	size1 = (!realloc_head || (realloc_head && !add_size)) ? size0 :
 		calculate_memsize(size, min_size, add_size,
-				resource_size(b_res), min_align);
+				resource_size(b_res),
+				min_align <= max_align ? min_align :
+					window_alignment(bus, b_res->flags));
 	if (!size0 && !size1) {
 		if (b_res->start || b_res->end)
 			dev_info(&bus->self->dev, "disabling bridge window %pR to %pR (unused)\n",

Yijing Wang

author

2015-04-02 08:36 UTC #5 lore raw

Quoted 5 lines · Yinghai Lu

Quoted 4 lines · Yijing Wang
Quoted 3 lines · Yinghai Lu
Quoted 2 lines · Yijing Wang
...

Quoted 8 lines

Quoted 6 lines
Quoted 3 lines
Quoted 3 lines
I compared above log and found after we did remove and rescan, the bridge requested resource size extended to 0x06000000, and when system boot up, it requested only 0x4800000.
I tested it, remove and rescan 05:19.0 device is ok now, but
if do the operations for the parent device of 05:19.0, the result is still fail.
Found the problem, attached patch should fix the problem.

Thanks, will test soon.

Quoted 5 lines

Thanks
Yinghai

Signature · 2 lines

Thanks!
Yijing

Yijing Wang

author

2015-04-09 11:12 UTC #6 lore raw

In reply to Yinghai Lu

Quoted 5 lines · Yinghai Lu

Quoted 4 lines · Yijing Wang
Quoted 3 lines · Yinghai Lu
Quoted 2 lines · Yijing Wang
...

Quoted 8 lines

Quoted 6 lines
Quoted 3 lines
Quoted 3 lines
I compared above log and found after we did remove and rescan, the bridge requested resource size extended to 0x06000000, and when system boot up, it requested only 0x4800000.
I tested it, remove and rescan 05:19.0 device is ok now, but
if do the operations for the parent device of 05:19.0, the result is still fail.
Found the problem, attached patch should fix the problem.

Hi Yinghai, sorry for the delay reply, this fix works, dmesg info:

...

[  181.567872] pci_bus 0000:06: busn_res: [bus 06-ff] end is updated to 12
[  181.567886] pci_bus 0000:05: busn_res: [bus 05-ff] end is updated to 12
[  181.567899] pci_bus 0000:04: busn_res: [bus 04-12] end is updated to 12
[  181.567928] pci_bus 0000:13: busn_res: [bus 13] end is updated to 13
[  181.568177] pci_bus 0000:16: busn_res: [bus 16] end is updated to 16
[  181.568233] pci_bus 0000:17: busn_res: [bus 17] end is updated to 17
[  181.568263] pci_bus 0000:18: busn_res: [bus 18-20] end is updated to 20
[  181.568274] pci_bus 0000:15: busn_res: [bus 15-20] end is updated to 20
[  181.568286] pci_bus 0000:14: busn_res: [bus 14-20] end is updated to 20
[  181.568313] pci_bus 0000:21: busn_res: [bus 21] end is updated to 21
[  181.568323] pci_bus 0000:03: busn_res: [bus 03-21] end is updated to 21
[  181.568333] pci_bus 0000:02: busn_res: [bus 02-21] end is updated to 21
[  181.568355] pci_bus 0000:22: busn_res: [bus 22] end is updated to 22
[  181.568545] pci_bus 0000:24: busn_res: [bus 24] end is updated to 24
[  181.568558] pci_bus 0000:23: busn_res: [bus 23-24] end is updated to 24
[  181.574379] pci 0000:04:00.0: BAR 14: assigned [mem 0xe4000000-0xe87fffff]
[  181.574390] pci 0000:05:19.0: BAR 14: assigned [mem 0xe4000000-0xe87fffff]
[  181.574398] pci 0000:06:00.0: BAR 2: assigned [mem 0xe4000000-0xe7ffffff 64bit]
[  181.574429] pci 0000:06:00.0: BAR 0: assigned [mem 0xe8000000-0xe87fffff]
[  181.574441] pci 0000:05:19.0: PCI bridge to [bus 06-12]
[  181.574456] pci 0000:05:19.0:   bridge window [mem 0xe4000000-0xe87fffff]
[  181.574482] pci 0000:04:00.0: PCI bridge to [bus 05-12]
[  181.574496] pci 0000:04:00.0:   bridge window [mem 0xe4000000-0xe87fffff]
[  181.574528] pci 0000:23:00.0: PCI bridge to [bus 24]

...

Quoted 5 lines

Thanks
Yinghai

Signature · 2 lines

Thanks!
Yijing

Bjorn Helgaas

maintainer

2015-05-05 19:07 UTC #7 lore raw

In reply to Yinghai Lu

Quoted 36 lines · Yinghai Lu

Quoted 10 lines · Yijing Wang
Quoted 6 lines · Yinghai Lu
Quoted 5 lines · Yijing Wang
...
I compared above log and found after we did remove and rescan, the bridge requested resource size extended to 0x06000000, and when system boot up, it requested only 0x4800000.
I tested it, remove and rescan 05:19.0 device is ok now, but
if do the operations for the parent device of 05:19.0, the result is still fail.
Found the problem, attached patch should fix the problem.
Thanks
Yinghai
Subject: [PATCH] PCI: Optimize bus mem sizing to small size
Current code try to get min_align as possible and use that to align final size.
That could cause generate wrong align/size or too big size in some case.
when we have align/size: 16M/64M
min_align/size0 will be 8M/64M, that is wrong, align must be 16M.
when we have align/size: 1M/1M, 64M/64M,
min_align/size0 will be 32M/96M, that is way too big for sum size 65M.
That will cuase allocation fails.
The patch introduce max_align/size0_max, and size0_max is just sum of all children resource.

Quoted 6 lines

Prefer small size instead of small align. Only use min_align when two size is the same.
The new size will only need to be aligned to bus window alignment.
Signed-off-by Yinghai Lu <[email protected]>

I'm sorry, I can't make any sense out of this. I can't understand what you're trying to say.

A specific simple example might help.

Is this a regression? Should it be marked for stable? If so, how far back?

It doesn't apply on v4.1-rc2 (I would apply it manually if I could understand the changelog and comments, but you might as well refresh it at the same time as you rewrite those).

Please include a link to the problem report.

Please include the patch inline in the message. It is a significant hassle for me to deal with attachments, and you are the only major contributor who uses them.

Bjorn

Quoted 95 lines

---
 drivers/pci/setup-bus.c |   50 +++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 47 insertions(+), 3 deletions(-)

Index: linux-2.6/drivers/pci/setup-bus.c
===================================================================

drivers/pci/setup-bus.c +47 -3

@@ -882,12 +882,14 @@ static void pbus_size_io(struct pci_bus
 	}
 
 	size0 = calculate_iosize(size, min_size, size1,
-			resource_size(b_res), min_align);
+				 resource_size(b_res),
+				 window_alignment(bus, IORESOURCE_IO));
 	if (children_add_size > add_size)
 		add_size = children_add_size;
 	size1 = (!realloc_head || (realloc_head && !add_size)) ? size0 :
 		calculate_iosize(size, min_size, add_size + size1,
-			resource_size(b_res), min_align);
+				 resource_size(b_res),
+				 window_alignment(bus, IORESOURCE_IO));
 	if (!size0 && !size1) {
 		if (b_res->start || b_res->end)
 			dev_info(&bus->self->dev, "disabling bridge window %pR to %pR (unused)\n",

@@ -962,6 +964,8 @@ static int pbus_size_mem(struct pci_bus
 	struct resource *b_res = find_free_bus_resource(bus,
 					mask | IORESOURCE_PREFETCH, type);
 	resource_size_t children_add_size = 0;
+	resource_size_t max_align = 0, size0_max;
+	int count = 0;
 
 	if (!b_res)
 		return -ENOSPC;

@@ -1016,19 +1020,59 @@ static int pbus_size_mem(struct pci_bus
 			if (order > max_order)
 				max_order = order;
 
+			count++;
+			if (align > max_align)
+				max_align = align;
+
 			if (realloc_head)
 				children_add_size += get_res_add_size(realloc_head, r);
 		}
 	}
 
+	/*
+	 * New rule: Prefer to small size instead of small align,
+	 * when we have align/size: 1M/1M, 2M/2M,
+	 *  min_align/size0: 1M/3M, max_align/size0_max: 2M/3M
+	 *  pick 1M/3M.
+	 * when we have align/size: 1M/1M, 64M/64M,
+	 *  min_align/size0: 32M/96M, max_align/size0_max: 64M/65M
+	 *  pick 64M/65M.
+	 * when we have align/size: 1M/1M, 16M/64M,
+	 *  min_align/size0: 8M/72M, max_align/size0_max: 16M/65M
+	 *  pick 16M/65M.
+	 * when we have align/size: 32M/64M, 128M/512M
+	 *  min_align/size0: 64M/576M, max_align/size0_max: 128M/576M
+	 *  pick 64M/576M.
+	 * when we have align/size: 16M/32M, 128M/512M
+	 *  min_align/size0: 64M/576M, max_align/size0_max: 128M/554M
+	 *  pick 128M/554M.
+	 * when we have align/size: 16M/64M
+	 *  min_align/size0: 8M/64M, max_align/size0_max: 16M/64M
+	 *  have to use 16M/64M.
+	 */
 	min_align = calculate_mem_align(aligns, max_order);
 	min_align = max(min_align, window_alignment(bus, b_res->flags));
+	max_align = max(max_align, window_alignment(bus, b_res->flags));
+	if (count == 1)
+		min_align = max_align;
+
 	size0 = calculate_memsize(size, min_size, 0, resource_size(b_res), min_align);
+	size0_max = calculate_memsize(size, min_size, 0, resource_size(b_res),
+					window_alignment(bus, b_res->flags));
+
+	if (size0_max < size0) {
+		size0 = size0_max;
+		min_align = max_align;
+		max_align--; /* to use small align for size1 calculation */
+	}
+
 	if (children_add_size > add_size)
 		add_size = children_add_size;
 	size1 = (!realloc_head || (realloc_head && !add_size)) ? size0 :
 		calculate_memsize(size, min_size, add_size,
-				resource_size(b_res), min_align);
+				resource_size(b_res),
+				min_align <= max_align ? min_align :
+					window_alignment(bus, b_res->flags));
 	if (!size0 && !size1) {
 		if (b_res->start || b_res->end)
 			dev_info(&bus->self->dev, "disabling bridge window %pR to %pR (unused)\n",

Yijing Wang

author

2015-06-25 01:59 UTC #8 lore raw

Quoted 37 lines

Quoted 18 lines
Subject: [PATCH] PCI: Optimize bus mem sizing to small size
Current code try to get min_align as possible and use that to align final size.
That could cause generate wrong align/size or too big size in some case.
when we have align/size: 16M/64M
min_align/size0 will be 8M/64M, that is wrong, align must be 16M.
when we have align/size: 1M/1M, 64M/64M,
min_align/size0 will be 32M/96M, that is way too big for sum size 65M.
That will cuase allocation fails.
The patch introduce max_align/size0_max, and size0_max is just sum of all children resource.
Quoted 6 lines
Prefer small size instead of small align. Only use min_align when two size is the same.
The new size will only need to be aligned to bus window alignment.
Signed-off-by Yinghai Lu <[email protected]>
I'm sorry, I can't make any sense out of this. I can't understand what you're trying to say.
A specific simple example might help.
Is this a regression? Should it be marked for stable? If so, how far back?
It doesn't apply on v4.1-rc2 (I would apply it manually if I could understand the changelog and comments, but you might as well refresh it at the same time as you rewrite those).

Quoted 5 lines

Please include a link to the problem report.
Please include the patch inline in the message. It is a significant hassle for me to deal with attachments, and you are the only major contributor who uses them.

Hi Yinghai, do you have some updates for this patch ?
I open a bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=100451

I hope this fix could be merged in upstream.

Thanks!
Yijing.

Quoted 102 lines

Bjorn

Quoted 85 lines

---
 drivers/pci/setup-bus.c |   50 +++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 47 insertions(+), 3 deletions(-)

Index: linux-2.6/drivers/pci/setup-bus.c
===================================================================

drivers/pci/setup-bus.c +44 -2

@@ -882,12 +882,14 @@ static void pbus_size_io(struct pci_bus
 	}
 
 	size0 = calculate_iosize(size, min_size, size1,
-			resource_size(b_res), min_align);
+				 resource_size(b_res),
+				 window_alignment(bus, IORESOURCE_IO));
 	if (children_add_size > add_size)
 		add_size = children_add_size;
 	size1 = (!realloc_head || (realloc_head && !add_size)) ? size0 :
 		calculate_iosize(size, min_size, add_size + size1,
-			resource_size(b_res), min_align);
+				 resource_size(b_res),
+				 window_alignment(bus, IORESOURCE_IO));
 	if (!size0 && !size1) {
 		if (b_res->start || b_res->end)
 			dev_info(&bus->self->dev, "disabling bridge window %pR to %pR (unused)\n",

@@ -962,6 +964,8 @@ static int pbus_size_mem(struct pci_bus
 	struct resource *b_res = find_free_bus_resource(bus,
 					mask | IORESOURCE_PREFETCH, type);
 	resource_size_t children_add_size = 0;
+	resource_size_t max_align = 0, size0_max;
+	int count = 0;
 
 	if (!b_res)
 		return -ENOSPC;

@@ -1016,19 +1020,59 @@ static int pbus_size_mem(struct pci_bus
 			if (order > max_order)
 				max_order = order;
 
+			count++;
+			if (align > max_align)
+				max_align = align;
+
 			if (realloc_head)
 				children_add_size += get_res_add_size(realloc_head, r);
 		}
 	}
 
+	/*
+	 * New rule: Prefer to small size instead of small align,
+	 * when we have align/size: 1M/1M, 2M/2M,
+	 *  min_align/size0: 1M/3M, max_align/size0_max: 2M/3M
+	 *  pick 1M/3M.
+	 * when we have align/size: 1M/1M, 64M/64M,
+	 *  min_align/size0: 32M/96M, max_align/size0_max: 64M/65M
+	 *  pick 64M/65M.
+	 * when we have align/size: 1M/1M, 16M/64M,
+	 *  min_align/size0: 8M/72M, max_align/size0_max: 16M/65M
+	 *  pick 16M/65M.
+	 * when we have align/size: 32M/64M, 128M/512M
+	 *  min_align/size0: 64M/576M, max_align/size0_max: 128M/576M
+	 *  pick 64M/576M.
+	 * when we have align/size: 16M/32M, 128M/512M
+	 *  min_align/size0: 64M/576M, max_align/size0_max: 128M/554M
+	 *  pick 128M/554M.
+	 * when we have align/size: 16M/64M
+	 *  min_align/size0: 8M/64M, max_align/size0_max: 16M/64M
+	 *  have to use 16M/64M.
+	 */
 	min_align = calculate_mem_align(aligns, max_order);
 	min_align = max(min_align, window_alignment(bus, b_res->flags));
+	max_align = max(max_align, window_alignment(bus, b_res->flags));
+	if (count == 1)
+		min_align = max_align;
+
 	size0 = calculate_memsize(size, min_size, 0, resource_size(b_res), min_align);
+	size0_max = calculate_memsize(size, min_size, 0, resource_size(b_res),
+					window_alignment(bus, b_res->flags));
+
+	if (size0_max < size0) {
+		size0 = size0_max;
+		min_align = max_align;
+		max_align--; /* to use small align for size1 calculation */
+	}
+
 	if (children_add_size > add_size)

drivers/pci/setup-bus.c static int pbus_size_mem(struct pci_bus · line 1078

Quoted 10 lines

drivers/pci/setup-bus.c +3 -1

 		add_size = children_add_size;
 	size1 = (!realloc_head || (realloc_head && !add_size)) ? size0 :
 		calculate_memsize(size, min_size, add_size,
-				resource_size(b_res), min_align);
+				resource_size(b_res),
+				min_align <= max_align ? min_align :
+					window_alignment(bus, b_res->flags));
 	if (!size0 && !size1) {
 		if (b_res->start || b_res->end)
 			dev_info(&bus->self->dev, "disabling bridge window %pR to %pR (unused)\n",

Signature · 2 lines

Thanks!
Yijing

Yinghai Lu

2015-06-25 07:05 UTC #9 lore raw

Quoted 5 lines · Yijing Wang

Hi Yinghai, do you have some updates for this patch ?
I open a bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=100451
I hope this fix could be merged in upstream.

Found problem with that patch on system with several bridges.

I am reworking allocation code.

Please check if you can test

git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git
for-pci-v4.2-rc1

on the setup.

It includes patches that support more smart bus align calculation and alt_align etc.

Thanks

Yinghai

Yijing Wang

author

2015-06-25 07:25 UTC #10 lore raw

Quoted 13 lines · Yinghai Lu

Quoted 5 lines · Yijing Wang
Hi Yinghai, do you have some updates for this patch ?
I open a bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=100451
I hope this fix could be merged in upstream.
Found problem with that patch on system with several bridges.
I am reworking allocation code.
Please check if you can test

Quoted 7 lines

git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git
for-pci-v4.2-rc1
on the setup.
It includes patches that support more smart bus align calculation and alt_align etc.

OK, I will look at it and try to test in local machine.

Thanks!
Yijing.

Quoted 6 lines

Thanks
Yinghai

Signature · 2 lines

Thanks!
Yijing