| Message ID | 20190328114527.171178-1-wangkefeng.wang@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: skip dump_backtrace() in show_regs when user mode pt_regs | expand |
On Thu, Mar 28, 2019 at 07:45:27PM +0800, Kefeng Wang wrote: > There is no need to call dump_backtrace() when the pt_regs > corresponds to user mode, eg, > > [ 334.634005] potentially unexpected fatal signal 11. > [ 334.634476] CPU: 3 PID: 2049 Comm: test Not tainted 5.1.0-rc2 #24 > [ 334.634745] Hardware name: QEMU QEMU Virtual Machine, BIOS 0.0.0 02/06/2015 > [ 334.635786] pstate: 60001000 (nZCv daif -PAN -UAO) > [ 334.636131] pc : 0000aaaaad4c0788 > [ 334.636725] lr : 0000ffff83aac6e0 > [ 334.637006] sp : 0000ffffd14e9670 > [ 334.637352] x29: 0000ffffd14e9690 x28: 0000000000000000 > [ 334.637814] x27: 0000000000000000 x26: 0000000000000000 > [ 334.638143] x25: 0000000000000000 x24: 0000000000000000 > [ 334.638460] x23: 0000000000000000 x22: 0000000000000000 > [ 334.639499] x21: 0000aaaaad4c0650 x20: 0000000000000000 > [ 334.640336] x19: 0000aaaaad4c0790 x18: 0000000000000002 > [ 334.641062] x17: 0000ffff83aac600 x16: 0000aaaaad4d0fa0 > [ 334.641474] x15: 0000ffff83c10150 x14: 0000ffff83c101e0 > [ 334.641894] x13: 0000ffff83c12048 x12: 0000000000000000 > [ 334.642421] x11: 0000000000010000 x10: 0000000008000000 > [ 334.642749] x9 : 000000000fffffff x8 : ffffffffffffffff > [ 334.643378] x7 : 0000000000010000 x6 : 0000ffff83be0b00 > [ 334.643669] x5 : 0000000000000000 x4 : 0000000000000000 > [ 334.644012] x3 : 0000aaaaad4c0754 x2 : 0000ffffd14e97d8 > [ 334.644302] x1 : 0000fffefffffffc x0 : 0000ffff00000000 > [ 334.645211] Call trace: > > Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> > --- > arch/arm64/kernel/process.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c > index 3767fb21a5b8..de224f1512ca 100644 > --- a/arch/arm64/kernel/process.c > +++ b/arch/arm64/kernel/process.c > @@ -298,7 +298,8 @@ void __show_regs(struct pt_regs *regs) > void show_regs(struct pt_regs * regs) > { > __show_regs(regs); > - dump_backtrace(regs, NULL); > + if (!user_mode(regs)) > + dump_backtrace(regs, NULL); > } Wouldn't this check be better off inside dump_backtrace()? Will
On 2019/4/3 20:44, Will Deacon wrote: > On Thu, Mar 28, 2019 at 07:45:27PM +0800, Kefeng Wang wrote: >> There is no need to call dump_backtrace() when the pt_regs >> corresponds to user mode, eg, >> >> [ 334.634005] potentially unexpected fatal signal 11. >> [ 334.634476] CPU: 3 PID: 2049 Comm: test Not tainted 5.1.0-rc2 #24 >> [ 334.634745] Hardware name: QEMU QEMU Virtual Machine, BIOS 0.0.0 02/06/2015 >> [ 334.635786] pstate: 60001000 (nZCv daif -PAN -UAO) >> [ 334.636131] pc : 0000aaaaad4c0788 >> [ 334.636725] lr : 0000ffff83aac6e0 >> [ 334.637006] sp : 0000ffffd14e9670 >> [ 334.637352] x29: 0000ffffd14e9690 x28: 0000000000000000 >> [ 334.637814] x27: 0000000000000000 x26: 0000000000000000 >> [ 334.638143] x25: 0000000000000000 x24: 0000000000000000 >> [ 334.638460] x23: 0000000000000000 x22: 0000000000000000 >> [ 334.639499] x21: 0000aaaaad4c0650 x20: 0000000000000000 >> [ 334.640336] x19: 0000aaaaad4c0790 x18: 0000000000000002 >> [ 334.641062] x17: 0000ffff83aac600 x16: 0000aaaaad4d0fa0 >> [ 334.641474] x15: 0000ffff83c10150 x14: 0000ffff83c101e0 >> [ 334.641894] x13: 0000ffff83c12048 x12: 0000000000000000 >> [ 334.642421] x11: 0000000000010000 x10: 0000000008000000 >> [ 334.642749] x9 : 000000000fffffff x8 : ffffffffffffffff >> [ 334.643378] x7 : 0000000000010000 x6 : 0000ffff83be0b00 >> [ 334.643669] x5 : 0000000000000000 x4 : 0000000000000000 >> [ 334.644012] x3 : 0000aaaaad4c0754 x2 : 0000ffffd14e97d8 >> [ 334.644302] x1 : 0000fffefffffffc x0 : 0000ffff00000000 >> [ 334.645211] Call trace: >> >> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> >> --- >> arch/arm64/kernel/process.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c >> index 3767fb21a5b8..de224f1512ca 100644 >> --- a/arch/arm64/kernel/process.c >> +++ b/arch/arm64/kernel/process.c >> @@ -298,7 +298,8 @@ void __show_regs(struct pt_regs *regs) >> void show_regs(struct pt_regs * regs) >> { >> __show_regs(regs); >> - dump_backtrace(regs, NULL); >> + if (!user_mode(regs)) >> + dump_backtrace(regs, NULL); >> } > Wouldn't this check be better off inside dump_backtrace()? show_stack() call dump_backtrace() with pr_regs=NULL, so only judge it in show_regs(). Or using following changes? diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c index 8ad119c3f665..66abe3ada3c7 100644 --- a/arch/arm64/kernel/traps.c +++ b/arch/arm64/kernel/traps.c @@ -106,6 +106,9 @@ void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk) pr_debug("%s(regs = %p tsk = %p)\n", __func__, regs, tsk); + if (regs && user_mode(regs)) + return; + if (!tsk) tsk = current; @@ -181,10 +184,8 @@ static int __die(const char *str, int err, struct pt_regs *regs) TASK_COMM_LEN, tsk->comm, task_pid_nr(tsk), end_of_stack(tsk)); - if (!user_mode(regs)) { - dump_backtrace(regs, tsk); - dump_instr(KERN_EMERG, regs); - } + dump_backtrace(regs, tsk); + dump_instr(KERN_EMERG, regs); return ret; }
On Wed, Apr 03, 2019 at 11:58:44PM +0800, Kefeng Wang wrote: > On 2019/4/3 20:44, Will Deacon wrote: > > On Thu, Mar 28, 2019 at 07:45:27PM +0800, Kefeng Wang wrote: > >> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c > >> index 3767fb21a5b8..de224f1512ca 100644 > >> --- a/arch/arm64/kernel/process.c > >> +++ b/arch/arm64/kernel/process.c > >> @@ -298,7 +298,8 @@ void __show_regs(struct pt_regs *regs) > >> void show_regs(struct pt_regs * regs) > >> { > >> __show_regs(regs); > >> - dump_backtrace(regs, NULL); > >> + if (!user_mode(regs)) > >> + dump_backtrace(regs, NULL); > >> } > > Wouldn't this check be better off inside dump_backtrace()? > > show_stack() call dump_backtrace() with pr_regs=NULL, so only judge it in show_regs(). > > Or using following changes? > > diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c > index 8ad119c3f665..66abe3ada3c7 100644 > --- a/arch/arm64/kernel/traps.c > +++ b/arch/arm64/kernel/traps.c > @@ -106,6 +106,9 @@ void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk) > > pr_debug("%s(regs = %p tsk = %p)\n", __func__, regs, tsk); > > + if (regs && user_mode(regs)) > + return; > + > if (!tsk) > tsk = current; > > @@ -181,10 +184,8 @@ static int __die(const char *str, int err, struct pt_regs *regs) > TASK_COMM_LEN, tsk->comm, task_pid_nr(tsk), > end_of_stack(tsk)); > > - if (!user_mode(regs)) { > - dump_backtrace(regs, tsk); > - dump_instr(KERN_EMERG, regs); > - } > + dump_backtrace(regs, tsk); > + dump_instr(KERN_EMERG, regs); > > return ret; > } Yeah, something like that. See my version below. Will --->8 diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c index 8ad119c3f665..29755989f616 100644 --- a/arch/arm64/kernel/traps.c +++ b/arch/arm64/kernel/traps.c @@ -102,10 +102,16 @@ static void dump_instr(const char *lvl, struct pt_regs *regs) void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk) { struct stackframe frame; - int skip; + int skip = 0; pr_debug("%s(regs = %p tsk = %p)\n", __func__, regs, tsk); + if (regs) { + if (user_mode(regs)) + return; + skip = 1; + } + if (!tsk) tsk = current; @@ -126,7 +132,6 @@ void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk) frame.graph = 0; #endif - skip = !!regs; printk("Call trace:\n"); do { /* skip until specified stack frame */ @@ -176,15 +181,13 @@ static int __die(const char *str, int err, struct pt_regs *regs) return ret; print_modules(); - __show_regs(regs); pr_emerg("Process %.*s (pid: %d, stack limit = 0x%p)\n", TASK_COMM_LEN, tsk->comm, task_pid_nr(tsk), end_of_stack(tsk)); + show_regs(regs); - if (!user_mode(regs)) { - dump_backtrace(regs, tsk); + if (!user_mode(regs)) dump_instr(KERN_EMERG, regs); - } return ret; }
On 2019/4/6 0:42, Will Deacon wrote: > On Wed, Apr 03, 2019 at 11:58:44PM +0800, Kefeng Wang wrote: >> On 2019/4/3 20:44, Will Deacon wrote: >>> On Thu, Mar 28, 2019 at 07:45:27PM +0800, Kefeng Wang wrote: >>> > Yeah, something like that. See my version below. It's better, should I resend it or you will pick it directly? > > Will > > --->8 > > diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c > index 8ad119c3f665..29755989f616 100644 > --- a/arch/arm64/kernel/traps.c > +++ b/arch/arm64/kernel/traps.c > @@ -102,10 +102,16 @@ static void dump_instr(const char *lvl, struct pt_regs *regs) > void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk) > { > struct stackframe frame; > - int skip; > + int skip = 0; > > pr_debug("%s(regs = %p tsk = %p)\n", __func__, regs, tsk); > > + if (regs) { > + if (user_mode(regs)) > + return; > + skip = 1; > + } > + > if (!tsk) > tsk = current; > > @@ -126,7 +132,6 @@ void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk) > frame.graph = 0; > #endif > > - skip = !!regs; > printk("Call trace:\n"); > do { > /* skip until specified stack frame */ > @@ -176,15 +181,13 @@ static int __die(const char *str, int err, struct pt_regs *regs) > return ret; > > print_modules(); > - __show_regs(regs); > pr_emerg("Process %.*s (pid: %d, stack limit = 0x%p)\n", > TASK_COMM_LEN, tsk->comm, task_pid_nr(tsk), > end_of_stack(tsk)); > + show_regs(regs); > > - if (!user_mode(regs)) { > - dump_backtrace(regs, tsk); > + if (!user_mode(regs)) > dump_instr(KERN_EMERG, regs); > - } > > return ret; > } > > . >
On Sun, Apr 07, 2019 at 03:38:19PM +0800, Kefeng Wang wrote: > > On 2019/4/6 0:42, Will Deacon wrote: > > On Wed, Apr 03, 2019 at 11:58:44PM +0800, Kefeng Wang wrote: > >> On 2019/4/3 20:44, Will Deacon wrote: > >>> On Thu, Mar 28, 2019 at 07:45:27PM +0800, Kefeng Wang wrote: > >>> > > Yeah, something like that. See my version below. > > It's better, should I resend it or you will pick it directly? I'll send it out as a patch. Cheers, Will
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c index 3767fb21a5b8..de224f1512ca 100644 --- a/arch/arm64/kernel/process.c +++ b/arch/arm64/kernel/process.c @@ -298,7 +298,8 @@ void __show_regs(struct pt_regs *regs) void show_regs(struct pt_regs * regs) { __show_regs(regs); - dump_backtrace(regs, NULL); + if (!user_mode(regs)) + dump_backtrace(regs, NULL); } static void tls_thread_flush(void)
There is no need to call dump_backtrace() when the pt_regs corresponds to user mode, eg, [ 334.634005] potentially unexpected fatal signal 11. [ 334.634476] CPU: 3 PID: 2049 Comm: test Not tainted 5.1.0-rc2 #24 [ 334.634745] Hardware name: QEMU QEMU Virtual Machine, BIOS 0.0.0 02/06/2015 [ 334.635786] pstate: 60001000 (nZCv daif -PAN -UAO) [ 334.636131] pc : 0000aaaaad4c0788 [ 334.636725] lr : 0000ffff83aac6e0 [ 334.637006] sp : 0000ffffd14e9670 [ 334.637352] x29: 0000ffffd14e9690 x28: 0000000000000000 [ 334.637814] x27: 0000000000000000 x26: 0000000000000000 [ 334.638143] x25: 0000000000000000 x24: 0000000000000000 [ 334.638460] x23: 0000000000000000 x22: 0000000000000000 [ 334.639499] x21: 0000aaaaad4c0650 x20: 0000000000000000 [ 334.640336] x19: 0000aaaaad4c0790 x18: 0000000000000002 [ 334.641062] x17: 0000ffff83aac600 x16: 0000aaaaad4d0fa0 [ 334.641474] x15: 0000ffff83c10150 x14: 0000ffff83c101e0 [ 334.641894] x13: 0000ffff83c12048 x12: 0000000000000000 [ 334.642421] x11: 0000000000010000 x10: 0000000008000000 [ 334.642749] x9 : 000000000fffffff x8 : ffffffffffffffff [ 334.643378] x7 : 0000000000010000 x6 : 0000ffff83be0b00 [ 334.643669] x5 : 0000000000000000 x4 : 0000000000000000 [ 334.644012] x3 : 0000aaaaad4c0754 x2 : 0000ffffd14e97d8 [ 334.644302] x1 : 0000fffefffffffc x0 : 0000ffff00000000 [ 334.645211] Call trace: Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> --- arch/arm64/kernel/process.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)