Re: [PATCHv2 1/8] ftrace: add ftrace_init_nop()

[Amit Kachhap]

On 11/6/19 7:45 PM, Mark Rutland wrote:
> On Tue, Nov 05, 2019 at 12:17:26PM +0530, Amit Kachhap wrote:
> > On 11/4/19 7:06 PM, Mark Rutland wrote:
> > > On Sat, Nov 02, 2019 at 05:49:00PM +0530, Amit Daniel Kachhap wrote:
> > > > On 10/29/19 10:28 PM, Mark Rutland wrote:
> > > > > +/**
> > > > > + * ftrace_init_nop - initialize a nop call site
> > > > > + * @mod: module structure if called by module load initialization
> > > > > + * @rec: the call site record (e.g. mcount/fentry)
> > > > > + *
> > > > > + * This is a very sensitive operation and great care needs
> > > > > + * to be taken by the arch.  The operation should carefully
> > > > > + * read the location, check to see if what is read is indeed
> > > > > + * what we expect it to be, and then on success of the compare,
> > > > > + * it should write to the location.
> > > > > + *
> > > > > + * The code segment at @rec->ip should contain the contents created by
> > > > > + * the compiler
> > > > Nit: Will it be better to write it as "@rec->ip should store the adjusted
> > > > ftrace entry address of the call site" or something like that.
> > >
> > > This was the specific wording requested by Steve, and it's trying to
> > > describe the instructions at rec->ip, rather than the value of rec->ip,
> > > so I think it's better to leave this as-is.
> > ok Its fine this way too. Actually from the comment, I could not understand
> > which one of the compiler contents this points to as in this case there are
> > 2 nops.
>
> We can't say what the compiler contents will be. An architecture may use
> this callback if it's using mcount, mfentry, patchable-function-entry,
> or some other mechanism we're not aware of today. Depending on the
> architecture and mechanism, the callsite could contain a number of
> distinct things.
>
> All the comment is trying to say is that when ftrace_init_nop() is
> called, the callsite has not been modified in any way since being
> compiled, so we can expect the contents to be whatever the compiler
> generated.

ok. Your details seems reasonable.

Thanks,
Amit Daniel
> Thanks,
> Mark.