MIME-Version: 1.0
References: <20230201163420.1579014-1-revest@chromium.org> <20230201163420.1579014-6-revest@chromium.org>
 <Y9vcua0+JzjmTICO@FVFF77S0Q05N.cambridge.arm.com> <Y9vrWUM8ypNNwHyv@FVFF77S0Q05N.cambridge.arm.com>
 <CABRcYm+nwsyyKEhvz9dr8sDiOWfha-YxOkGMFSx4mH9O+HAiYg@mail.gmail.com> <Y9zb+uKqQN9gJJBI@FVFF77S0Q05N>
In-Reply-To: <Y9zb+uKqQN9gJJBI@FVFF77S0Q05N>
From:   Florent Revest <revest@chromium.org>
Date:   Fri, 3 Feb 2023 12:01:36 +0100
Message-ID: <CABRcYmKMdoEAH=ua+AKJyMTt_xMvV0jisAFcr4UuVOJGDzHdfw@mail.gmail.com>
Subject: Re: [PATCH 5/8] ftrace: Make DIRECT_CALLS work WITH_ARGS and !WITH_REGS
To:     Mark Rutland <mark.rutland@arm.com>
Cc:     linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        linux-trace-kernel@vger.kernel.org, bpf@vger.kernel.org,
        catalin.marinas@arm.com, will@kernel.org, rostedt@goodmis.org,
        mhiramat@kernel.org, ast@kernel.org, daniel@iogearbox.net,
        andrii@kernel.org, kpsingh@kernel.org, jolsa@kernel.org,
        xukuohai@huaweicloud.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, Feb 3, 2023 at 11:03 AM Mark Rutland <mark.rutland@arm.com> wrote:
>
> On Thu, Feb 02, 2023 at 07:19:58PM +0100, Florent Revest wrote:
> > On Thu, Feb 2, 2023 at 5:57 PM Mark Rutland <mark.rutland@arm.com> wrote:
> > > diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> > > index 84f717f8959e..3d2156e335d7 100644
> > > --- a/include/linux/ftrace.h
> > > +++ b/include/linux/ftrace.h
> > > @@ -241,6 +241,12 @@ enum {
> > >         FTRACE_OPS_FL_DIRECT                    = BIT(17),
> > >  };
> > >
> > > +#ifndef CONFIG_DYNAMIC_FTRACE_WITH_ARGS
> > > +#define FTRACE_OPS_FL_SAVE_ARGS                        FTRACE_OPS_FL_SAVE_REGS
> > > +#else
> > > +#define FTRACE_OPS_FL_SAVE_ARGS                        0
> >
> > Mh, could we (theoretically) be in a situation where an arch supports
> > WITH_ARGS but it also has two ftrace_caller trampolines: one that
> > saves the args and the other that saves nothing ? (For example if x86
> > migrates their WITH_REGS to WITH_ARGS only)
>
> I don't think so -- the point of WITH_ARGS is that we always have to
> save/restore the args, and when WITH_ARGS is selected they're unconditionally
> available (though not necessarily a full pt_regs), which is what other code
> assumes when WITH_ARGS is selected.

Perfect then!

> > Wouldn't it make sense then to define FTRACE_OPS_FL_SAVE_ARGS as an
> > extra bit to tell ftrace that we need the args, similarly to
> > FTRACE_OPS_FL_SAVE_REGS ?
> >
> > If that can't happen or if we want to leave this up for later, the
> > patch lgtm and I can squash it into my patch 5 in v2. ;)
>
> I think that can't happen, and for now the above should be fine.

Yep