arm64: fix MAX_ORDER for 64K pagesize

PATCH In review Mark Salter 4 participants 14 messages Jun 11, 2014 – Jun 23, 2014

Subsystems (3)

Memory management arm64 Page allocator

lore

Share & tools

lore

Mark Salter

author

2014-06-11 21:33 UTC #0 lore raw

PATCH 1/1 arm64: fix MAX_ORDER for 64K pagesize

With a kernel configured with ARM64_64K_PAGES && !TRANSPARENT_HUGEPAGE I get this at early boot:

SMP: Total of 8 processors activated.
devtmpfs: initialized

  Unable to handle kernel NULL pointer dereference at virtual address 00000008
  pgd = fffffe0000050000
  [00000008] *pgd=00000043fba00003, *pmd=00000043fba00003, *pte=00e0000078010407
  Internal error: Oops: 96000006 [#1] SMP
  Modules linked in:
  CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.15.0-rc864k+ #44
  task: fffffe03bc040000 ti: fffffe03bc080000 task.ti: fffffe03bc080000
  PC is at __list_add+0x10/0xd4
  LR is at free_one_page+0x270/0x638
  ...
  Call trace:
  [<fffffe00003ee970>] __list_add+0x10/0xd4
  [<fffffe000019c478>] free_one_page+0x26c/0x638
  [<fffffe000019c8c8>] __free_pages_ok.part.52+0x84/0xbc
  [<fffffe000019d5e8>] __free_pages+0x74/0xbc
  [<fffffe0000c01350>] init_cma_reserved_pageblock+0xe8/0x104
  [<fffffe0000c24de0>] cma_init_reserved_areas+0x190/0x1e4
  [<fffffe0000090418>] do_one_initcall+0xc4/0x154
  [<fffffe0000bf0a50>] kernel_init_freeable+0x204/0x2a8
  [<fffffe00007520a0>] kernel_init+0xc/0xd4

This happens in this configuration because __free_one_page() is called with an order greater than MAX_ORDER, accesses past zone->free_list[] and passes a bogus list_head to list_add().

arch/arm64/Kconfig has:

config FORCE_MAX_ZONEORDER

	int
	default "14" if (ARM64_64K_PAGES && TRANSPARENT_HUGEPAGE)
	default "11"

So with THP turned off MAX_ORDER == 11 but init_cma_reserved_pageblock() passes __free_pages() an order of pageblock_order which is based on (HPAGE_SHIFT - PAGE_SHIFT) which is 13 for 64K pages. I worked around this by removing the THP test so FORCE_MAX_ZONEORDER is always 14 for ARM64_64K_PAGES.

Signed-off-by Mark Salter <[email protected]>

---
 arch/arm64/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/arm64/Kconfig +1 -1

index 7295419..42a334e 100644
@@ -269,7 +269,7 @@ config XEN
 
 config FORCE_MAX_ZONEORDER
 	int
-	default "14" if (ARM64_64K_PAGES && TRANSPARENT_HUGEPAGE)
+	default "14" if ARM64_64K_PAGES
 	default "11"
 
 endmenu

Signature · 1 line

1.9.0

David Rientjes

2014-06-11 23:03 UTC #1 lore raw

Quoted 53 lines · Mark Salter

With a kernel configured with ARM64_64K_PAGES && !TRANSPARENT_HUGEPAGE I get this at early boot:
SMP: Total of 8 processors activated.
devtmpfs: initialized
  Unable to handle kernel NULL pointer dereference at virtual address 00000008
  pgd = fffffe0000050000
  [00000008] *pgd=00000043fba00003, *pmd=00000043fba00003, *pte=00e0000078010407
  Internal error: Oops: 96000006 [#1] SMP
  Modules linked in:
  CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.15.0-rc864k+ #44
  task: fffffe03bc040000 ti: fffffe03bc080000 task.ti: fffffe03bc080000
  PC is at __list_add+0x10/0xd4
  LR is at free_one_page+0x270/0x638
  ...
  Call trace:
  [<fffffe00003ee970>] __list_add+0x10/0xd4
  [<fffffe000019c478>] free_one_page+0x26c/0x638
  [<fffffe000019c8c8>] __free_pages_ok.part.52+0x84/0xbc
  [<fffffe000019d5e8>] __free_pages+0x74/0xbc
  [<fffffe0000c01350>] init_cma_reserved_pageblock+0xe8/0x104
  [<fffffe0000c24de0>] cma_init_reserved_areas+0x190/0x1e4
  [<fffffe0000090418>] do_one_initcall+0xc4/0x154
  [<fffffe0000bf0a50>] kernel_init_freeable+0x204/0x2a8
  [<fffffe00007520a0>] kernel_init+0xc/0xd4
This happens in this configuration because __free_one_page() is called with an order greater than MAX_ORDER, accesses past zone->free_list[] and passes a bogus list_head to list_add().
arch/arm64/Kconfig has:
config FORCE_MAX_ZONEORDER
	int
	default "14" if (ARM64_64K_PAGES && TRANSPARENT_HUGEPAGE)
	default "11"
So with THP turned off MAX_ORDER == 11 but init_cma_reserved_pageblock() passes __free_pages() an order of pageblock_order which is based on (HPAGE_SHIFT - PAGE_SHIFT) which is 13 for 64K pages. I worked around this by removing the THP test so FORCE_MAX_ZONEORDER is always 14 for ARM64_64K_PAGES.
Signed-off-by Mark Salter <[email protected]>
---
 arch/arm64/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/Kconfig +0 -0
index 7295419..42a334e 100644

arch/arm64/Kconfig config XEN · line 275

Quoted 8 lines

arch/arm64/Kconfig +1 -1

@@ -269,7 +269,7 @@ config XEN
 
 config FORCE_MAX_ZONEORDER
 	int
-	default "14" if (ARM64_64K_PAGES && TRANSPARENT_HUGEPAGE)
+	default "14" if ARM64_64K_PAGES
 	default "11"
 
 endmenu

Any reason to not switch this to

	ARM64_64K_PAGES && TRANSPARENT_HUGEPAGE && CMA

instead? If pageblock_order > MAX_ORDER because of HPAGE_SHIFT > PAGE_SHIFT, then cma is always going to be passing a too-large-order to free_pages_prepare() via this path.

Adding Michal and Marek to the cc.

David Rientjes

2014-06-11 23:04 UTC #2 lore raw

Quoted 4 lines · David Rientjes

Any reason to not switch this to
	ARM64_64K_PAGES && TRANSPARENT_HUGEPAGE && CMA

(ARM64_64K_PAGES) && (TRANSPARENT_HUGEPAGE || CMA)

Quoted 6 lines

instead? If pageblock_order > MAX_ORDER because of HPAGE_SHIFT > PAGE_SHIFT, then cma is always going to be passing a too-large-order to free_pages_prepare() via this path.
Adding Michal and Marek to the cc.

Mark Salter

author

2014-06-12 13:57 UTC #3 lore raw

Quoted 8 lines · David Rientjes

Quoted 4 lines · David Rientjes
Any reason to not switch this to
	ARM64_64K_PAGES && TRANSPARENT_HUGEPAGE && CMA
(ARM64_64K_PAGES) && (TRANSPARENT_HUGEPAGE || CMA)

And add HUGETLB to the list also? I'm not sure of all the trade offs here, so I kept it simple. I don't have a strong opinion one way or the other.

Michal Nazarewicz

2014-06-17 18:32 UTC #4 lore raw

In reply to David Rientjes

Quoted 66 lines · David Rientjes

Quoted 53 lines · Mark Salter
With a kernel configured with ARM64_64K_PAGES && !TRANSPARENT_HUGEPAGE I get this at early boot:
SMP: Total of 8 processors activated.
devtmpfs: initialized
  Unable to handle kernel NULL pointer dereference at virtual address 00000008
  pgd = fffffe0000050000
  [00000008] *pgd=00000043fba00003, *pmd=00000043fba00003, *pte=00e0000078010407
  Internal error: Oops: 96000006 [#1] SMP
  Modules linked in:
  CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.15.0-rc864k+ #44
  task: fffffe03bc040000 ti: fffffe03bc080000 task.ti: fffffe03bc080000
  PC is at __list_add+0x10/0xd4
  LR is at free_one_page+0x270/0x638
  ...
  Call trace:
  [<fffffe00003ee970>] __list_add+0x10/0xd4
  [<fffffe000019c478>] free_one_page+0x26c/0x638
  [<fffffe000019c8c8>] __free_pages_ok.part.52+0x84/0xbc
  [<fffffe000019d5e8>] __free_pages+0x74/0xbc
  [<fffffe0000c01350>] init_cma_reserved_pageblock+0xe8/0x104
  [<fffffe0000c24de0>] cma_init_reserved_areas+0x190/0x1e4
  [<fffffe0000090418>] do_one_initcall+0xc4/0x154
  [<fffffe0000bf0a50>] kernel_init_freeable+0x204/0x2a8
  [<fffffe00007520a0>] kernel_init+0xc/0xd4
This happens in this configuration because __free_one_page() is called with an order greater than MAX_ORDER, accesses past zone->free_list[] and passes a bogus list_head to list_add().
arch/arm64/Kconfig has:
config FORCE_MAX_ZONEORDER
	int
	default "14" if (ARM64_64K_PAGES && TRANSPARENT_HUGEPAGE)
	default "11"
So with THP turned off MAX_ORDER == 11 but init_cma_reserved_pageblock() passes __free_pages() an order of pageblock_order which is based on (HPAGE_SHIFT - PAGE_SHIFT) which is 13 for 64K pages. I worked around this by removing the THP test so FORCE_MAX_ZONEORDER is always 14 for ARM64_64K_PAGES.
Signed-off-by Mark Salter <[email protected]>
---
 arch/arm64/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/Kconfig +0 -0
index 7295419..42a334e 100644
arch/arm64/Kconfig config XEN · line 275
Quoted 8 lines
arch/arm64/Kconfig +1 -1
@@ -269,7 +269,7 @@ config XEN
 
 config FORCE_MAX_ZONEORDER
 	int
-	default "14" if (ARM64_64K_PAGES && TRANSPARENT_HUGEPAGE)
+	default "14" if ARM64_64K_PAGES
 	default "11"
 
 endmenu
Any reason to not switch this to

Quoted 7 lines

	ARM64_64K_PAGES && TRANSPARENT_HUGEPAGE && CMA
instead? If pageblock_order > MAX_ORDER because of HPAGE_SHIFT > PAGE_SHIFT, then cma is always going to be passing a too-large-order to free_pages_prepare() via this path.
Adding Michal and Marek to the cc.

The correct fix would be to change init_cma_reserved_pageblock such that it checks whether pageblock_order > MAX_ORDER and if so frees each max order page of the pageblock individually:

--------- >8 ---------------------------------------------------------
From: Michal Nazarewicz <[email protected]>
Subject: [PATCH] mm: cma: fix cases where pageblock is bigger then MAX_ORDER

With a kernel configured with ARM64_64K_PAGES && !TRANSPARENT_HUGEPAGE, the following is triggered at early boot:

SMP: Total of 8 processors activated.
devtmpfs: initialized

  Unable to handle kernel NULL pointer dereference at virtual address 00000008
  pgd = fffffe0000050000
  [00000008] *pgd=00000043fba00003, *pmd=00000043fba00003, *pte=00e0000078010407
  Internal error: Oops: 96000006 [#1] SMP
  Modules linked in:
  CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.15.0-rc864k+ #44
  task: fffffe03bc040000 ti: fffffe03bc080000 task.ti: fffffe03bc080000
  PC is at __list_add+0x10/0xd4
  LR is at free_one_page+0x270/0x638
  ...
  Call trace:
  [<fffffe00003ee970>] __list_add+0x10/0xd4
  [<fffffe000019c478>] free_one_page+0x26c/0x638
  [<fffffe000019c8c8>] __free_pages_ok.part.52+0x84/0xbc
  [<fffffe000019d5e8>] __free_pages+0x74/0xbc
  [<fffffe0000c01350>] init_cma_reserved_pageblock+0xe8/0x104
  [<fffffe0000c24de0>] cma_init_reserved_areas+0x190/0x1e4
  [<fffffe0000090418>] do_one_initcall+0xc4/0x154
  [<fffffe0000bf0a50>] kernel_init_freeable+0x204/0x2a8
  [<fffffe00007520a0>] kernel_init+0xc/0xd4

This happens in this configuration because __free_one_page() is called with an order greater than MAX_ORDER, accesses past zone->free_list[] and passes a bogus list_head to list_add().

arch/arm64/Kconfig has:

config FORCE_MAX_ZONEORDER

	int
	default "14" if (ARM64_64K_PAGES && TRANSPARENT_HUGEPAGE)
	default "11"

Fix the problem by changing init_cma_reserved_pageblock() such that it splits pageblock into individual MAX_ORDER pages if pageblock is bigger than a MAX_ORDER page.

Signed-off-by Michal Nazarewicz <[email protected]>

Reported-by Mark Salter <[email protected]>

---
 mm/page_alloc.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

mm/page_alloc.c +9 -1

index 5dba293..6e657ce 100644
@@ -801,7 +801,15 @@ void __init init_cma_reserved_pageblock(struct page *page)
 
 	set_page_refcounted(page);
 	set_pageblock_migratetype(page, MIGRATE_CMA);
-	__free_pages(page, pageblock_order);
+	if (pageblock_order > MAX_ORDER) {
+		struct page *subpage = p;
+		unsigned count = 1 << (pageblock_order - MAX_ORDER);
+		do {
+			__free_pages(subpage, pageblock_order);
+		} while (subpage += MAX_ORDER_NR_PAGES, --count);
+	} else {
+		__free_pages(page, pageblock_order);
+	}
 	adjust_managed_page_count(page, pageblock_nr_pages);
 }
 #endif

--------- >8 ---------------------------------------------------------

Thoughts? This has not been tested and I think it may cause performance degradation in some cases since pageblock_order is not always a constant, so the comparison may end up not being stripped away even on systems where it's always false.

Signature · 4 lines

Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Micha? ?mina86? Nazarewicz    (o o)
ooo +--<[email protected]>--<xmpp:[email protected]>--ooO--(_)--Ooo--

Mark Salter

author

2014-06-19 18:12 UTC #5 lore raw

Quoted 139 lines · Michal Nazarewicz

Quoted 66 lines · David Rientjes
Quoted 53 lines · Mark Salter
With a kernel configured with ARM64_64K_PAGES && !TRANSPARENT_HUGEPAGE I get this at early boot:
SMP: Total of 8 processors activated.
devtmpfs: initialized
  Unable to handle kernel NULL pointer dereference at virtual address 00000008
  pgd = fffffe0000050000
  [00000008] *pgd=00000043fba00003, *pmd=00000043fba00003, *pte=00e0000078010407
  Internal error: Oops: 96000006 [#1] SMP
  Modules linked in:
  CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.15.0-rc864k+ #44
  task: fffffe03bc040000 ti: fffffe03bc080000 task.ti: fffffe03bc080000
  PC is at __list_add+0x10/0xd4
  LR is at free_one_page+0x270/0x638
  ...
  Call trace:
  [<fffffe00003ee970>] __list_add+0x10/0xd4
  [<fffffe000019c478>] free_one_page+0x26c/0x638
  [<fffffe000019c8c8>] __free_pages_ok.part.52+0x84/0xbc
  [<fffffe000019d5e8>] __free_pages+0x74/0xbc
  [<fffffe0000c01350>] init_cma_reserved_pageblock+0xe8/0x104
  [<fffffe0000c24de0>] cma_init_reserved_areas+0x190/0x1e4
  [<fffffe0000090418>] do_one_initcall+0xc4/0x154
  [<fffffe0000bf0a50>] kernel_init_freeable+0x204/0x2a8
  [<fffffe00007520a0>] kernel_init+0xc/0xd4
This happens in this configuration because __free_one_page() is called with an order greater than MAX_ORDER, accesses past zone->free_list[] and passes a bogus list_head to list_add().
arch/arm64/Kconfig has:
config FORCE_MAX_ZONEORDER
	int
	default "14" if (ARM64_64K_PAGES && TRANSPARENT_HUGEPAGE)
	default "11"
So with THP turned off MAX_ORDER == 11 but init_cma_reserved_pageblock() passes __free_pages() an order of pageblock_order which is based on (HPAGE_SHIFT - PAGE_SHIFT) which is 13 for 64K pages. I worked around this by removing the THP test so FORCE_MAX_ZONEORDER is always 14 for ARM64_64K_PAGES.
Signed-off-by Mark Salter <[email protected]>
---
 arch/arm64/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/Kconfig +0 -0
index 7295419..42a334e 100644
arch/arm64/Kconfig config XEN · line 275
Quoted 8 lines
arch/arm64/Kconfig +1 -1
@@ -269,7 +269,7 @@ config XEN
 
 config FORCE_MAX_ZONEORDER
 	int
-	default "14" if (ARM64_64K_PAGES && TRANSPARENT_HUGEPAGE)
+	default "14" if ARM64_64K_PAGES
 	default "11"
 
 endmenu
Any reason to not switch this to
Quoted 7 lines
	ARM64_64K_PAGES && TRANSPARENT_HUGEPAGE && CMA
instead? If pageblock_order > MAX_ORDER because of HPAGE_SHIFT > PAGE_SHIFT, then cma is always going to be passing a too-large-order to free_pages_prepare() via this path.
Adding Michal and Marek to the cc.
The correct fix would be to change init_cma_reserved_pageblock such that it checks whether pageblock_order > MAX_ORDER and if so frees each max order page of the pageblock individually:
--------- >8 ---------------------------------------------------------
From: Michal Nazarewicz <[email protected]>
Subject: [PATCH] mm: cma: fix cases where pageblock is bigger then MAX_ORDER
With a kernel configured with ARM64_64K_PAGES && !TRANSPARENT_HUGEPAGE, the following is triggered at early boot:
SMP: Total of 8 processors activated.
devtmpfs: initialized
  Unable to handle kernel NULL pointer dereference at virtual address 00000008
  pgd = fffffe0000050000
  [00000008] *pgd=00000043fba00003, *pmd=00000043fba00003, *pte=00e0000078010407
  Internal error: Oops: 96000006 [#1] SMP
  Modules linked in:
  CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.15.0-rc864k+ #44
  task: fffffe03bc040000 ti: fffffe03bc080000 task.ti: fffffe03bc080000
  PC is at __list_add+0x10/0xd4
  LR is at free_one_page+0x270/0x638
  ...
  Call trace:
  [<fffffe00003ee970>] __list_add+0x10/0xd4
  [<fffffe000019c478>] free_one_page+0x26c/0x638
  [<fffffe000019c8c8>] __free_pages_ok.part.52+0x84/0xbc
  [<fffffe000019d5e8>] __free_pages+0x74/0xbc
  [<fffffe0000c01350>] init_cma_reserved_pageblock+0xe8/0x104
  [<fffffe0000c24de0>] cma_init_reserved_areas+0x190/0x1e4
  [<fffffe0000090418>] do_one_initcall+0xc4/0x154
  [<fffffe0000bf0a50>] kernel_init_freeable+0x204/0x2a8
  [<fffffe00007520a0>] kernel_init+0xc/0xd4
This happens in this configuration because __free_one_page() is called with an order greater than MAX_ORDER, accesses past zone->free_list[] and passes a bogus list_head to list_add().
arch/arm64/Kconfig has:
config FORCE_MAX_ZONEORDER
	int
	default "14" if (ARM64_64K_PAGES && TRANSPARENT_HUGEPAGE)
	default "11"
So with THP turned off MAX_ORDER == 11 but init_cma_reserved_pageblock() passes __free_pages() an order of pageblock_order which is based on (HPAGE_SHIFT - PAGE_SHIFT) which is 13 for 64K pages.
Fix the problem by changing init_cma_reserved_pageblock() such that it splits pageblock into individual MAX_ORDER pages if pageblock is bigger than a MAX_ORDER page.
Signed-off-by Michal Nazarewicz <[email protected]>
Reported-by Mark Salter <[email protected]>
---
 mm/page_alloc.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)
mm/page_alloc.c +0 -0
index 5dba293..6e657ce 100644

mm/page_alloc.c void __init init_cma_reserved_pageblock(struct page *page) · line 808

Quoted 9 lines

mm/page_alloc.c +5 -1

@@ -801,7 +801,15 @@ void __init init_cma_reserved_pageblock(struct page *page)
 
 	set_page_refcounted(page);
 	set_pageblock_migratetype(page, MIGRATE_CMA);
-	__free_pages(page, pageblock_order);
+	if (pageblock_order > MAX_ORDER) {
+		struct page *subpage = p;
+		unsigned count = 1 << (pageblock_order - MAX_ORDER);
+		do {
+			__free_pages(subpage, pageblock_order);

                                               ^^^^^^^
                                               MAX_ORDER

Quoted 9 lines

mm/page_alloc.c +4 -1

+		} while (subpage += MAX_ORDER_NR_PAGES, --count);
+	} else {
+		__free_pages(page, pageblock_order);
+	}
 	adjust_managed_page_count(page, pageblock_nr_pages);
 }
 #endif
--------- >8 ---------------------------------------------------------

Quoted 5 lines

Thoughts? This has not been tested and I think it may cause performance degradation in some cases since pageblock_order is not always a constant, so the comparison may end up not being stripped away even on systems where it's always false.

This works with the above tweak. So it fixes the problm here, but I was not sure if we'd get bitten elsewhere by pageblock_order > MAX_ORDER. It will be slower, but does it only gets called a few time at most at boot time, right?

Michal Nazarewicz

2014-06-19 19:24 UTC #6 lore raw

Quoted 7 lines · Mark Salter

Quoted 6 lines · Michal Nazarewicz
mm/page_alloc.c +0 -0
index 5dba293..6e657ce 100644
@@ -801,7 +801,15 @@ void __init init_cma_reserved_pageblock(struct page *page)
 

Quoted 10 lines

mm/page_alloc.c void __init init_cma_reserved_pageblock(struct page *page) · line 808

Quoted 8 lines

mm/page_alloc.c +5 -1

 	set_page_refcounted(page);
 	set_pageblock_migratetype(page, MIGRATE_CMA);
-	__free_pages(page, pageblock_order);
+	if (pageblock_order > MAX_ORDER) {
+		struct page *subpage = p;
+		unsigned count = 1 << (pageblock_order - MAX_ORDER);
+		do {
+			__free_pages(subpage, pageblock_order);

                                               ^^^^^^^
                                               MAX_ORDER

D'oh! I'll send a revised patch.

Quoted 9 lines

mm/page_alloc.c +4 -1

+		} while (subpage += MAX_ORDER_NR_PAGES, --count);
+	} else {
+		__free_pages(page, pageblock_order);
+	}
 	adjust_managed_page_count(page, pageblock_nr_pages);
 }
 #endif
--------- >8 ---------------------------------------------------------

Quoted 7 lines

Quoted 4 lines
Thoughts? This has not been tested and I think it may cause performance degradation in some cases since pageblock_order is not always a constant, so the comparison may end up not being stripped away even on systems where it's always false.
This works with the above tweak. So it fixes the problm here, but I was not sure if we'd get bitten elsewhere by pageblock_order > MAX_ORDER.

This is always a possibility, but in such cases, it's a bug in CMA. I've tried to keep in mind that pageblock_order may be greater than MAX_ORDER when writing CMA, but I've never tested on such a system.

Quoted 2 lines

It will be slower, but does it only gets called a few time at most at boot time, right?

Yes. The performance degradation should be negligible since init_cma_reserved is hardly a critical path and is called at most MAX_CMA_AREAS times which by default is 8. And I mean it will be slower because it will have to perform a branch.

Signature · 4 lines

Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Micha? ?mina86? Nazarewicz    (o o)
ooo +--<[email protected]>--<xmpp:[email protected]>--ooO--(_)--Ooo--

Mark Salter

author

2014-06-20 17:37 UTC #7 lore raw

Quoted 42 lines · Michal Nazarewicz

Quoted 7 lines · Mark Salter
Quoted 6 lines · Michal Nazarewicz
mm/page_alloc.c +0 -0
index 5dba293..6e657ce 100644
@@ -801,7 +801,15 @@ void __init init_cma_reserved_pageblock(struct page *page)
 
Quoted 10 lines
mm/page_alloc.c void __init init_cma_reserved_pageblock(struct page *page) · line 808
Quoted 8 lines
mm/page_alloc.c +5 -1
 	set_page_refcounted(page);
 	set_pageblock_migratetype(page, MIGRATE_CMA);
-	__free_pages(page, pageblock_order);
+	if (pageblock_order > MAX_ORDER) {
+		struct page *subpage = p;
+		unsigned count = 1 << (pageblock_order - MAX_ORDER);
+		do {
+			__free_pages(subpage, pageblock_order);
                                               ^^^^^^^
                                               MAX_ORDER
D'oh! I'll send a revised patch.
Quoted 9 lines
Quoted 9 lines
mm/page_alloc.c +4 -1
+		} while (subpage += MAX_ORDER_NR_PAGES, --count);
+	} else {
+		__free_pages(page, pageblock_order);
+	}
 	adjust_managed_page_count(page, pageblock_nr_pages);
 }
 #endif
--------- >8 ---------------------------------------------------------
Quoted 7 lines
Quoted 4 lines
Thoughts? This has not been tested and I think it may cause performance degradation in some cases since pageblock_order is not always a constant, so the comparison may end up not being stripped away even on systems where it's always false.
This works with the above tweak. So it fixes the problm here, but I was not sure if we'd get bitten elsewhere by pageblock_order > MAX_ORDER.
This is always a possibility, but in such cases, it's a bug in CMA. I've tried to keep in mind that pageblock_order may be greater than MAX_ORDER when writing CMA, but I've never tested on such a system.

Quoted 8 lines

Quoted 2 lines
It will be slower, but does it only gets called a few time at most at boot time, right?
Yes. The performance degradation should be negligible since init_cma_reserved is hardly a critical path and is called at most MAX_CMA_AREAS times which by default is 8. And I mean it will be slower because it will have to perform a branch.

I ended up needing this (on top of your patch) to get the system to boot. Each MAX_ORDER-1 group needs the refcount and migratetype set so that __free_pages does the right thing.

mm/page_alloc.c +7 -6

index 02fb1ed..a7ca6cc 100644
@@ -799,17 +799,18 @@ void __init init_cma_reserved_pageblock(struct page *page)
 		set_page_count(p, 0);
 	} while (++p, --i);
 
