| Message ID | 20220714025901.359695-3-xianting.tian@linux.alibaba.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Improve vmcoreinfo and memory layout dump | expand |
On Thu, Jul 14, 2022 at 4:59 AM Xianting Tian <xianting.tian@linux.alibaba.com> wrote: > > As MODULES is only defined for CONFIG_64BIT, so we dump it when > CONFIG_64BIT. Doesn't this cause a compile-time error on 32-bit? > (unsigned long)VMEMMAP_END); > print_ml("vmalloc", (unsigned long)VMALLOC_START, > (unsigned long)VMALLOC_END); > + if (IS_ENABLED(CONFIG_64BIT)) > + print_ml("modules", (unsigned long)MODULES_VADDR, > + (unsigned long)MODULES_END); The IS_ENABLED() check prevents the line from getting executed, but unlike an #ifdef it still relies on it to be parsable. Arnd
在 2022/7/14 下午4:24, Arnd Bergmann 写道: > On Thu, Jul 14, 2022 at 4:59 AM Xianting Tian > <xianting.tian@linux.alibaba.com> wrote: >> As MODULES is only defined for CONFIG_64BIT, so we dump it when >> CONFIG_64BIT. > Doesn't this cause a compile-time error on 32-bit? I tested, rv32 compile is OK. > >> (unsigned long)VMEMMAP_END); >> print_ml("vmalloc", (unsigned long)VMALLOC_START, >> (unsigned long)VMALLOC_END); >> + if (IS_ENABLED(CONFIG_64BIT)) >> + print_ml("modules", (unsigned long)MODULES_VADDR, >> + (unsigned long)MODULES_END); > The IS_ENABLED() check prevents the line from getting executed, but > unlike an #ifdef it still relies on it to be parsable. Thanks, I will use #ifdef instead of IS_ENABLED > > Arnd
Am Donnerstag, 14. Juli 2022, 11:17:26 CEST schrieb Xianting Tian: > > 在 2022/7/14 下午4:24, Arnd Bergmann 写道: > > On Thu, Jul 14, 2022 at 4:59 AM Xianting Tian > > <xianting.tian@linux.alibaba.com> wrote: > >> As MODULES is only defined for CONFIG_64BIT, so we dump it when > >> CONFIG_64BIT. > > Doesn't this cause a compile-time error on 32-bit? > I tested, rv32 compile is OK. > >> (unsigned long)VMEMMAP_END); > >> print_ml("vmalloc", (unsigned long)VMALLOC_START, > >> (unsigned long)VMALLOC_END); > >> + if (IS_ENABLED(CONFIG_64BIT)) > >> + print_ml("modules", (unsigned long)MODULES_VADDR, > >> + (unsigned long)MODULES_END); > > The IS_ENABLED() check prevents the line from getting executed, but > > unlike an #ifdef it still relies on it to be parsable. > Thanks, I will use #ifdef instead of IS_ENABLED Patch1 also has that issue with the if (IS_ENABLED(CONFIG_64BIT)) { vmcoreinfo_append_str("NUMBER(MODULES_VADDR)=0x%lx\n", MODULES_VADDR); vmcoreinfo_append_str("NUMBER(MODULES_END)=0x%lx\n", MODULES_END); .... module_alloc falls back to a weak variant [0] which is the same as the riscv variant only then using VMALLOC_START - VMALLOC_END as range, as the riscv-variant conditional to CONFIG_64BIT. I'm wondering if it wouldn't be easier in the long run to just define MODULES_VADDR, MODULES_END for 32bit to use these values and get rid of the CONFIG_64BIT ifdef we already have for MODULES (and new ones we are introducing now). Heiko [0] https://elixir.bootlin.com/linux/latest/source/kernel/module.c#L2835
在 2022/7/14 下午5:46, Heiko Stübner 写道: > Am Donnerstag, 14. Juli 2022, 11:17:26 CEST schrieb Xianting Tian: >> 在 2022/7/14 下午4:24, Arnd Bergmann 写道: >>> On Thu, Jul 14, 2022 at 4:59 AM Xianting Tian >>> <xianting.tian@linux.alibaba.com> wrote: >>>> As MODULES is only defined for CONFIG_64BIT, so we dump it when >>>> CONFIG_64BIT. >>> Doesn't this cause a compile-time error on 32-bit? >> I tested, rv32 compile is OK. >>>> (unsigned long)VMEMMAP_END); >>>> print_ml("vmalloc", (unsigned long)VMALLOC_START, >>>> (unsigned long)VMALLOC_END); >>>> + if (IS_ENABLED(CONFIG_64BIT)) >>>> + print_ml("modules", (unsigned long)MODULES_VADDR, >>>> + (unsigned long)MODULES_END); >>> The IS_ENABLED() check prevents the line from getting executed, but >>> unlike an #ifdef it still relies on it to be parsable. >> Thanks, I will use #ifdef instead of IS_ENABLED > Patch1 also has that issue with the thanks, I will modify it in V3. > > if (IS_ENABLED(CONFIG_64BIT)) { > vmcoreinfo_append_str("NUMBER(MODULES_VADDR)=0x%lx\n", MODULES_VADDR); > vmcoreinfo_append_str("NUMBER(MODULES_END)=0x%lx\n", MODULES_END); > .... > > > module_alloc falls back to a weak variant [0] which is the same as the riscv variant > only then using VMALLOC_START - VMALLOC_END as range, as the riscv-variant > conditional to CONFIG_64BIT. yes, I ever checked, actually before 5.13, it doesn't define MODULES area but use VMALLOC for modules, crash> mod MODULE NAME BASE SIZE OBJECT FILE ffffffdf8167f280 galcore ffffffdf81646000 3075841 (not loaded) [CONFIG_KALLSYMS] [ 0.000000] vmalloc : 0xffffffd000000000 - 0xffffffdfffffffff (65535 MB) > > I'm wondering if it wouldn't be easier in the long run to just define > MODULES_VADDR, MODULES_END for 32bit to use these values and get rid of > the CONFIG_64BIT ifdef we already have for MODULES (and new ones we are > introducing now). > > > Heiko > > > [0] https://elixir.bootlin.com/linux/latest/source/kernel/module.c#L2835 > >
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c index d466ec670e1f..bbc9431e9042 100644 --- a/arch/riscv/mm/init.c +++ b/arch/riscv/mm/init.c @@ -135,6 +135,9 @@ static void __init print_vm_layout(void) (unsigned long)VMEMMAP_END); print_ml("vmalloc", (unsigned long)VMALLOC_START, (unsigned long)VMALLOC_END); + if (IS_ENABLED(CONFIG_64BIT)) + print_ml("modules", (unsigned long)MODULES_VADDR, + (unsigned long)MODULES_END); print_ml("lowmem", (unsigned long)PAGE_OFFSET, (unsigned long)high_memory); if (IS_ENABLED(CONFIG_64BIT)) {
Modules always live before the kernel, MODULES_END is fixed but MODULES_VADDR isn't fixed, it depends on the kernel size. Let's add it to virtual kernel memory layout dump. As MODULES is only defined for CONFIG_64BIT, so we dump it when CONFIG_64BIT. eg, MODULES_VADDR - MODULES_END 0xffffffff01133000 - 0xffffffff80000000 Signed-off-by: Xianting Tian <xianting.tian@linux.alibaba.com> --- arch/riscv/mm/init.c | 3 +++ 1 file changed, 3 insertions(+)