Messages in this thread | | | Date | Fri, 15 Sep 2023 22:19:15 +0800 | Subject | Re: [PATCH] LoongArch: Fix lockdep static memory detection | From | WANG Xuerui <> |
| |
Hi,
On 9/15/23 22:07, Guenter Roeck wrote: > Hi Helge, > > On 9/15/23 03:10, Helge Deller wrote: >> On 9/15/23 11:23, Huacai Chen wrote: >>> On Fri, Sep 15, 2023 at 4:16 PM Helge Deller <deller@gmx.de> wrote: >>>> >>>> On 9/15/23 05:22, Huacai Chen wrote: >>>>> Hi Helge, >>>>> >>>>> On Wed, Sep 13, 2023 at 3:18 AM Helge Deller <deller@gmx.de> wrote: >>>>>> >>>>>> Since commit 0a6b58c5cd0d ("lockdep: fix static memory detection >>>>>> even >>>>>> more") the lockdep code uses is_kernel_core_data(), >>>>>> is_kernel_rodata() >>>>>> and init_section_contains() to verify if a lock is located inside a >>>>>> kernel static data section. >>>>>> >>>>>> This change triggers a failure on LoongArch, for which the >>>>>> vmlinux.lds.S >>>>>> script misses to put the locks (as part of in the .data.rel symbols) >>>>>> into the Linux data section. >>>>>> This patch fixes the lockdep problem by moving *(.data.rel*) symbols >>>>>> into the kernel data section (from _sdata to _edata). >>>>>> >>>>>> Additionally, move other wrongly assigned symbols too: >>>>>> - altinstructions into the _initdata section, >>>> >>>>> I think altinstructions cannot be put into _initdata because it will >>>>> be used by modules. >>>> >>>> No. >>>> arch/loongarch/kernel/vmlinux.lds.S is used for the static parts of >>>> the kernel >>>> and altinstructions are replaced before modules are loaded. >>>> For altinstructions in modules the linker script >>>> scripts/module.lds.S is used. >> >>> OK, then what about .got/.plt? It seems arm64 also doesn't put them in >>> the data section. >> >> arm64 seems to throw away all plt entries already at link time (and >> just keeps >> the got.plt in the read-only data section). >> It even checks at link time, that there are no plt entries in the >> binary: >> ASSERT(SIZEOF(.plt) == 0, "Unexpected run-time procedure >> linkages detected!") >> >> I don't know for loongarch, but if you need the plt entries for >> loongarch, it's >> safest & best to put them into the read-only data section too, which >> is what my patch does. >> Up to now, you have them completely outside of code & data sections. >> >> In the end you need to decide for your platform. My patch is a >> suggestion, which I think >> is correct (untested by me, but Guenter replied he tested it). >> But to fix the lockdep problem at minimum the move of the .data.rel >> section >> is needed. >> > > Just my $0.02 .. it might make sense to concentrate on the minimum to > get the immediate > problem fixed. Loongarch maintainers can then decide at their own pace > if they want > to apply any of the other changes you suggested. After all, unless I > am missing > something, those additional changes are not really needed in stable > releases.
Sorry for coming late, but as reviewer of arch/loongarch, I'd agree with Guenter and Helge here: let's fix the immediate problem and investigate the rest later -- it's not like the problems are *definitely* orthogonal in this case, and at least *some* progress would be appreciated.
I'll try to reproduce the problem and test the fix during the weekend, so hopefully Huacai can get the fix in before -rc2 or -rc3. Thanks for the attention and fix.
-- WANG "xen0n" Xuerui
Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/
| |