-	set_page_refcounted(page);
-	set_pageblock_migratetype(page, MIGRATE_CMA);
-
-	if (pageblock_order > MAX_ORDER) {
-		i = pageblock_order - MAX_ORDER;
+	if (pageblock_order >= MAX_ORDER) {
+		i = pageblock_order - MAX_ORDER + 1;
 		i = 1 << i;
 		p = page;
 		do {
-			__free_pages(p, MAX_ORDER);
+			set_page_refcounted(p);
+			set_pageblock_migratetype(p, MIGRATE_CMA);
+			__free_pages(p, MAX_ORDER - 1);
 		} while (p += MAX_ORDER_NR_PAGES, --i);
 	} else {
+		set_page_refcounted(page);
+		set_pageblock_migratetype(page, MIGRATE_CMA);
 		__free_pages(page, pageblock_order);
 	}

Michal Nazarewicz

2014-06-23 19:40 UTC #8 lore raw

PATCH v3 1/1 mm: page_alloc: fix CMA area initialisation when pageblock > MAX_ORDER

With a kernel configured with ARM64_64K_PAGES && !TRANSPARENT_HUGEPAGE, the following is triggered at early boot:

SMP: Total of 8 processors activated.
devtmpfs: initialized

  Unable to handle kernel NULL pointer dereference at virtual address 00000008
  pgd = fffffe0000050000
  [00000008] *pgd=00000043fba00003, *pmd=00000043fba00003, *pte=00e0000078010407
  Internal error: Oops: 96000006 [#1] SMP
  Modules linked in:
  CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.15.0-rc864k+ #44
  task: fffffe03bc040000 ti: fffffe03bc080000 task.ti: fffffe03bc080000
  PC is at __list_add+0x10/0xd4
  LR is at free_one_page+0x270/0x638
  ...
  Call trace:
  [<fffffe00003ee970>] __list_add+0x10/0xd4
  [<fffffe000019c478>] free_one_page+0x26c/0x638
  [<fffffe000019c8c8>] __free_pages_ok.part.52+0x84/0xbc
  [<fffffe000019d5e8>] __free_pages+0x74/0xbc
  [<fffffe0000c01350>] init_cma_reserved_pageblock+0xe8/0x104
  [<fffffe0000c24de0>] cma_init_reserved_areas+0x190/0x1e4
  [<fffffe0000090418>] do_one_initcall+0xc4/0x154
  [<fffffe0000bf0a50>] kernel_init_freeable+0x204/0x2a8
  [<fffffe00007520a0>] kernel_init+0xc/0xd4

This happens because init_cma_reserved_pageblock() calls __free_one_page() with pageblock_order as page order but it is bigger han MAX_ORDER. This in turn causes accesses past zone->free_list[].

Fix the problem by changing init_cma_reserved_pageblock() such that it splits pageblock into individual MAX_ORDER pages if pageblock is bigger than a MAX_ORDER page.

In cases where !CONFIG_HUGETLB_PAGE_SIZE_VARIABLE, which is all architectures expect for ia64, powerpc and tile at the moment, the ?pageblock_order > MAX_ORDER? condition will be optimised out since both sides of the operator are constants. In cases where pageblock size is variable, the performance degradation should not be significant anyway since init_cma_reserved_pageblock() is called only at boot time at most MAX_CMA_AREAS times which by default is eight.

Cc stable at vger.kernel.org

Signed-off-by Michal Nazarewicz <[email protected]>

Reported-by Mark Salter <[email protected]>

Tested-by Christopher Covington <[email protected]>

---
 mm/page_alloc.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

Quoted 22 lines · Mark Salter

I ended up needing this (on top of your patch) to get the system to boot. Each MAX_ORDER-1 group needs the refcount and migratetype set so that __free_pages does the right thing.

mm/page_alloc.c +2 -5

index 02fb1ed..a7ca6cc 100644
@@ -799,17 +799,18 @@ void __init init_cma_reserved_pageblock(struct page *page)
 		set_page_count(p, 0);
 	} while (++p, --i);
 
-	set_page_refcounted(page);
-	set_pageblock_migratetype(page, MIGRATE_CMA);
-
-	if (pageblock_order > MAX_ORDER) {
-		i = pageblock_order - MAX_ORDER;
+	if (pageblock_order >= MAX_ORDER) {
+		i = pageblock_order - MAX_ORDER + 1;
 		i = 1 << i;
 		p = page;
 		do {

mm/page_alloc.c void __init init_cma_reserved_pageblock(struct page *page) · line 815

Quoted 10 lines

mm/page_alloc.c +5 -1

-			__free_pages(p, MAX_ORDER);
+			set_page_refcounted(p);
+			set_pageblock_migratetype(p, MIGRATE_CMA);
+			__free_pages(p, MAX_ORDER - 1);
 		} while (p += MAX_ORDER_NR_PAGES, --i);
 	} else {
+		set_page_refcounted(page);
+		set_pageblock_migratetype(page, MIGRATE_CMA);
 		__free_pages(page, pageblock_order);
 	}

This is kinda embarrassing, dunno how I missed that.

But each page actually does not need to have migratetype set, does it? All of those pages are in a single pageblock so a single call suffices. If you track set_pageblock_migratetype down to pfn_to_bitidx there is:

	return (pfn >> pageblock_order) * NR_PAGEBLOCK_BITS;

so for pfns inside of a pageblock, they get truncated. Or did I miss yet another thing?

mm/page_alloc.c +14 -2

index ee92384..fef9614 100644
@@ -816,9 +816,21 @@ void __init init_cma_reserved_pageblock(struct page *page)
 		set_page_count(p, 0);
 	} while (++p, --i);
 
-	set_page_refcounted(page);
 	set_pageblock_migratetype(page, MIGRATE_CMA);
-	__free_pages(page, pageblock_order);
+
+	if (pageblock_order >= MAX_ORDER) {
+		i = pageblock_nr_pages;
+		p = page;
+		do {
+			set_page_refcounted(p);
+			__free_pages(p, MAX_ORDER - 1);
+			p += MAX_ORDER_NR_PAGES;
+		} while (i -= MAX_ORDER_NR_PAGES);
+	} else {
+		set_page_refcounted(page);
+		__free_pages(page, pageblock_order);
+	}
+
 	adjust_managed_page_count(page, pageblock_nr_pages);
 }
 #endif

