Re: linux-next regression: SNP Guest boot hangs with certain cpu/mem config combination
From: Kirill A. Shutemov
Date: Fri Mar 28 2025 - 05:25:36 EST
On Fri, Mar 28, 2025 at 02:49:27PM +0530, Aithal, Srikanth wrote:
> On 3/28/2025 2:39 PM, Kirill A. Shutemov wrote:
> > On Fri, Mar 28, 2025 at 10:28:19AM +0200, Kirill A. Shutemov wrote:
> > > On Thu, Mar 27, 2025 at 07:39:22PM +0200, Kirill A. Shutemov wrote:
> > > > On Thu, Mar 27, 2025 at 11:02:24AM -0400, Steven Rostedt wrote:
> > > > > On Thu, 27 Mar 2025 16:43:43 +0200
> > > > > "Kirill A. Shutemov" <kirill.shutemov@xxxxxxxxxxxxxxx> wrote:
> > > > >
> > > > > > > > The only option I see so far is to drop static branch from this path.
> > > > > > > >
> > > > > > > > But I am not sure if it the only case were we use static branch from CPU
> > > > > > > > hotplug callbacks.
> > > > > > > >
> > > > > > > > Any other ideas?
> > > > > > >
> > > > > > >
> > > > > > > Hmmm, didn't take too close a look here, but there is the
> > > > > > > static_key_slow_dec_cpuslocked() variant, would that work here? Is the issue
> > > > > > > the caller may or may not have the cpu_hotplug lock?
> > > > > >
> > > > > > Yes. This is generic page alloc path and it can be called with and without
> > > > > > the lock.
> > > > >
> > > > > Note, it's not the static_branch that is an issue, it's enabling/disabling
> > > > > the static branch that is. Changing a static branch takes a bit of work as
> > > > > it does modify the kernel text.
> > > > >
> > > > > Is it possible to delay the update via a workqueue?
> > > >
> > > > Ah. Good point. Should work. I'll give it try.
> > >
> > > The patch below fixes problem for me.
> >
> > Ah. No, it won't work. We can get there before workqueues are initialized:
> > mm_core_init() is called before workqueue_init_early().
> >
> > We cannot queue a work. :/
> >
> > Steven, any other ideas?
> >
>
> I have booted the guest with different memory and CPU combinations and have
> not seen any failures with the fix so far. Are there any other scenarios
> that could trigger the above case? Please let me know.
I saw this:
[ 1.828379] BUG: kernel NULL pointer dereference, address: 00000000000001c1
[ 1.833122] #PF: supervisor read access in kernel mode
[ 1.834907] #PF: error_code(0x0000) - not-present page
[ 1.836681] PGD 0
[ 1.837386] Oops: Oops: 0000 [#1] PREEMPT SMP NOPTI
[ 1.839051] CPU: 0 UID: 0 PID: 0 Comm: swapper Tainted: G S 6.14.0-rc5-00015-ga250fa517231-dirty #2266
[ 1.842700] Tainted: [S]=CPU_OUT_OF_SPEC
[ 1.844036] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS unknown 02/02/2022
[ 1.846790] RIP: 0010:__queue_work+0x3f/0x610
[ 1.848330] Code: d3 49 89 f5 41 89 fc 83 3d 5d 51 cb 01 00 74 1a 65 8b 05 04 db 94 4a 85 c0 75 0f 65 8b 05 d1 d7 94 4a 85 c0 0f 85 59 03 00 00 <41> 0f b7 85 c1 01 00 00 a9 80 01 00 00 74 57 65 8b 05 23 ec 97 4a
[ 1.854803] RSP: 0000:ffffffffb7003c08 EFLAGS: 00010046
[ 1.856614] RAX: 0000000000000000 RBX: ff3d51307fb54488 RCX: 0000000000000000
[ 1.859083] RDX: ff3d51307fb54488 RSI: 0000000000000000 RDI: 0000000000000200
[ 1.861534] RBP: ffffffffb7003e18 R08: 0000000000000001 R09: 0000000000000001
[ 1.864018] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000200
[ 1.866480] R13: 0000000000000000 R14: 0000000000000092 R15: ff3d51307fb544d8
[ 1.868920] FS: 0000000000000000(0000) GS:ff3d513075000000(0000) knlGS:0000000000000000
[ 1.871721] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1.873710] CR2: 00000000000001c1 CR3: 0000000004a5c001 CR4: 00000000000210f0
[ 1.876194] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 1.878629] DR3: 0000000000000000 DR6: 00000000fffe07f0 DR7: 0000000000000400
[ 1.881105] Call Trace:
[ 1.881936] <TASK>
[ 1.882642] ? __die+0xda/0x120
[ 1.883715] ? page_fault_oops+0x330/0x3c0
[ 1.885144] ? exc_page_fault+0x43/0x100
[ 1.886483] ? exc_page_fault+0x62/0x100
[ 1.887825] ? __add_to_free_list+0xc0/0x160
[ 1.891278] ? asm_exc_page_fault+0x22/0x30
[ 1.892706] ? __queue_work+0x3f/0x610
[ 1.894000] ? __free_pages_ok+0x2b0/0x530
[ 1.895416] queue_work_on+0xb3/0x110
[ 1.896673] ? __accept_page+0x10b/0x170
[ 1.898042] cond_accept_memory+0x1dc/0x200
[ 1.899496] get_page_from_freelist+0x1ad/0x1380
[ 1.901086] __alloc_frozen_pages_noprof+0xff/0x280
[ 1.902760] alloc_pages_mpol+0x111/0x1c0
[ 1.904159] alloc_pages_noprof+0x7c/0xf0
[ 1.905535] __p4d_alloc+0x31/0x110
[ 1.906729] preallocate_vmalloc_pages+0xdf/0x160
[ 1.908397] mm_core_init+0x25/0x60
[ 1.909590] start_kernel+0x16a/0x400
[ 1.910850] x86_64_start_reservations+0x20/0x20
[ 1.912454] x86_64_start_kernel+0xaa/0xb0
[ 1.913856] common_startup_64+0x13e/0x140
[ 1.915269] </TASK>
[ 1.916023] CR2: 00000000000001c1
[ 1.917183] ---[ end trace 0000000000000000 ]---
[ 1.918764] RIP: 0010:__queue_work+0x3f/0x610
This patch fixes it:
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 9540b41894da..ea5f7e0b675d 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -964,6 +964,9 @@ struct zone {
#ifdef CONFIG_UNACCEPTED_MEMORY
/* Pages to be accepted. All pages on the list are MAX_PAGE_ORDER */
struct list_head unaccepted_pages;
+
+ /* To be called once last page in the zone is accepted */
+ struct work_struct unaccepted_cleanup;
#endif
/* zone flags, see below */
diff --git a/mm/internal.h b/mm/internal.h
index 109ef30fee11..f2e6d42af6eb 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -1516,6 +1516,7 @@ unsigned long move_page_tables(struct vm_area_struct *vma,
#ifdef CONFIG_UNACCEPTED_MEMORY
void accept_page(struct page *page);
+void unaccepted_cleanup_work(struct work_struct *work);
#else /* CONFIG_UNACCEPTED_MEMORY */
static inline void accept_page(struct page *page)
{
diff --git a/mm/mm_init.c b/mm/mm_init.c
index 2630cc30147e..d5a51f65dc4d 100644
--- a/mm/mm_init.c
+++ b/mm/mm_init.c
@@ -1404,6 +1404,7 @@ static void __meminit zone_init_free_lists(struct zone *zone)
#ifdef CONFIG_UNACCEPTED_MEMORY
INIT_LIST_HEAD(&zone->unaccepted_pages);
+ INIT_WORK(&zone->unaccepted_cleanup, unaccepted_cleanup_work);
#endif
}
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 4fe93029bcb6..1b776d0c7784 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6921,6 +6921,11 @@ static DEFINE_STATIC_KEY_FALSE(zones_with_unaccepted_pages);
static bool lazy_accept = true;
+void unaccepted_cleanup_work(struct work_struct *work)
+{
+ static_branch_dec(&zones_with_unaccepted_pages);
+}
+
static int __init accept_memory_parse(char *p)
{
if (!strcmp(p, "lazy")) {
@@ -6959,8 +6964,27 @@ static void __accept_page(struct zone *zone, unsigned long *flags,
__free_pages_ok(page, MAX_PAGE_ORDER, FPI_TO_TAIL);
- if (last)
- static_branch_dec(&zones_with_unaccepted_pages);
+ if (last) {
+ /*
+ * There are two corner cases:
+ *
+ * - If allocation occurs during the CPU bring up,
+ * static_branch_dec() cannot be used directly as
+ * it causes a deadlock on cpu_hotplug_lock.
+ *
+ * Instead, use schedule_work() to prevent deadlock.
+ *
+ * - If allocation occurs before workqueues are initialized,
+ * static_branch_dec() should be directly.
+ *
+ * Workqueues are initialized before CPU bring up, so this
+ * will not conflict with the first scenario.
+ */
+ if (system_wq)
+ schedule_work(&zone->unaccepted_cleanup);
+ else
+ unaccepted_cleanup_work(&zone->unaccepted_cleanup);
+ }
}
void accept_page(struct page *page)
--
Kiryl Shutsemau / Kirill A. Shutemov