| Message ID | 20230311135113.711824-1-trix@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 8732565549011cabbea08329a1aefd78a68d96c7 |
| Headers | show |
| Series | ftrace: set direct_ops storage-class-specifier to static | expand |
On Sat, 11 Mar 2023 08:51:13 -0500 Tom Rix <trix@redhat.com> wrote: > smatch reports this warning > kernel/trace/ftrace.c:2594:19: warning: > symbol 'direct_ops' was not declared. Should it be static? > > The variable direct_ops is only used in ftrace.c, so it should be static > > Signed-off-by: Tom Rix <trix@redhat.com> This looks good to me. Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> Thank you! > --- > kernel/trace/ftrace.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c > index 29baa97d0d53..05f76dc1f0c5 100644 > --- a/kernel/trace/ftrace.c > +++ b/kernel/trace/ftrace.c > @@ -2591,7 +2591,7 @@ static void call_direct_funcs(unsigned long ip, unsigned long pip, > arch_ftrace_set_direct_caller(fregs, addr); > } > > -struct ftrace_ops direct_ops = { > +static struct ftrace_ops direct_ops = { > .func = call_direct_funcs, > .flags = FTRACE_OPS_FL_DIRECT | FTRACE_OPS_FL_SAVE_REGS > | FTRACE_OPS_FL_PERMANENT, > -- > 2.27.0 >
On Sat, Mar 11, 2023 at 08:51:13AM -0500, Tom Rix wrote: > smatch reports this warning > kernel/trace/ftrace.c:2594:19: warning: > symbol 'direct_ops' was not declared. Should it be static? > > The variable direct_ops is only used in ftrace.c, so it should be static > > Signed-off-by: Tom Rix <trix@redhat.com> This makes sense, and as this stands: Acked-by: Mark Rutland <mark.rutland@arm.com> ... but Florent's rework of the API is going to remove this anyway: https://lore.kernel.org/all/20230316173811.1223508-1-revest@chromium.org/ https://lore.kernel.org/all/20230316173811.1223508-4-revest@chromium.org/ ... so if we take Florent's series, this patch isn't necessary. Mark. > --- > kernel/trace/ftrace.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c > index 29baa97d0d53..05f76dc1f0c5 100644 > --- a/kernel/trace/ftrace.c > +++ b/kernel/trace/ftrace.c > @@ -2591,7 +2591,7 @@ static void call_direct_funcs(unsigned long ip, unsigned long pip, > arch_ftrace_set_direct_caller(fregs, addr); > } > > -struct ftrace_ops direct_ops = { > +static struct ftrace_ops direct_ops = { > .func = call_direct_funcs, > .flags = FTRACE_OPS_FL_DIRECT | FTRACE_OPS_FL_SAVE_REGS > | FTRACE_OPS_FL_PERMANENT, > -- > 2.27.0 >
On Mon, 20 Mar 2023 17:44:13 +0000 Mark Rutland <mark.rutland@arm.com> wrote: > On Sat, Mar 11, 2023 at 08:51:13AM -0500, Tom Rix wrote: > > smatch reports this warning > > kernel/trace/ftrace.c:2594:19: warning: > > symbol 'direct_ops' was not declared. Should it be static? > > > > The variable direct_ops is only used in ftrace.c, so it should be static > > > > Signed-off-by: Tom Rix <trix@redhat.com> > > This makes sense, and as this stands: > > Acked-by: Mark Rutland <mark.rutland@arm.com> > > ... but Florent's rework of the API is going to remove this anyway: > > https://lore.kernel.org/all/20230316173811.1223508-1-revest@chromium.org/ > https://lore.kernel.org/all/20230316173811.1223508-4-revest@chromium.org/ > > ... so if we take Florent's series, this patch isn't necessary. Except that it's already upstream: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8732565549011cabbea08329a1aefd78a68d96c7 I had to rebase Florent's patch to accommodate this change. -- Steve
On Mon, Mar 20, 2023 at 01:51:32PM -0400, Steven Rostedt wrote: > On Mon, 20 Mar 2023 17:44:13 +0000 > Mark Rutland <mark.rutland@arm.com> wrote: > > On Sat, Mar 11, 2023 at 08:51:13AM -0500, Tom Rix wrote: > > > smatch reports this warning > > > kernel/trace/ftrace.c:2594:19: warning: > > > symbol 'direct_ops' was not declared. Should it be static? > > > > > > The variable direct_ops is only used in ftrace.c, so it should be static > > > > > > Signed-off-by: Tom Rix <trix@redhat.com> > > > > This makes sense, and as this stands: > > > > Acked-by: Mark Rutland <mark.rutland@arm.com> > > > > ... but Florent's rework of the API is going to remove this anyway: > > > > https://lore.kernel.org/all/20230316173811.1223508-1-revest@chromium.org/ > > https://lore.kernel.org/all/20230316173811.1223508-4-revest@chromium.org/ > > > > ... so if we take Florent's series, this patch isn't necessary. > > Except that it's already upstream: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8732565549011cabbea08329a1aefd78a68d96c7 > > I had to rebase Florent's patch to accommodate this change. Sorry for the noise; my fault for being so far behind with patch review! Mark.
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c index 29baa97d0d53..05f76dc1f0c5 100644 --- a/kernel/trace/ftrace.c +++ b/kernel/trace/ftrace.c @@ -2591,7 +2591,7 @@ static void call_direct_funcs(unsigned long ip, unsigned long pip, arch_ftrace_set_direct_caller(fregs, addr); } -struct ftrace_ops direct_ops = { +static struct ftrace_ops direct_ops = { .func = call_direct_funcs, .flags = FTRACE_OPS_FL_DIRECT | FTRACE_OPS_FL_SAVE_REGS | FTRACE_OPS_FL_PERMANENT,
smatch reports this warning kernel/trace/ftrace.c:2594:19: warning: symbol 'direct_ops' was not declared. Should it be static? The variable direct_ops is only used in ftrace.c, so it should be static Signed-off-by: Tom Rix <trix@redhat.com> --- kernel/trace/ftrace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)