Signature · 1 line

2.0.0.526.g5318336

Mark Salter

author

2014-06-23 21:10 UTC #9 lore raw

Re: [PATCHv3] mm: page_alloc: fix CMA area initialisation when pageblock > MAX_ORDER

Quoted 92 lines · Michal Nazarewicz

With a kernel configured with ARM64_64K_PAGES && !TRANSPARENT_HUGEPAGE, the following is triggered at early boot:
SMP: Total of 8 processors activated.
devtmpfs: initialized
  Unable to handle kernel NULL pointer dereference at virtual address 00000008
  pgd = fffffe0000050000
  [00000008] *pgd=00000043fba00003, *pmd=00000043fba00003, *pte=00e0000078010407
  Internal error: Oops: 96000006 [#1] SMP
  Modules linked in:
  CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.15.0-rc864k+ #44
  task: fffffe03bc040000 ti: fffffe03bc080000 task.ti: fffffe03bc080000
  PC is at __list_add+0x10/0xd4
  LR is at free_one_page+0x270/0x638
  ...
  Call trace:
  [<fffffe00003ee970>] __list_add+0x10/0xd4
  [<fffffe000019c478>] free_one_page+0x26c/0x638
  [<fffffe000019c8c8>] __free_pages_ok.part.52+0x84/0xbc
  [<fffffe000019d5e8>] __free_pages+0x74/0xbc
  [<fffffe0000c01350>] init_cma_reserved_pageblock+0xe8/0x104
  [<fffffe0000c24de0>] cma_init_reserved_areas+0x190/0x1e4
  [<fffffe0000090418>] do_one_initcall+0xc4/0x154
  [<fffffe0000bf0a50>] kernel_init_freeable+0x204/0x2a8
  [<fffffe00007520a0>] kernel_init+0xc/0xd4
This happens because init_cma_reserved_pageblock() calls __free_one_page() with pageblock_order as page order but it is bigger han MAX_ORDER. This in turn causes accesses past zone->free_list[].
Fix the problem by changing init_cma_reserved_pageblock() such that it splits pageblock into individual MAX_ORDER pages if pageblock is bigger than a MAX_ORDER page.
In cases where !CONFIG_HUGETLB_PAGE_SIZE_VARIABLE, which is all architectures expect for ia64, powerpc and tile at the moment, the ?pageblock_order > MAX_ORDER? condition will be optimised out since both sides of the operator are constants. In cases where pageblock size is variable, the performance degradation should not be significant anyway since init_cma_reserved_pageblock() is called only at boot time at most MAX_CMA_AREAS times which by default is eight.
Cc stable at vger.kernel.org
Signed-off-by Michal Nazarewicz <[email protected]>
Reported-by Mark Salter <[email protected]>
Tested-by Christopher Covington <[email protected]>
---
 mm/page_alloc.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)
