Re: [PATCH v7 2/3] arm64: implement ftrace with regs

From: Torsten Duwe
Date: Mon Feb 04 2019 - 07:03:13 EST


On Tue, Jan 22, 2019 at 02:55:12PM +0100, Ard Biesheuvel wrote:
> On Tue, 22 Jan 2019 at 14:28, Torsten Duwe <duwe@xxxxxx> wrote:
> >
> > On Tue, Jan 22, 2019 at 10:18:17AM +0000, Julien Thierry wrote:
> > > Hi Torsten,
> > >
> > > A few suggestions below.
> > >
> > > > +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> > > > +#define ARCH_SUPPORTS_FTRACE_OPS 1
> > > > +#define REC_IP_BRANCH_OFFSET AARCH64_INSN_SIZE
> > > > +/* All we need is some magic value. Simply use "_mCount:" */
> > >
> > > Nit: Should the casing be "_mcount" ?
> >
> > No! The string makes it clear what it's supposed to be and the peculiar
> > casing makes it unique and leaves no doubt were it came from. So whenever
> > you see this register value in a crash dump you don't have to wonder about
> > weird linkage errors, as it surely did not originate from a symtab.
> >
>
> In that case, do you need to deal with endianness here?
>
> > > > +#define MCOUNT_ADDR (0x5f6d436f756e743a)

Strictly speaking, yes. OTOH "wrong-andian" machines always show a difference
when memory is dumped in bigger units than bytes, so when you see the register
value in hex...

Since all that's needed is a somewhat unique constant, let's not over-engineer
this ok?

If there are no other comments I'll send out v8 with the discussed changes.

Torsten