Currently, instruction pointers are printed in [<%08lx>] format to make
them more visible. However, it is not necessary in __show_regs() because
they have the prefix 'pc :' or 'lr :', and it is also inconsistent with
that of other registers, which causes misalignment.
Before:
pc : [<8019a48c>] lr : [<8019a48c>] psr: 60000013
sp : c0965f28 ip : 00000001 fp : 00000001
r10: be6052d8 r9 : 431bde82 r8 : d7b634db
After:
pc : 8019a46c lr : 8019a46c psr: 60000013
sp : c8a71f28 ip : 00000001 fp : 00000002
r10: 1ef6a458 r9 : 431bde82 r8 : d7b634db
Signed-off-by: Zhen Lei <[email protected]>
---
KernelVersion: v5.19
arch/arm/kernel/process.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index 3d9cace63884013..3fb30d734c3568a 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -136,7 +136,7 @@ void __show_regs(struct pt_regs *regs)
printk("PC is at %pS\n", (void *)instruction_pointer(regs));
printk("LR is at %pS\n", (void *)regs->ARM_lr);
- printk("pc : [<%08lx>] lr : [<%08lx>] psr: %08lx\n",
+ printk("pc : %08lx lr : %08lx psr: %08lx\n",
regs->ARM_pc, regs->ARM_lr, regs->ARM_cpsr);
printk("sp : %08lx ip : %08lx fp : %08lx\n",
regs->ARM_sp, regs->ARM_ip, regs->ARM_fp);
--
2.25.1
On Mon, Aug 01, 2022 at 11:20:16AM +0800, Zhen Lei wrote:
> Currently, instruction pointers are printed in [<%08lx>] format to make
> them more visible. However, it is not necessary in __show_regs() because
> they have the prefix 'pc :' or 'lr :', and it is also inconsistent with
> that of other registers, which causes misalignment.
The formatting is not "to make them more visible" - it was to mark the
addresses that we wanted the ksymoops utility to translate to kernel
symbols before we had kallsyms in the kernel. If one disables kallsyms,
then we still need a way to translate kernel addresses to symbols.
I notice there is a script which helps with this that is part of the
kernel source - scripts/decode_stacktrace.sh. I haven't tried this on
arm32 since I always use kallsyms - and I suspect that is rather
universally true as it avoids needing System.map files etc to decode
the oops. That said, if you're building a kernel for small systems,
you probably don't want the overhead of kallsyms.
So, there's an argument for keeping it - it's an API in that it
provides hints to scripting to identify which values need to be
converted to symbols. There's also the argument for getting rid of it,
which is that hardly anyone does that anymore.
The question is, which is the more important argument, and I don't
think there's a definite answer. So I'm inclined to leave this
as-is.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
On 2022/8/8 17:41, Russell King (Oracle) wrote:
> On Mon, Aug 01, 2022 at 11:20:16AM +0800, Zhen Lei wrote:
>> Currently, instruction pointers are printed in [<%08lx>] format to make
>> them more visible. However, it is not necessary in __show_regs() because
>> they have the prefix 'pc :' or 'lr :', and it is also inconsistent with
>> that of other registers, which causes misalignment.
>
> The formatting is not "to make them more visible" - it was to mark the
> addresses that we wanted the ksymoops utility to translate to kernel
> symbols before we had kallsyms in the kernel. If one disables kallsyms,
> then we still need a way to translate kernel addresses to symbols.
I searched the git log and found that the ksymoops utility is discarded.
See:
073a9ecb3a73401662430bb955aedeac1de643d1
However, a commit in the pre-git era [1] had added the statement,
"ksymoops is useless on 2.6. Please use the Oops in its original format".
That statement existed until commit 4eb9241127a0 ("Documentation:
admin-guide: update bug-hunting.rst") finally removed the stale
ksymoops information.
4eb9241127a0b5ac3aaaf1b246728009527ebc86
- delete all references to ksymoops since it is no longer applicable;
>
> I notice there is a script which helps with this that is part of the
> kernel source - scripts/decode_stacktrace.sh. I haven't tried this on
> arm32 since I always use kallsyms - and I suspect that is rather
> universally true as it avoids needing System.map files etc to decode
> the oops. That said, if you're building a kernel for small systems,
> you probably don't want the overhead of kallsyms.
Yes, I read scripts/decode_stacktrace.sh, it requires the format "[<...>]".
But if that's the only concern, maybe we can do the conversion from
"pc: addr" and "lr: addr" to "[<addr>]" first in scripts/decode_stacktrace.sh
I'm usually "objdump -d vmlinux > asm_file", then search "addr:" in asm_file.
Honestly, I think format "[<...>]" is dump_backtrace()'s requirement, not __show_regs()'s.
>
> So, there's an argument for keeping it - it's an API in that it
> provides hints to scripting to identify which values need to be
> converted to symbols. There's also the argument for getting rid of it,
> which is that hardly anyone does that anymore.
>
> The question is, which is the more important argument, and I don't
> think there's a definite answer. So I'm inclined to leave this
> as-is.
OK
>
--
Regards,
Zhen Lei
On 2022/8/9 10:09, Leizhen (ThunderTown) wrote:
>
>
> On 2022/8/8 17:41, Russell King (Oracle) wrote:
>> On Mon, Aug 01, 2022 at 11:20:16AM +0800, Zhen Lei wrote:
>>> Currently, instruction pointers are printed in [<%08lx>] format to make
>>> them more visible. However, it is not necessary in __show_regs() because
>>> they have the prefix 'pc :' or 'lr :', and it is also inconsistent with
>>> that of other registers, which causes misalignment.
>>
>> The formatting is not "to make them more visible" - it was to mark the
>> addresses that we wanted the ksymoops utility to translate to kernel
>> symbols before we had kallsyms in the kernel. If one disables kallsyms,
>> then we still need a way to translate kernel addresses to symbols.
>
> I searched the git log and found that the ksymoops utility is discarded.
>
> See:
> 073a9ecb3a73401662430bb955aedeac1de643d1
> However, a commit in the pre-git era [1] had added the statement,
> "ksymoops is useless on 2.6. Please use the Oops in its original format".
>
> That statement existed until commit 4eb9241127a0 ("Documentation:
> admin-guide: update bug-hunting.rst") finally removed the stale
> ksymoops information.
>
> 4eb9241127a0b5ac3aaaf1b246728009527ebc86
> - delete all references to ksymoops since it is no longer applicable;
>
>>
>> I notice there is a script which helps with this that is part of the
>> kernel source - scripts/decode_stacktrace.sh. I haven't tried this on
>> arm32 since I always use kallsyms - and I suspect that is rather
>> universally true as it avoids needing System.map files etc to decode
>> the oops. That said, if you're building a kernel for small systems,
>> you probably don't want the overhead of kallsyms.
Hi, Russell:
I re-examined the code and found that 'pc' and 'lr' had extra printing
in __show_regs(). Therefore, maybe v2 should be changed as follows, as
is done in dump_backtrace_entry():
diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index 96f3fbd51764292..2b0b49821cbf2f5 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -134,9 +134,15 @@ void __show_regs(struct pt_regs *regs)
show_regs_print_info(KERN_DEFAULT);
+#ifndef CONFIG_KALLSYMS
+ printk("PC is at [<%08lx>]\n", instruction_pointer(regs));
+ printk("LR is at [<%08lx>]\n", regs->ARM_lr);
+#else
printk("PC is at %pS\n", (void *)instruction_pointer(regs));
printk("LR is at %pS\n", (void *)regs->ARM_lr);
- printk("pc : [<%08lx>] lr : [<%08lx>] psr: %08lx\n",
+#endif
+
+ printk("pc : %08lx lr : %08lx psr: %08lx\n",
regs->ARM_pc, regs->ARM_lr, regs->ARM_cpsr);
printk("sp : %08lx ip : %08lx fp : %08lx\n",
regs->ARM_sp, regs->ARM_ip, regs->ARM_fp);
So that there are no concerns that you mentioned.
CONFIG_KALLSYMS=n:
PC is at [<801993d4>]
LR is at [<801993d4>]
pc : 801993d4 lr : 801993d4 psr: 60000013
sp : c49f9f28 ip : 00000001 fp : 00000001
CONFIG_KALLSYMS=y:
PC is at ktime_get+0x4c/0xe8
LR is at ktime_get+0x4c/0xe8
pc : 8019a4ac lr : 8019a4ac psr: 60000013
sp : c49f9f28 ip : 00000001 fp : 00000001
>
> Yes, I read scripts/decode_stacktrace.sh, it requires the format "[<...>]".
> But if that's the only concern, maybe we can do the conversion from
> "pc: addr" and "lr: addr" to "[<addr>]" first in scripts/decode_stacktrace.sh
>
> I'm usually "objdump -d vmlinux > asm_file", then search "addr:" in asm_file.
>
> Honestly, I think format "[<...>]" is dump_backtrace()'s requirement, not __show_regs()'s.
>
>
>>
>> So, there's an argument for keeping it - it's an API in that it
>> provides hints to scripting to identify which values need to be
>> converted to symbols. There's also the argument for getting rid of it,
>> which is that hardly anyone does that anymore.
>>
>> The question is, which is the more important argument, and I don't
>> think there's a definite answer. So I'm inclined to leave this
>> as-is.
>
> OK
>
>>
>
--
Regards,
Zhen Lei