Quoted 22 lines · Mark Salter
I ended up needing this (on top of your patch) to get the system to boot. Each MAX_ORDER-1 group needs the refcount and migratetype set so that __free_pages does the right thing.
mm/page_alloc.c +2 -5
index 02fb1ed..a7ca6cc 100644
@@ -799,17 +799,18 @@ void __init init_cma_reserved_pageblock(struct page *page)
 		set_page_count(p, 0);
 	} while (++p, --i);
 
-	set_page_refcounted(page);
-	set_pageblock_migratetype(page, MIGRATE_CMA);
-
-	if (pageblock_order > MAX_ORDER) {
-		i = pageblock_order - MAX_ORDER;
+	if (pageblock_order >= MAX_ORDER) {
+		i = pageblock_order - MAX_ORDER + 1;
 		i = 1 << i;
 		p = page;
 		do {
mm/page_alloc.c void __init init_cma_reserved_pageblock(struct page *page) · line 815
Quoted 10 lines
mm/page_alloc.c +5 -1
-			__free_pages(p, MAX_ORDER);
+			set_page_refcounted(p);
+			set_pageblock_migratetype(p, MIGRATE_CMA);
+			__free_pages(p, MAX_ORDER - 1);
 		} while (p += MAX_ORDER_NR_PAGES, --i);
 	} else {
+		set_page_refcounted(page);
+		set_pageblock_migratetype(page, MIGRATE_CMA);
 		__free_pages(page, pageblock_order);
 	}
This is kinda embarrassing, dunno how I missed that.
But each page actually does not need to have migratetype set, does it? All of those pages are in a single pageblock so a single call suffices. If you track set_pageblock_migratetype down to pfn_to_bitidx there is:

Quoted 4 lines

	return (pfn >> pageblock_order) * NR_PAGEBLOCK_BITS;
so for pfns inside of a pageblock, they get truncated. Or did I miss yet another thing?

Nope, my turn to miss something. You only need to set migrate type once per pageblock.

Quoted 19 lines

mm/page_alloc.c +7 -2

index ee92384..fef9614 100644
@@ -816,9 +816,21 @@ void __init init_cma_reserved_pageblock(struct page *page)
 		set_page_count(p, 0);
 	} while (++p, --i);
 
