| Message ID | ZNZcBkiVkm87+Tvr@p100 (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | init: Add lockdep annotation to kthreadd_done completer | expand |
On Fri, 11 Aug 2023 18:04:22 +0200 Helge Deller <deller@gmx.de> wrote: > Add the missing lockdep annotation to avoid this warning: > > INFO: trying to register non-static key. > The code is fine but needs lockdep annotation, or maybe > you didn't initialize this object before use? > turning off the locking correctness validator. > CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.5.0-rc5+ #681 > Hardware name: 9000/785/C3700 > Backtrace: > [<000000004030bcd0>] show_stack+0x74/0xb0 > [<0000000041469c7c>] dump_stack_lvl+0x104/0x180 > [<0000000041469d2c>] dump_stack+0x34/0x48 > [<000000004040e5b4>] register_lock_class+0xd24/0xd30 > [<000000004040c21c>] __lock_acquire.isra.0+0xb4/0xac8 > [<000000004040cd60>] lock_acquire+0x130/0x298 > [<000000004146df54>] _raw_spin_lock_irq+0x60/0xb8 > [<0000000041472044>] wait_for_completion+0xa0/0x2d0 > [<000000004146b544>] kernel_init+0x48/0x3a8 > [<0000000040302020>] ret_from_kernel_thread+0x20/0x28 > > ... > > --- a/init/main.c > +++ b/init/main.c > @@ -682,6 +682,8 @@ noinline void __ref __noreturn rest_init(void) > struct task_struct *tsk; > int pid; > > + init_completion(&kthreadd_done); > + > rcu_scheduler_starting(); > /* > * We need to spawn init first so that it obtains pid 1, however This is pretty old code, as is the page_alloc_init_late() change. Do we know why this warning has just turned up lately? I'll add cc:stable to these, but might take that away again if we can answer the above. btw, please don't forget the "^---$" between changelog and patch.
On 8/11/23 18:43, Andrew Morton wrote: > On Fri, 11 Aug 2023 18:04:22 +0200 Helge Deller <deller@gmx.de> wrote: > >> Add the missing lockdep annotation to avoid this warning: >> >> INFO: trying to register non-static key. >> The code is fine but needs lockdep annotation, or maybe >> you didn't initialize this object before use? >> turning off the locking correctness validator. >> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.5.0-rc5+ #681 >> Hardware name: 9000/785/C3700 >> Backtrace: >> [<000000004030bcd0>] show_stack+0x74/0xb0 >> [<0000000041469c7c>] dump_stack_lvl+0x104/0x180 >> [<0000000041469d2c>] dump_stack+0x34/0x48 >> [<000000004040e5b4>] register_lock_class+0xd24/0xd30 >> [<000000004040c21c>] __lock_acquire.isra.0+0xb4/0xac8 >> [<000000004040cd60>] lock_acquire+0x130/0x298 >> [<000000004146df54>] _raw_spin_lock_irq+0x60/0xb8 >> [<0000000041472044>] wait_for_completion+0xa0/0x2d0 >> [<000000004146b544>] kernel_init+0x48/0x3a8 >> [<0000000040302020>] ret_from_kernel_thread+0x20/0x28 >> >> ... >> >> --- a/init/main.c >> +++ b/init/main.c >> @@ -682,6 +682,8 @@ noinline void __ref __noreturn rest_init(void) >> struct task_struct *tsk; >> int pid; >> >> + init_completion(&kthreadd_done); >> + >> rcu_scheduler_starting(); >> /* >> * We need to spawn init first so that it obtains pid 1, however > > This is pretty old code, as is the page_alloc_init_late() change. Do > we know why this warning has just turned up lately? I haven't debugged in depth yet, but here is what I believe is the reason why I do see those lockdep warnings and others not. I'm building & testing on the parisc platform. Just recently I added lockdep support to parisc for kernel 6.4 and backported it to v6.0+. Since then I've seen the warnings. And I think the main reason why I see those warnings on parisc and others on other platforms don't, is that parisc is the only architecture where __ARCH_SPIN_LOCK_UNLOCKED_VAL is NOT zero (0x1a46 actually). The reason is that parisc offers only one assembler instruction which operates atomicly on memory, and which we use to lock spinlocks: ldcw ("load and clear word"). So, a "zero" spinlock word means the lock is locked, while non-zero means it's unlocked. For other platforms it's the other way around. So, for a structure in e.g. __initdata[] which is pre-initialized by the compiler, the spinlocks are locked by default (lockword = 0) on parisc, if they haven't been initialized correctly, and thus the kernel will complain at runtime. Now, maybe lockdep doesn't use spinlocks per se. I have't checked in depth yet, but I'm sure it's somehow related to the odd __ARCH_SPIN_LOCK_UNLOCKED_VAL value of parisc. I wonder if the same bug appears if you use a non-zero __ARCH_SPIN_LOCK_UNLOCKED_VAL on other platforms too (if it's possible). > I'll add cc:stable to these, but might take that away again if we can > answer the above. Thanks for adding the patches. I did sent two other patches as well: for watchdog and devtmpfs.. > btw, please don't forget the "^---$" between changelog and patch. Ok. Helge
Hi Andrew, On 8/11/23 19:44, Helge Deller wrote: > On 8/11/23 18:43, Andrew Morton wrote: >> On Fri, 11 Aug 2023 18:04:22 +0200 Helge Deller <deller@gmx.de> wrote: >> >>> Add the missing lockdep annotation to avoid this warning: >>> >>> INFO: trying to register non-static key. >>> The code is fine but needs lockdep annotation, or maybe >>> you didn't initialize this object before use? >>> turning off the locking correctness validator. >>> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.5.0-rc5+ #681 >>> Hardware name: 9000/785/C3700 >>> Backtrace: >>> [<000000004030bcd0>] show_stack+0x74/0xb0 >>> [<0000000041469c7c>] dump_stack_lvl+0x104/0x180 >>> [<0000000041469d2c>] dump_stack+0x34/0x48 >>> [<000000004040e5b4>] register_lock_class+0xd24/0xd30 >>> [<000000004040c21c>] __lock_acquire.isra.0+0xb4/0xac8 >>> [<000000004040cd60>] lock_acquire+0x130/0x298 >>> [<000000004146df54>] _raw_spin_lock_irq+0x60/0xb8 >>> [<0000000041472044>] wait_for_completion+0xa0/0x2d0 >>> [<000000004146b544>] kernel_init+0x48/0x3a8 >>> [<0000000040302020>] ret_from_kernel_thread+0x20/0x28 >>> >>> ... >>> >>> --- a/init/main.c >>> +++ b/init/main.c >>> @@ -682,6 +682,8 @@ noinline void __ref __noreturn rest_init(void) >>> struct task_struct *tsk; >>> int pid; >>> >>> + init_completion(&kthreadd_done); >>> + >>> rcu_scheduler_starting(); >>> /* >>> * We need to spawn init first so that it obtains pid 1, however >> >> This is pretty old code, as is the page_alloc_init_late() change. Do >> we know why this warning has just turned up lately? [dropped Helge's assumptions that it's related to parisc's unusual __ARCH_SPIN_LOCK_UNLOCKED_VAL value. It turned out to be wrong. Now I was able to trace down why I see those lockdep warnings on parisc. The short answer is: On parisc the _initdata section lies outside of the usual kernel _stext ... _end range. Lockdep calls static_obj() which currently assumes that __initdata is inside that range and thus returns "false". That's why lockdep then reports INFO: trying to register non-static key. which is wrong. Please drop those 3 lockdep patches from your mm-queue: mm-add-lockdep-annotation-to-pgdat_init_all_done_comp-completer.patch init-add-lockdep-annotation-to-kthreadd_done-completer.patch watchdog-fix-lockdep-warning.patch I'll send one single patch which fixes static_obj() instead. Thanks, Helge
diff --git a/init/main.c b/init/main.c index ad920fac325c..11870ca752de 100644 --- a/init/main.c +++ b/init/main.c @@ -682,6 +682,8 @@ noinline void __ref __noreturn rest_init(void) struct task_struct *tsk; int pid; + init_completion(&kthreadd_done); + rcu_scheduler_starting(); /* * We need to spawn init first so that it obtains pid 1, however
Add the missing lockdep annotation to avoid this warning: INFO: trying to register non-static key. The code is fine but needs lockdep annotation, or maybe you didn't initialize this object before use? turning off the locking correctness validator. CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.5.0-rc5+ #681 Hardware name: 9000/785/C3700 Backtrace: [<000000004030bcd0>] show_stack+0x74/0xb0 [<0000000041469c7c>] dump_stack_lvl+0x104/0x180 [<0000000041469d2c>] dump_stack+0x34/0x48 [<000000004040e5b4>] register_lock_class+0xd24/0xd30 [<000000004040c21c>] __lock_acquire.isra.0+0xb4/0xac8 [<000000004040cd60>] lock_acquire+0x130/0x298 [<000000004146df54>] _raw_spin_lock_irq+0x60/0xb8 [<0000000041472044>] wait_for_completion+0xa0/0x2d0 [<000000004146b544>] kernel_init+0x48/0x3a8 [<0000000040302020>] ret_from_kernel_thread+0x20/0x28 Signed-off-by: Helge Deller <deller@gmx.de>