Ingo,
Please pull the tracing/core branch that can be found at:
git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git
tracing/core
Thanks,
Frederic
---
Heiko Carstens (2):
tracing/kprobes: Make Kconfig dependencies generic
s390: Add pt_regs register and stack access API
Mike Frysinger (1):
tracing: Unify arch_syscall_addr() implementations
Documentation/trace/ftrace-design.txt | 5 +--
arch/Kconfig | 3 ++
arch/s390/Kconfig | 1 +
arch/s390/include/asm/ptrace.h | 13 +++++++-
arch/s390/include/asm/syscall.h | 7 ++++
arch/s390/kernel/ftrace.c | 10 ------
arch/s390/kernel/ptrace.c | 58 +++++++++++++++++++++++++++++++++
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/Kconfig | 1 +
arch/x86/include/asm/syscall.h | 2 +
arch/x86/kernel/ftrace.c | 10 ------
include/linux/ftrace.h | 6 +++
kernel/trace/Kconfig | 2 +-
kernel/trace/trace_syscalls.c | 5 +++
17 files changed, 107 insertions(+), 45 deletions(-)
From: Mike Frysinger <[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]>
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;
--
1.6.2.3
From: Heiko Carstens <[email protected]>
This API is needed for the kprobe-based event tracer.
Signed-off-by: Heiko Carstens <[email protected]>
Reviewed-by: Masami Hiramatsu <[email protected]>
Cc: Martin Schwidefsky <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Frederic Weisbecker <[email protected]>
---
arch/s390/Kconfig | 1 +
arch/s390/include/asm/ptrace.h | 13 ++++++++-
arch/s390/kernel/ptrace.c | 58 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 71 insertions(+), 1 deletions(-)
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index c802352..2590ce2 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -87,6 +87,7 @@ config S390
select HAVE_SYSCALL_TRACEPOINTS
select HAVE_DYNAMIC_FTRACE
select HAVE_FUNCTION_GRAPH_TRACER
+ select HAVE_REGS_AND_STACK_ACCESS_API
select HAVE_DEFAULT_NO_SPIN_MUTEXES
select HAVE_OPROFILE
select HAVE_KPROBES
diff --git a/arch/s390/include/asm/ptrace.h b/arch/s390/include/asm/ptrace.h
index 95dcf18..dd2d913 100644
--- a/arch/s390/include/asm/ptrace.h
+++ b/arch/s390/include/asm/ptrace.h
@@ -492,13 +492,24 @@ struct user_regs_struct
struct task_struct;
extern void user_enable_single_step(struct task_struct *);
extern void user_disable_single_step(struct task_struct *);
+extern void show_regs(struct pt_regs * regs);
#define user_mode(regs) (((regs)->psw.mask & PSW_MASK_PSTATE) != 0)
#define instruction_pointer(regs) ((regs)->psw.addr & PSW_ADDR_INSN)
#define user_stack_pointer(regs)((regs)->gprs[15])
#define regs_return_value(regs)((regs)->gprs[2])
#define profile_pc(regs) instruction_pointer(regs)
-extern void show_regs(struct pt_regs * regs);
+
+int regs_query_register_offset(const char *name);
+const char *regs_query_register_name(unsigned int offset);
+unsigned long regs_get_register(struct pt_regs *regs, unsigned int offset);
+unsigned long regs_get_kernel_stack_nth(struct pt_regs *regs, unsigned int n);
+
+static inline unsigned long kernel_stack_pointer(struct pt_regs *regs)
+{
+ return regs->gprs[15] & PSW_ADDR_INSN;
+}
+
#endif /* __KERNEL__ */
#endif /* __ASSEMBLY__ */
diff --git a/arch/s390/kernel/ptrace.c b/arch/s390/kernel/ptrace.c
index 13815d3..1720f38 100644
--- a/arch/s390/kernel/ptrace.c
+++ b/arch/s390/kernel/ptrace.c
@@ -984,3 +984,61 @@ const struct user_regset_view *task_user_regset_view(struct task_struct *task)
#endif
return &user_s390_view;
}
+
+static const char *gpr_names[NUM_GPRS] = {
+ "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7",
+ "r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15",
+};
+
+unsigned long regs_get_register(struct pt_regs *regs, unsigned int offset)
+{
+ if (offset >= NUM_GPRS)
+ return 0;
+ return regs->gprs[offset];
+}
+
+int regs_query_register_offset(const char *name)
+{
+ unsigned long offset;
+
+ if (!name || *name != 'r')
+ return -EINVAL;
+ if (strict_strtoul(name + 1, 10, &offset))
+ return -EINVAL;
+ if (offset >= NUM_GPRS)
+ return -EINVAL;
+ return offset;
+}
+
+const char *regs_query_register_name(unsigned int offset)
+{
+ if (offset >= NUM_GPRS)
+ return NULL;
+ return gpr_names[offset];
+}
+
+static int regs_within_kernel_stack(struct pt_regs *regs, unsigned long addr)
+{
+ unsigned long ksp = kernel_stack_pointer(regs);
+
+ return (addr & ~(THREAD_SIZE - 1)) == (ksp & ~(THREAD_SIZE - 1));
+}
+
+/**
+ * regs_get_kernel_stack_nth() - get Nth entry of the stack
+ * @regs:pt_regs which contains kernel stack pointer.
+ * @n:stack entry number.
+ *
+ * regs_get_kernel_stack_nth() returns @n th entry of the kernel stack which
+ * is specifined by @regs. If the @n th entry is NOT in the kernel stack,
+ * this returns 0.
+ */
+unsigned long regs_get_kernel_stack_nth(struct pt_regs *regs, unsigned int n)
+{
+ unsigned long addr;
+
+ addr = kernel_stack_pointer(regs) + n * sizeof(long);
+ if (!regs_within_kernel_stack(regs, addr))
+ return 0;
+ return *(unsigned long *)addr;
+}
--
1.6.2.3
From: Heiko Carstens <[email protected]>
KPROBES_EVENT actually depends on the regs and stack access API
(b1cf540f) and not on x86.
So introduce a new config option which architectures can select if
they have the API implemented and switch x86.
Signed-off-by: Heiko Carstens <[email protected]>
Acked-by: Masami Hiramatsu <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Martin Schwidefsky <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Frederic Weisbecker <[email protected]>
---
arch/Kconfig | 3 +++
arch/x86/Kconfig | 1 +
kernel/trace/Kconfig | 2 +-
3 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/arch/Kconfig b/arch/Kconfig
index 9d055b4..04e3aa7 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -121,6 +121,9 @@ config HAVE_DMA_ATTRS
config USE_GENERIC_SMP_HELPERS
bool
+config HAVE_REGS_AND_STACK_ACCESS_API
+ bool
+
config HAVE_CLK
bool
help
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 55298e8..07baa12 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -45,6 +45,7 @@ config X86
select HAVE_GENERIC_DMA_COHERENT if X86_32
select HAVE_EFFICIENT_UNALIGNED_ACCESS
select USER_STACKTRACE_SUPPORT
+ select HAVE_REGS_AND_STACK_ACCESS_API
select HAVE_DMA_API_DEBUG
select HAVE_KERNEL_GZIP
select HAVE_KERNEL_BZIP2
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index 6c22d8a..40fef55 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -451,7 +451,7 @@ config BLK_DEV_IO_TRACE
config KPROBE_EVENT
depends on KPROBES
- depends on X86
+ depends on HAVE_REGS_AND_STACK_ACCESS_API
bool "Enable kprobes-based dynamic events"
select TRACING
default y
--
1.6.2.3
On Thu, Feb 18, 2010 at 06:13, Frederic Weisbecker wrote:
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -121,6 +121,9 @@ config HAVE_DMA_ATTRS
> config USE_GENERIC_SMP_HELPERS
> bool
>
> +config HAVE_REGS_AND_STACK_ACCESS_API
> + bool
> +
could you add an appropriate help/comment so arch peeps know what
needs to be implemented before they can select this
-mike
On Thu, Feb 18, 2010 at 06:18:20AM -0500, Mike Frysinger wrote:
> On Thu, Feb 18, 2010 at 06:13, Frederic Weisbecker wrote:
> > --- a/arch/Kconfig
> > +++ b/arch/Kconfig
> > @@ -121,6 +121,9 @@ config HAVE_DMA_ATTRS
> > ?config USE_GENERIC_SMP_HELPERS
> > ? ? ? ?bool
> >
> > +config HAVE_REGS_AND_STACK_ACCESS_API
> > + ? ? ? bool
> > +
>
> could you add an appropriate help/comment so arch peeps know what
> needs to be implemented before they can select this
That's why I added the commit ID for the regs and stack access api
to the changelog. imho that should be sufficient.
Besides that the next commit would implement it for s390 as a
blueprint for others. That is... for those that missed the initial
x86 implementation.
On Thu, Feb 18, 2010 at 07:09, Heiko Carstens wrote:
> On Thu, Feb 18, 2010 at 06:18:20AM -0500, Mike Frysinger wrote:
>> On Thu, Feb 18, 2010 at 06:13, Frederic Weisbecker wrote:
>> > --- a/arch/Kconfig
>> > +++ b/arch/Kconfig
>> > @@ -121,6 +121,9 @@ config HAVE_DMA_ATTRS
>> > config USE_GENERIC_SMP_HELPERS
>> > bool
>> >
>> > +config HAVE_REGS_AND_STACK_ACCESS_API
>> > + bool
>> > +
>>
>> could you add an appropriate help/comment so arch peeps know what
>> needs to be implemented before they can select this
>
> That's why I added the commit ID for the regs and stack access api
> to the changelog. imho that should be sufficient.
> Besides that the next commit would implement it for s390 as a
> blueprint for others. That is... for those that missed the initial
> x86 implementation.
people shouldnt have to dive into the changelog to try and divine
documentation. it's hardly standard, so people fall on it in a
pima-last-resort kind of way. being explicit in the file up front by
writing real documentation says other people a lot more time.
-mike
On Thu, Feb 18, 2010 at 07:12:08AM -0500, Mike Frysinger wrote:
> On Thu, Feb 18, 2010 at 07:09, Heiko Carstens wrote:
> > On Thu, Feb 18, 2010 at 06:18:20AM -0500, Mike Frysinger wrote:
> >> On Thu, Feb 18, 2010 at 06:13, Frederic Weisbecker wrote:
> >> > --- a/arch/Kconfig
> >> > +++ b/arch/Kconfig
> >> > @@ -121,6 +121,9 @@ config HAVE_DMA_ATTRS
> >> > ?config USE_GENERIC_SMP_HELPERS
> >> > ? ? ? ?bool
> >> >
> >> > +config HAVE_REGS_AND_STACK_ACCESS_API
> >> > + ? ? ? bool
> >> > +
> >>
> >> could you add an appropriate help/comment so arch peeps know what
> >> needs to be implemented before they can select this
> >
> > That's why I added the commit ID for the regs and stack access api
> > to the changelog. imho that should be sufficient.
> > Besides that the next commit would implement it for s390 as a
> > blueprint for others. That is... for those that missed the initial
> > x86 implementation.
>
> people shouldnt have to dive into the changelog to try and divine
> documentation. it's hardly standard, so people fall on it in a
> pima-last-resort kind of way. being explicit in the file up front by
> writing real documentation says other people a lot more time.
> -mike
Yeah, would be nice to have a comment above the config definition
to explain what it implies.
Heiko, mind sending a delta patch for that?
Thanks.
On Thu, Feb 18, 2010 at 01:34:57PM +0100, Frederic Weisbecker wrote:
> On Thu, Feb 18, 2010 at 07:12:08AM -0500, Mike Frysinger wrote:
> > On Thu, Feb 18, 2010 at 07:09, Heiko Carstens wrote:
> > > On Thu, Feb 18, 2010 at 06:18:20AM -0500, Mike Frysinger wrote:
> > >> On Thu, Feb 18, 2010 at 06:13, Frederic Weisbecker wrote:
> > >> > --- a/arch/Kconfig
> > >> > +++ b/arch/Kconfig
> > >> > @@ -121,6 +121,9 @@ config HAVE_DMA_ATTRS
> > >> > ?config USE_GENERIC_SMP_HELPERS
> > >> > ? ? ? ?bool
> > >> >
> > >> > +config HAVE_REGS_AND_STACK_ACCESS_API
> > >> > + ? ? ? bool
> > >> > +
> > >>
> > >> could you add an appropriate help/comment so arch peeps know what
> > >> needs to be implemented before they can select this
> > >
> > > That's why I added the commit ID for the regs and stack access api
> > > to the changelog. imho that should be sufficient.
> > > Besides that the next commit would implement it for s390 as a
> > > blueprint for others. That is... for those that missed the initial
> > > x86 implementation.
> >
> > people shouldnt have to dive into the changelog to try and divine
> > documentation. it's hardly standard, so people fall on it in a
> > pima-last-resort kind of way. being explicit in the file up front by
> > writing real documentation says other people a lot more time.
> > -mike
>
>
> Yeah, would be nice to have a comment above the config definition
> to explain what it implies.
>
> Heiko, mind sending a delta patch for that?
Subject: [PATCH] tracing/kprobes: add short documentation for HAVE_REGS_AND_STACK_ACCESS_API
From: Heiko Carstens <[email protected]>
Signed-off-by: Heiko Carstens <[email protected]>
---
arch/Kconfig | 4 ++++
1 file changed, 4 insertions(+)
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -123,6 +123,10 @@ config USE_GENERIC_SMP_HELPERS
config HAVE_REGS_AND_STACK_ACCESS_API
bool
+ help
+ This symbol should be selected by an architecure if it supports
+ the API needed to access registers and stack entries from pt_regs.
+ For example the kprobes-based event tracer needs this API.
config HAVE_CLK
bool
On Thu, Feb 18, 2010 at 08:25, Heiko Carstens wrote:
> On Thu, Feb 18, 2010 at 01:34:57PM +0100, Frederic Weisbecker wrote:
>> On Thu, Feb 18, 2010 at 07:12:08AM -0500, Mike Frysinger wrote:
>> > On Thu, Feb 18, 2010 at 07:09, Heiko Carstens wrote:
>> > > On Thu, Feb 18, 2010 at 06:18:20AM -0500, Mike Frysinger wrote:
>> > >> On Thu, Feb 18, 2010 at 06:13, Frederic Weisbecker wrote:
>> > >> > --- a/arch/Kconfig
>> > >> > +++ b/arch/Kconfig
>> > >> > @@ -121,6 +121,9 @@ config HAVE_DMA_ATTRS
>> > >> > config USE_GENERIC_SMP_HELPERS
>> > >> > bool
>> > >> >
>> > >> > +config HAVE_REGS_AND_STACK_ACCESS_API
>> > >> > + bool
>> > >> > +
>> > >>
>> > >> could you add an appropriate help/comment so arch peeps know what
>> > >> needs to be implemented before they can select this
>> > >
>> > > That's why I added the commit ID for the regs and stack access api
>> > > to the changelog. imho that should be sufficient.
>> > > Besides that the next commit would implement it for s390 as a
>> > > blueprint for others. That is... for those that missed the initial
>> > > x86 implementation.
>> >
>> > people shouldnt have to dive into the changelog to try and divine
>> > documentation. it's hardly standard, so people fall on it in a
>> > pima-last-resort kind of way. being explicit in the file up front by
>> > writing real documentation says other people a lot more time.
>> > -mike
>>
>>
>> Yeah, would be nice to have a comment above the config definition
>> to explain what it implies.
>>
>> Heiko, mind sending a delta patch for that?
>
> Subject: [PATCH] tracing/kprobes: add short documentation for HAVE_REGS_AND_STACK_ACCESS_API
>
> From: Heiko Carstens <[email protected]>
>
> Signed-off-by: Heiko Carstens <[email protected]>
> ---
> arch/Kconfig | 4 ++++
> 1 file changed, 4 insertions(+)
>
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -123,6 +123,10 @@ config USE_GENERIC_SMP_HELPERS
>
> config HAVE_REGS_AND_STACK_ACCESS_API
> bool
> + help
> + This symbol should be selected by an architecure if it supports
> + the API needed to access registers and stack entries from pt_regs.
> + For example the kprobes-based event tracer needs this API.
a bit vague ... arent there headers/functions people could look at ?
perhaps you're talking about the regset functions (which is an API to
access registers in pt_regs) ? or you're talking about asm/syscall.h
(which is an API to access registers in pt_regs) ?
i'm not asking to be a pain, i'm asking because i really havent a
clue. if i wanted to add support for this stuff to the Blackfin arch,
i wouldnt know where to start. even after reading this help i'd fall
back to grepping arch/x86/ and trying to divine a starting point from
there.
-mike
On Thu, Feb 18, 2010 at 02:25:21PM +0100, Heiko Carstens wrote:
> Subject: [PATCH] tracing/kprobes: add short documentation for HAVE_REGS_AND_STACK_ACCESS_API
>
> From: Heiko Carstens <[email protected]>
>
> Signed-off-by: Heiko Carstens <[email protected]>
Thanks, I'm adding it in the set.
> ---
> arch/Kconfig | 4 ++++
> 1 file changed, 4 insertions(+)
>
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -123,6 +123,10 @@ config USE_GENERIC_SMP_HELPERS
>
> config HAVE_REGS_AND_STACK_ACCESS_API
> bool
> + help
> + This symbol should be selected by an architecure if it supports
> + the API needed to access registers and stack entries from pt_regs.
> + For example the kprobes-based event tracer needs this API.
>
> config HAVE_CLK
> bool
On Thu, Feb 18, 2010 at 09:01:12AM -0500, Mike Frysinger wrote:
> > --- a/arch/Kconfig
> > +++ b/arch/Kconfig
> > @@ -123,6 +123,10 @@ config USE_GENERIC_SMP_HELPERS
> >
> > ?config HAVE_REGS_AND_STACK_ACCESS_API
> > ? ? ? ?bool
> > + ? ? ? help
> > + ? ? ? ? This symbol should be selected by an architecure if it supports
> > + ? ? ? ? the API needed to access registers and stack entries from pt_regs.
> > + ? ? ? ? For example the kprobes-based event tracer needs this API.
>
> a bit vague ... arent there headers/functions people could look at ?
> perhaps you're talking about the regset functions (which is an API to
> access registers in pt_regs) ? or you're talking about asm/syscall.h
> (which is an API to access registers in pt_regs) ?
>
> i'm not asking to be a pain, i'm asking because i really havent a
> clue. if i wanted to add support for this stuff to the Blackfin arch,
> i wouldnt know where to start. even after reading this help i'd fall
> back to grepping arch/x86/ and trying to divine a starting point from
> there.
If an arch support kprobes, it just needs to select
HAVE_REGS_AND_STACK_ACCESS_API to figure out quickly what is missing,
as gcc will barf every missing clues you need.
For now it is stored is asm/ptrace.h, but that might be split in
the future, especially as ptrace has initially nothing related to
that. A documentation that deals with filenames or API enumerations
tend to be incidentally async with API evolutions.
Hm?
On Thu, Feb 18, 2010 at 09:01:12AM -0500, Mike Frysinger wrote:
> On Thu, Feb 18, 2010 at 08:25, Heiko Carstens wrote:
> > --- a/arch/Kconfig
> > +++ b/arch/Kconfig
> > @@ -123,6 +123,10 @@ config USE_GENERIC_SMP_HELPERS
> >
> > ?config HAVE_REGS_AND_STACK_ACCESS_API
> > ? ? ? ?bool
> > + ? ? ? help
> > + ? ? ? ? This symbol should be selected by an architecure if it supports
> > + ? ? ? ? the API needed to access registers and stack entries from pt_regs.
> > + ? ? ? ? For example the kprobes-based event tracer needs this API.
>
> a bit vague ... arent there headers/functions people could look at ?
> perhaps you're talking about the regset functions (which is an API to
> access registers in pt_regs) ? or you're talking about asm/syscall.h
> (which is an API to access registers in pt_regs) ?
>
> i'm not asking to be a pain, i'm asking because i really havent a
> clue. if i wanted to add support for this stuff to the Blackfin arch,
> i wouldnt know where to start. even after reading this help i'd fall
> back to grepping arch/x86/ and trying to divine a starting point from
> there.
git show b1cf540f would be your friend. That's why I pointed out the
id in the changelog.
I have no idea what your workflow is, but doing something like
gitk v2.6.32..v2.6.33-rc1 arch/<whatever>/
is what I do to figure out what other archs did during the merge
window and if there's something that needs an arch backend on s390.
This reveals also bug fixes that need to be ported from time to time.
That worked pretty well for me during the last few years.
On Thu, Feb 18, 2010 at 09:50, Frederic Weisbecker wrote:
> On Thu, Feb 18, 2010 at 09:01:12AM -0500, Mike Frysinger wrote:
>> > --- a/arch/Kconfig
>> > +++ b/arch/Kconfig
>> > @@ -123,6 +123,10 @@ config USE_GENERIC_SMP_HELPERS
>> >
>> > config HAVE_REGS_AND_STACK_ACCESS_API
>> > bool
>> > + help
>> > + This symbol should be selected by an architecure if it supports
>> > + the API needed to access registers and stack entries from pt_regs.
>> > + For example the kprobes-based event tracer needs this API.
>>
>> a bit vague ... arent there headers/functions people could look at ?
>> perhaps you're talking about the regset functions (which is an API to
>> access registers in pt_regs) ? or you're talking about asm/syscall.h
>> (which is an API to access registers in pt_regs) ?
>>
>> i'm not asking to be a pain, i'm asking because i really havent a
>> clue. if i wanted to add support for this stuff to the Blackfin arch,
>> i wouldnt know where to start. even after reading this help i'd fall
>> back to grepping arch/x86/ and trying to divine a starting point from
>> there.
>
> If an arch support kprobes, it just needs to select
> HAVE_REGS_AND_STACK_ACCESS_API to figure out quickly what is missing,
> as gcc will barf every missing clues you need.
so should this new Kconfig option have an appropriate depends on
KPROBES or something ?
> For now it is stored is asm/ptrace.h, but that might be split in
> the future, especially as ptrace has initially nothing related to
> that. A documentation that deals with filenames or API enumerations
> tend to be incidentally async with API evolutions.
i dont expect there to be per-function documentation here ... such
things below in the header files themselves (linux/regset.h is an
example of how to approach this). but having a tip of reading a file
or two (like HAVE_ARCH_TRACEHOOK) doesnt bit rot nearly as often. if
the common API expected of headers hasnt yet been split, then i guess
not much left to be done here.
-mike
On Thu, Feb 18, 2010 at 10:00, Heiko Carstens wrote:
> On Thu, Feb 18, 2010 at 09:01:12AM -0500, Mike Frysinger wrote:
>> On Thu, Feb 18, 2010 at 08:25, Heiko Carstens wrote:
>> > --- a/arch/Kconfig
>> > +++ b/arch/Kconfig
>> > @@ -123,6 +123,10 @@ config USE_GENERIC_SMP_HELPERS
>> >
>> > config HAVE_REGS_AND_STACK_ACCESS_API
>> > bool
>> > + help
>> > + This symbol should be selected by an architecure if it supports
>> > + the API needed to access registers and stack entries from pt_regs.
>> > + For example the kprobes-based event tracer needs this API.
>>
>> a bit vague ... arent there headers/functions people could look at ?
>> perhaps you're talking about the regset functions (which is an API to
>> access registers in pt_regs) ? or you're talking about asm/syscall.h
>> (which is an API to access registers in pt_regs) ?
>>
>> i'm not asking to be a pain, i'm asking because i really havent a
>> clue. if i wanted to add support for this stuff to the Blackfin arch,
>> i wouldnt know where to start. even after reading this help i'd fall
>> back to grepping arch/x86/ and trying to divine a starting point from
>> there.
>
> git show b1cf540f would be your friend. That's why I pointed out the
> id in the changelog.
> I have no idea what your workflow is, but doing something like
> gitk v2.6.32..v2.6.33-rc1 arch/<whatever>/
> is what I do to figure out what other archs did during the merge
> window and if there's something that needs an arch backend on s390.
> This reveals also bug fixes that need to be ported from time to time.
> That worked pretty well for me during the last few years.
most of the time though such implementations are tied to the arch, and
for people unfamiliar with the arch in question, they can have a hard
time separating the common requirements from the arch requirements.
changelog entries also really shouldnt be the norm for documentation
of APIs, especially as APIs change over time (by design -- linux has
no stable API). so while this commit may be useful for the next
release or two, it isnt uncommon for them to be partially if not
wholly irrelevant down the line. which is when us smaller arches get
around to implementing this cooler features.
-mike
On Thu, Feb 18, 2010 at 10:02:52AM -0500, Mike Frysinger wrote:
> On Thu, Feb 18, 2010 at 09:50, Frederic Weisbecker wrote:
> > On Thu, Feb 18, 2010 at 09:01:12AM -0500, Mike Frysinger wrote:
> >> > --- a/arch/Kconfig
> >> > +++ b/arch/Kconfig
> >> > @@ -123,6 +123,10 @@ config USE_GENERIC_SMP_HELPERS
> >> >
> >> > ?config HAVE_REGS_AND_STACK_ACCESS_API
> >> > ? ? ? ?bool
> >> > + ? ? ? help
> >> > + ? ? ? ? This symbol should be selected by an architecure if it supports
> >> > + ? ? ? ? the API needed to access registers and stack entries from pt_regs.
> >> > + ? ? ? ? For example the kprobes-based event tracer needs this API.
> >>
> >> a bit vague ... arent there headers/functions people could look at ?
> >> perhaps you're talking about the regset functions (which is an API to
> >> access registers in pt_regs) ? ?or you're talking about asm/syscall.h
> >> (which is an API to access registers in pt_regs) ?
> >>
> >> i'm not asking to be a pain, i'm asking because i really havent a
> >> clue. ?if i wanted to add support for this stuff to the Blackfin arch,
> >> i wouldnt know where to start. ?even after reading this help i'd fall
> >> back to grepping arch/x86/ and trying to divine a starting point from
> >> there.
> >
> > If an arch support kprobes, it just needs to select
> > HAVE_REGS_AND_STACK_ACCESS_API to figure out quickly what is missing,
> > as gcc will barf every missing clues you need.
>
> so should this new Kconfig option have an appropriate depends on
> KPROBES or something ?
No, kprobes events need KPROBES and the above one, but KPROBES
alone can perfectely work without HAVE_REGS_AND_STACK_ACCESS_API.
> > For now it is stored is asm/ptrace.h, but that might be split in
> > the future, especially as ptrace has initially nothing related to
> > that. A documentation that deals with filenames or API enumerations
> > tend to be incidentally async with API evolutions.
>
> i dont expect there to be per-function documentation here ... such
> things below in the header files themselves (linux/regset.h is an
> example of how to approach this). but having a tip of reading a file
> or two (like HAVE_ARCH_TRACEHOOK) doesnt bit rot nearly as often. if
> the common API expected of headers hasnt yet been split, then i guess
> not much left to be done here.
Yeah. Once it's split up from ptrace, we can think about a more dedicated
header file and then it can be reasonable to put the header filename in the
documentation. But not for now IMHO.
Mike Frysinger wrote:
>>
>> config HAVE_REGS_AND_STACK_ACCESS_API
>> bool
>> + help
>> + This symbol should be selected by an architecure if it supports
>> + the API needed to access registers and stack entries from pt_regs.
>> + For example the kprobes-based event tracer needs this API.
>
> a bit vague ... arent there headers/functions people could look at ?
> perhaps you're talking about the regset functions (which is an API to
> access registers in pt_regs) ? or you're talking about asm/syscall.h
> (which is an API to access registers in pt_regs) ?
We're talking about new API which was introduced by b1cf540f
(regs_get_argument_nth has been dropped on -tip tree anyway)
> i'm not asking to be a pain, i'm asking because i really havent a
> clue. if i wanted to add support for this stuff to the Blackfin arch,
> i wouldnt know where to start. even after reading this help i'd fall
> back to grepping arch/x86/ and trying to divine a starting point from
> there.
yeah, I think the comment might be better to refer that APIs are
in arch/*/include/asm/ptrace.h :)
Thank you,
--
Masami Hiramatsu
Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division
e-mail: [email protected]
Frederic Weisbecker wrote:
> On Thu, Feb 18, 2010 at 09:01:12AM -0500, Mike Frysinger wrote:
>>> --- a/arch/Kconfig
>>> +++ b/arch/Kconfig
>>> @@ -123,6 +123,10 @@ config USE_GENERIC_SMP_HELPERS
>>>
>>> config HAVE_REGS_AND_STACK_ACCESS_API
>>> bool
>>> + help
>>> + This symbol should be selected by an architecure if it supports
>>> + the API needed to access registers and stack entries from pt_regs.
>>> + For example the kprobes-based event tracer needs this API.
>>
>> a bit vague ... arent there headers/functions people could look at ?
>> perhaps you're talking about the regset functions (which is an API to
>> access registers in pt_regs) ? or you're talking about asm/syscall.h
>> (which is an API to access registers in pt_regs) ?
>>
>> i'm not asking to be a pain, i'm asking because i really havent a
>> clue. if i wanted to add support for this stuff to the Blackfin arch,
>> i wouldnt know where to start. even after reading this help i'd fall
>> back to grepping arch/x86/ and trying to divine a starting point from
>> there.
>
>
> If an arch support kprobes, it just needs to select
> HAVE_REGS_AND_STACK_ACCESS_API to figure out quickly what is missing,
> as gcc will barf every missing clues you need.
These APIs are needed for kprobe-tracer on ftrace(kernel/trace/trace_kprobe.c),
which is just a consumer of kprobes (so depends on CONFIG_KPROBES).
I mean, if you wanna figure out that, you have to select CONFIG_KPROBE_EVENT too :)
And then, gcc tries to compile trace_kprobe.c and quickly dumps error messages
what APIs are not found :)
> For now it is stored is asm/ptrace.h, but that might be split in
> the future, especially as ptrace has initially nothing related to
> that. A documentation that deals with filenames or API enumerations
> tend to be incidentally async with API evolutions.
yeah, now those APIs depend on pt_regs, so I put it in ptrace.h.
Thank you,
--
Masami Hiramatsu
Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division
e-mail: [email protected]
On Thu, Feb 18, 2010 at 11:19:54AM -0500, Masami Hiramatsu wrote:
> Frederic Weisbecker wrote:
> > On Thu, Feb 18, 2010 at 09:01:12AM -0500, Mike Frysinger wrote:
> >>> --- a/arch/Kconfig
> >>> +++ b/arch/Kconfig
> >>> @@ -123,6 +123,10 @@ config USE_GENERIC_SMP_HELPERS
> >>>
> >>> config HAVE_REGS_AND_STACK_ACCESS_API
> >>> bool
> >>> + help
> >>> + This symbol should be selected by an architecure if it supports
> >>> + the API needed to access registers and stack entries from pt_regs.
> >>> + For example the kprobes-based event tracer needs this API.
> >>
> >> a bit vague ... arent there headers/functions people could look at ?
> >> perhaps you're talking about the regset functions (which is an API to
> >> access registers in pt_regs) ? or you're talking about asm/syscall.h
> >> (which is an API to access registers in pt_regs) ?
> >>
> >> i'm not asking to be a pain, i'm asking because i really havent a
> >> clue. if i wanted to add support for this stuff to the Blackfin arch,
> >> i wouldnt know where to start. even after reading this help i'd fall
> >> back to grepping arch/x86/ and trying to divine a starting point from
> >> there.
> >
> >
> > If an arch support kprobes, it just needs to select
> > HAVE_REGS_AND_STACK_ACCESS_API to figure out quickly what is missing,
> > as gcc will barf every missing clues you need.
>
> These APIs are needed for kprobe-tracer on ftrace(kernel/trace/trace_kprobe.c),
> which is just a consumer of kprobes (so depends on CONFIG_KPROBES).
>
> I mean, if you wanna figure out that, you have to select CONFIG_KPROBE_EVENT too :)
> And then, gcc tries to compile trace_kprobe.c and quickly dumps error messages
> what APIs are not found :)
Yeah.
> > For now it is stored is asm/ptrace.h, but that might be split in
> > the future, especially as ptrace has initially nothing related to
> > that. A documentation that deals with filenames or API enumerations
> > tend to be incidentally async with API evolutions.
>
> yeah, now those APIs depend on pt_regs, so I put it in ptrace.h.
>
> Thank you,
But anyway, if you prefer I can add a reference to ptrace.h in
the config help. We just need to not forgot that if we move
these functions later :)
Thanks.
On Thu, Feb 18, 2010 at 11:19:54AM -0500, Masami Hiramatsu wrote:
> > For now it is stored is asm/ptrace.h, but that might be split in
> > the future, especially as ptrace has initially nothing related to
> > that. A documentation that deals with filenames or API enumerations
> > tend to be incidentally async with API evolutions.
>
> yeah, now those APIs depend on pt_regs, so I put it in ptrace.h.
>
> Thank you,
Are you guys fine with the following patch or should I bring
more details somewhere?
Thanks.
---
commit d3643132cfeac24ee20d2b6cd8cb06d73e6a6da9
Author: Heiko Carstens <[email protected]>
Date: Thu Feb 18 14:25:21 2010 +0100
tracing/kprobes: Add short documentation for HAVE_REGS_AND_STACK_ACCESS_API
So that arch developers know how to implement it without the
need to dig into changelogs.
Reported-by: Mike Frysinger <[email protected]>
Signed-off-by: Heiko Carstens <[email protected]>
Cc: Masami Hiramatsu <[email protected]>
Cc: Martin Schwidefsky <[email protected]>
Cc: "David S . Miller" <[email protected]>
Cc: Paul Mundt <[email protected]>
Cc: Steven Rostedt <[email protected]>
LKML-Reference: <[email protected]>
[added reference to ptrace.h in the config help]
Signed-off-by: Frederic Weisbecker <[email protected]>
diff --git a/arch/Kconfig b/arch/Kconfig
index 04e3aa7..50877ef 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -123,6 +123,11 @@ config USE_GENERIC_SMP_HELPERS
config HAVE_REGS_AND_STACK_ACCESS_API
bool
+ help
+ This symbol should be selected by an architecure if it supports
+ the API needed to access registers and stack entries from pt_regs,
+ declared in asm/ptrace.h
+ For example the kprobes-based event tracer needs this API.
config HAVE_CLK
bool
Frederic Weisbecker wrote:
> On Thu, Feb 18, 2010 at 11:19:54AM -0500, Masami Hiramatsu wrote:
>>> For now it is stored is asm/ptrace.h, but that might be split in
>>> the future, especially as ptrace has initially nothing related to
>>> that. A documentation that deals with filenames or API enumerations
>>> tend to be incidentally async with API evolutions.
>>
>> yeah, now those APIs depend on pt_regs, so I put it in ptrace.h.
>>
>> Thank you,
>
>
> Are you guys fine with the following patch or should I bring
> more details somewhere?
I'm fine with it :-)
Acked-by: Masami Hiramatsu <[email protected]>
>
> Thanks.
>
> ---
> commit d3643132cfeac24ee20d2b6cd8cb06d73e6a6da9
> Author: Heiko Carstens <[email protected]>
> Date: Thu Feb 18 14:25:21 2010 +0100
>
> tracing/kprobes: Add short documentation for HAVE_REGS_AND_STACK_ACCESS_API
>
> So that arch developers know how to implement it without the
> need to dig into changelogs.
>
> Reported-by: Mike Frysinger <[email protected]>
> Signed-off-by: Heiko Carstens <[email protected]>
> Cc: Masami Hiramatsu <[email protected]>
> Cc: Martin Schwidefsky <[email protected]>
> Cc: "David S . Miller" <[email protected]>
> Cc: Paul Mundt <[email protected]>
> Cc: Steven Rostedt <[email protected]>
> LKML-Reference: <[email protected]>
> [added reference to ptrace.h in the config help]
> Signed-off-by: Frederic Weisbecker <[email protected]>
>
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 04e3aa7..50877ef 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -123,6 +123,11 @@ config USE_GENERIC_SMP_HELPERS
>
> config HAVE_REGS_AND_STACK_ACCESS_API
> bool
> + help
> + This symbol should be selected by an architecure if it supports
> + the API needed to access registers and stack entries from pt_regs,
> + declared in asm/ptrace.h
> + For example the kprobes-based event tracer needs this API.
>
> config HAVE_CLK
> bool
>
>
--
Masami Hiramatsu
Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division
e-mail: [email protected]
On Sun, Feb 21, 2010 at 20:27, Frederic Weisbecker wrote:
> + This symbol should be selected by an architecure if it supports
architecure -> architecture
otherwise, i'm placated ... thanks
-mike
Commit-ID: e01292b1fd68ff2abe234d584b06e64344d2c1de
Gitweb: http://git.kernel.org/tip/e01292b1fd68ff2abe234d584b06e64344d2c1de
Author: Heiko Carstens <[email protected]>
AuthorDate: Thu, 18 Feb 2010 14:25:21 +0100
Committer: Frederic Weisbecker <[email protected]>
CommitDate: Mon, 22 Feb 2010 17:05:51 +0100
tracing/kprobes: Add short documentation for HAVE_REGS_AND_STACK_ACCESS_API
So that arch developers know how to implement it without the
need to dig into changelogs.
Reported-by: Mike Frysinger <[email protected]>
Signed-off-by: Heiko Carstens <[email protected]>
Acked-by: Masami Hiramatsu <[email protected]>
Cc: Martin Schwidefsky <[email protected]>
Cc: "David S . Miller" <[email protected]>
Cc: Paul Mundt <[email protected]>
Cc: Steven Rostedt <[email protected]>
LKML-Reference: <[email protected]>
[added reference to ptrace.h in the config help]
Signed-off-by: Frederic Weisbecker <[email protected]>
---
arch/Kconfig | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/arch/Kconfig b/arch/Kconfig
index 04e3aa7..50877ef 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -123,6 +123,11 @@ config USE_GENERIC_SMP_HELPERS
config HAVE_REGS_AND_STACK_ACCESS_API
bool
+ help
+ This symbol should be selected by an architecure if it supports
+ the API needed to access registers and stack entries from pt_regs,
+ declared in asm/ptrace.h
+ For example the kprobes-based event tracer needs this API.
config HAVE_CLK
bool