lkml.org 
[lkml]   [2021]   [Mar]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2 02/43] powerpc/traps: Declare unrecoverable_exception() as __noreturn
    Excerpts from Christophe Leroy's message of March 9, 2021 10:09 pm:
    > unrecoverable_exception() is never expected to return, most callers
    > have an infiniteloop in case it returns.
    >
    > Ensure it really never returns by terminating it with a BUG(), and
    > declare it __no_return.
    >
    > It always GCC to really simplify functions calling it. In the exemple
    > below, it avoids the stack frame in the likely fast path and avoids
    > code duplication for the exit.
    >
    > With this patch:

    [snip]

    Nice.

    > diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
    > index a44a30b0688c..d5c9d9ddd186 100644
    > --- a/arch/powerpc/kernel/traps.c
    > +++ b/arch/powerpc/kernel/traps.c
    > @@ -2170,11 +2170,15 @@ DEFINE_INTERRUPT_HANDLER(SPEFloatingPointRoundException)
    > * in the MSR is 0. This indicates that SRR0/1 are live, and that
    > * we therefore lost state by taking this exception.
    > */
    > -void unrecoverable_exception(struct pt_regs *regs)
    > +void __noreturn unrecoverable_exception(struct pt_regs *regs)
    > {
    > pr_emerg("Unrecoverable exception %lx at %lx (msr=%lx)\n",
    > regs->trap, regs->nip, regs->msr);
    > die("Unrecoverable exception", regs, SIGABRT);
    > + /* die() should not return */
    > + WARN(true, "die() unexpectedly returned");
    > + for (;;)
    > + ;
    > }

    I don't think the WARN should be added because that will cause another
    interrupt after something is already badly wrong, so this might just
    make it harder to debug.

    For example if die() is falling through for some reason, we warn and
    cause a program check here, and that might also be unrecoverable so it
    might come through here and fall through again and warn again, etc.

    Putting the infinite loop is good enough I think (and better than there
    was previously).

    Otherwise

    Reviewed-by: Nicholas Piggin <npiggin@gmail.com>

    Thanks,
    Nick

    \
     
     \ /
      Last update: 2021-03-10 02:25    [W:4.148 / U:0.336 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site