Hi Kazu
Please help to review the squashed version of the patch.
Thanks
Hong
________________________________
From: HAGIO KAZUHITO(�尾 一仁) <k-hagio-ab(a)nec.com>
Sent: Monday, November 22, 2021 8:00
To: Hong YANG3 杨红 <hong.yang3(a)nio.com>
Cc: Discussion list for crash utility usage, maintenance and development
<crash-utility(a)redhat.com>; kazuhito.hagio(a)gmail.com
<kazuhito.hagio(a)gmail.com>
Subject: RE: arm64: Support overflow stack panic
注意:此封邮件来自于公司外部,请注意信息安全!
Attention: This email comes from outside of the company, please pay attention to the
information security!
-----Original Message-----
Hi Kazu
Thanks for your time to review the patches and all the detailed comments, the 3rd patch
attached with all
the refinement.
Thanks, it looks good.
By the way, do I need to squash all the 3 patches as one before them get accepted?
yes, that'll be better.
Thanks,
Kazu
Thanks
Hong
________________________________
From: HAGIO KAZUHITO(�尾 一仁) <k-hagio-ab(a)nec.com>
Sent: Friday, November 19, 2021 16:21
To: Hong Yang3 杨红 <hong.yang3(a)nio.com>
Cc: Discussion list for crash utility usage, maintenance and development
<crash-utility(a)redhat.com>
Subject: RE: arm64: Support overflow stack panic
注意:此封邮件来自于公司外部,请注意信息安全!
Attention: This email comes from outside of the company, please pay attention to the
information security!
Hi Hong,
Thanks for the refinement.
> + if (symbol_exists("overflow_stack") &&
> + (sp = per_cpu_symbol_search("overflow_stack")) &&
> + get_symbol_type("overflow_stack", NULL, req)) {
> + if (CRASHDEBUG(1)) {
> + fprintf(fp, "overflow_stack: \n");
> + fprintf(fp, " type: %x, %s\n",
> + (int)req->typecode,
> + (req->typecode == TYPE_CODE_PTR) ?
> + "TYPE_CODE_PTR" :
"other");
Given that overflow_stack is array, TYPE_CODE_ARRAY would be better.
> @@ -2673,6 +2724,12 @@ arm64_back_trace_cmd(struct bt_info *bt)
> bt->hp->eip : GET_STACK_ULONG(bt->hp->esp);
> stackframe.sp = bt->hp->esp + 8;
> bt->flags &= ~BT_REGS_NOT_FOUND;
> + } else if (arm64_on_overflow_stack(bt->tc->processor, bt->frameptr))
{
> + arm64_set_overflow_stack(bt);
> + bt->flags |= BT_OVERFLOW_STACK;
I would prefer to place this into the else block below
> + stackframe.sp = bt->stkptr;
> + stackframe.pc = bt->instptr;
> + stackframe.fp = bt->frameptr;
then we can remove these three lines.
> } else {
> if (arm64_on_irq_stack(bt->tc->processor, bt->frameptr)) {
> arm64_set_irq_stack(bt);
> +static int
> +arm64_in_alternate_stack(int cpu, ulong stkptr)
> +{
> + struct machine_specific *ms = machdep->machspec;
> +
> + return arm64_in_alternate_stackv(cpu, stkptr,
> + ms->irq_stacks, ms->irq_stack_size);
> +}
Given that in_alternate_stack() should check if it's in non-process stacks,
I think we should check also for overflow_stacks here. So how about making
these function like this?
arm64_in_alternate_stack
arm64_on_irq_stack
arm64_on_overflow_stack
arm64_on_irq_stack
arm64_in_alternate_stackv(irq_stacks)
arm64_on_overflow_stack
arm64_in_alternate_stackv(overflow_stacks)
Thanks,
Kazu
-----Original Message-----
> Hi Kazu
>
> Here are the latest patches for supporting to run bt command against a core dump
with kernel stack overflow
> exception for arm64.
>
> Please help to review and advise if any further change needed.
>
> Tested bt command with options:
>
> bt
> bt -a
> bt -c 3
>
> By the way, 'mach' command also updated to show overflow stacks info as same
as IRQ stacks.
>
> Thanks
> Hong
> ________________________________
>
> From: HAGIO KAZUHITO(�尾 一仁) <k-hagio-ab(a)nec.com>
> Sent: Wednesday, November 17, 2021 15:23
> To: Hong Yang3 杨红 <hong.yang3(a)nio.com>
> Cc: Discussion list for crash utility usage, maintenance and development
<crash-utility(a)redhat.com>
> Subject: RE: arm64: Support overflow stack panic
>
> 注意:此封邮件来自于公司外部,请注意信息安全!
> Attention: This email comes from outside of the company, please pay attention to the
information security!
>
> Hi Hong,
>
> Thank you for the patch and sending it to this list.
>
> -----Original Message-----
> > Hi Crash
> >
> > I'll keep refining the patch before it get approved:
>
> OK, so we will wait for the refined patch.
>
> Thanks,
> Kazu
>
> >
> >
> > 1. Fix the error in arm64_overflow_stack_init() which saved the overflow
stack address into
> > ms->irqstacks[], which would cause bt command crash on other cpus. The
normal IRQ stacks should be used
> > for bt command for other cpus.
> > 2. In addition to unwind on the overflow stack, try to go through the IRQ
stack to find more useful
> > information
> >
> > Kernel stack overflow case would be rarely but I'd like to sharp the crash
to cover this kind of issue.
> >
> > Best regards
> > Hong
> > ________________________________
> >
> > From: Hong Yang3 杨红
> > Sent: Tuesday, November 16, 2021 9:55
> > To: crash-utility(a)redhat.com <crash-utility(a)redhat.com>
> > Subject: arm64: Support overflow stack panic
> >
> > Hi All
> >
> > When I was trying to open a core of an overflow stack panic result, the bt
command caused a segment
fault,
> > after a while I figured out the overflow stack is not supported by crash
utility.
> >
> > This patch is trying to initialize the overflow stack information on startup
stage, and the bt command
> works
> > as expected to dump the correct call trace in the overflow stack, currently it
only apply to arm64
target.
> >
> > I'm not sure if any other sub command also need to be fixed for full
support for the overflow stack,
please
> > advise and I'll try to improve the patch.
> >
> > Thanks
> > Hong YANG