* [PATCH] x86/ftrace: avoid an unused-function warning
@ 2019-03-04 20:07 Arnd Bergmann
2019-03-04 21:43 ` Steven Rostedt
0 siblings, 1 reply; 3+ messages in thread
From: Arnd Bergmann @ 2019-03-04 20:07 UTC (permalink / raw)
To: Steven Rostedt, Ingo Molnar, Thomas Gleixner, Borislav Petkov, x86
Cc: Arnd Bergmann, H. Peter Anvin, Masami Hiramatsu, linux-kernel
Depending on the configuration, ftrace_jmp_replace() sometimes
has no callers at all, since one of the users was removed:
arch/x86/kernel/ftrace.c:669:23: error: 'ftrace_jmp_replace' defined but not used [-Werror=unused-function]
Rather than trying to figure out the correct #ifdef check for this
time, just mark it __maybe_unused and let the compiler drop it
silently.
Fixes: d2a68c4effd8 ("x86/ftrace: Do not call function graph from dynamic trampolines")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
arch/x86/kernel/ftrace.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index 3e3789c8f8e1..905056240411 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -665,8 +665,7 @@ int __init ftrace_dyn_arch_init(void)
return 0;
}
-#if defined(CONFIG_X86_64) || defined(CONFIG_FUNCTION_GRAPH_TRACER)
-static unsigned char *ftrace_jmp_replace(unsigned long ip, unsigned long addr)
+static __maybe_unused unsigned char *ftrace_jmp_replace(unsigned long ip, unsigned long addr)
{
static union ftrace_code_union calc;
@@ -679,7 +678,6 @@ static unsigned char *ftrace_jmp_replace(unsigned long ip, unsigned long addr)
*/
return calc.code;
}
-#endif
/* Currently only x86_64 supports dynamic trampolines */
#ifdef CONFIG_X86_64
--
2.20.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] x86/ftrace: avoid an unused-function warning
2019-03-04 20:07 [PATCH] x86/ftrace: avoid an unused-function warning Arnd Bergmann
@ 2019-03-04 21:43 ` Steven Rostedt
2019-03-05 7:52 ` Arnd Bergmann
0 siblings, 1 reply; 3+ messages in thread
From: Steven Rostedt @ 2019-03-04 21:43 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Ingo Molnar, Thomas Gleixner, Borislav Petkov, x86,
H. Peter Anvin, Masami Hiramatsu, linux-kernel
On Mon, 4 Mar 2019 21:07:39 +0100
Arnd Bergmann <arnd@arndb.de> wrote:
> Depending on the configuration, ftrace_jmp_replace() sometimes
> has no callers at all, since one of the users was removed:
>
> arch/x86/kernel/ftrace.c:669:23: error: 'ftrace_jmp_replace' defined but not used [-Werror=unused-function]
>
> Rather than trying to figure out the correct #ifdef check for this
> time, just mark it __maybe_unused and let the compiler drop it
> silently.
>
Actually, its now only used in one place (that git grep shows).
Changing it to "__maybe_unused" is a copout, and really shouldn't be
something we do for a static function, especially since it's only used
in a single location.
Also, it's basically identical to ftrace_call_replace(). I'm going to
combine the two and move the jmp replace and have it do this instead.
-- Steve
From 8e5c6a3255de709ee0d699494243328cd5a11276 Mon Sep 17 00:00:00 2001
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
Date: Mon, 4 Mar 2019 16:35:22 -0500
Subject: [PATCH] x86/ftrace: Fix warning and considate ftrace_jmp_replace()
and ftrace_call_replace()
Arnd reported the following compiler warning:
arch/x86/kernel/ftrace.c:669:23: error: 'ftrace_jmp_replace' defined but not used [-Werror=unused-function]
The ftrace_jmp_replace() function now only has a single user and should be
simply moved by that user. But looking at the code, it shows that
ftrace_jmp_replace() is similar to ftrace_call_replace() except that instead
of using the opcode of 0xe8 it uses 0xe9. It makes more sense to consolidate
that function into one implementation that both ftrace_jmp_replace() and
ftrace_call_replace() use by passing in the op code separate.
The structure in ftrace_code_union is also modified to replace the "e8"
field with the more appropriate name "op".
Cc: stable@vger.kernel.org
Reported-by: Arnd Bergmann <arnd@arndb.de>
Link: http://lkml.kernel.org/r/20190304200748.1418790-1-arnd@arndb.de
Fixes: d2a68c4effd8 ("x86/ftrace: Do not call function graph from dynamic trampolines")
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
arch/x86/kernel/ftrace.c | 42 ++++++++++++++++------------------------
1 file changed, 17 insertions(+), 25 deletions(-)
diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index 8257a59704ae..763d4264d16a 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -49,7 +49,7 @@ int ftrace_arch_code_modify_post_process(void)
union ftrace_code_union {
char code[MCOUNT_INSN_SIZE];
struct {
- unsigned char e8;
+ unsigned char op;
int offset;
} __attribute__((packed));
};
@@ -59,20 +59,23 @@ static int ftrace_calc_offset(long ip, long addr)
return (int)(addr - ip);
}
-static unsigned char *ftrace_call_replace(unsigned long ip, unsigned long addr)
+static unsigned char *
+ftrace_text_replace(unsigned char op, unsigned long ip, unsigned long addr)
{
static union ftrace_code_union calc;
- calc.e8 = 0xe8;
+ calc.op = op;
calc.offset = ftrace_calc_offset(ip + MCOUNT_INSN_SIZE, addr);
- /*
- * No locking needed, this must be called via kstop_machine
- * which in essence is like running on a uniprocessor machine.
- */
return calc.code;
}
+static unsigned char *
+ftrace_call_replace(unsigned long ip, unsigned long addr)
+{
+ return ftrace_text_replace(0xe8, ip, addr);
+}
+
static inline int
within(unsigned long addr, unsigned long start, unsigned long end)
{
@@ -664,22 +667,6 @@ int __init ftrace_dyn_arch_init(void)
return 0;
}
-#if defined(CONFIG_X86_64) || defined(CONFIG_FUNCTION_GRAPH_TRACER)
-static unsigned char *ftrace_jmp_replace(unsigned long ip, unsigned long addr)
-{
- static union ftrace_code_union calc;
-
- /* Jmp not a call (ignore the .e8) */
- calc.e8 = 0xe9;
- calc.offset = ftrace_calc_offset(ip + MCOUNT_INSN_SIZE, addr);
-
- /*
- * ftrace external locks synchronize the access to the static variable.
- */
- return calc.code;
-}
-#endif
-
/* Currently only x86_64 supports dynamic trampolines */
#ifdef CONFIG_X86_64
@@ -891,8 +878,8 @@ static void *addr_from_call(void *ptr)
return NULL;
/* Make sure this is a call */
- if (WARN_ON_ONCE(calc.e8 != 0xe8)) {
- pr_warn("Expected e8, got %x\n", calc.e8);
+ if (WARN_ON_ONCE(calc.op != 0xe8)) {
+ pr_warn("Expected e8, got %x\n", calc.op);
return NULL;
}
@@ -963,6 +950,11 @@ void arch_ftrace_trampoline_free(struct ftrace_ops *ops)
#ifdef CONFIG_DYNAMIC_FTRACE
extern void ftrace_graph_call(void);
+static unsigned char *ftrace_jmp_replace(unsigned long ip, unsigned long addr)
+{
+ return ftrace_text_replace(0xe9, ip, addr);
+}
+
static int ftrace_mod_jmp(unsigned long ip, void *func)
{
unsigned char *new;
--
2.20.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] x86/ftrace: avoid an unused-function warning
2019-03-04 21:43 ` Steven Rostedt
@ 2019-03-05 7:52 ` Arnd Bergmann
0 siblings, 0 replies; 3+ messages in thread
From: Arnd Bergmann @ 2019-03-05 7:52 UTC (permalink / raw)
To: Steven Rostedt
Cc: Ingo Molnar, Thomas Gleixner, Borislav Petkov,
the arch/x86 maintainers, H. Peter Anvin, Masami Hiramatsu,
Linux Kernel Mailing List
On Mon, Mar 4, 2019 at 10:43 PM Steven Rostedt <rostedt@goodmis.org> wrote:
> On Mon, 4 Mar 2019 21:07:39 +0100 Arnd Bergmann <arnd@arndb.de> wrote:
...
> Also, it's basically identical to ftrace_call_replace(). I'm going to
> combine the two and move the jmp replace and have it do this instead.
Sounds good.
> arch/x86/kernel/ftrace.c:669:23: error: 'ftrace_jmp_replace' defined but not used [-Werror=unused-function]
>
> The ftrace_jmp_replace() function now only has a single user and should be
> simply moved by that user. But looking at the code, it shows that
> ftrace_jmp_replace() is similar to ftrace_call_replace() except that instead
> of using the opcode of 0xe8 it uses 0xe9. It makes more sense to consolidate
> that function into one implementation that both ftrace_jmp_replace() and
> ftrace_call_replace() use by passing in the op code separate.
>
> The structure in ftrace_code_union is also modified to replace the "e8"
> field with the more appropriate name "op".
>
> Cc: stable@vger.kernel.org
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Link: http://lkml.kernel.org/r/20190304200748.1418790-1-arnd@arndb.de
> Fixes: d2a68c4effd8 ("x86/ftrace: Do not call function graph from dynamic trampolines")
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Acked-by: Arnd Bergmann <arnd@arndb.de>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-03-05 7:52 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-04 20:07 [PATCH] x86/ftrace: avoid an unused-function warning Arnd Bergmann
2019-03-04 21:43 ` Steven Rostedt
2019-03-05 7:52 ` Arnd Bergmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).