-	set_page_refcounted(page);
 	set_pageblock_migratetype(page, MIGRATE_CMA);
-	__free_pages(page, pageblock_order);
+
+	if (pageblock_order >= MAX_ORDER) {
+		i = pageblock_nr_pages;
+		p = page;
+		do {
+			set_page_refcounted(p);
+			__free_pages(p, MAX_ORDER - 1);

mm/page_alloc.c void __init init_cma_reserved_pageblock(struct page *page) · line 836

Quoted 10 lines

mm/page_alloc.c +7 -0

+			p += MAX_ORDER_NR_PAGES;
+		} while (i -= MAX_ORDER_NR_PAGES);
+	} else {
+		set_page_refcounted(page);
+		__free_pages(page, pageblock_order);
+	}
+
 	adjust_managed_page_count(page, pageblock_nr_pages);
 }
 #endif

This version works for me. Thanks.

Michal Nazarewicz

2014-06-19 19:53 UTC #10 lore raw

PATCH v2 1/1 mm: page_alloc: fix CMA area initialisation when pageblock > MAX_ORDER

In reply to Mark Salter

With a kernel configured with ARM64_64K_PAGES && !TRANSPARENT_HUGEPAGE, the following is triggered at early boot:

SMP: Total of 8 processors activated.
devtmpfs: initialized

  Unable to handle kernel NULL pointer dereference at virtual address 00000008
  pgd = fffffe0000050000
  [00000008] *pgd=00000043fba00003, *pmd=00000043fba00003, *pte=00e0000078010407
  Internal error: Oops: 96000006 [#1] SMP
  Modules linked in:
  CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.15.0-rc864k+ #44
  task: fffffe03bc040000 ti: fffffe03bc080000 task.ti: fffffe03bc080000
  PC is at __list_add+0x10/0xd4
  LR is at free_one_page+0x270/0x638
  ...
  Call trace:
  [<fffffe00003ee970>] __list_add+0x10/0xd4
  [<fffffe000019c478>] free_one_page+0x26c/0x638
  [<fffffe000019c8c8>] __free_pages_ok.part.52+0x84/0xbc
  [<fffffe000019d5e8>] __free_pages+0x74/0xbc
  [<fffffe0000c01350>] init_cma_reserved_pageblock+0xe8/0x104
  [<fffffe0000c24de0>] cma_init_reserved_areas+0x190/0x1e4
  [<fffffe0000090418>] do_one_initcall+0xc4/0x154
  [<fffffe0000bf0a50>] kernel_init_freeable+0x204/0x2a8
  [<fffffe00007520a0>] kernel_init+0xc/0xd4

Fix the problem by changing init_cma_reserved_pageblock() such that it splits pageblock into individual MAX_ORDER pages if pageblock is bigger than a MAX_ORDER page.

Signed-off-by Michal Nazarewicz <[email protected]>

Reported-by Mark Salter <[email protected]>

---
 mm/page_alloc.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

mm/page_alloc.c +12 -1

index 7f97767..fe114db 100644
@@ -817,7 +817,18 @@ void __init init_cma_reserved_pageblock(struct page *page)
 
 	set_page_refcounted(page);
 	set_pageblock_migratetype(page, MIGRATE_CMA);
-	__free_pages(page, pageblock_order);
+
+	if (pageblock_order > MAX_ORDER) {
+		i = pageblock_order - MAX_ORDER;
+		i = 1 << i;
+		p = page;
+		do {
+			__free_pages(p, MAX_ORDER);
+		} while (p += MAX_ORDER_NR_PAGES, --i);
+	} else {
+		__free_pages(page, pageblock_order);
+	}
+
 	adjust_managed_page_count(page, pageblock_nr_pages);
 }
 #endif

