Re: [PATCH 3/8] arm64: stacktrace: move SDEI stack helpers to stacktrace code

[Mark Rutland <mark.rutland@arm.com>]

Hi Kalesh,

On Mon, Aug 01, 2022 at 09:53:35PM -0700, Kalesh Singh wrote:
> On Mon, Aug 1, 2022 at 5:12 AM Mark Rutland <mark.rutland@arm.com> wrote:
> > diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
> > index 4c8865e495fea..04a9b56b114c1 100644
> > --- a/arch/arm64/kernel/stacktrace.c
> > +++ b/arch/arm64/kernel/stacktrace.c
> > @@ -86,8 +86,17 @@ static bool on_accessible_stack(const struct task_struct *tsk,
> >                 return true;
> >         if (on_overflow_stack(sp, size, info))
> >                 return true;
> > -       if (on_sdei_stack(sp, size, info))
> > -               return true;
> > +
> > +       if (IS_ENABLED(CONFIG_VMAP_STACK) &&
> > +           IS_ENABLED(CONFIG_ARM_SDE_INTERFACE) &&
> > +           in_nmi())
> 
> I think we can remove the IS_ENABLED() checks since it's handled by
> ifdefs in asm/stacktrace.h
> 
> Otherwise, Reviewed-by: Kalesh Singh <kaleshsingh@google.com>

I'd kept the IS_ENABLED() checks here to avoid code being generated of in_nmi()
when SDEI is not configured in. Since in_nmi() uses preempt_count() and that
uses READ_ONCE(), the compiler can't optimize the read away in case there are
side effects (but I imagine will discard the value immediately).

With that in mind, are you happy if I leave this as is, and take your
Reviewed-by?

Thanks,
Mark.

> 
> > +       {
> > +               if (on_sdei_critical_stack(sp, size, info))
> > +                       return true;
> > +
> > +               if (on_sdei_normal_stack(sp, size, info))
> > +                       return true;
> > +       }
> >
> >         return false;
> >  }
> > --
> > 2.30.2
> >

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel