kho: align kho_scratch to MAX_ORDER_NR_PAGES pages
Subsystems (3)
PATCH 1/1 kho: align kho_scratch to MAX_ORDER_NR_PAGES pages
I caught this crash:
BUG: unable to handle page fault for address: ff19164fffff8328 RIP: 0010:__free_one_page+0x1a1/0x6b0 Call Trace: <TASK> [<ffffffff913208bf>] free_one_page+0xaf/0x240 [<ffffffff93973288>] deferred_free_pages+0xa8/0xd0 [<ffffffff93971b4f>] deferred_init_memmap_chunk+0x10f/0x1b0 [<ffffffff9396e265>] padata_mt_helper+0x65/0xa0 [<ffffffff90fac402>] process_scheduled_works+0x202/0x410 [<ffffffff90fae739>] worker_thread+0x1f9/0x2d0 [<ffffffff90fb62fd>] kthread+0x27d/0x2f0 [<ffffffff90fae540>] ? __pfx_worker_thread+0x10/0x10 [<ffffffff90fb6080>] ? __pfx_kthread+0x10/0x10 [<ffffffff90efdc55>] ret_from_fork+0x145/0x280 [<ffffffff90fb6080>] ? __pfx_kthread+0x10/0x10 [<ffffffff90e2e46a>] ret_from_fork_asm+0x1a/0x30 </TASK>
deferred_init_memmap_chunk() works fine without KHO because free regions will never be neighbors. However, with KHO, free memory will be split into (free && scratch) and (free && !scratch). KHO scratch is aligned to CMA_MIN_ALIGNMENT_BYTES (which on my setup is equal to 1 << 9 pages) but buddy can look at the neighborhood of MAX_ORDER_NR_PAGES pages (which is equal to 1 << 10 pages). This sometimes crashes when one half of the neighborhood is being initialized but the other part is yet to be initialized.
To fix this, let's just align KHO scratch to MAX_ORDER_NR_PAGES pages.
---
Unfortunately, this is very hard to catch, so I don't have a good reproducer. But I run the code with the fix through extensive testing and it seems fine.
I think I've never caught it before because enabling HUGETLB is what causes CMA_MIN_ALIGNMENT_PAGES to be 1 << 9, instead of 1 << 10.
If this passes review, please cherry-pick it to 7.2 so we don't release a broken kernel.
--- kernel/liveupdate/kexec_handover.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-)
kernel/liveupdate/kexec_handover.c +16 -6
@@ -38,6 +38,16 @@#include "../kexec_internal.h" #include "kexec_handover_internal.h" +/* +* This is the minimal alignment required by deferred struct page init. +* deferred_init_memmap_chunk frees memory to the buddy allocator, which looks +* at the neighboring pages (up to MAX_PAGE_ORDER) to merge them. +* If KHO scratch is not aligned to that value, buddy can access uninitialized +* struct pages, which can cause a crash. +*/ +#define SCRATCH_ALIGNMENT_BYTES (1 << (PAGE_SHIFT + MAX_PAGE_ORDER)) +static_assert (SCRATCH_ALIGNMENT_BYTES >=CMA_MIN_ALIGNMENT_BYTES ); +/* The magic token for preserved pages */ #define KHO_PAGE_MAGIC 0x4b484f50U/* ASCII for 'KHOP' */
@@ -640,8 +650,8 @@ static void __init scratch_size_update(void)* Scratch areas are released as MIGRATE_CMA. Round them up to the right * size. */ - scratch_size_lowmem =round_up (scratch_size_lowmem,CMA_MIN_ALIGNMENT_BYTES ); - scratch_size_global =round_up (scratch_size_global,CMA_MIN_ALIGNMENT_BYTES ); + scratch_size_lowmem =round_up (scratch_size_lowmem,SCRATCH_ALIGNMENT_BYTES ); + scratch_size_global =round_up (scratch_size_global,SCRATCH_ALIGNMENT_BYTES ); }static phys_addr_t __init scratch_size_node (int nid)
@@ -656,7 +666,7 @@ static phys_addr_t __init scratch_size_node(int nid) size = scratch_size_pernode; } -return round_up (size,CMA_MIN_ALIGNMENT_BYTES ); +return round_up (size,SCRATCH_ALIGNMENT_BYTES ); }/**
@@ -692,7 +702,7 @@ static void __init kho_reserve_scratch(void)* next kernel */ size = scratch_size_lowmem; - addr =memblock_phys_alloc_range (size,CMA_MIN_ALIGNMENT_BYTES ,0 , + addr =memblock_phys_alloc_range (size,SCRATCH_ALIGNMENT_BYTES ,0 ,ARCH_LOW_ADDRESS_LIMIT );if (!addr) {pr_err ("Failed to reserve lowmem scratch buffer\n" );
@@ -705,7 +715,7 @@ static void __init kho_reserve_scratch(void)/* reserve large contiguous area for allocations without nid */ size = scratch_size_global; - addr =memblock_phys_alloc (size,CMA_MIN_ALIGNMENT_BYTES ); + addr =memblock_phys_alloc (size,SCRATCH_ALIGNMENT_BYTES );if (!addr) {pr_err ("Failed to reserve global scratch buffer\n" ); goto err_free_scratch_areas;
@@ -721,7 +731,7 @@ static void __init kho_reserve_scratch(void)*/ for_each_node_state (nid,N_MEMORY ) { size =scratch_size_node (nid); - addr =memblock_alloc_range_nid (size,CMA_MIN_ALIGNMENT_BYTES , + addr =memblock_alloc_range_nid (size,SCRATCH_ALIGNMENT_BYTES ,0 ,MEMBLOCK_ALLOC_ACCESSIBLE , nid, true);if (!addr) {
Signature · 1 line
2.55.0.141.g00534a21ce-goog
Mike Rapoport
maintainerHi Michal,
Quoted 40 lines
I caught this crash:
BUG: unable to handle page fault for address: ff19164fffff8328 RIP: 0010:__free_one_page+0x1a1/0x6b0 Call Trace: <TASK> [<ffffffff913208bf>] free_one_page+0xaf/0x240 [<ffffffff93973288>] deferred_free_pages+0xa8/0xd0 [<ffffffff93971b4f>] deferred_init_memmap_chunk+0x10f/0x1b0 [<ffffffff9396e265>] padata_mt_helper+0x65/0xa0 [<ffffffff90fac402>] process_scheduled_works+0x202/0x410 [<ffffffff90fae739>] worker_thread+0x1f9/0x2d0 [<ffffffff90fb62fd>] kthread+0x27d/0x2f0 [<ffffffff90fae540>] ? __pfx_worker_thread+0x10/0x10 [<ffffffff90fb6080>] ? __pfx_kthread+0x10/0x10 [<ffffffff90efdc55>] ret_from_fork+0x145/0x280 [<ffffffff90fb6080>] ? __pfx_kthread+0x10/0x10 [<ffffffff90e2e46a>] ret_from_fork_asm+0x1a/0x30 </TASK>
deferred_init_memmap_chunk()works fine without KHO because free regions will never be neighbors. However, with KHO, free memory will be split into (free && scratch) and (free && !scratch). KHO scratch is aligned toCMA_MIN_ALIGNMENT_BYTES(which on my setup is equal to 1 << 9 pages) but buddy can look at the neighborhood ofMAX_ORDER_NR_PAGESpages (which is equal to 1 << 10 pages). This sometimes crashes when one half of the neighborhood is being initialized but the other part is yet to be initialized.To fix this, let's just align KHO scratch to
MAX_ORDER_NR_PAGESpages.Fixes c6073743d0c7 ("kho: make preserved pages compatible with deferred struct page init")Signed-off-by Michal Clapinski <[email protected]>kernel/liveupdate/kexec_handover.c +0
-0@@ -38,6 +38,16 @@#include "../kexec_internal.h" #include "kexec_handover_internal.h"
kernel/liveupdate/kexec_handover.c line 49
Quoted 10 lines
kernel/liveupdate/kexec_handover.c +9
-0+/* +* This is the minimal alignment required by deferred struct page init. +* deferred_init_memmap_chunk frees memory to the buddy allocator, which looks +* at the neighboring pages (up to MAX_PAGE_ORDER) to merge them. +* If KHO scratch is not aligned to that value, buddy can access uninitialized +* struct pages, which can cause a crash. +*/ +#define SCRATCH_ALIGNMENT_BYTES (1 << (PAGE_SHIFT + MAX_PAGE_ORDER)) +static_assert (SCRATCH_ALIGNMENT_BYTES >=CMA_MIN_ALIGNMENT_BYTES );
Maybe make it a variable and set it at runtime?
We won't need to worry about different confugurations that override MAX_ORDER and potentail divergence of pageblock size from MAX_ORDER in the future.
Signature · 2 lines
Sincerely yours, Mike.
Quoted 44 lines · Mike Rapoport
Hi Michal,
Quoted 41 lines
I caught this crash:
BUG: unable to handle page fault for address: ff19164fffff8328 RIP: 0010:__free_one_page+0x1a1/0x6b0 Call Trace: <TASK> [<ffffffff913208bf>] free_one_page+0xaf/0x240 [<ffffffff93973288>] deferred_free_pages+0xa8/0xd0 [<ffffffff93971b4f>] deferred_init_memmap_chunk+0x10f/0x1b0 [<ffffffff9396e265>] padata_mt_helper+0x65/0xa0 [<ffffffff90fac402>] process_scheduled_works+0x202/0x410 [<ffffffff90fae739>] worker_thread+0x1f9/0x2d0 [<ffffffff90fb62fd>] kthread+0x27d/0x2f0 [<ffffffff90fae540>] ? __pfx_worker_thread+0x10/0x10 [<ffffffff90fb6080>] ? __pfx_kthread+0x10/0x10 [<ffffffff90efdc55>] ret_from_fork+0x145/0x280 [<ffffffff90fb6080>] ? __pfx_kthread+0x10/0x10 [<ffffffff90e2e46a>] ret_from_fork_asm+0x1a/0x30 </TASK>
deferred_init_memmap_chunk()works fine without KHO because free regions will never be neighbors. However, with KHO, free memory will be split into (free && scratch) and (free && !scratch). KHO scratch is aligned toCMA_MIN_ALIGNMENT_BYTES(which on my setup is equal to 1 << 9 pages) but buddy can look at the neighborhood ofMAX_ORDER_NR_PAGESpages (which is equal to 1 << 10 pages). This sometimes crashes when one half of the neighborhood is being initialized but the other part is yet to be initialized.To fix this, let's just align KHO scratch to
MAX_ORDER_NR_PAGESpages.Fixes c6073743d0c7 ("kho: make preserved pages compatible with deferred struct page init")Signed-off-by Michal Clapinski <[email protected]>kernel/liveupdate/kexec_handover.c +0
-0@@ -38,6 +38,16 @@#include "../kexec_internal.h" #include "kexec_handover_internal.h"
Quoted 14 lines
kernel/liveupdate/kexec_handover.c line 49
Quoted 9 lines
kernel/liveupdate/kexec_handover.c +9
-0+/* +* This is the minimal alignment required by deferred struct page init. +* deferred_init_memmap_chunk frees memory to the buddy allocator, which looks +* at the neighboring pages (up to MAX_PAGE_ORDER) to merge them. +* If KHO scratch is not aligned to that value, buddy can access uninitialized +* struct pages, which can cause a crash. +*/ +#define SCRATCH_ALIGNMENT_BYTES (1 << (PAGE_SHIFT + MAX_PAGE_ORDER)) +static_assert (SCRATCH_ALIGNMENT_BYTES >=CMA_MIN_ALIGNMENT_BYTES );Maybe make it a variable and set it at runtime?
We won't need to worry about different confugurations that overrideMAX_ORDERand potentail divergence of pageblock size fromMAX_ORDERin the future.
Sorry, I don't track. What would I set it to at runtime? MAX_PAGE_ORDER must be >= PAGE_BLOCK_MAX_ORDER and I don't understand what's the problem if some configurations override MAX_PAGE_ORDER. They can't override it to be smaller than PAGE_BLOCK_MAX_ORDER.
Are you saying that in the future PAGE_BLOCK_MAX_ORDER could be bigger than MAX_PAGE_ORDER?
Mike Rapoport
maintainerQuoted 60 lines · Michał Cłapiński
Quoted 44 lines · Mike Rapoport
Hi Michal,
Quoted 41 lines
I caught this crash:
BUG: unable to handle page fault for address: ff19164fffff8328 RIP: 0010:__free_one_page+0x1a1/0x6b0 Call Trace: <TASK> [<ffffffff913208bf>] free_one_page+0xaf/0x240 [<ffffffff93973288>] deferred_free_pages+0xa8/0xd0 [<ffffffff93971b4f>] deferred_init_memmap_chunk+0x10f/0x1b0 [<ffffffff9396e265>] padata_mt_helper+0x65/0xa0 [<ffffffff90fac402>] process_scheduled_works+0x202/0x410 [<ffffffff90fae739>] worker_thread+0x1f9/0x2d0 [<ffffffff90fb62fd>] kthread+0x27d/0x2f0 [<ffffffff90fae540>] ? __pfx_worker_thread+0x10/0x10 [<ffffffff90fb6080>] ? __pfx_kthread+0x10/0x10 [<ffffffff90efdc55>] ret_from_fork+0x145/0x280 [<ffffffff90fb6080>] ? __pfx_kthread+0x10/0x10 [<ffffffff90e2e46a>] ret_from_fork_asm+0x1a/0x30 </TASK>
deferred_init_memmap_chunk()works fine without KHO because free regions will never be neighbors. However, with KHO, free memory will be split into (free && scratch) and (free && !scratch). KHO scratch is aligned toCMA_MIN_ALIGNMENT_BYTES(which on my setup is equal to 1 << 9 pages) but buddy can look at the neighborhood ofMAX_ORDER_NR_PAGESpages (which is equal to 1 << 10 pages). This sometimes crashes when one half of the neighborhood is being initialized but the other part is yet to be initialized.To fix this, let's just align KHO scratch to
MAX_ORDER_NR_PAGESpages.Fixes c6073743d0c7 ("kho: make preserved pages compatible with deferred struct page init")Signed-off-by Michal Clapinski <[email protected]>kernel/liveupdate/kexec_handover.c +0
-0@@ -38,6 +38,16 @@#include "../kexec_internal.h" #include "kexec_handover_internal.h" Quoted 14 lines
kernel/liveupdate/kexec_handover.c line 49
Quoted 9 lines
kernel/liveupdate/kexec_handover.c +9
-0+/* +* This is the minimal alignment required by deferred struct page init. +* deferred_init_memmap_chunk frees memory to the buddy allocator, which looks +* at the neighboring pages (up to MAX_PAGE_ORDER) to merge them. +* If KHO scratch is not aligned to that value, buddy can access uninitialized +* struct pages, which can cause a crash. +*/ +#define SCRATCH_ALIGNMENT_BYTES (1 << (PAGE_SHIFT + MAX_PAGE_ORDER)) +static_assert (SCRATCH_ALIGNMENT_BYTES >=CMA_MIN_ALIGNMENT_BYTES );Maybe make it a variable and set it at runtime?
We won't need to worry about different confugurations that overrideMAX_ORDERand potentail divergence of pageblock size fromMAX_ORDERin the future.
Quoted 7 lines
Sorry, I don't track. What would I set it to at runtime?
MAX_PAGE_ORDERmust be >=PAGE_BLOCK_MAX_ORDERand I don't understand what's the problem if some configurations overrideMAX_PAGE_ORDER. They can't override it to be smaller thanPAGE_BLOCK_MAX_ORDER.Are you saying that in the future
PAGE_BLOCK_MAX_ORDERcould be bigger thanMAX_PAGE_ORDER?
That's very unlikely, indeed.
Signature · 2 lines
Sincerely yours, Mike.
Pasha Tatashin
maintainerQuoted 43 lines · Mike Rapoport
Hi Michal,
Quoted 41 lines
I caught this crash:
BUG: unable to handle page fault for address: ff19164fffff8328 RIP: 0010:__free_one_page+0x1a1/0x6b0 Call Trace: <TASK> [<ffffffff913208bf>] free_one_page+0xaf/0x240 [<ffffffff93973288>] deferred_free_pages+0xa8/0xd0 [<ffffffff93971b4f>] deferred_init_memmap_chunk+0x10f/0x1b0 [<ffffffff9396e265>] padata_mt_helper+0x65/0xa0 [<ffffffff90fac402>] process_scheduled_works+0x202/0x410 [<ffffffff90fae739>] worker_thread+0x1f9/0x2d0 [<ffffffff90fb62fd>] kthread+0x27d/0x2f0 [<ffffffff90fae540>] ? __pfx_worker_thread+0x10/0x10 [<ffffffff90fb6080>] ? __pfx_kthread+0x10/0x10 [<ffffffff90efdc55>] ret_from_fork+0x145/0x280 [<ffffffff90fb6080>] ? __pfx_kthread+0x10/0x10 [<ffffffff90e2e46a>] ret_from_fork_asm+0x1a/0x30 </TASK>
deferred_init_memmap_chunk()works fine without KHO because free regions will never be neighbors. However, with KHO, free memory will be split into (free && scratch) and (free && !scratch). KHO scratch is aligned toCMA_MIN_ALIGNMENT_BYTES(which on my setup is equal to 1 << 9 pages) but buddy can look at the neighborhood ofMAX_ORDER_NR_PAGESpages (which is equal to 1 << 10 pages). This sometimes crashes when one half of the neighborhood is being initialized but the other part is yet to be initialized.To fix this, let's just align KHO scratch to
MAX_ORDER_NR_PAGESpages.Fixes c6073743d0c7 ("kho: make preserved pages compatible with deferred struct page init")Signed-off-by Michal Clapinski <[email protected]>kernel/liveupdate/kexec_handover.c +0
-0@@ -38,6 +38,16 @@#include "../kexec_internal.h" #include "kexec_handover_internal.h"
Quoted 13 lines
kernel/liveupdate/kexec_handover.c line 49
Quoted 9 lines
kernel/liveupdate/kexec_handover.c +9
-0+/* +* This is the minimal alignment required by deferred struct page init. +* deferred_init_memmap_chunk frees memory to the buddy allocator, which looks +* at the neighboring pages (up to MAX_PAGE_ORDER) to merge them. +* If KHO scratch is not aligned to that value, buddy can access uninitialized +* struct pages, which can cause a crash. +*/ +#define SCRATCH_ALIGNMENT_BYTES (1 << (PAGE_SHIFT + MAX_PAGE_ORDER)) +static_assert (SCRATCH_ALIGNMENT_BYTES >=CMA_MIN_ALIGNMENT_BYTES );Maybe make it a variable and set it at runtime?
We won't need to worry about different confugurations that overrideMAX_ORDERand potentail divergence of pageblock size fromMAX_ORDERin
Do those architectures reduce MAX_ORDER when they override it? I would hope we could set the alignment once and make it large enough to work everywhere, rather than depending on dynamic changes even during boot...
Quoted 6 lines
the future.
Signature · 2 lines
Sincerely yours, Mike.
Quoted 59 lines · Pasha Tatashin
Quoted 43 lines · Mike Rapoport
Hi Michal,
Quoted 41 lines
I caught this crash:
BUG: unable to handle page fault for address: ff19164fffff8328 RIP: 0010:__free_one_page+0x1a1/0x6b0 Call Trace: <TASK> [<ffffffff913208bf>] free_one_page+0xaf/0x240 [<ffffffff93973288>] deferred_free_pages+0xa8/0xd0 [<ffffffff93971b4f>] deferred_init_memmap_chunk+0x10f/0x1b0 [<ffffffff9396e265>] padata_mt_helper+0x65/0xa0 [<ffffffff90fac402>] process_scheduled_works+0x202/0x410 [<ffffffff90fae739>] worker_thread+0x1f9/0x2d0 [<ffffffff90fb62fd>] kthread+0x27d/0x2f0 [<ffffffff90fae540>] ? __pfx_worker_thread+0x10/0x10 [<ffffffff90fb6080>] ? __pfx_kthread+0x10/0x10 [<ffffffff90efdc55>] ret_from_fork+0x145/0x280 [<ffffffff90fb6080>] ? __pfx_kthread+0x10/0x10 [<ffffffff90e2e46a>] ret_from_fork_asm+0x1a/0x30 </TASK>
deferred_init_memmap_chunk()works fine without KHO because free regions will never be neighbors. However, with KHO, free memory will be split into (free && scratch) and (free && !scratch). KHO scratch is aligned toCMA_MIN_ALIGNMENT_BYTES(which on my setup is equal to 1 << 9 pages) but buddy can look at the neighborhood ofMAX_ORDER_NR_PAGESpages (which is equal to 1 << 10 pages). This sometimes crashes when one half of the neighborhood is being initialized but the other part is yet to be initialized.To fix this, let's just align KHO scratch to
MAX_ORDER_NR_PAGESpages.Fixes c6073743d0c7 ("kho: make preserved pages compatible with deferred struct page init")Signed-off-by Michal Clapinski <[email protected]>kernel/liveupdate/kexec_handover.c +0
-0@@ -38,6 +38,16 @@#include "../kexec_internal.h" #include "kexec_handover_internal.h" Quoted 13 lines
kernel/liveupdate/kexec_handover.c line 49
Quoted 9 lines
kernel/liveupdate/kexec_handover.c +9
-0+/* +* This is the minimal alignment required by deferred struct page init. +* deferred_init_memmap_chunk frees memory to the buddy allocator, which looks +* at the neighboring pages (up to MAX_PAGE_ORDER) to merge them. +* If KHO scratch is not aligned to that value, buddy can access uninitialized +* struct pages, which can cause a crash. +*/ +#define SCRATCH_ALIGNMENT_BYTES (1 << (PAGE_SHIFT + MAX_PAGE_ORDER)) +static_assert (SCRATCH_ALIGNMENT_BYTES >=CMA_MIN_ALIGNMENT_BYTES );Maybe make it a variable and set it at runtime?
We won't need to worry about different confugurations that overrideMAX_ORDERand potentail divergence of pageblock size fromMAX_ORDERin
Quoted 3 lines
Do those architectures reduce
MAX_ORDERwhen they override it? I would hope we could set the alignment once and make it large enough to work everywhere, rather than depending on dynamic changes even during boot...
MAX_PAGE_ORDER can't be modified at runtime. It's either 10 or it's CONFIG_ARCH_FORCE_MAX_ORDER.
Mike Rapoport
maintainerQuoted 58 lines · Pasha Tatashin
Quoted 43 lines · Mike Rapoport
Hi Michal,
Quoted 41 lines
I caught this crash:
BUG: unable to handle page fault for address: ff19164fffff8328 RIP: 0010:__free_one_page+0x1a1/0x6b0 Call Trace: <TASK> [<ffffffff913208bf>] free_one_page+0xaf/0x240 [<ffffffff93973288>] deferred_free_pages+0xa8/0xd0 [<ffffffff93971b4f>] deferred_init_memmap_chunk+0x10f/0x1b0 [<ffffffff9396e265>] padata_mt_helper+0x65/0xa0 [<ffffffff90fac402>] process_scheduled_works+0x202/0x410 [<ffffffff90fae739>] worker_thread+0x1f9/0x2d0 [<ffffffff90fb62fd>] kthread+0x27d/0x2f0 [<ffffffff90fae540>] ? __pfx_worker_thread+0x10/0x10 [<ffffffff90fb6080>] ? __pfx_kthread+0x10/0x10 [<ffffffff90efdc55>] ret_from_fork+0x145/0x280 [<ffffffff90fb6080>] ? __pfx_kthread+0x10/0x10 [<ffffffff90e2e46a>] ret_from_fork_asm+0x1a/0x30 </TASK>
deferred_init_memmap_chunk()works fine without KHO because free regions will never be neighbors. However, with KHO, free memory will be split into (free && scratch) and (free && !scratch). KHO scratch is aligned toCMA_MIN_ALIGNMENT_BYTES(which on my setup is equal to 1 << 9 pages) but buddy can look at the neighborhood ofMAX_ORDER_NR_PAGESpages (which is equal to 1 << 10 pages). This sometimes crashes when one half of the neighborhood is being initialized but the other part is yet to be initialized.To fix this, let's just align KHO scratch to
MAX_ORDER_NR_PAGESpages.Fixes c6073743d0c7 ("kho: make preserved pages compatible with deferred struct page init")Signed-off-by Michal Clapinski <[email protected]>kernel/liveupdate/kexec_handover.c +0
-0@@ -38,6 +38,16 @@#include "../kexec_internal.h" #include "kexec_handover_internal.h" Quoted 13 lines
kernel/liveupdate/kexec_handover.c line 49
Quoted 9 lines
kernel/liveupdate/kexec_handover.c +9
-0+/* +* This is the minimal alignment required by deferred struct page init. +* deferred_init_memmap_chunk frees memory to the buddy allocator, which looks +* at the neighboring pages (up to MAX_PAGE_ORDER) to merge them. +* If KHO scratch is not aligned to that value, buddy can access uninitialized +* struct pages, which can cause a crash. +*/ +#define SCRATCH_ALIGNMENT_BYTES (1 << (PAGE_SHIFT + MAX_PAGE_ORDER)) +static_assert (SCRATCH_ALIGNMENT_BYTES >=CMA_MIN_ALIGNMENT_BYTES );Maybe make it a variable and set it at runtime?
We won't need to worry about different confugurations that overrideMAX_ORDERand potentail divergence of pageblock size fromMAX_ORDERin
Quoted 3 lines
Do those architectures reduce
MAX_ORDERwhen they override it? I would hope we could set the alignment once and make it large enough to work everywhere, rather than depending on dynamic changes even during boot...
They could, e.g. arm64 has this:
config ARCH_FORCE_MAX_ORDER
int default "13" if ARM64_64K_PAGES default "11" if ARM64_16K_PAGES default "10"
Quoted 6 lines
Quoted 6 lines
the future.
Signature · 2 lines
Sincerely yours, Mike.
Signature · 2 lines
Sincerely yours, Mike.
Mike Rapoport
maintainerQuoted 1 line · Michal Clapinski
I caught this crash:
We are not as strict as x86 folks, but imperative voice is preferred.
Quoted 18 lines
BUG: unable to handle page fault for address: ff19164fffff8328 RIP: 0010:__free_one_page+0x1a1/0x6b0 Call Trace: <TASK> [<ffffffff913208bf>] free_one_page+0xaf/0x240 [<ffffffff93973288>] deferred_free_pages+0xa8/0xd0 [<ffffffff93971b4f>] deferred_init_memmap_chunk+0x10f/0x1b0 [<ffffffff9396e265>] padata_mt_helper+0x65/0xa0 [<ffffffff90fac402>] process_scheduled_works+0x202/0x410 [<ffffffff90fae739>] worker_thread+0x1f9/0x2d0 [<ffffffff90fb62fd>] kthread+0x27d/0x2f0 [<ffffffff90fae540>] ? __pfx_worker_thread+0x10/0x10 [<ffffffff90fb6080>] ? __pfx_kthread+0x10/0x10 [<ffffffff90efdc55>] ret_from_fork+0x145/0x280 [<ffffffff90fb6080>] ? __pfx_kthread+0x10/0x10 [<ffffffff90e2e46a>] ret_from_fork_asm+0x1a/0x30 </TASK>
Quoted 2 lines
deferred_init_memmap_chunk()works fine without KHO because free regions will never be neighbors. However, with KHO, free memory
I'd change it to something like
deferred_init_memmap_chunk() interleaves initialization of struct pages
with freeing them. This works fine without KHO ...
Quoted 4 lines
will be split into (free && scratch) and (free && !scratch). KHO scratch is aligned to
CMA_MIN_ALIGNMENT_BYTES(which on my setup is equal to 1 << 9 pages) but buddy can look at the neighborhood ofMAX_ORDER_NR_PAGESpages (which is equal to 1 << 10 pages).
Please make it clearer that the values correspond to a particular configuration. E.g.
in a configuration where CMA_MIN_ALIGNMENT_PAGES is 512 and MAX_ORDER_NR_PAGES is 1024
Quoted 26 lines
This sometimes crashes when one half of the neighborhood is being initialized but the other part is yet to be initialized.
To fix this, let's just align KHO scratch to
MAX_ORDER_NR_PAGESpages.Fixes c6073743d0c7 ("kho: make preserved pages compatible with deferred struct page init")Signed-off-by Michal Clapinski <[email protected]>---
Unfortunately, this is very hard to catch, so I don't have a good reproducer. But I run the code with the fix through extensive testing and it seems fine.
I think I've never caught it before because enabling HUGETLB is what causesCMA_MIN_ALIGNMENT_PAGESto be 1 << 9, instead of 1 << 10.If this passes review, please cherry-pick it to 7.2 so we don't release a broken kernel.
--- kernel/liveupdate/kexec_handover.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-)kernel/liveupdate/kexec_handover.c +0
-0@@ -38,6 +38,16 @@#include "../kexec_internal.h"
kernel/liveupdate/kexec_handover.c line 48
Quoted 10 lines
kernel/liveupdate/kexec_handover.c +8
-0#include "kexec_handover_internal.h" +/* +* This is the minimal alignment required by deferred struct page init. +* deferred_init_memmap_chunk frees memory to the buddy allocator, which looks +* at the neighboring pages (up to MAX_PAGE_ORDER) to merge them. +* If KHO scratch is not aligned to that value, buddy can access uninitialized +* struct pages, which can cause a crash. +*/ +#define SCRATCH_ALIGNMENT_BYTES (1 << (PAGE_SHIFT + MAX_PAGE_ORDER))
I believe PAGE_SIZE * MAX_ORDER_NR_PAGES looks better
Quoted 55 lines
kernel/liveupdate/kexec_handover.c +8
-6+static_assert (SCRATCH_ALIGNMENT_BYTES >=CMA_MIN_ALIGNMENT_BYTES ); +/* The magic token for preserved pages */ #define KHO_PAGE_MAGIC 0x4b484f50U/* ASCII for 'KHOP' */ @@ -640,8 +650,8 @@ static void __init scratch_size_update(void)* Scratch areas are released as MIGRATE_CMA. Round them up to the right * size. */ - scratch_size_lowmem =round_up (scratch_size_lowmem,CMA_MIN_ALIGNMENT_BYTES ); - scratch_size_global =round_up (scratch_size_global,CMA_MIN_ALIGNMENT_BYTES ); + scratch_size_lowmem =round_up (scratch_size_lowmem,SCRATCH_ALIGNMENT_BYTES ); + scratch_size_global =round_up (scratch_size_global,SCRATCH_ALIGNMENT_BYTES ); }static phys_addr_t __init scratch_size_node (int nid)@@ -656,7 +666,7 @@ static phys_addr_t __init scratch_size_node(int nid) size = scratch_size_pernode; } -return round_up (size,CMA_MIN_ALIGNMENT_BYTES ); +return round_up (size,SCRATCH_ALIGNMENT_BYTES ); }/** @@ -692,7 +702,7 @@ static void __init kho_reserve_scratch(void)* next kernel */ size = scratch_size_lowmem; - addr =memblock_phys_alloc_range (size,CMA_MIN_ALIGNMENT_BYTES ,0 , + addr =memblock_phys_alloc_range (size,SCRATCH_ALIGNMENT_BYTES ,0 ,ARCH_LOW_ADDRESS_LIMIT );if (!addr) {pr_err ("Failed to reserve lowmem scratch buffer\n" );@@ -705,7 +715,7 @@ static void __init kho_reserve_scratch(void)/* reserve large contiguous area for allocations without nid */ size = scratch_size_global; - addr =memblock_phys_alloc (size,CMA_MIN_ALIGNMENT_BYTES ); + addr =memblock_phys_alloc (size,SCRATCH_ALIGNMENT_BYTES );if (!addr) {pr_err ("Failed to reserve global scratch buffer\n" ); goto err_free_scratch_areas;@@ -721,7 +731,7 @@ static void __init kho_reserve_scratch(void)*/ for_each_node_state (nid,N_MEMORY ) { size =scratch_size_node (nid); - addr =memblock_alloc_range_nid (size,CMA_MIN_ALIGNMENT_BYTES , + addr =memblock_alloc_range_nid (size,SCRATCH_ALIGNMENT_BYTES ,0 ,MEMBLOCK_ALLOC_ACCESSIBLE , nid, true);if (!addr) {Signature · 1 line
2.55.0.141.g00534a21ce-goog
Signature · 2 lines
Sincerely yours, Mike.