Signature · 1 line

2.0.0.526.g5318336

Christopher Covington

Tested-by on v2 1/1

2014-06-20 13:54 UTC #11 lore raw

Re: [PATCHv2] mm: page_alloc: fix CMA area initialisation when pageblock > MAX_ORDER

Quoted 17 lines · Michal Nazarewicz

With a kernel configured with ARM64_64K_PAGES && !TRANSPARENT_HUGEPAGE, the following is triggered at early boot:

SMP: Total of 8 processors activated.
devtmpfs: initialized

  Unable to handle kernel NULL pointer dereference at virtual address 00000008
  pgd = fffffe0000050000
  [00000008] *pgd=00000043fba00003, *pmd=00000043fba00003, *pte=00e0000078010407
  Internal error: Oops: 96000006 [#1] SMP
  Modules linked in:
  CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.15.0-rc864k+ #44
  task: fffffe03bc040000 ti: fffffe03bc080000 task.ti: fffffe03bc080000
  PC is at __list_add+0x10/0xd4
  LR is at free_one_page+0x270/0x638
  ...
  Call trace:
  [<fffffe00003ee970>] __list_add+0x10/0xd4

Quoted 8 lines

  [<fffffe000019c478>] free_one_page+0x26c/0x638
  [<fffffe000019c8c8>] __free_pages_ok.part.52+0x84/0xbc
  [<fffffe000019d5e8>] __free_pages+0x74/0xbc
  [<fffffe0000c01350>] init_cma_reserved_pageblock+0xe8/0x104
  [<fffffe0000c24de0>] cma_init_reserved_areas+0x190/0x1e4
  [<fffffe0000090418>] do_one_initcall+0xc4/0x154
  [<fffffe0000bf0a50>] kernel_init_freeable+0x204/0x2a8
  [<fffffe00007520a0>] kernel_init+0xc/0xd4

I just ran into this. Thanks for the fix.

Tested-by Christopher Covington <[email protected]>

Signature · 3 lines

Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by the Linux Foundation.

Mark Salter

author

2014-06-20 15:48 UTC #12 lore raw

Re: [PATCHv2] mm: page_alloc: fix CMA area initialisation when pageblock > MAX_ORDER

In reply to Michal Nazarewicz

Quoted 63 lines · Michal Nazarewicz

With a kernel configured with ARM64_64K_PAGES && !TRANSPARENT_HUGEPAGE, the following is triggered at early boot:
SMP: Total of 8 processors activated.
devtmpfs: initialized
  Unable to handle kernel NULL pointer dereference at virtual address 00000008
  pgd = fffffe0000050000
  [00000008] *pgd=00000043fba00003, *pmd=00000043fba00003, *pte=00e0000078010407
  Internal error: Oops: 96000006 [#1] SMP
  Modules linked in:
  CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.15.0-rc864k+ #44
  task: fffffe03bc040000 ti: fffffe03bc080000 task.ti: fffffe03bc080000
  PC is at __list_add+0x10/0xd4
  LR is at free_one_page+0x270/0x638
  ...
  Call trace:
  [<fffffe00003ee970>] __list_add+0x10/0xd4
  [<fffffe000019c478>] free_one_page+0x26c/0x638
  [<fffffe000019c8c8>] __free_pages_ok.part.52+0x84/0xbc
  [<fffffe000019d5e8>] __free_pages+0x74/0xbc
  [<fffffe0000c01350>] init_cma_reserved_pageblock+0xe8/0x104
  [<fffffe0000c24de0>] cma_init_reserved_areas+0x190/0x1e4
  [<fffffe0000090418>] do_one_initcall+0xc4/0x154
  [<fffffe0000bf0a50>] kernel_init_freeable+0x204/0x2a8
  [<fffffe00007520a0>] kernel_init+0xc/0xd4
This happens because init_cma_reserved_pageblock() calls __free_one_page() with pageblock_order as page order but it is bigger han MAX_ORDER. This in turn causes accesses past zone->free_list[].
Fix the problem by changing init_cma_reserved_pageblock() such that it splits pageblock into individual MAX_ORDER pages if pageblock is bigger than a MAX_ORDER page.
In cases where !CONFIG_HUGETLB_PAGE_SIZE_VARIABLE, which is all architectures expect for ia64, powerpc and tile at the moment, the ?pageblock_order > MAX_ORDER? condition will be optimised out since both sides of the operator are constants. In cases where pageblock size is variable, the performance degradation should not be significant anyway since init_cma_reserved_pageblock() is called only at boot time at most MAX_CMA_AREAS times which by default is eight.
Signed-off-by Michal Nazarewicz <[email protected]>
Reported-by Mark Salter <[email protected]>
---
 mm/page_alloc.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)
mm/page_alloc.c +5 -1
index 7f97767..fe114db 100644
@@ -817,7 +817,18 @@ void __init init_cma_reserved_pageblock(struct page *page)
 
 	set_page_refcounted(page);
 	set_pageblock_migratetype(page, MIGRATE_CMA);
-	__free_pages(page, pageblock_order);
+
+	if (pageblock_order > MAX_ORDER) {
+		i = pageblock_order - MAX_ORDER;
+		i = 1 << i;
+		p = page;

mm/page_alloc.c void __init init_cma_reserved_pageblock(struct page *page) · line 834

Quoted 10 lines

mm/page_alloc.c +7 -0

+		do {
+			__free_pages(p, MAX_ORDER);
+		} while (p += MAX_ORDER_NR_PAGES, --i);
+	} else {
+		__free_pages(page, pageblock_order);
+	}
+
 	adjust_managed_page_count(page, pageblock_nr_pages);
 }
 #endif

This still isn't quite right. __free_pages can only take up to MAX_ORDER-1 (MAX_ORDER_NR_PAGES is 1 << (MAX_ORDER - 1)). But I'm hitting a slightly different issue even with that fixed up. Still looking...

Michal Nazarewicz

2014-06-20 16:36 UTC #13 lore raw

Re: [PATCHv2] mm: page_alloc: fix CMA area initialisation when pageblock > MAX_ORDER

Quoted 2 lines · Mark Salter

This still isn't quite right. __free_pages can only take up to MAX_ORDER-1 (MAX_ORDER_NR_PAGES is 1 << (MAX_ORDER - 1)).

Good catch. I'll send v3 in a few days then.

Signature · 4 lines

Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Micha? ?mina86? Nazarewicz    (o o)
ooo +--<[email protected]>--<xmpp:[email protected]>--ooO--(_)--Ooo--