Re: [PATCH v3] lockdep: add lockdep_cleanup_dead_cpu()

[David Woodhouse <dwmw2@infradead.org>]

[-- Attachment #1: Type: text/plain, Size: 2472 bytes --]

On Thu, 2024-09-26 at 05:34 -0700, Boqun Feng wrote:
> On Thu, Sep 26, 2024 at 01:16:32PM +0100, David Woodhouse wrote:
> > On Thu, 2024-09-26 at 05:09 -0700, Boqun Feng wrote:
> > > 
> > > 
> > > I think this is already fixed by:
> > > 
> > >         9bb69ba4c177 ("ACPI: processor_idle: use raw_safe_halt() in acpi_idle_play_dead()")
> > > 
> > > , no?
> > 
> > That patch fixed the bug.
> > 
> > *This* patch fixes the fact that lockdep didn't *tell* us about the bug.
> 
> But I thought along with the above commit, Peter also made it possible
> that objtool can detect leaving noinstr section in the offline path? Do
> you have a case where you can alter hardirqs_enabled flag in offline
> path but don't hit the objtool warning?

I do not recall such. Peter?

IIRC the bug fixed by commit 9bb69ba4c177 only showed up under real
Xen, as QEMU doesn't expose processor C-states. So I reintroduced the
equivalent bug by doing this instead:

--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1390,7 +1390,7 @@ void __noreturn hlt_play_dead(void)
                wbinvd();
 
        while (1)
-               native_halt();
+               safe_halt();
 }
 

Without this patch, I get a triple-fault on bringing the CPU back
online as before. With it, as intended, I get a warning, but success:

[root@localhost ~]# echo 0 > /sys/devices/system/cpu/cpu1/online 
[   42.090839] smpboot: CPU 1 is now offline
[   42.091989] CPU 1 left hardirqs enabled!
[   42.091997] irq event stamp: 144559
[   42.094155] hardirqs last  enabled at (144559): [<ffffffff89098b2e>] hlt_play_dead+0x1e/0x30
[   42.096196] hardirqs last disabled at (144558): [<ffffffff891800ee>] do_idle+0xae/0x260
[   42.098062] softirqs last  enabled at (144530): [<ffffffff89107260>] __irq_exit_rcu+0xb0/0xd0
[   42.100056] softirqs last disabled at (144519): [<ffffffff89107260>] __irq_exit_rcu+0xb0/0xd0
[root@localhost ~]# echo 1 > /sys/devices/system/cpu/cpu1/online 
[   47.480889] installing Xen timer for CPU 1
[   47.485308] smpboot: Booting Node 0 Processor 1 APIC 0x1
[   47.491569] cpu 1 spinlock event irq 35


So I think the patch is still applicable.

> Anyway, the commit log needs a rework.

Sure. Other than to refer to commit 9bb69ba4c177 instead of the mailing
list message, is there anything else that needs changing? I suppose I
should drop the word 'recently' from '...was recently observed'? :)


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]