* Re: [PATCH] lockdep: Fix static memory detection even more [not found] ` <081537c0-0d74-0242-451a-e6bd6f71cdd9@roeck-us.net> @ 2023-09-07 7:40 ` Helge Deller 2023-09-08 20:52 ` Guenter Roeck 0 siblings, 1 reply; 2+ messages in thread From: Helge Deller @ 2023-09-07 7:40 UTC (permalink / raw) To: Guenter Roeck, Huacai Chen, loongarch Cc: linux-kernel, Rafael J. Wysocki, Borislav Petkov, Peter Zijlstra * Guenter Roeck <linux@roeck-us.net>: > On 9/6/23 00:18, Helge Deller wrote: > > * Guenter Roeck <linux@roeck-us.net>: > > > On 9/3/23 14:11, Helge Deller wrote: > > > > * Guenter Roeck <linux@roeck-us.net>: > > > > > Hi, > > > > > > > > > > On Sat, Aug 12, 2023 at 05:48:52PM +0200, Helge Deller wrote: > > > > > > On the parisc architecture, lockdep reports for all static objects which > > > > > > are in the __initdata section (e.g. "setup_done" in devtmpfs, > > > > > > "kthreadd_done" in init/main.c) this warning: > > > > > > > > > > > > INFO: trying to register non-static key. > > > > > > > > > > > > The warning itself is wrong, because those objects are in the __initdata > > > > > > section, but the section itself is on parisc outside of range from > > > > > > _stext to _end, which is why the static_obj() functions returns a wrong > > > > > > answer. > > > > > > > > > > > > While fixing this issue, I noticed that the whole existing check can > > > > > > be simplified a lot. > > > > > > Instead of checking against the _stext and _end symbols (which include > > > > > > code areas too) just check for the .data and .bss segments (since we check a > > > > > > data object). This can be done with the existing is_kernel_core_data() > > > > > > macro. > > > > > > > > > > > > In addition objects in the __initdata section can be checked with > > > > > > init_section_contains(). > > > > > > > > > > > > This partly reverts and simplifies commit bac59d18c701 ("x86/setup: Fix static > > > > > > memory detection"). > > > > > > > > > > > > Tested on x86-64 and parisc. > > > > > > > > > > > > Signed-off-by: Helge Deller <deller@gmx.de> > > > > > > Fixes: bac59d18c701 ("x86/setup: Fix static memory detection") > > > > > > > > > > On loongarch, this patch results in the following backtrace. > > > > > > > > > > EFI stub: Loaded initrd from LINUX_EFI_INITRD_MEDIA_GUID device path > > > > > EFI stub: Exiting boot services > > > > > [ 0.000000] INFO: trying to register non-static key. > > > > > [ 0.000000] The code is fine but needs lockdep annotation, or maybe > > > > > [ 0.000000] you didn't initialize this object before use? > > > > > [ 0.000000] turning off the locking correctness validator. > > > > > [ 0.000000] ... > > > > > [ 0.000000] Call Trace: > > > > > [ 0.000000] [<9000000000223d84>] show_stack+0x5c/0x180 > > > > > [ 0.000000] [<900000000153e0b4>] dump_stack_lvl+0x88/0xd0 > > > > > [ 0.000000] [<90000000002bc548>] register_lock_class+0x768/0x770 > > > > > [ 0.000000] [<90000000002bc710>] __lock_acquire+0xb0/0x2a18 > > > > > [ 0.000000] [<90000000002bba1c>] lock_acquire+0x11c/0x328 > > > > > [ 0.000000] [<9000000000b34a60>] __debug_object_init+0x60/0x244 > > > > > [ 0.000000] [<9000000000337f94>] init_cgroup_housekeeping+0xe8/0x144 > > > > > [ 0.000000] [<900000000033e364>] init_cgroup_root+0x38/0xa0 > > > > > [ 0.000000] [<90000000017801ac>] cgroup_init_early+0x44/0x16c > > > > > [ 0.000000] [<9000000001770758>] start_kernel+0x50/0x624 > > > > > [ 0.000000] [<90000000015410b4>] kernel_entry+0xb4/0xc4 > > > > > > > > > > Reverting it fixes the problem. Bisect log attached. > > > > > > > > > > This is also seen in v6.5.y and v6.4.y since the patch has been applied > > > > > to those branches. > > > > > > > > Does this happens with CONFIG_SMP=n ? > > > > If so, I think the untested patch below might fix the issue. > > > > > > > > > > No, this is loongarch:defconfig with various debug options enabled. > > > That has CONFIG_SMP=y. > > > > Could you apply below patch and verify with the contents of the > > System.map file where the lock is located ? > > > > diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c > > index e85b5ad3e206..db0a301f9740 100644 > > --- a/kernel/locking/lockdep.c > > +++ b/kernel/locking/lockdep.c > > @@ -969,7 +969,7 @@ static bool assign_lock_key(struct lockdep_map *lock) > > else { > > /* Debug-check: all keys must be persistent! */ > > debug_locks_off(); > > - pr_err("INFO: trying to register non-static key.\n"); > > + pr_err("INFO: trying to register non-static key at %08lx.\n", addr); > > pr_err("The code is fine but needs lockdep annotation, or maybe\n"); > > pr_err("you didn't initialize this object before use?\n"); > > pr_err("turning off the locking correctness validator.\n"); > > 90000000015602d0 D __la_abs_end > ... > 90000000016815c0 d fill_pool_map.3 <--- lock pointer > ... > 9000000001770000 T __init_begin The problem is, that loongarch's vmlinux.lds.S file puts data into areas which are not marked "data" or "code". can you try below patch? The relevant part is here the move of ".data.rel : { *(.data.rel*) }" into the data section. The other parts are suggestions. Helge diff --git a/arch/loongarch/kernel/vmlinux.lds.S b/arch/loongarch/kernel/vmlinux.lds.S index b1686afcf876..bb2ec86f37a8 100644 --- a/arch/loongarch/kernel/vmlinux.lds.S +++ b/arch/loongarch/kernel/vmlinux.lds.S @@ -53,33 +53,6 @@ SECTIONS . = ALIGN(PECOFF_SEGMENT_ALIGN); _etext = .; - /* - * struct alt_inst entries. From the header (alternative.h): - * "Alternative instructions for different CPU types or capabilities" - * Think locking instructions on spinlocks. - */ - . = ALIGN(4); - .altinstructions : AT(ADDR(.altinstructions) - LOAD_OFFSET) { - __alt_instructions = .; - *(.altinstructions) - __alt_instructions_end = .; - } - -#ifdef CONFIG_RELOCATABLE - . = ALIGN(8); - .la_abs : AT(ADDR(.la_abs) - LOAD_OFFSET) { - __la_abs_begin = .; - *(.la_abs) - __la_abs_end = .; - } -#endif - - .got : ALIGN(16) { *(.got) } - .plt : ALIGN(16) { *(.plt) } - .got.plt : ALIGN(16) { *(.got.plt) } - - .data.rel : { *(.data.rel*) } - . = ALIGN(PECOFF_SEGMENT_ALIGN); __init_begin = .; __inittext_begin = .; @@ -94,6 +67,18 @@ SECTIONS __initdata_begin = .; + /* + * struct alt_inst entries. From the header (alternative.h): + * "Alternative instructions for different CPU types or capabilities" + * Think locking instructions on spinlocks. + */ + . = ALIGN(4); + .altinstructions : AT(ADDR(.altinstructions) - LOAD_OFFSET) { + __alt_instructions = .; + *(.altinstructions) + __alt_instructions_end = .; + } + INIT_DATA_SECTION(16) .exit.data : { EXIT_DATA @@ -113,6 +98,11 @@ SECTIONS _sdata = .; RO_DATA(4096) + + .got : ALIGN(16) { *(.got) } + .plt : ALIGN(16) { *(.plt) } + .got.plt : ALIGN(16) { *(.got.plt) } + RW_DATA(1 << CONFIG_L1_CACHE_SHIFT, PAGE_SIZE, THREAD_SIZE) .rela.dyn : ALIGN(8) { @@ -121,6 +111,17 @@ SECTIONS __rela_dyn_end = .; } + .data.rel : { *(.data.rel*) } + +#ifdef CONFIG_RELOCATABLE + . = ALIGN(8); + .la_abs : AT(ADDR(.la_abs) - LOAD_OFFSET) { + __la_abs_begin = .; + *(.la_abs) + __la_abs_end = .; + } +#endif + .sdata : { *(.sdata) } ^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] lockdep: Fix static memory detection even more 2023-09-07 7:40 ` [PATCH] lockdep: Fix static memory detection even more Helge Deller @ 2023-09-08 20:52 ` Guenter Roeck 0 siblings, 0 replies; 2+ messages in thread From: Guenter Roeck @ 2023-09-08 20:52 UTC (permalink / raw) To: Helge Deller, Huacai Chen, loongarch Cc: linux-kernel, Rafael J. Wysocki, Borislav Petkov, Peter Zijlstra On 9/7/23 00:40, Helge Deller wrote: > * Guenter Roeck <linux@roeck-us.net>: >> On 9/6/23 00:18, Helge Deller wrote: >>> * Guenter Roeck <linux@roeck-us.net>: >>>> On 9/3/23 14:11, Helge Deller wrote: >>>>> * Guenter Roeck <linux@roeck-us.net>: >>>>>> Hi, >>>>>> >>>>>> On Sat, Aug 12, 2023 at 05:48:52PM +0200, Helge Deller wrote: >>>>>>> On the parisc architecture, lockdep reports for all static objects which >>>>>>> are in the __initdata section (e.g. "setup_done" in devtmpfs, >>>>>>> "kthreadd_done" in init/main.c) this warning: >>>>>>> >>>>>>> INFO: trying to register non-static key. >>>>>>> >>>>>>> The warning itself is wrong, because those objects are in the __initdata >>>>>>> section, but the section itself is on parisc outside of range from >>>>>>> _stext to _end, which is why the static_obj() functions returns a wrong >>>>>>> answer. >>>>>>> >>>>>>> While fixing this issue, I noticed that the whole existing check can >>>>>>> be simplified a lot. >>>>>>> Instead of checking against the _stext and _end symbols (which include >>>>>>> code areas too) just check for the .data and .bss segments (since we check a >>>>>>> data object). This can be done with the existing is_kernel_core_data() >>>>>>> macro. >>>>>>> >>>>>>> In addition objects in the __initdata section can be checked with >>>>>>> init_section_contains(). >>>>>>> >>>>>>> This partly reverts and simplifies commit bac59d18c701 ("x86/setup: Fix static >>>>>>> memory detection"). >>>>>>> >>>>>>> Tested on x86-64 and parisc. >>>>>>> >>>>>>> Signed-off-by: Helge Deller <deller@gmx.de> >>>>>>> Fixes: bac59d18c701 ("x86/setup: Fix static memory detection") >>>>>> >>>>>> On loongarch, this patch results in the following backtrace. >>>>>> >>>>>> EFI stub: Loaded initrd from LINUX_EFI_INITRD_MEDIA_GUID device path >>>>>> EFI stub: Exiting boot services >>>>>> [ 0.000000] INFO: trying to register non-static key. >>>>>> [ 0.000000] The code is fine but needs lockdep annotation, or maybe >>>>>> [ 0.000000] you didn't initialize this object before use? >>>>>> [ 0.000000] turning off the locking correctness validator. >>>>>> [ 0.000000] ... >>>>>> [ 0.000000] Call Trace: >>>>>> [ 0.000000] [<9000000000223d84>] show_stack+0x5c/0x180 >>>>>> [ 0.000000] [<900000000153e0b4>] dump_stack_lvl+0x88/0xd0 >>>>>> [ 0.000000] [<90000000002bc548>] register_lock_class+0x768/0x770 >>>>>> [ 0.000000] [<90000000002bc710>] __lock_acquire+0xb0/0x2a18 >>>>>> [ 0.000000] [<90000000002bba1c>] lock_acquire+0x11c/0x328 >>>>>> [ 0.000000] [<9000000000b34a60>] __debug_object_init+0x60/0x244 >>>>>> [ 0.000000] [<9000000000337f94>] init_cgroup_housekeeping+0xe8/0x144 >>>>>> [ 0.000000] [<900000000033e364>] init_cgroup_root+0x38/0xa0 >>>>>> [ 0.000000] [<90000000017801ac>] cgroup_init_early+0x44/0x16c >>>>>> [ 0.000000] [<9000000001770758>] start_kernel+0x50/0x624 >>>>>> [ 0.000000] [<90000000015410b4>] kernel_entry+0xb4/0xc4 >>>>>> >>>>>> Reverting it fixes the problem. Bisect log attached. >>>>>> >>>>>> This is also seen in v6.5.y and v6.4.y since the patch has been applied >>>>>> to those branches. >>>>> >>>>> Does this happens with CONFIG_SMP=n ? >>>>> If so, I think the untested patch below might fix the issue. >>>>> >>>> >>>> No, this is loongarch:defconfig with various debug options enabled. >>>> That has CONFIG_SMP=y. >>> >>> Could you apply below patch and verify with the contents of the >>> System.map file where the lock is located ? >>> >>> diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c >>> index e85b5ad3e206..db0a301f9740 100644 >>> --- a/kernel/locking/lockdep.c >>> +++ b/kernel/locking/lockdep.c >>> @@ -969,7 +969,7 @@ static bool assign_lock_key(struct lockdep_map *lock) >>> else { >>> /* Debug-check: all keys must be persistent! */ >>> debug_locks_off(); >>> - pr_err("INFO: trying to register non-static key.\n"); >>> + pr_err("INFO: trying to register non-static key at %08lx.\n", addr); >>> pr_err("The code is fine but needs lockdep annotation, or maybe\n"); >>> pr_err("you didn't initialize this object before use?\n"); >>> pr_err("turning off the locking correctness validator.\n"); >> >> 90000000015602d0 D __la_abs_end >> ... >> 90000000016815c0 d fill_pool_map.3 <--- lock pointer >> ... >> 9000000001770000 T __init_begin > > The problem is, that loongarch's vmlinux.lds.S file puts data into > areas which are not marked "data" or "code". > > can you try below patch? > The relevant part is here the move of ".data.rel : { *(.data.rel*) }" > into the data section. > The other parts are suggestions. > The patch below fixes the problem for me. Guenter > Helge > > > diff --git a/arch/loongarch/kernel/vmlinux.lds.S b/arch/loongarch/kernel/vmlinux.lds.S > index b1686afcf876..bb2ec86f37a8 100644 > --- a/arch/loongarch/kernel/vmlinux.lds.S > +++ b/arch/loongarch/kernel/vmlinux.lds.S > @@ -53,33 +53,6 @@ SECTIONS > . = ALIGN(PECOFF_SEGMENT_ALIGN); > _etext = .; > > - /* > - * struct alt_inst entries. From the header (alternative.h): > - * "Alternative instructions for different CPU types or capabilities" > - * Think locking instructions on spinlocks. > - */ > - . = ALIGN(4); > - .altinstructions : AT(ADDR(.altinstructions) - LOAD_OFFSET) { > - __alt_instructions = .; > - *(.altinstructions) > - __alt_instructions_end = .; > - } > - > -#ifdef CONFIG_RELOCATABLE > - . = ALIGN(8); > - .la_abs : AT(ADDR(.la_abs) - LOAD_OFFSET) { > - __la_abs_begin = .; > - *(.la_abs) > - __la_abs_end = .; > - } > -#endif > - > - .got : ALIGN(16) { *(.got) } > - .plt : ALIGN(16) { *(.plt) } > - .got.plt : ALIGN(16) { *(.got.plt) } > - > - .data.rel : { *(.data.rel*) } > - > . = ALIGN(PECOFF_SEGMENT_ALIGN); > __init_begin = .; > __inittext_begin = .; > @@ -94,6 +67,18 @@ SECTIONS > > __initdata_begin = .; > > + /* > + * struct alt_inst entries. From the header (alternative.h): > + * "Alternative instructions for different CPU types or capabilities" > + * Think locking instructions on spinlocks. > + */ > + . = ALIGN(4); > + .altinstructions : AT(ADDR(.altinstructions) - LOAD_OFFSET) { > + __alt_instructions = .; > + *(.altinstructions) > + __alt_instructions_end = .; > + } > + > INIT_DATA_SECTION(16) > .exit.data : { > EXIT_DATA > @@ -113,6 +98,11 @@ SECTIONS > > _sdata = .; > RO_DATA(4096) > + > + .got : ALIGN(16) { *(.got) } > + .plt : ALIGN(16) { *(.plt) } > + .got.plt : ALIGN(16) { *(.got.plt) } > + > RW_DATA(1 << CONFIG_L1_CACHE_SHIFT, PAGE_SIZE, THREAD_SIZE) > > .rela.dyn : ALIGN(8) { > @@ -121,6 +111,17 @@ SECTIONS > __rela_dyn_end = .; > } > > + .data.rel : { *(.data.rel*) } > + > +#ifdef CONFIG_RELOCATABLE > + . = ALIGN(8); > + .la_abs : AT(ADDR(.la_abs) - LOAD_OFFSET) { > + __la_abs_begin = .; > + *(.la_abs) > + __la_abs_end = .; > + } > +#endif > + > .sdata : { > *(.sdata) > } > ^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-09-08 20:52 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <ZNep5EcYskP9HtGD@p100>
[not found] ` <b7526bf6-886f-457a-beba-84ae9f75bc77@roeck-us.net>
[not found] ` <ZPT2cINsHd+sWJQU@ls3530>
[not found] ` <78473084-d4d5-685f-9778-4bbe8878a43e@roeck-us.net>
[not found] ` <ZPgnslFTlwImJ+Aq@ls3530>
[not found] ` <081537c0-0d74-0242-451a-e6bd6f71cdd9@roeck-us.net>
2023-09-07 7:40 ` [PATCH] lockdep: Fix static memory detection even more Helge Deller
2023-09-08 20:52 ` Guenter Roeck
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox