| Message ID | e1429923d9eda92a3cf5ee9e33c7eacce539781d.1558115654.git.naveen.n.rao@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Nop out the preceding mflr with -mprofile-kernel | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch next (8150a153c013aa2dd1ffae43370b89ac1347a7fb) |
| snowpatch_ozlabs/checkpatch | fail | total: 1 errors, 0 warnings, 0 checks, 10 lines checked |
On Sat, 18 May 2019 00:32:46 +0530 "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote: > In commit a0572f687fb3c ("ftrace: Allow ftrace_replace_code() to be > schedulable), the generic ftrace_replace_code() function was modified to > accept a flags argument in place of a single 'enable' flag. However, the > x86 version of this function was not updated. Fix the same. > > Fixes: a0572f687fb3c ("ftrace: Allow ftrace_replace_code() to be schedulable") > Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> > --- > I haven't yet tested this patch on x86, but this looked wrong so sending > this as a RFC. This code has been through a bit of updates, and I need to go through and clean it up. I'll have to take a look and convert "int" to "bool" so that "enable" is not confusing. Thanks, I think I'll try to do a clean up first, and then this patch shouldn't "look wrong" after that. -- Steve > > - Naveen > > > arch/x86/kernel/ftrace.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c > index 0caf8122d680..0c01b344ba16 100644 > --- a/arch/x86/kernel/ftrace.c > +++ b/arch/x86/kernel/ftrace.c > @@ -554,8 +554,9 @@ static void run_sync(void) > local_irq_disable(); > } > > -void ftrace_replace_code(int enable) > +void ftrace_replace_code(int mod_flags) > { > + int enable = mod_flags & FTRACE_MODIFY_ENABLE_FL; > struct ftrace_rec_iter *iter; > struct dyn_ftrace *rec; > const char *report = "adding breakpoints";
On Mon, 20 May 2019 09:13:20 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > > I haven't yet tested this patch on x86, but this looked wrong so sending > > this as a RFC. > > This code has been through a bit of updates, and I need to go through > and clean it up. I'll have to take a look and convert "int" to "bool" > so that "enable" is not confusing. > > Thanks, I think I'll try to do a clean up first, and then this patch > shouldn't "look wrong" after that. > I'm going to apply the attached two patches. There may be some conflicts between yours here and these, but nothing that Linus can't figure out. Do you feel more comfortable with this code, if these patches are applied? -- Steve
Hi Steven, Steven Rostedt wrote: > On Mon, 20 May 2019 09:13:20 -0400 > Steven Rostedt <rostedt@goodmis.org> wrote: > > >> > I haven't yet tested this patch on x86, but this looked wrong so sending >> > this as a RFC. >> >> This code has been through a bit of updates, and I need to go through >> and clean it up. I'll have to take a look and convert "int" to "bool" >> so that "enable" is not confusing. >> >> Thanks, I think I'll try to do a clean up first, and then this patch >> shouldn't "look wrong" after that. >> > > I'm going to apply the attached two patches. There may be some > conflicts between yours here and these, but nothing that Linus can't > figure out. Do you feel more comfortable with this code, if these > patches are applied? Thanks, that definitely helps make things clearer. A very small nit from your first patch -- it would be good to also convert the calls to ftrace_check_record() to use 'true' or 'false' for the 'update' field. I will test my series in more detail and post a v1. - Naveen
On Mon, 20 May 2019 20:12:48 +0530 "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote: > Thanks, that definitely helps make things clearer. A very small nit from > your first patch -- it would be good to also convert the calls to > ftrace_check_record() to use 'true' or 'false' for the 'update' field. Heh, I was so focused on the "enable" part, I did the "update" as a second thought, and forgot to update the callers. Thanks for pointing that out. > > I will test my series in more detail and post a v1. Great. -- Steve
diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c index 0caf8122d680..0c01b344ba16 100644 --- a/arch/x86/kernel/ftrace.c +++ b/arch/x86/kernel/ftrace.c @@ -554,8 +554,9 @@ static void run_sync(void) local_irq_disable(); } -void ftrace_replace_code(int enable) +void ftrace_replace_code(int mod_flags) { + int enable = mod_flags & FTRACE_MODIFY_ENABLE_FL; struct ftrace_rec_iter *iter; struct dyn_ftrace *rec; const char *report = "adding breakpoints";
In commit a0572f687fb3c ("ftrace: Allow ftrace_replace_code() to be schedulable), the generic ftrace_replace_code() function was modified to accept a flags argument in place of a single 'enable' flag. However, the x86 version of this function was not updated. Fix the same. Fixes: a0572f687fb3c ("ftrace: Allow ftrace_replace_code() to be schedulable") Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> --- I haven't yet tested this patch on x86, but this looked wrong so sending this as a RFC. - Naveen arch/x86/kernel/ftrace.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)