| Message ID | 1445328019-23330-2-git-send-email-takahiro.akashi@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 10/20/2015 10:26 PM, Jungseok Lee wrote: > On Oct 20, 2015, at 5:00 PM, AKASHI Takahiro wrote: >> This patch allows unwind_frame() to traverse from interrupt stack >> to process stack correctly by having a dummy stack frame for irq >> exception entry created at its prologue. >> >> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> >> --- >> arch/arm64/kernel/entry.S | 22 ++++++++++++++++++++-- >> arch/arm64/kernel/stacktrace.c | 14 +++++++++++++- >> 2 files changed, 33 insertions(+), 3 deletions(-) >> >> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S >> index c8e0bcf..779f807 100644 >> --- a/arch/arm64/kernel/entry.S >> +++ b/arch/arm64/kernel/entry.S >> @@ -186,8 +186,26 @@ alternative_endif >> and x23, x23, #~(IRQ_STACK_SIZE - 1) >> cmp x20, x23 // check irq re-enterance >> mov x19, sp >> - csel x23, x19, x24, eq // x24 = top of irq stack >> - mov sp, x23 >> + beq 1f >> + mov sp, x24 // x24 = top of irq stack >> + stp x29, x19, [sp, #-16]! // for sanity check >> + stp x29, x22, [sp, #-16]! // dummy stack frame >> + mov x29, sp >> +1: >> + /* >> + * Layout of interrupt stack after this macro is invoked: >> + * >> + * | | >> + *-0x20+----------------+ <= dummy stack frame >> + * | fp | : fp on process stack >> + *-0x18+----------------+ >> + * | lr | : return address >> + *-0x10+----------------+ >> + * | fp (copy) | : for sanity check >> + * -0x8+----------------+ >> + * | sp | : sp on process stack >> + * 0x0+----------------+ >> + */ >> .endm >> >> /* >> diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c >> index 407991b..03611a1 100644 >> --- a/arch/arm64/kernel/stacktrace.c >> +++ b/arch/arm64/kernel/stacktrace.c >> @@ -43,12 +43,24 @@ int notrace unwind_frame(struct stackframe *frame) >> low = frame->sp; >> high = ALIGN(low, THREAD_SIZE); >> >> - if (fp < low || fp > high - 0x18 || fp & 0xf) >> + if (fp < low || fp > high - 0x20 || fp & 0xf) >> return -EINVAL; >> >> frame->sp = fp + 0x10; >> frame->fp = *(unsigned long *)(fp); >> /* >> + * check whether we are going to walk trough from interrupt stack >> + * to process stack >> + * If the previous frame is the initial (dummy) stack frame on >> + * interrupt stack, frame->sp now points to just below the frame >> + * (dummy frame + 0x10). >> + * See entry.S >> + */ >> +#define STACK_LOW(addr) round_down((addr), THREAD_SIZE) >> + if ((STACK_LOW(frame->sp) != STACK_LOW(frame->fp)) && >> + (frame->fp == *(unsigned long *)frame->sp)) >> + frame->sp = *((unsigned long *)(frame->sp + 8)); >> + /* >> * -4 here because we care about the PC at time of bl, >> * not where the return will go. >> */ >> -- >> 1.7.9.5 > > How about folding the following hunk into this patch? > The comment would be helpful for people to follow this code. Thanks. A frame pointer(x29) is a frame pointer, and I'm not sure that the comment is very useful. But it's much better than "fp on process stack" in my ascii-art. -Takahiro AKASHI > ----8<---- > > diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S > index f1303c5..0ff7db3 100644 > --- a/arch/arm64/kernel/entry.S > +++ b/arch/arm64/kernel/entry.S > @@ -122,7 +122,8 @@ > * x21 - aborted SP > * x22 - aborted PC > * x23 - aborted PSTATE > - */ > + * x29 - aborted FP > + */ > .endm > > .macro kernel_exit, el > > ----8<---- > > Best Regards > Jungseok Lee > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[Only Akashi and James] On Oct 21, 2015, at 3:38 PM, AKASHI Takahiro wrote: Hi Akashi and James, Am I the only person who have experienced kernel panic with this series? I have observed NULL pointer kernel panic with the following two ways. - $ sudo echo c > /proc/sysrq-trigger - perf record -e mem:[address of do_softirq]:x -ag -- sleep 2 The kernel panic is triggered when the last stack frame of swapper is unwound. At that time, the value of fp, low, high is 0, 0, 0, respectively. My tree includes "Synchronise dump_backtrace() with perf call chain" patch which is queued into for-next/core. Best Regards Jungseok Lee > On 10/20/2015 10:26 PM, Jungseok Lee wrote: >> On Oct 20, 2015, at 5:00 PM, AKASHI Takahiro wrote: >>> This patch allows unwind_frame() to traverse from interrupt stack >>> to process stack correctly by having a dummy stack frame for irq >>> exception entry created at its prologue. >>> >>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> >>> --- >>> arch/arm64/kernel/entry.S | 22 ++++++++++++++++++++-- >>> arch/arm64/kernel/stacktrace.c | 14 +++++++++++++- >>> 2 files changed, 33 insertions(+), 3 deletions(-) >>> >>> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S >>> index c8e0bcf..779f807 100644 >>> --- a/arch/arm64/kernel/entry.S >>> +++ b/arch/arm64/kernel/entry.S >>> @@ -186,8 +186,26 @@ alternative_endif >>> and x23, x23, #~(IRQ_STACK_SIZE - 1) >>> cmp x20, x23 // check irq re-enterance >>> mov x19, sp >>> - csel x23, x19, x24, eq // x24 = top of irq stack >>> - mov sp, x23 >>> + beq 1f >>> + mov sp, x24 // x24 = top of irq stack >>> + stp x29, x19, [sp, #-16]! // for sanity check >>> + stp x29, x22, [sp, #-16]! // dummy stack frame >>> + mov x29, sp >>> +1: >>> + /* >>> + * Layout of interrupt stack after this macro is invoked: >>> + * >>> + * | | >>> + *-0x20+----------------+ <= dummy stack frame >>> + * | fp | : fp on process stack >>> + *-0x18+----------------+ >>> + * | lr | : return address >>> + *-0x10+----------------+ >>> + * | fp (copy) | : for sanity check >>> + * -0x8+----------------+ >>> + * | sp | : sp on process stack >>> + * 0x0+----------------+ >>> + */ >>> .endm >>> >>> /* >>> diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c >>> index 407991b..03611a1 100644 >>> --- a/arch/arm64/kernel/stacktrace.c >>> +++ b/arch/arm64/kernel/stacktrace.c >>> @@ -43,12 +43,24 @@ int notrace unwind_frame(struct stackframe *frame) >>> low = frame->sp; >>> high = ALIGN(low, THREAD_SIZE); >>> >>> - if (fp < low || fp > high - 0x18 || fp & 0xf) >>> + if (fp < low || fp > high - 0x20 || fp & 0xf) >>> return -EINVAL; >>> >>> frame->sp = fp + 0x10; >>> frame->fp = *(unsigned long *)(fp); >>> /* >>> + * check whether we are going to walk trough from interrupt stack >>> + * to process stack >>> + * If the previous frame is the initial (dummy) stack frame on >>> + * interrupt stack, frame->sp now points to just below the frame >>> + * (dummy frame + 0x10). >>> + * See entry.S >>> + */ >>> +#define STACK_LOW(addr) round_down((addr), THREAD_SIZE) >>> + if ((STACK_LOW(frame->sp) != STACK_LOW(frame->fp)) && >>> + (frame->fp == *(unsigned long *)frame->sp)) >>> + frame->sp = *((unsigned long *)(frame->sp + 8)); >>> + /* >>> * -4 here because we care about the PC at time of bl, >>> * not where the return will go. >>> */ >>> -- >>> 1.7.9.5 >> >> How about folding the following hunk into this patch? >> The comment would be helpful for people to follow this code. > > Thanks. > A frame pointer(x29) is a frame pointer, and I'm not sure that the comment is > very useful. > But it's much better than "fp on process stack" in my ascii-art. > > -Takahiro AKASHI > >> ----8<---- >> >> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S >> index f1303c5..0ff7db3 100644 >> --- a/arch/arm64/kernel/entry.S >> +++ b/arch/arm64/kernel/entry.S >> @@ -122,7 +122,8 @@ >> * x21 - aborted SP >> * x22 - aborted PC >> * x23 - aborted PSTATE >> - */ >> + * x29 - aborted FP >> + */ >> .endm >> >> .macro kernel_exit, el >> >> ----8<---- >> >> Best Regards >> Jungseok Lee >> -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On Oct 21, 2015, at 9:14 PM, Jungseok Lee wrote: Whoops! > [Only Akashi and James] > > On Oct 21, 2015, at 3:38 PM, AKASHI Takahiro wrote: > > Hi Akashi and James, > > Am I the only person who have experienced kernel panic with this series? > I have observed NULL pointer kernel panic with the following two ways. > > - $ sudo echo c > /proc/sysrq-trigger > - perf record -e mem:[address of do_softirq]:x -ag -- sleep 2 > > The kernel panic is triggered when the last stack frame of swapper is unwound. > At that time, the value of fp, low, high is 0, 0, 0, respectively. > > My tree includes "Synchronise dump_backtrace() with perf call chain" patch > which is queued into for-next/core. > > Best Regards > Jungseok Lee Really sorry for the noise.. It won't happen again.. Best Regards Jungseok Lee-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S index c8e0bcf..779f807 100644 --- a/arch/arm64/kernel/entry.S +++ b/arch/arm64/kernel/entry.S @@ -186,8 +186,26 @@ alternative_endif and x23, x23, #~(IRQ_STACK_SIZE - 1) cmp x20, x23 // check irq re-enterance mov x19, sp - csel x23, x19, x24, eq // x24 = top of irq stack - mov sp, x23 + beq 1f + mov sp, x24 // x24 = top of irq stack + stp x29, x19, [sp, #-16]! // for sanity check + stp x29, x22, [sp, #-16]! // dummy stack frame + mov x29, sp +1: + /* + * Layout of interrupt stack after this macro is invoked: + * + * | | + *-0x20+----------------+ <= dummy stack frame + * | fp | : fp on process stack + *-0x18+----------------+ + * | lr | : return address + *-0x10+----------------+ + * | fp (copy) | : for sanity check + * -0x8+----------------+ + * | sp | : sp on process stack + * 0x0+----------------+ + */ .endm /* diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c index 407991b..03611a1 100644 --- a/arch/arm64/kernel/stacktrace.c +++ b/arch/arm64/kernel/stacktrace.c @@ -43,12 +43,24 @@ int notrace unwind_frame(struct stackframe *frame) low = frame->sp; high = ALIGN(low, THREAD_SIZE); - if (fp < low || fp > high - 0x18 || fp & 0xf) + if (fp < low || fp > high - 0x20 || fp & 0xf) return -EINVAL; frame->sp = fp + 0x10; frame->fp = *(unsigned long *)(fp); /* + * check whether we are going to walk trough from interrupt stack + * to process stack + * If the previous frame is the initial (dummy) stack frame on + * interrupt stack, frame->sp now points to just below the frame + * (dummy frame + 0x10). + * See entry.S + */ +#define STACK_LOW(addr) round_down((addr), THREAD_SIZE) + if ((STACK_LOW(frame->sp) != STACK_LOW(frame->fp)) && + (frame->fp == *(unsigned long *)frame->sp)) + frame->sp = *((unsigned long *)(frame->sp + 8)); + /* * -4 here because we care about the PC at time of bl, * not where the return will go. */
This patch allows unwind_frame() to traverse from interrupt stack to process stack correctly by having a dummy stack frame for irq exception entry created at its prologue. Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> --- arch/arm64/kernel/entry.S | 22 ++++++++++++++++++++-- arch/arm64/kernel/stacktrace.c | 14 +++++++++++++- 2 files changed, 33 insertions(+), 3 deletions(-)