Every arch_syscall_addr() implementation thus far is the same, so unify
them as a default weak in common code so more arches don't have to waste
time copying & pasting this simple function. The Blackfin version is
going to be exactly the same.
Signed-off-by: Mike Frysinger <[email protected]>
---
note: only thing that needs double checking is s390 and sparc where they
declared the sys_call_table as an array of ints. considering this table
is supposed to be an array of function pointers, this seems like more of
a typo to me ...
also, does the arch_syscall_addr() even really need to be weak ? or can
we assume that everyone is going to be sane and do it the same way ...
Documentation/trace/ftrace-design.txt | 6 +++++-
arch/s390/kernel/ftrace.c | 10 ----------
arch/sh/kernel/ftrace.c | 9 ---------
arch/sparc/kernel/ftrace.c | 11 -----------
arch/x86/kernel/ftrace.c | 10 ----------
include/linux/ftrace.h | 6 ++++++
kernel/trace/trace_syscalls.c | 6 ++++++
7 files changed, 17 insertions(+), 41 deletions(-)
diff --git a/Documentation/trace/ftrace-design.txt b/Documentation/trace/ftrace-design.txt
index 6a5a579..1a67b8c 100644
--- a/Documentation/trace/ftrace-design.txt
+++ b/Documentation/trace/ftrace-design.txt
@@ -240,8 +240,12 @@ You need very few things to get the syscalls tracing in an arch.
- Have a NR_syscalls variable in <asm/unistd.h> that provides the number
of syscalls supported by the arch.
+- Implement <asm/syscall.h>.
- Implement arch_syscall_addr() that resolves a syscall address from a
- syscall number.
+ syscall number. For the simple arches where your syscall table is an
+ array of longs named "sys_call_table", there is a default implementation
+ in kernel/trace/trace_syscalls.c. If your arch needs something weird,
+ then you'll have to define the function yourself.
- Support the TIF_SYSCALL_TRACEPOINT thread flags
- Put the trace_sys_enter() and trace_sys_exit() tracepoints calls from ptrace
in the ptrace syscalls tracing path.
diff --git a/arch/s390/kernel/ftrace.c b/arch/s390/kernel/ftrace.c
index 5a82bc6..9e69449 100644
--- a/arch/s390/kernel/ftrace.c
+++ b/arch/s390/kernel/ftrace.c
@@ -200,13 +200,3 @@ out:
return parent;
}
#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
-
-#ifdef CONFIG_FTRACE_SYSCALLS
-
-extern unsigned int sys_call_table[];
-
-unsigned long __init arch_syscall_addr(int nr)
-{
- return (unsigned long)sys_call_table[nr];
-}
-#endif
diff --git a/arch/sh/kernel/ftrace.c b/arch/sh/kernel/ftrace.c
index a48cded..30e1319 100644
--- a/arch/sh/kernel/ftrace.c
+++ b/arch/sh/kernel/ftrace.c
@@ -399,12 +399,3 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr)
}
}
#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
-
-#ifdef CONFIG_FTRACE_SYSCALLS
-extern unsigned long *sys_call_table;
-
-unsigned long __init arch_syscall_addr(int nr)
-{
- return (unsigned long)sys_call_table[nr];
-}
-#endif /* CONFIG_FTRACE_SYSCALLS */
diff --git a/arch/sparc/kernel/ftrace.c b/arch/sparc/kernel/ftrace.c
index 29973da..9103a56 100644
--- a/arch/sparc/kernel/ftrace.c
+++ b/arch/sparc/kernel/ftrace.c
@@ -91,14 +91,3 @@ int __init ftrace_dyn_arch_init(void *data)
return 0;
}
#endif
-
-#ifdef CONFIG_FTRACE_SYSCALLS
-
-extern unsigned int sys_call_table[];
-
-unsigned long __init arch_syscall_addr(int nr)
-{
- return (unsigned long)sys_call_table[nr];
-}
-
-#endif
diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index 3096892..0d93a94 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -484,13 +484,3 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr,
}
}
#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
-
-#ifdef CONFIG_FTRACE_SYSCALLS
-
-extern unsigned long *sys_call_table;
-
-unsigned long __init arch_syscall_addr(int nr)
-{
- return (unsigned long)(&sys_call_table)[nr];
-}
-#endif
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 0b4f97d..1cbb36f 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -511,4 +511,10 @@ static inline void trace_hw_branch_oops(void) {}
#endif /* CONFIG_HW_BRANCH_TRACER */
+#ifdef CONFIG_FTRACE_SYSCALLS
+
+unsigned long arch_syscall_addr(int nr);
+
+#endif /* CONFIG_FTRACE_SYSCALLS */
+
#endif /* _LINUX_FTRACE_H */
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index 75289f3..671c670 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -394,6 +394,12 @@ int init_syscall_trace(struct ftrace_event_call *call)
return 0;
}
+unsigned long __init __weak arch_syscall_addr(int nr)
+{
+ extern const unsigned long sys_call_table[];
+ return sys_call_table[nr];
+}
+
int __init init_ftrace_syscalls(void)
{
struct syscall_metadata *meta;
--
1.6.6.1
[ Added Heiko Carstens and Paul Mundt to Cc ]
On Fri, 2010-01-22 at 08:43 -0500, Mike Frysinger wrote:
> Every arch_syscall_addr() implementation thus far is the same, so unify
> them as a default weak in common code so more arches don't have to waste
> time copying & pasting this simple function. The Blackfin version is
> going to be exactly the same.
>
> Signed-off-by: Mike Frysinger <[email protected]>
> ---
> note: only thing that needs double checking is s390 and sparc where they
> declared the sys_call_table as an array of ints. considering this table
> is supposed to be an array of function pointers, this seems like more of
> a typo to me ...
I would not be too sure. s390 is very strange, and I would definitely
want to get an Ack from the arch maintainers first.
>
> also, does the arch_syscall_addr() even really need to be weak ? or can
> we assume that everyone is going to be sane and do it the same way ...
>
> Documentation/trace/ftrace-design.txt | 6 +++++-
> arch/s390/kernel/ftrace.c | 10 ----------
> arch/sh/kernel/ftrace.c | 9 ---------
> arch/sparc/kernel/ftrace.c | 11 -----------
> arch/x86/kernel/ftrace.c | 10 ----------
> include/linux/ftrace.h | 6 ++++++
> kernel/trace/trace_syscalls.c | 6 +++++
Actually for this patch to be accepted, we need to get the acks from the
maintainers of s390, sh, and sparc since it touches their code. Doesn't
matter if it is just removing duplicate code. Still need their acks.
Thanks,
-- Steve
> +
> 7 files changed, 17 insertions(+), 41 deletions(-)
>
> diff --git a/Documentation/trace/ftrace-design.txt b/Documentation/trace/ftrace-design.txt
> index 6a5a579..1a67b8c 100644
> --- a/Documentation/trace/ftrace-design.txt
> +++ b/Documentation/trace/ftrace-design.txt
> @@ -240,8 +240,12 @@ You need very few things to get the syscalls tracing in an arch.
>
> - Have a NR_syscalls variable in <asm/unistd.h> that provides the number
> of syscalls supported by the arch.
> +- Implement <asm/syscall.h>.
> - Implement arch_syscall_addr() that resolves a syscall address from a
> - syscall number.
> + syscall number. For the simple arches where your syscall table is an
> + array of longs named "sys_call_table", there is a default implementation
> + in kernel/trace/trace_syscalls.c. If your arch needs something weird,
> + then you'll have to define the function yourself.
> - Support the TIF_SYSCALL_TRACEPOINT thread flags
> - Put the trace_sys_enter() and trace_sys_exit() tracepoints calls from ptrace
> in the ptrace syscalls tracing path.
> diff --git a/arch/s390/kernel/ftrace.c b/arch/s390/kernel/ftrace.c
> index 5a82bc6..9e69449 100644
> --- a/arch/s390/kernel/ftrace.c
> +++ b/arch/s390/kernel/ftrace.c
> @@ -200,13 +200,3 @@ out:
> return parent;
> }
> #endif /* CONFIG_FUNCTION_GRAPH_TRACER */
> -
> -#ifdef CONFIG_FTRACE_SYSCALLS
> -
> -extern unsigned int sys_call_table[];
> -
> -unsigned long __init arch_syscall_addr(int nr)
> -{
> - return (unsigned long)sys_call_table[nr];
> -}
> -#endif
> diff --git a/arch/sh/kernel/ftrace.c b/arch/sh/kernel/ftrace.c
> index a48cded..30e1319 100644
> --- a/arch/sh/kernel/ftrace.c
> +++ b/arch/sh/kernel/ftrace.c
> @@ -399,12 +399,3 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr)
> }
> }
> #endif /* CONFIG_FUNCTION_GRAPH_TRACER */
> -
> -#ifdef CONFIG_FTRACE_SYSCALLS
> -extern unsigned long *sys_call_table;
> -
> -unsigned long __init arch_syscall_addr(int nr)
> -{
> - return (unsigned long)sys_call_table[nr];
> -}
> -#endif /* CONFIG_FTRACE_SYSCALLS */
> diff --git a/arch/sparc/kernel/ftrace.c b/arch/sparc/kernel/ftrace.c
> index 29973da..9103a56 100644
> --- a/arch/sparc/kernel/ftrace.c
> +++ b/arch/sparc/kernel/ftrace.c
> @@ -91,14 +91,3 @@ int __init ftrace_dyn_arch_init(void *data)
> return 0;
> }
> #endif
> -
> -#ifdef CONFIG_FTRACE_SYSCALLS
> -
> -extern unsigned int sys_call_table[];
> -
> -unsigned long __init arch_syscall_addr(int nr)
> -{
> - return (unsigned long)sys_call_table[nr];
> -}
> -
> -#endif
> diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
> index 3096892..0d93a94 100644
> --- a/arch/x86/kernel/ftrace.c
> +++ b/arch/x86/kernel/ftrace.c
> @@ -484,13 +484,3 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr,
> }
> }
> #endif /* CONFIG_FUNCTION_GRAPH_TRACER */
> -
> -#ifdef CONFIG_FTRACE_SYSCALLS
> -
> -extern unsigned long *sys_call_table;
> -
> -unsigned long __init arch_syscall_addr(int nr)
> -{
> - return (unsigned long)(&sys_call_table)[nr];
> -}
> -#endif
> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> index 0b4f97d..1cbb36f 100644
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
> @@ -511,4 +511,10 @@ static inline void trace_hw_branch_oops(void) {}
>
> #endif /* CONFIG_HW_BRANCH_TRACER */
>
> +#ifdef CONFIG_FTRACE_SYSCALLS
> +
> +unsigned long arch_syscall_addr(int nr);
> +
> +#endif /* CONFIG_FTRACE_SYSCALLS */
> +
> #endif /* _LINUX_FTRACE_H */
> diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
> index 75289f3..671c670 100644
> --- a/kernel/trace/trace_syscalls.c
> +++ b/kernel/trace/trace_syscalls.c
> @@ -394,6 +394,12 @@ int init_syscall_trace(struct ftrace_event_call *call)
> return 0;
> }
>
> +unsigned long __init __weak arch_syscall_addr(int nr)
> +{
> + extern const unsigned long sys_call_table[];
> + return sys_call_table[nr];
> +}
> +
> int __init init_ftrace_syscalls(void)
> {
> struct syscall_metadata *meta;
On Fri, Jan 22, 2010 at 09:36:17AM -0500, Steven Rostedt wrote:
> [ Added Heiko Carstens and Paul Mundt to Cc ]
>
> On Fri, 2010-01-22 at 08:43 -0500, Mike Frysinger wrote:
> > Every arch_syscall_addr() implementation thus far is the same, so unify
> > them as a default weak in common code so more arches don't have to waste
> > time copying & pasting this simple function. The Blackfin version is
> > going to be exactly the same.
> >
> > Signed-off-by: Mike Frysinger <[email protected]>
> > ---
> > note: only thing that needs double checking is s390 and sparc where they
> > declared the sys_call_table as an array of ints. considering this table
> > is supposed to be an array of function pointers, this seems like more of
> > a typo to me ...
>
> I would not be too sure. s390 is very strange, and I would definitely
> want to get an Ack from the arch maintainers first.
It's not a typo. The syscall table on s390 contains always 32 bit pointers
since we know that the address of the function to be called is (way) below
4GB. So this saves us a few bytes.
In addition this makes syscall patching done by some security modules a
bit more difficult, since they would need to store a 64 bit pointer.
That's because we make sure that module addresses are always above 4GB.
On Fri, Jan 22, 2010 at 11:37, Heiko Carstens wrote:
> On Fri, Jan 22, 2010 at 09:36:17AM -0500, Steven Rostedt wrote:
>> On Fri, 2010-01-22 at 08:43 -0500, Mike Frysinger wrote:
>> > Every arch_syscall_addr() implementation thus far is the same, so unify
>> > them as a default weak in common code so more arches don't have to waste
>> > time copying & pasting this simple function. The Blackfin version is
>> > going to be exactly the same.
>> >
>> > Signed-off-by: Mike Frysinger <[email protected]>
>> > ---
>> > note: only thing that needs double checking is s390 and sparc where they
>> > declared the sys_call_table as an array of ints. considering this table
>> > is supposed to be an array of function pointers, this seems like more of
>> > a typo to me ...
>>
>> I would not be too sure. s390 is very strange, and I would definitely
>> want to get an Ack from the arch maintainers first.
>
> It's not a typo. The syscall table on s390 contains always 32 bit pointers
> since we know that the address of the function to be called is (way) below
> 4GB. So this saves us a few bytes.
> In addition this makes syscall patching done by some security modules a
> bit more difficult, since they would need to store a 64 bit pointer.
> That's because we make sure that module addresses are always above 4GB.
i'll update the s390 code to add an explanatory comment then and post
a new version
-mike
From: Steven Rostedt <[email protected]>
Date: Fri, 22 Jan 2010 09:36:17 -0500
> [ Added Heiko Carstens and Paul Mundt to Cc ]
>
> On Fri, 2010-01-22 at 08:43 -0500, Mike Frysinger wrote:
>> also, does the arch_syscall_addr() even really need to be weak ? or can
>> we assume that everyone is going to be sane and do it the same way ...
>>
>> Documentation/trace/ftrace-design.txt | 6 +++++-
>> arch/s390/kernel/ftrace.c | 10 ----------
>> arch/sh/kernel/ftrace.c | 9 ---------
>> arch/sparc/kernel/ftrace.c | 11 -----------
>> arch/x86/kernel/ftrace.c | 10 ----------
>> include/linux/ftrace.h | 6 ++++++
>> kernel/trace/trace_syscalls.c | 6 +++++
>
> Actually for this patch to be accepted, we need to get the acks from the
> maintainers of s390, sh, and sparc since it touches their code. Doesn't
> matter if it is just removing duplicate code. Still need their acks.
Acked-by: David S. Miller <[email protected]>
From: Heiko Carstens <[email protected]>
Date: Fri, 22 Jan 2010 17:37:00 +0100
> On Fri, Jan 22, 2010 at 09:36:17AM -0500, Steven Rostedt wrote:
>> I would not be too sure. s390 is very strange, and I would definitely
>> want to get an Ack from the arch maintainers first.
>
> It's not a typo. The syscall table on s390 contains always 32 bit pointers
> since we know that the address of the function to be called is (way) below
> 4GB. So this saves us a few bytes.
> In addition this makes syscall patching done by some security modules a
> bit more difficult, since they would need to store a 64 bit pointer.
> That's because we make sure that module addresses are always above 4GB.
Sparc64 is the same way.
Most implementations of arch_syscall_addr() are the same, so create a
default weak version in common code. New arch ports don't have to waste
time copying & pasting this simple function. The Blackfin version is
going to be exactly the same for example.
The s390/sparc versions need to be different, so document why.
Signed-off-by: Mike Frysinger <[email protected]>
---
v2
- document s390/sparc funkiness
Documentation/trace/ftrace-design.txt | 6 +++++-
arch/s390/kernel/ftrace.c | 7 ++++++-
arch/sh/kernel/ftrace.c | 9 ---------
arch/sparc/kernel/ftrace.c | 7 ++++++-
arch/x86/kernel/ftrace.c | 10 ----------
include/linux/ftrace.h | 6 ++++++
kernel/trace/trace_syscalls.c | 6 ++++++
7 files changed, 29 insertions(+), 22 deletions(-)
diff --git a/Documentation/trace/ftrace-design.txt b/Documentation/trace/ftrace-design.txt
index 6a5a579..1a67b8c 100644
--- a/Documentation/trace/ftrace-design.txt
+++ b/Documentation/trace/ftrace-design.txt
@@ -240,8 +240,12 @@ You need very few things to get the syscalls tracing in an arch.
- Have a NR_syscalls variable in <asm/unistd.h> that provides the number
of syscalls supported by the arch.
+- Implement <asm/syscall.h>.
- Implement arch_syscall_addr() that resolves a syscall address from a
- syscall number.
+ syscall number. For the simple arches where your syscall table is an
+ array of longs named "sys_call_table", there is a default implementation
+ in kernel/trace/trace_syscalls.c. If your arch needs something weird,
+ then you'll have to define the function yourself.
- Support the TIF_SYSCALL_TRACEPOINT thread flags
- Put the trace_sys_enter() and trace_sys_exit() tracepoints calls from ptrace
in the ptrace syscalls tracing path.
diff --git a/arch/s390/kernel/ftrace.c b/arch/s390/kernel/ftrace.c
index 5a82bc6..d5fa352 100644
--- a/arch/s390/kernel/ftrace.c
+++ b/arch/s390/kernel/ftrace.c
@@ -203,7 +203,12 @@ out:
#ifdef CONFIG_FTRACE_SYSCALLS
-extern unsigned int sys_call_table[];
+/*
+ * The syscall table always contains 32 bit pointers since we know that the
+ * address of the function to be called is (way) below 4GB. So the "int"
+ * type here is what we want [need] for both 32 bit and 64 bit systems.
+ */
+extern const unsigned int sys_call_table[];
unsigned long __init arch_syscall_addr(int nr)
{
diff --git a/arch/sh/kernel/ftrace.c b/arch/sh/kernel/ftrace.c
index a48cded..30e1319 100644
--- a/arch/sh/kernel/ftrace.c
+++ b/arch/sh/kernel/ftrace.c
@@ -399,12 +399,3 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr)
}
}
#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
-
-#ifdef CONFIG_FTRACE_SYSCALLS
-extern unsigned long *sys_call_table;
-
-unsigned long __init arch_syscall_addr(int nr)
-{
- return (unsigned long)sys_call_table[nr];
-}
-#endif /* CONFIG_FTRACE_SYSCALLS */
diff --git a/arch/sparc/kernel/ftrace.c b/arch/sparc/kernel/ftrace.c
index 29973da..deeeaa6 100644
--- a/arch/sparc/kernel/ftrace.c
+++ b/arch/sparc/kernel/ftrace.c
@@ -94,7 +94,12 @@ int __init ftrace_dyn_arch_init(void *data)
#ifdef CONFIG_FTRACE_SYSCALLS
-extern unsigned int sys_call_table[];
+/*
+ * The syscall table always contains 32 bit pointers since we know that the
+ * address of the function to be called is (way) below 4GB. So the "int"
+ * type here is what we want [need] for both 32 bit and 64 bit systems.
+ */
+extern const unsigned int sys_call_table[];
unsigned long __init arch_syscall_addr(int nr)
{
diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index 3096892..0d93a94 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -484,13 +484,3 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr,
}
}
#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
-
-#ifdef CONFIG_FTRACE_SYSCALLS
-
-extern unsigned long *sys_call_table;
-
-unsigned long __init arch_syscall_addr(int nr)
-{
- return (unsigned long)(&sys_call_table)[nr];
-}
-#endif
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 0b4f97d..1cbb36f 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -511,4 +511,10 @@ static inline void trace_hw_branch_oops(void) {}
#endif /* CONFIG_HW_BRANCH_TRACER */
+#ifdef CONFIG_FTRACE_SYSCALLS
+
+unsigned long arch_syscall_addr(int nr);
+
+#endif /* CONFIG_FTRACE_SYSCALLS */
+
#endif /* _LINUX_FTRACE_H */
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index 75289f3..671c670 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -394,6 +394,12 @@ int init_syscall_trace(struct ftrace_event_call *call)
return 0;
}
+unsigned long __init __weak arch_syscall_addr(int nr)
+{
+ extern const unsigned long sys_call_table[];
+ return sys_call_table[nr];
+}
+
int __init init_ftrace_syscalls(void)
{
struct syscall_metadata *meta;
--
1.6.6.1
From: Mike Frysinger <[email protected]>
Date: Sat, 23 Jan 2010 02:10:43 -0500
> Most implementations of arch_syscall_addr() are the same, so create a
> default weak version in common code. New arch ports don't have to waste
> time copying & pasting this simple function. The Blackfin version is
> going to be exactly the same for example.
>
> The s390/sparc versions need to be different, so document why.
>
> Signed-off-by: Mike Frysinger <[email protected]>
You can just have the extern declaration match whatever the
arch needs (long or int) in the generic ftrace.c code and
then unconditionally do the (unsigned long) case.
That way even s390/sparc can use the generic implementation.
On Sat, Jan 23, 2010 at 03:28, David Miller <[email protected]> wrote:
> From: Mike Frysinger <[email protected]>
> Date: Sat, 23 Jan 2010 02:10:43 -0500
>
>> Most implementations of arch_syscall_addr() are the same, so create a
>> default weak version in common code. New arch ports don't have to waste
>> time copying & pasting this simple function. The Blackfin version is
>> going to be exactly the same for example.
>>
>> The s390/sparc versions need to be different, so document why.
>>
>> Signed-off-by: Mike Frysinger <[email protected]>
>
> You can just have the extern declaration match whatever the
> arch needs (long or int) in the generic ftrace.c code and
> then unconditionally do the (unsigned long) case.
>
> That way even s390/sparc can use the generic implementation.
asm/syscall.h is the most reasonable location to move the decls to in
asm/ ... people OK with that ?
-mike
On Fri, Jan 22, 2010 at 09:36:17AM -0500, Steven Rostedt wrote:
> [ Added Heiko Carstens and Paul Mundt to Cc ]
>
> On Fri, 2010-01-22 at 08:43 -0500, Mike Frysinger wrote:
> > also, does the arch_syscall_addr() even really need to be weak ? or can
> > we assume that everyone is going to be sane and do it the same way ...
> >
> > Documentation/trace/ftrace-design.txt | 6 +++++-
> > arch/s390/kernel/ftrace.c | 10 ----------
> > arch/sh/kernel/ftrace.c | 9 ---------
> > arch/sparc/kernel/ftrace.c | 11 -----------
> > arch/x86/kernel/ftrace.c | 10 ----------
> > include/linux/ftrace.h | 6 ++++++
> > kernel/trace/trace_syscalls.c | 6 +++++
>
> Actually for this patch to be accepted, we need to get the acks from the
> maintainers of s390, sh, and sparc since it touches their code. Doesn't
> matter if it is just removing duplicate code. Still need their acks.
>
Acked-by: Paul Mundt <[email protected]>
Most implementations of arch_syscall_addr() are the same, so create a
default version in common code and move the one piece that differs (the
syscall table) to asm/syscall.h. New arch ports don't have to waste
time copying & pasting this simple function.
The s390/sparc versions need to be different, so document why.
Signed-off-by: Mike Frysinger <[email protected]>
Acked-by: David S. Miller <[email protected]>
Acked-by: Paul Mundt <[email protected]>
---
v3
- move sys_call_table[] decl to asm/syscall.h
- unify everyone's arch_syscall_addr() again
- mention right tracehook requirements
Documentation/trace/ftrace-design.txt | 5 ++---
arch/s390/include/asm/syscall.h | 7 +++++++
arch/s390/kernel/ftrace.c | 10 ----------
arch/sh/include/asm/syscall.h | 2 ++
arch/sh/kernel/ftrace.c | 9 ---------
arch/sparc/include/asm/syscall.h | 7 +++++++
arch/sparc/kernel/ftrace.c | 11 -----------
arch/x86/include/asm/syscall.h | 2 ++
arch/x86/kernel/ftrace.c | 10 ----------
include/linux/ftrace.h | 6 ++++++
kernel/trace/trace_syscalls.c | 5 +++++
11 files changed, 31 insertions(+), 43 deletions(-)
diff --git a/Documentation/trace/ftrace-design.txt b/Documentation/trace/ftrace-design.txt
index 6a5a579..f1f81af 100644
--- a/Documentation/trace/ftrace-design.txt
+++ b/Documentation/trace/ftrace-design.txt
@@ -238,11 +238,10 @@ HAVE_SYSCALL_TRACEPOINTS
You need very few things to get the syscalls tracing in an arch.
+- Support HAVE_ARCH_TRACEHOOK (see arch/Kconfig).
- Have a NR_syscalls variable in <asm/unistd.h> that provides the number
of syscalls supported by the arch.
-- Implement arch_syscall_addr() that resolves a syscall address from a
- syscall number.
-- Support the TIF_SYSCALL_TRACEPOINT thread flags
+- Support the TIF_SYSCALL_TRACEPOINT thread flags.
- Put the trace_sys_enter() and trace_sys_exit() tracepoints calls from ptrace
in the ptrace syscalls tracing path.
- Tag this arch as HAVE_SYSCALL_TRACEPOINTS.
diff --git a/arch/s390/include/asm/syscall.h b/arch/s390/include/asm/syscall.h
index e0a73d3..8429686 100644
--- a/arch/s390/include/asm/syscall.h
+++ b/arch/s390/include/asm/syscall.h
@@ -15,6 +15,13 @@
#include <linux/sched.h>
#include <asm/ptrace.h>
+/*
+ * The syscall table always contains 32 bit pointers since we know that the
+ * address of the function to be called is (way) below 4GB. So the "int"
+ * type here is what we want [need] for both 32 bit and 64 bit systems.
+ */
+extern const unsigned int sys_call_table[];
+
static inline long syscall_get_nr(struct task_struct *task,
struct pt_regs *regs)
{
diff --git a/arch/s390/kernel/ftrace.c b/arch/s390/kernel/ftrace.c
index 5a82bc6..9e69449 100644
--- a/arch/s390/kernel/ftrace.c
+++ b/arch/s390/kernel/ftrace.c
@@ -200,13 +200,3 @@ out:
return parent;
}
#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
-
-#ifdef CONFIG_FTRACE_SYSCALLS
-
-extern unsigned int sys_call_table[];
-
-unsigned long __init arch_syscall_addr(int nr)
-{
- return (unsigned long)sys_call_table[nr];
-}
-#endif
diff --git a/arch/sh/include/asm/syscall.h b/arch/sh/include/asm/syscall.h
index 6a38142..aa7777b 100644
--- a/arch/sh/include/asm/syscall.h
+++ b/arch/sh/include/asm/syscall.h
@@ -1,6 +1,8 @@
#ifndef __ASM_SH_SYSCALL_H
#define __ASM_SH_SYSCALL_H
+extern const unsigned long sys_call_table[];
+
#ifdef CONFIG_SUPERH32
# include "syscall_32.h"
#else
diff --git a/arch/sh/kernel/ftrace.c b/arch/sh/kernel/ftrace.c
index a48cded..30e1319 100644
--- a/arch/sh/kernel/ftrace.c
+++ b/arch/sh/kernel/ftrace.c
@@ -399,12 +399,3 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr)
}
}
#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
-
-#ifdef CONFIG_FTRACE_SYSCALLS
-extern unsigned long *sys_call_table;
-
-unsigned long __init arch_syscall_addr(int nr)
-{
- return (unsigned long)sys_call_table[nr];
-}
-#endif /* CONFIG_FTRACE_SYSCALLS */
diff --git a/arch/sparc/include/asm/syscall.h b/arch/sparc/include/asm/syscall.h
index 7486c60..025a02a 100644
--- a/arch/sparc/include/asm/syscall.h
+++ b/arch/sparc/include/asm/syscall.h
@@ -5,6 +5,13 @@
#include <linux/sched.h>
#include <asm/ptrace.h>
+/*
+ * The syscall table always contains 32 bit pointers since we know that the
+ * address of the function to be called is (way) below 4GB. So the "int"
+ * type here is what we want [need] for both 32 bit and 64 bit systems.
+ */
+extern const unsigned int sys_call_table[];
+
/* The system call number is given by the user in %g1 */
static inline long syscall_get_nr(struct task_struct *task,
struct pt_regs *regs)
diff --git a/arch/sparc/kernel/ftrace.c b/arch/sparc/kernel/ftrace.c
index 29973da..9103a56 100644
--- a/arch/sparc/kernel/ftrace.c
+++ b/arch/sparc/kernel/ftrace.c
@@ -91,14 +91,3 @@ int __init ftrace_dyn_arch_init(void *data)
return 0;
}
#endif
-
-#ifdef CONFIG_FTRACE_SYSCALLS
-
-extern unsigned int sys_call_table[];
-
-unsigned long __init arch_syscall_addr(int nr)
-{
- return (unsigned long)sys_call_table[nr];
-}
-
-#endif
diff --git a/arch/x86/include/asm/syscall.h b/arch/x86/include/asm/syscall.h
index 8d33bc5..c4a348f 100644
--- a/arch/x86/include/asm/syscall.h
+++ b/arch/x86/include/asm/syscall.h
@@ -16,6 +16,8 @@
#include <linux/sched.h>
#include <linux/err.h>
+extern const unsigned long sys_call_table[];
+
/*
* Only the low 32 bits of orig_ax are meaningful, so we return int.
* This importantly ignores the high bits on 64-bit, so comparisons
diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index 3096892..0d93a94 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -484,13 +484,3 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr,
}
}
#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
-
-#ifdef CONFIG_FTRACE_SYSCALLS
-
-extern unsigned long *sys_call_table;
-
-unsigned long __init arch_syscall_addr(int nr)
-{
- return (unsigned long)(&sys_call_table)[nr];
-}
-#endif
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 0b4f97d..1cbb36f 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -511,4 +511,10 @@ static inline void trace_hw_branch_oops(void) {}
#endif /* CONFIG_HW_BRANCH_TRACER */
+#ifdef CONFIG_FTRACE_SYSCALLS
+
+unsigned long arch_syscall_addr(int nr);
+
+#endif /* CONFIG_FTRACE_SYSCALLS */
+
#endif /* _LINUX_FTRACE_H */
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index 75289f3..a72732e 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -394,6 +394,11 @@ int init_syscall_trace(struct ftrace_event_call *call)
return 0;
}
+unsigned long __init arch_syscall_addr(int nr)
+{
+ return (unsigned long)sys_call_table[nr];
+}
+
int __init init_ftrace_syscalls(void)
{
struct syscall_metadata *meta;
--
1.6.6.1
On Tue, Jan 26, 2010 at 04:40:03AM -0500, Mike Frysinger wrote:
> Most implementations of arch_syscall_addr() are the same, so create a
> default version in common code and move the one piece that differs (the
> syscall table) to asm/syscall.h. New arch ports don't have to waste
> time copying & pasting this simple function.
>
> The s390/sparc versions need to be different, so document why.
>
> Signed-off-by: Mike Frysinger <[email protected]>
> Acked-by: David S. Miller <[email protected]>
> Acked-by: Paul Mundt <[email protected]>
> ---
> v3
> - move sys_call_table[] decl to asm/syscall.h
> - unify everyone's arch_syscall_addr() again
> - mention right tracehook requirements
>
> Documentation/trace/ftrace-design.txt | 5 ++---
> arch/s390/include/asm/syscall.h | 7 +++++++
> arch/s390/kernel/ftrace.c | 10 ----------
Acked-by: Heiko Carstens <[email protected]>
On Fri, Jan 22, 2010 at 09:36, Steven Rostedt wrote:
> On Fri, 2010-01-22 at 08:43 -0500, Mike Frysinger wrote:
>> Every arch_syscall_addr() implementation thus far is the same, so unify
>> them as a default weak in common code so more arches don't have to waste
>> time copying & pasting this simple function. The Blackfin version is
>> going to be exactly the same.
>>
>> Signed-off-by: Mike Frysinger <[email protected]>
>> ---
>> note: only thing that needs double checking is s390 and sparc where they
>> declared the sys_call_table as an array of ints. considering this table
>> is supposed to be an array of function pointers, this seems like more of
>> a typo to me ...
>
> I would not be too sure. s390 is very strange, and I would definitely
> want to get an Ack from the arch maintainers first.
i think everyone has checked in with v3 ...
-mike
On Tue, Jan 26, 2010 at 04:40:03AM -0500, Mike Frysinger wrote:
> Most implementations of arch_syscall_addr() are the same, so create a
> default version in common code and move the one piece that differs (the
> syscall table) to asm/syscall.h. New arch ports don't have to waste
> time copying & pasting this simple function.
>
> The s390/sparc versions need to be different, so document why.
>
> Signed-off-by: Mike Frysinger <[email protected]>
> Acked-by: David S. Miller <[email protected]>
> Acked-by: Paul Mundt <[email protected]>
Thanks!
I'm queuing it, unless Steve has already beaten me at it :)
Commit-ID: e7b8e675d9c71b868b66f62f725a948047514719
Gitweb: http://git.kernel.org/tip/e7b8e675d9c71b868b66f62f725a948047514719
Author: Mike Frysinger <[email protected]>
AuthorDate: Tue, 26 Jan 2010 04:40:03 -0500
Committer: Frederic Weisbecker <[email protected]>
CommitDate: Wed, 17 Feb 2010 13:07:21 +0100
tracing: Unify arch_syscall_addr() implementations
Most implementations of arch_syscall_addr() are the same, so create a
default version in common code and move the one piece that differs (the
syscall table) to asm/syscall.h. New arch ports don't have to waste
time copying & pasting this simple function.
The s390/sparc versions need to be different, so document why.
Signed-off-by: Mike Frysinger <[email protected]>
Acked-by: David S. Miller <[email protected]>
Acked-by: Paul Mundt <[email protected]>
Acked-by: Heiko Carstens <[email protected]>
Cc: Steven Rostedt <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Frederic Weisbecker <[email protected]>
---
Documentation/trace/ftrace-design.txt | 5 ++---
arch/s390/include/asm/syscall.h | 7 +++++++
arch/s390/kernel/ftrace.c | 10 ----------
arch/sh/include/asm/syscall.h | 2 ++
arch/sh/kernel/ftrace.c | 9 ---------
arch/sparc/include/asm/syscall.h | 7 +++++++
arch/sparc/kernel/ftrace.c | 11 -----------
arch/x86/include/asm/syscall.h | 2 ++
arch/x86/kernel/ftrace.c | 10 ----------
include/linux/ftrace.h | 6 ++++++
kernel/trace/trace_syscalls.c | 5 +++++
11 files changed, 31 insertions(+), 43 deletions(-)
diff --git a/Documentation/trace/ftrace-design.txt b/Documentation/trace/ftrace-design.txt
index 239f14b..99df110 100644
--- a/Documentation/trace/ftrace-design.txt
+++ b/Documentation/trace/ftrace-design.txt
@@ -218,11 +218,10 @@ HAVE_SYSCALL_TRACEPOINTS
You need very few things to get the syscalls tracing in an arch.
+- Support HAVE_ARCH_TRACEHOOK (see arch/Kconfig).
- Have a NR_syscalls variable in <asm/unistd.h> that provides the number
of syscalls supported by the arch.
-- Implement arch_syscall_addr() that resolves a syscall address from a
- syscall number.
-- Support the TIF_SYSCALL_TRACEPOINT thread flags
+- Support the TIF_SYSCALL_TRACEPOINT thread flags.
- Put the trace_sys_enter() and trace_sys_exit() tracepoints calls from ptrace
in the ptrace syscalls tracing path.
- Tag this arch as HAVE_SYSCALL_TRACEPOINTS.
diff --git a/arch/s390/include/asm/syscall.h b/arch/s390/include/asm/syscall.h
index e0a73d3..8429686 100644
--- a/arch/s390/include/asm/syscall.h
+++ b/arch/s390/include/asm/syscall.h
@@ -15,6 +15,13 @@
#include <linux/sched.h>
#include <asm/ptrace.h>
+/*
+ * The syscall table always contains 32 bit pointers since we know that the
+ * address of the function to be called is (way) below 4GB. So the "int"
+ * type here is what we want [need] for both 32 bit and 64 bit systems.
+ */
+extern const unsigned int sys_call_table[];
+
static inline long syscall_get_nr(struct task_struct *task,
struct pt_regs *regs)
{
diff --git a/arch/s390/kernel/ftrace.c b/arch/s390/kernel/ftrace.c
index 5a82bc6..9e69449 100644
--- a/arch/s390/kernel/ftrace.c
+++ b/arch/s390/kernel/ftrace.c
@@ -200,13 +200,3 @@ out:
return parent;
}
#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
-
-#ifdef CONFIG_FTRACE_SYSCALLS
-
-extern unsigned int sys_call_table[];
-
-unsigned long __init arch_syscall_addr(int nr)
-{
- return (unsigned long)sys_call_table[nr];
-}
-#endif
diff --git a/arch/sh/include/asm/syscall.h b/arch/sh/include/asm/syscall.h
index 6a38142..aa7777b 100644
--- a/arch/sh/include/asm/syscall.h
+++ b/arch/sh/include/asm/syscall.h
@@ -1,6 +1,8 @@
#ifndef __ASM_SH_SYSCALL_H
#define __ASM_SH_SYSCALL_H
+extern const unsigned long sys_call_table[];
+
#ifdef CONFIG_SUPERH32
# include "syscall_32.h"
#else
diff --git a/arch/sh/kernel/ftrace.c b/arch/sh/kernel/ftrace.c
index a48cded..30e1319 100644
--- a/arch/sh/kernel/ftrace.c
+++ b/arch/sh/kernel/ftrace.c
@@ -399,12 +399,3 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr)
}
}
#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
-
-#ifdef CONFIG_FTRACE_SYSCALLS
-extern unsigned long *sys_call_table;
-
-unsigned long __init arch_syscall_addr(int nr)
-{
- return (unsigned long)sys_call_table[nr];
-}
-#endif /* CONFIG_FTRACE_SYSCALLS */
diff --git a/arch/sparc/include/asm/syscall.h b/arch/sparc/include/asm/syscall.h
index 7486c60..025a02a 100644
--- a/arch/sparc/include/asm/syscall.h
+++ b/arch/sparc/include/asm/syscall.h
@@ -5,6 +5,13 @@
#include <linux/sched.h>
#include <asm/ptrace.h>
+/*
+ * The syscall table always contains 32 bit pointers since we know that the
+ * address of the function to be called is (way) below 4GB. So the "int"
+ * type here is what we want [need] for both 32 bit and 64 bit systems.
+ */
+extern const unsigned int sys_call_table[];
+
/* The system call number is given by the user in %g1 */
static inline long syscall_get_nr(struct task_struct *task,
struct pt_regs *regs)
diff --git a/arch/sparc/kernel/ftrace.c b/arch/sparc/kernel/ftrace.c
index 29973da..9103a56 100644
--- a/arch/sparc/kernel/ftrace.c
+++ b/arch/sparc/kernel/ftrace.c
@@ -91,14 +91,3 @@ int __init ftrace_dyn_arch_init(void *data)
return 0;
}
#endif
-
-#ifdef CONFIG_FTRACE_SYSCALLS
-
-extern unsigned int sys_call_table[];
-
-unsigned long __init arch_syscall_addr(int nr)
-{
- return (unsigned long)sys_call_table[nr];
-}
-
-#endif
diff --git a/arch/x86/include/asm/syscall.h b/arch/x86/include/asm/syscall.h
index 8d33bc5..c4a348f 100644
--- a/arch/x86/include/asm/syscall.h
+++ b/arch/x86/include/asm/syscall.h
@@ -16,6 +16,8 @@
#include <linux/sched.h>
#include <linux/err.h>
+extern const unsigned long sys_call_table[];
+
/*
* Only the low 32 bits of orig_ax are meaningful, so we return int.
* This importantly ignores the high bits on 64-bit, so comparisons
diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index 3096892..0d93a94 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -484,13 +484,3 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr,
}
}
#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
-
-#ifdef CONFIG_FTRACE_SYSCALLS
-
-extern unsigned long *sys_call_table;
-
-unsigned long __init arch_syscall_addr(int nr)
-{
- return (unsigned long)(&sys_call_table)[nr];
-}
-#endif
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 0b4f97d..1cbb36f 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -511,4 +511,10 @@ static inline void trace_hw_branch_oops(void) {}
#endif /* CONFIG_HW_BRANCH_TRACER */
+#ifdef CONFIG_FTRACE_SYSCALLS
+
+unsigned long arch_syscall_addr(int nr);
+
+#endif /* CONFIG_FTRACE_SYSCALLS */
+
#endif /* _LINUX_FTRACE_H */
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index 49cea70..ecf0078 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -394,6 +394,11 @@ int init_syscall_trace(struct ftrace_event_call *call)
return id;
}
+unsigned long __init arch_syscall_addr(int nr)
+{
+ return (unsigned long)sys_call_table[nr];
+}
+
int __init init_ftrace_syscalls(void)
{
struct syscall_metadata *meta;