Commit 50e782a86c98 ("efi/unaccepted: Fix soft lockups caused
by parallel memory acceptance") has released the spinlock so
other CPUs can do memory acceptance in parallel and not
triggers softlockup on other CPUs.
However the softlock up was intermittent shown up if the memory
of the TD guest is large, and the timeout of softlockup is set
to 1 second:
RIP: 0010:_raw_spin_unlock_irqrestore
Call Trace:
? __hrtimer_run_queues
<IRQ>
? hrtimer_interrupt
? watchdog_timer_fn
? __sysvec_apic_timer_interrupt
? __pfx_watchdog_timer_fn
? sysvec_apic_timer_interrupt
</IRQ>
? __hrtimer_run_queues
<TASK>
? hrtimer_interrupt
? asm_sysvec_apic_timer_interrupt
? _raw_spin_unlock_irqrestore
? __sysvec_apic_timer_interrupt
? sysvec_apic_timer_interrupt
accept_memory
try_to_accept_memory
do_huge_pmd_anonymous_page
get_page_from_freelist
__handle_mm_fault
__alloc_pages
__folio_alloc
? __tdx_hypercall
handle_mm_fault
vma_alloc_folio
do_user_addr_fault
do_huge_pmd_anonymous_page
exc_page_fault
? __do_huge_pmd_anonymous_page
asm_exc_page_fault
__handle_mm_fault
When the local irq is enabled at the end of accept_memory(),
the softlockup detects that the watchdog on single CPU has
not been fed for a while. That is to say, even other CPUs
will not be blocked by spinlock, the current CPU might be
stunk with local irq disabled for a while, which hurts not
only nmi watchdog but also softlockup.
Chao Gao pointed out that the memory accept could be time
costly and there was similar report before. Thus to avoid
any softlocup detection during this stage, give the
softlockup a flag to skip the timeout check at the end of
accept_memory(), by invoking touch_softlockup_watchdog().
Reported-by: "Hossain, Md Iqbal" <[email protected]>
Signed-off-by: Chen Yu <[email protected]>
---
drivers/firmware/efi/unaccepted_memory.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/firmware/efi/unaccepted_memory.c b/drivers/firmware/efi/unaccepted_memory.c
index 5b439d04079c..50f6503fe49f 100644
--- a/drivers/firmware/efi/unaccepted_memory.c
+++ b/drivers/firmware/efi/unaccepted_memory.c
@@ -4,6 +4,7 @@
#include <linux/memblock.h>
#include <linux/spinlock.h>
#include <linux/crash_dump.h>
+#include <linux/nmi.h>
#include <asm/unaccepted_memory.h>
/* Protects unaccepted memory bitmap and accepting_list */
@@ -149,6 +150,9 @@ void accept_memory(phys_addr_t start, phys_addr_t end)
}
list_del(&range.list);
+
+ touch_softlockup_watchdog();
+
spin_unlock_irqrestore(&unaccepted_memory_lock, flags);
}
--
2.25.1
On Wed, Apr 10, 2024 at 06:23:01PM +0800, Chen Yu wrote:
> Commit 50e782a86c98 ("efi/unaccepted: Fix soft lockups caused
> by parallel memory acceptance") has released the spinlock so
> other CPUs can do memory acceptance in parallel and not
> triggers softlockup on other CPUs.
>
> However the softlock up was intermittent shown up if the memory
> of the TD guest is large, and the timeout of softlockup is set
> to 1 second:
>
> RIP: 0010:_raw_spin_unlock_irqrestore
> Call Trace:
> ? __hrtimer_run_queues
> <IRQ>
> ? hrtimer_interrupt
> ? watchdog_timer_fn
> ? __sysvec_apic_timer_interrupt
> ? __pfx_watchdog_timer_fn
> ? sysvec_apic_timer_interrupt
> </IRQ>
> ? __hrtimer_run_queues
> <TASK>
> ? hrtimer_interrupt
> ? asm_sysvec_apic_timer_interrupt
> ? _raw_spin_unlock_irqrestore
> ? __sysvec_apic_timer_interrupt
> ? sysvec_apic_timer_interrupt
> accept_memory
> try_to_accept_memory
> do_huge_pmd_anonymous_page
> get_page_from_freelist
> __handle_mm_fault
> __alloc_pages
> __folio_alloc
> ? __tdx_hypercall
> handle_mm_fault
> vma_alloc_folio
> do_user_addr_fault
> do_huge_pmd_anonymous_page
> exc_page_fault
> ? __do_huge_pmd_anonymous_page
> asm_exc_page_fault
> __handle_mm_fault
Stacktrace doesn't add much value here. Just drop it.
> When the local irq is enabled at the end of accept_memory(),
> the softlockup detects that the watchdog on single CPU has
> not been fed for a while. That is to say, even other CPUs
> will not be blocked by spinlock, the current CPU might be
> stunk with local irq disabled for a while, which hurts not
> only nmi watchdog but also softlockup.
>
> Chao Gao pointed out that the memory accept could be time
> costly and there was similar report before. Thus to avoid
> any softlocup detection during this stage, give the
> softlockup a flag to skip the timeout check at the end of
> accept_memory(), by invoking touch_softlockup_watchdog().
>
> Reported-by: "Hossain, Md Iqbal" <[email protected]>
> Signed-off-by: Chen Yu <[email protected]>
Reviewed-by: Kirill A. Shutemov <[email protected]>
And I think it is justified to add Fixes tag:
Fixes: 50e782a86c98 ("efi/unaccepted: Fix soft lockups caused by parallel memory acceptance")
--
Kiryl Shutsemau / Kirill A. Shutemov
On 2024-04-10 at 15:35:39 +0300, Kirill A. Shutemov wrote:
> On Wed, Apr 10, 2024 at 06:23:01PM +0800, Chen Yu wrote:
> > Commit 50e782a86c98 ("efi/unaccepted: Fix soft lockups caused
> > by parallel memory acceptance") has released the spinlock so
> > other CPUs can do memory acceptance in parallel and not
> > triggers softlockup on other CPUs.
> >
> > However the softlock up was intermittent shown up if the memory
> > of the TD guest is large, and the timeout of softlockup is set
> > to 1 second:
> >
> > RIP: 0010:_raw_spin_unlock_irqrestore
> > Call Trace:
> > ? __hrtimer_run_queues
> > <IRQ>
> > ? hrtimer_interrupt
> > ? watchdog_timer_fn
> > ? __sysvec_apic_timer_interrupt
> > ? __pfx_watchdog_timer_fn
> > ? sysvec_apic_timer_interrupt
> > </IRQ>
> > ? __hrtimer_run_queues
> > <TASK>
> > ? hrtimer_interrupt
> > ? asm_sysvec_apic_timer_interrupt
> > ? _raw_spin_unlock_irqrestore
> > ? __sysvec_apic_timer_interrupt
> > ? sysvec_apic_timer_interrupt
> > accept_memory
> > try_to_accept_memory
> > do_huge_pmd_anonymous_page
> > get_page_from_freelist
> > __handle_mm_fault
> > __alloc_pages
> > __folio_alloc
> > ? __tdx_hypercall
> > handle_mm_fault
> > vma_alloc_folio
> > do_user_addr_fault
> > do_huge_pmd_anonymous_page
> > exc_page_fault
> > ? __do_huge_pmd_anonymous_page
> > asm_exc_page_fault
> > __handle_mm_fault
>
> Stacktrace doesn't add much value here. Just drop it.
>
OK, will do.
> > When the local irq is enabled at the end of accept_memory(),
> > the softlockup detects that the watchdog on single CPU has
> > not been fed for a while. That is to say, even other CPUs
> > will not be blocked by spinlock, the current CPU might be
> > stunk with local irq disabled for a while, which hurts not
> > only nmi watchdog but also softlockup.
> >
> > Chao Gao pointed out that the memory accept could be time
> > costly and there was similar report before. Thus to avoid
> > any softlocup detection during this stage, give the
> > softlockup a flag to skip the timeout check at the end of
> > accept_memory(), by invoking touch_softlockup_watchdog().
> >
> > Reported-by: "Hossain, Md Iqbal" <[email protected]>
> > Signed-off-by: Chen Yu <[email protected]>
>
> Reviewed-by: Kirill A. Shutemov <[email protected]>
>
> And I think it is justified to add Fixes tag:
>
> Fixes: 50e782a86c98 ("efi/unaccepted: Fix soft lockups caused by parallel memory acceptance")
>
OK, I'll refine the patch and send v2, thanks!
thanks,Chenyu