From: Steven Rostedt <rostedt@goodmis.org> To: Mark Rutland <mark.rutland@arm.com> Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, amit.kachhap@arm.com, ard.biesheuvel@linaro.org, catalin.marinas@arm.com, deller@gmx.de, duwe@suse.de, james.morse@arm.com, jeyu@kernel.org, jpoimboe@redhat.com, jthierry@redhat.com, mingo@redhat.com, peterz@infradead.org, svens@stackframe.org, takahiro.akashi@linaro.org, will@kernel.org Subject: Re: [PATCH 1/8] ftrace: add ftrace_init_nop() Date: Tue, 22 Oct 2019 08:54:28 -0400 [thread overview] Message-ID: <20191022085428.75cfaad6@gandalf.local.home> (raw) In-Reply-To: <20191022112811.GA11583@lakrids.cambridge.arm.com> On Tue, 22 Oct 2019 12:28:11 +0100 Mark Rutland <mark.rutland@arm.com> wrote: > > To make the name even better, let's just rename it to: > > > > ftrace_nop_initialization() > > > > I think that may be the best description for it. > > Perhaps ftrace_nop_initialize(), so that it's not a noun? > > I've made it ftrace_nop_initialization() in my branch for now. I'm fine with ftrace_nop_initialize(). > > > > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c > > > index f296d89be757..afd7e210e595 100644 > > > --- a/kernel/trace/ftrace.c > > > +++ b/kernel/trace/ftrace.c > > > @@ -2493,15 +2493,22 @@ struct dyn_ftrace *ftrace_rec_iter_record(struct ftrace_rec_iter *iter) > > > return &iter->pg->records[iter->index]; > > > } > > > > > > +#ifndef ftrace_init_nop > > > +static int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec) > > > +{ > > > + return ftrace_make_nop(mod, rec, MCOUNT_ADDR); > > > +} > > > +#endif > > > > Can you place the above in the ftrace.h header. That's where that would > > belong. > > > > #ifndef ftrace_init_nop > > struct module; > > static inline int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec) > > { > > return ftrace_make_nop(mod, rec, MCOUNT_ADDR); > > } > > #endif > > True. > > I've put this immediately after ftrace_make_nop() in the header, and > given it a kerneldoc comment. There's a declaration for struct module at > the top of the header, so I've just relied on that > > That looks like: > > | /** > | * ftrace_init_nop - initialize a nop call site > | * @mod: module structure if called by module load initialization > | * @rec: the mcount call site record Perhaps say "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 be as initialized by the "should be as" is a bit confusing. What about? "The code segment at @rec->ip should contain the contents created by the compiler". -- Steve > | * compiler > | * > | * Return must be: > | * 0 on success > | * -EFAULT on error reading the location > | * -EINVAL on a failed compare of the contents > | * -EPERM on error writing to the location > | * Any other value will be considered a failure. > | */ > | #ifndef ftrace_init_nop > | static int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec) > | { > | return ftrace_make_nop(mod, rec, MCOUNT_ADDR); > | } > | #endif > > Thanks, > Mark.
WARNING: multiple messages have this Message-ID (diff)
From: Steven Rostedt <rostedt@goodmis.org> To: Mark Rutland <mark.rutland@arm.com> Cc: jthierry@redhat.com, will@kernel.org, ard.biesheuvel@linaro.org, peterz@infradead.org, catalin.marinas@arm.com, deller@gmx.de, jpoimboe@redhat.com, linux-kernel@vger.kernel.org, takahiro.akashi@linaro.org, mingo@redhat.com, james.morse@arm.com, jeyu@kernel.org, amit.kachhap@arm.com, svens@stackframe.org, duwe@suse.de, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH 1/8] ftrace: add ftrace_init_nop() Date: Tue, 22 Oct 2019 08:54:28 -0400 [thread overview] Message-ID: <20191022085428.75cfaad6@gandalf.local.home> (raw) In-Reply-To: <20191022112811.GA11583@lakrids.cambridge.arm.com> On Tue, 22 Oct 2019 12:28:11 +0100 Mark Rutland <mark.rutland@arm.com> wrote: > > To make the name even better, let's just rename it to: > > > > ftrace_nop_initialization() > > > > I think that may be the best description for it. > > Perhaps ftrace_nop_initialize(), so that it's not a noun? > > I've made it ftrace_nop_initialization() in my branch for now. I'm fine with ftrace_nop_initialize(). > > > > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c > > > index f296d89be757..afd7e210e595 100644 > > > --- a/kernel/trace/ftrace.c > > > +++ b/kernel/trace/ftrace.c > > > @@ -2493,15 +2493,22 @@ struct dyn_ftrace *ftrace_rec_iter_record(struct ftrace_rec_iter *iter) > > > return &iter->pg->records[iter->index]; > > > } > > > > > > +#ifndef ftrace_init_nop > > > +static int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec) > > > +{ > > > + return ftrace_make_nop(mod, rec, MCOUNT_ADDR); > > > +} > > > +#endif > > > > Can you place the above in the ftrace.h header. That's where that would > > belong. > > > > #ifndef ftrace_init_nop > > struct module; > > static inline int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec) > > { > > return ftrace_make_nop(mod, rec, MCOUNT_ADDR); > > } > > #endif > > True. > > I've put this immediately after ftrace_make_nop() in the header, and > given it a kerneldoc comment. There's a declaration for struct module at > the top of the header, so I've just relied on that > > That looks like: > > | /** > | * ftrace_init_nop - initialize a nop call site > | * @mod: module structure if called by module load initialization > | * @rec: the mcount call site record Perhaps say "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 be as initialized by the "should be as" is a bit confusing. What about? "The code segment at @rec->ip should contain the contents created by the compiler". -- Steve > | * compiler > | * > | * Return must be: > | * 0 on success > | * -EFAULT on error reading the location > | * -EINVAL on a failed compare of the contents > | * -EPERM on error writing to the location > | * Any other value will be considered a failure. > | */ > | #ifndef ftrace_init_nop > | static int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec) > | { > | return ftrace_make_nop(mod, rec, MCOUNT_ADDR); > | } > | #endif > > Thanks, > Mark. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2019-10-22 12:54 UTC|newest] Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-10-21 16:34 [PATCH 0/8] arm64: ftrace cleanup + FTRACE_WITH_REGS Mark Rutland 2019-10-21 16:34 ` Mark Rutland 2019-10-21 16:34 ` [PATCH 1/8] ftrace: add ftrace_init_nop() Mark Rutland 2019-10-21 16:34 ` Mark Rutland 2019-10-21 18:07 ` Steven Rostedt 2019-10-21 18:07 ` Steven Rostedt 2019-10-22 11:28 ` Mark Rutland 2019-10-22 11:28 ` Mark Rutland 2019-10-22 12:54 ` Steven Rostedt [this message] 2019-10-22 12:54 ` Steven Rostedt 2019-10-22 15:30 ` Mark Rutland 2019-10-22 15:30 ` Mark Rutland 2019-10-22 15:33 ` Mark Rutland 2019-10-22 15:33 ` Mark Rutland 2019-10-22 16:01 ` Steven Rostedt 2019-10-22 16:01 ` Steven Rostedt 2019-10-21 16:34 ` [PATCH 2/8] module/ftrace: handle patchable-function-entry Mark Rutland 2019-10-21 16:34 ` Mark Rutland 2019-10-21 16:34 ` [PATCH 3/8] arm64: module: rework special section handling Mark Rutland 2019-10-21 16:34 ` Mark Rutland 2019-10-21 16:34 ` [PATCH 4/8] arm64: module/ftrace: intialize PLT at load time Mark Rutland 2019-10-21 16:34 ` Mark Rutland 2019-10-21 16:34 ` [PATCH 5/8] arm64: insn: add encoder for MOV (register) Mark Rutland 2019-10-21 16:34 ` Mark Rutland 2019-10-21 16:34 ` [PATCH 6/8] arm64: asm-offsets: add S_FP Mark Rutland 2019-10-21 16:34 ` Mark Rutland 2019-10-21 16:34 ` [PATCH 7/8] arm64: implement ftrace with regs Mark Rutland 2019-10-21 16:34 ` Mark Rutland 2019-10-21 16:34 ` [PATCH 8/8] arm64: ftrace: minimize ifdeffery Mark Rutland 2019-10-21 16:34 ` Mark Rutland 2019-10-24 16:32 ` [PATCH 0/8] arm64: ftrace cleanup + FTRACE_WITH_REGS Ard Biesheuvel 2019-10-24 16:32 ` Ard Biesheuvel
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20191022085428.75cfaad6@gandalf.local.home \ --to=rostedt@goodmis.org \ --cc=amit.kachhap@arm.com \ --cc=ard.biesheuvel@linaro.org \ --cc=catalin.marinas@arm.com \ --cc=deller@gmx.de \ --cc=duwe@suse.de \ --cc=james.morse@arm.com \ --cc=jeyu@kernel.org \ --cc=jpoimboe@redhat.com \ --cc=jthierry@redhat.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=mark.rutland@arm.com \ --cc=mingo@redhat.com \ --cc=peterz@infradead.org \ --cc=svens@stackframe.org \ --cc=takahiro.akashi@linaro.org \ --cc=will@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.