* [GIT PULL] perf updates
@ 2010-06-08 20:13 Frederic Weisbecker
2010-06-08 20:13 ` [PATCH 1/2] x86: Make save_stack_address() !CONFIG_FRAME_POINTER friendly Frederic Weisbecker
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Frederic Weisbecker @ 2010-06-08 20:13 UTC (permalink / raw)
To: Ingo Molnar
Cc: LKML, Frederic Weisbecker, Steven Rostedt, Peter Zijlstra,
Paul Mackerras, Arnaldo Carvalho de Melo
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2558 bytes --]
Ingo,
Please pull the perf/core branch that can be found at:
git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git
perf/core
I'm only posting Oleg's patches as only those are new in my queues.
Thanks,
Frederic
---
Frederic Weisbecker (2):
x86: Unify dumpstack.h and stacktrace.h
perf: Drop the skip argument from perf_arch_fetch_regs_caller
Oleg Nesterov (2):
x86: Make save_stack_address() !CONFIG_FRAME_POINTER friendly
x86: Unify save_stack_address() and save_stack_address_nosched()
Américo Wang (1):
tracing: Remove boot tracer
Li Zefan (1):
tracing: Remove kmemtrace ftrace plugin
Documentation/ABI/testing/debugfs-kmemtrace | 71 ----
Documentation/trace/kmemtrace.txt | 126 -------
MAINTAINERS | 7 -
arch/powerpc/include/asm/perf_event.h | 12 +
arch/powerpc/kernel/misc.S | 26 --
arch/sparc/include/asm/perf_event.h | 8 +
arch/sparc/kernel/helpers.S | 6 +-
arch/x86/include/asm/perf_event.h | 13 +
arch/x86/include/asm/stacktrace.h | 49 +++
arch/x86/kernel/cpu/perf_event.c | 18 -
arch/x86/kernel/dumpstack.c | 1 -
arch/x86/kernel/dumpstack.h | 56 ---
arch/x86/kernel/dumpstack_32.c | 2 -
arch/x86/kernel/dumpstack_64.c | 1 -
arch/x86/kernel/stacktrace.c | 31 +-
include/linux/kmemtrace.h | 25 --
include/linux/perf_event.h | 32 +--
include/linux/slab_def.h | 3 +-
include/linux/slub_def.h | 3 +-
include/trace/boot.h | 60 ---
include/trace/ftrace.h | 2 +-
init/main.c | 29 +-
kernel/perf_event.c | 5 -
kernel/trace/Kconfig | 37 --
kernel/trace/Makefile | 2 -
kernel/trace/kmemtrace.c | 529 ---------------------------
kernel/trace/trace.c | 3 -
kernel/trace/trace.h | 20 -
kernel/trace/trace_boot.c | 185 ----------
kernel/trace/trace_entries.h | 62 ----
kernel/trace/trace_event_perf.c | 2 -
mm/slab.c | 1 -
mm/slub.c | 1 -
33 files changed, 123 insertions(+), 1305 deletions(-)
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 1/2] x86: Make save_stack_address() !CONFIG_FRAME_POINTER friendly
2010-06-08 20:13 [GIT PULL] perf updates Frederic Weisbecker
@ 2010-06-08 20:13 ` Frederic Weisbecker
2010-06-08 20:13 ` [PATCH 2/2] x86: Unify save_stack_address() and save_stack_address_nosched() Frederic Weisbecker
2010-06-08 21:13 ` [GIT PULL] perf updates Ingo Molnar
2 siblings, 0 replies; 11+ messages in thread
From: Frederic Weisbecker @ 2010-06-08 20:13 UTC (permalink / raw)
To: Ingo Molnar
Cc: LKML, Oleg Nesterov, Roland McGrath, Arjan van de Ven,
Vegard Nossum, Ingo Molnar, Andrew Morton, Frederic Weisbecker
From: Oleg Nesterov <oleg@redhat.com>
If CONFIG_FRAME_POINTER=n, print_context_stack() shouldn't neglect the
non-reliable addresses on stack, this is all we have if dump_trace(bp)
is called with the wrong or zero bp.
For example, /proc/pid/stack doesn't work if CONFIG_FRAME_POINTER=n.
This patch obviously has no effect if CONFIG_FRAME_POINTER=y, otherwise
it reverts 1650743c "x86: don't save unreliable stack trace entries".
Also, remove the unnecessary type-cast.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Cc: Roland McGrath <roland@redhat.com>
Cc: Arjan van de Ven <arjan@linux.intel.com>
Cc: Vegard Nossum <vegard.nossum@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Andrew Morton <akpm@linux-foundation.org>
LKML-Reference: <20100603193239.GA31530@redhat.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
arch/x86/kernel/stacktrace.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/arch/x86/kernel/stacktrace.c b/arch/x86/kernel/stacktrace.c
index ea54d02..abc321d 100644
--- a/arch/x86/kernel/stacktrace.c
+++ b/arch/x86/kernel/stacktrace.c
@@ -26,8 +26,10 @@ static int save_stack_stack(void *data, char *name)
static void save_stack_address(void *data, unsigned long addr, int reliable)
{
struct stack_trace *trace = data;
+#ifdef CONFIG_FRAME_POINTER
if (!reliable)
return;
+#endif
if (trace->skip > 0) {
trace->skip--;
return;
@@ -39,9 +41,11 @@ static void save_stack_address(void *data, unsigned long addr, int reliable)
static void
save_stack_address_nosched(void *data, unsigned long addr, int reliable)
{
- struct stack_trace *trace = (struct stack_trace *)data;
+ struct stack_trace *trace = data;
+#ifdef CONFIG_FRAME_POINTER
if (!reliable)
return;
+#endif
if (in_sched_functions(addr))
return;
if (trace->skip > 0) {
--
1.6.2.3
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 2/2] x86: Unify save_stack_address() and save_stack_address_nosched()
2010-06-08 20:13 [GIT PULL] perf updates Frederic Weisbecker
2010-06-08 20:13 ` [PATCH 1/2] x86: Make save_stack_address() !CONFIG_FRAME_POINTER friendly Frederic Weisbecker
@ 2010-06-08 20:13 ` Frederic Weisbecker
2010-06-08 21:13 ` [GIT PULL] perf updates Ingo Molnar
2 siblings, 0 replies; 11+ messages in thread
From: Frederic Weisbecker @ 2010-06-08 20:13 UTC (permalink / raw)
To: Ingo Molnar
Cc: LKML, Oleg Nesterov, Roland McGrath, Arjan van de Ven,
Vegard Nossum, Ingo Molnar, Frederic Weisbecker
From: Oleg Nesterov <oleg@redhat.com>
Cleanup. Factor the common code in save_stack_address() and
save_stack_address_nosched().
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Cc: Roland McGrath <roland@redhat.com>
Cc: Arjan van de Ven <arjan@linux.intel.com>
Cc: Vegard Nossum <vegard.nossum@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
LKML-Reference: <20100603193243.GA31534@redhat.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
arch/x86/kernel/stacktrace.c | 24 ++++++++++--------------
1 files changed, 10 insertions(+), 14 deletions(-)
diff --git a/arch/x86/kernel/stacktrace.c b/arch/x86/kernel/stacktrace.c
index abc321d..b53c525 100644
--- a/arch/x86/kernel/stacktrace.c
+++ b/arch/x86/kernel/stacktrace.c
@@ -23,13 +23,16 @@ static int save_stack_stack(void *data, char *name)
return 0;
}
-static void save_stack_address(void *data, unsigned long addr, int reliable)
+static void
+__save_stack_address(void *data, unsigned long addr, bool reliable, bool nosched)
{
struct stack_trace *trace = data;
#ifdef CONFIG_FRAME_POINTER
if (!reliable)
return;
#endif
+ if (nosched && in_sched_functions(addr))
+ return;
if (trace->skip > 0) {
trace->skip--;
return;
@@ -38,22 +41,15 @@ static void save_stack_address(void *data, unsigned long addr, int reliable)
trace->entries[trace->nr_entries++] = addr;
}
+static void save_stack_address(void *data, unsigned long addr, int reliable)
+{
+ return __save_stack_address(data, addr, reliable, false);
+}
+
static void
save_stack_address_nosched(void *data, unsigned long addr, int reliable)
{
- struct stack_trace *trace = data;
-#ifdef CONFIG_FRAME_POINTER
- if (!reliable)
- return;
-#endif
- if (in_sched_functions(addr))
- return;
- if (trace->skip > 0) {
- trace->skip--;
- return;
- }
- if (trace->nr_entries < trace->max_entries)
- trace->entries[trace->nr_entries++] = addr;
+ return __save_stack_address(data, addr, reliable, true);
}
static const struct stacktrace_ops save_stack_ops = {
--
1.6.2.3
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [GIT PULL] perf updates
2010-06-08 20:13 [GIT PULL] perf updates Frederic Weisbecker
2010-06-08 20:13 ` [PATCH 1/2] x86: Make save_stack_address() !CONFIG_FRAME_POINTER friendly Frederic Weisbecker
2010-06-08 20:13 ` [PATCH 2/2] x86: Unify save_stack_address() and save_stack_address_nosched() Frederic Weisbecker
@ 2010-06-08 21:13 ` Ingo Molnar
2 siblings, 0 replies; 11+ messages in thread
From: Ingo Molnar @ 2010-06-08 21:13 UTC (permalink / raw)
To: Frederic Weisbecker
Cc: LKML, Steven Rostedt, Peter Zijlstra, Paul Mackerras,
Arnaldo Carvalho de Melo
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=unknown-8bit, Size: 2773 bytes --]
* Frederic Weisbecker <fweisbec@gmail.com> wrote:
> Ingo,
>
> Please pull the perf/core branch that can be found at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git
> perf/core
>
> I'm only posting Oleg's patches as only those are new in my queues.
>
> Thanks,
> Frederic
> ---
>
> Frederic Weisbecker (2):
> x86: Unify dumpstack.h and stacktrace.h
> perf: Drop the skip argument from perf_arch_fetch_regs_caller
>
> Oleg Nesterov (2):
> x86: Make save_stack_address() !CONFIG_FRAME_POINTER friendly
> x86: Unify save_stack_address() and save_stack_address_nosched()
>
> Américo Wang (1):
> tracing: Remove boot tracer
>
> Li Zefan (1):
> tracing: Remove kmemtrace ftrace plugin
>
>
> Documentation/ABI/testing/debugfs-kmemtrace | 71 ----
> Documentation/trace/kmemtrace.txt | 126 -------
> MAINTAINERS | 7 -
> arch/powerpc/include/asm/perf_event.h | 12 +
> arch/powerpc/kernel/misc.S | 26 --
> arch/sparc/include/asm/perf_event.h | 8 +
> arch/sparc/kernel/helpers.S | 6 +-
> arch/x86/include/asm/perf_event.h | 13 +
> arch/x86/include/asm/stacktrace.h | 49 +++
> arch/x86/kernel/cpu/perf_event.c | 18 -
> arch/x86/kernel/dumpstack.c | 1 -
> arch/x86/kernel/dumpstack.h | 56 ---
> arch/x86/kernel/dumpstack_32.c | 2 -
> arch/x86/kernel/dumpstack_64.c | 1 -
> arch/x86/kernel/stacktrace.c | 31 +-
> include/linux/kmemtrace.h | 25 --
> include/linux/perf_event.h | 32 +--
> include/linux/slab_def.h | 3 +-
> include/linux/slub_def.h | 3 +-
> include/trace/boot.h | 60 ---
> include/trace/ftrace.h | 2 +-
> init/main.c | 29 +-
> kernel/perf_event.c | 5 -
> kernel/trace/Kconfig | 37 --
> kernel/trace/Makefile | 2 -
> kernel/trace/kmemtrace.c | 529 ---------------------------
> kernel/trace/trace.c | 3 -
> kernel/trace/trace.h | 20 -
> kernel/trace/trace_boot.c | 185 ----------
> kernel/trace/trace_entries.h | 62 ----
> kernel/trace/trace_event_perf.c | 2 -
> mm/slab.c | 1 -
> mm/slub.c | 1 -
> 33 files changed, 123 insertions(+), 1305 deletions(-)
Pulled, thanks a lot Frederic!
Ingo
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] x86: make save_stack_address() !CONFIG_FRAME_POINTER friendly
@ 2010-06-03 19:32 Oleg Nesterov
2010-06-03 19:52 ` Frederic Weisbecker
2010-06-03 19:53 ` Arjan van de Ven
0 siblings, 2 replies; 11+ messages in thread
From: Oleg Nesterov @ 2010-06-03 19:32 UTC (permalink / raw)
To: Andrew Morton
Cc: Arjan van de Ven, Frederic Weisbecker, Ingo Molnar,
Roland McGrath, Vegard Nossum, linux-kernel
If CONFIG_FRAME_POINTER=n, print_context_stack() shouldn't neglect the
non-reliable addresses on stack, this is all we have if dump_trace(bp)
is called with the wrong or zero bp.
For example, /proc/pid/stack doesn't work if CONFIG_FRAME_POINTER=n.
This patch obviously has no effect if CONFIG_FRAME_POINTER=y, otherwise
it reverts 1650743c "x86: don't save unreliable stack trace entries".
Also, remove the unnecessary type-cast.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
arch/x86/kernel/stacktrace.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
--- 34-rc1/arch/x86/kernel/stacktrace.c~SAVE_STACK_WO_FP 2010-06-03 18:43:27.000000000 +0200
+++ 34-rc1/arch/x86/kernel/stacktrace.c 2010-06-03 21:29:52.000000000 +0200
@@ -26,8 +26,10 @@ static int save_stack_stack(void *data,
static void save_stack_address(void *data, unsigned long addr, int reliable)
{
struct stack_trace *trace = data;
+#ifdef CONFIG_FRAME_POINTER
if (!reliable)
return;
+#endif
if (trace->skip > 0) {
trace->skip--;
return;
@@ -39,9 +41,11 @@ static void save_stack_address(void *dat
static void
save_stack_address_nosched(void *data, unsigned long addr, int reliable)
{
- struct stack_trace *trace = (struct stack_trace *)data;
+ struct stack_trace *trace = data;
+#ifdef CONFIG_FRAME_POINTER
if (!reliable)
return;
+#endif
if (in_sched_functions(addr))
return;
if (trace->skip > 0) {
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 1/2] x86: make save_stack_address() !CONFIG_FRAME_POINTER friendly
2010-06-03 19:32 [PATCH 1/2] x86: make save_stack_address() !CONFIG_FRAME_POINTER friendly Oleg Nesterov
@ 2010-06-03 19:52 ` Frederic Weisbecker
2010-06-03 19:53 ` Arjan van de Ven
1 sibling, 0 replies; 11+ messages in thread
From: Frederic Weisbecker @ 2010-06-03 19:52 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Andrew Morton, Arjan van de Ven, Ingo Molnar, Roland McGrath,
Vegard Nossum, linux-kernel
On Thu, Jun 03, 2010 at 09:32:39PM +0200, Oleg Nesterov wrote:
> If CONFIG_FRAME_POINTER=n, print_context_stack() shouldn't neglect the
> non-reliable addresses on stack, this is all we have if dump_trace(bp)
> is called with the wrong or zero bp.
>
> For example, /proc/pid/stack doesn't work if CONFIG_FRAME_POINTER=n.
>
> This patch obviously has no effect if CONFIG_FRAME_POINTER=y, otherwise
> it reverts 1650743c "x86: don't save unreliable stack trace entries".
>
> Also, remove the unnecessary type-cast.
>
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] x86: make save_stack_address() !CONFIG_FRAME_POINTER friendly
2010-06-03 19:32 [PATCH 1/2] x86: make save_stack_address() !CONFIG_FRAME_POINTER friendly Oleg Nesterov
2010-06-03 19:52 ` Frederic Weisbecker
@ 2010-06-03 19:53 ` Arjan van de Ven
2010-06-03 20:06 ` Frederic Weisbecker
1 sibling, 1 reply; 11+ messages in thread
From: Arjan van de Ven @ 2010-06-03 19:53 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Andrew Morton, Frederic Weisbecker, Ingo Molnar, Roland McGrath,
Vegard Nossum, linux-kernel
On 6/3/2010 12:32 PM, Oleg Nesterov wrote:
> If CONFIG_FRAME_POINTER=n, print_context_stack() shouldn't neglect the
> non-reliable addresses on stack, this is all we have if dump_trace(bp)
> is called with the wrong or zero bp.
>
> For example, /proc/pid/stack doesn't work if CONFIG_FRAME_POINTER=n.
>
> This patch obviously has no effect if CONFIG_FRAME_POINTER=y, otherwise
> it reverts 1650743c "x86: don't save unreliable stack trace entries".
>
would be nice if there was a compile time thing to detect if frame
pointers are on ratehr than an ifdef.
you're now also changing the rules; until now, you would ALWAYS get a
backtrace without noise....
now that's changing quite a bit. How are various tools (like perf and
sysprof) going to cope with that?
> Also, remove the unnecessary type-cast.
>
> Signed-off-by: Oleg Nesterov<oleg@redhat.com>
> ---
>
> arch/x86/kernel/stacktrace.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> --- 34-rc1/arch/x86/kernel/stacktrace.c~SAVE_STACK_WO_FP 2010-06-03 18:43:27.000000000 +0200
> +++ 34-rc1/arch/x86/kernel/stacktrace.c 2010-06-03 21:29:52.000000000 +0200
> @@ -26,8 +26,10 @@ static int save_stack_stack(void *data,
> static void save_stack_address(void *data, unsigned long addr, int reliable)
> {
> struct stack_trace *trace = data;
> +#ifdef CONFIG_FRAME_POINTER
> if (!reliable)
> return;
> +#endif
> if (trace->skip> 0) {
> trace->skip--;
> return;
> @@ -39,9 +41,11 @@ static void save_stack_address(void *dat
> static void
> save_stack_address_nosched(void *data, unsigned long addr, int reliable)
> {
> - struct stack_trace *trace = (struct stack_trace *)data;
> + struct stack_trace *trace = data;
> +#ifdef CONFIG_FRAME_POINTER
> if (!reliable)
> return;
> +#endif
> if (in_sched_functions(addr))
> return;
> if (trace->skip> 0) {
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 1/2] x86: make save_stack_address() !CONFIG_FRAME_POINTER friendly
2010-06-03 19:53 ` Arjan van de Ven
@ 2010-06-03 20:06 ` Frederic Weisbecker
2010-06-03 20:31 ` Oleg Nesterov
0 siblings, 1 reply; 11+ messages in thread
From: Frederic Weisbecker @ 2010-06-03 20:06 UTC (permalink / raw)
To: Arjan van de Ven
Cc: Oleg Nesterov, Andrew Morton, Ingo Molnar, Roland McGrath,
Vegard Nossum, linux-kernel
On Thu, Jun 03, 2010 at 12:53:52PM -0700, Arjan van de Ven wrote:
> On 6/3/2010 12:32 PM, Oleg Nesterov wrote:
>> If CONFIG_FRAME_POINTER=n, print_context_stack() shouldn't neglect the
>> non-reliable addresses on stack, this is all we have if dump_trace(bp)
>> is called with the wrong or zero bp.
>>
>> For example, /proc/pid/stack doesn't work if CONFIG_FRAME_POINTER=n.
>>
>> This patch obviously has no effect if CONFIG_FRAME_POINTER=y, otherwise
>> it reverts 1650743c "x86: don't save unreliable stack trace entries".
>>
>
> would be nice if there was a compile time thing to detect if frame
> pointers are on ratehr than an ifdef.
I wanted to suggest that too, but since only one place got the ifdef
after the second patch.
But yeah, something like this could be reused:
if (reliable_frame_pointer(reliable))
return ...;
> you're now also changing the rules; until now, you would ALWAYS get a
> backtrace without noise....
> now that's changing quite a bit. How are various tools (like perf and
> sysprof) going to cope with that?
perf and sysprof have their own stacktrace ops, so they aren't affected.
I think the rest is /proc/pid/task, lockdep, latencytop, ftrace, kmemleak,
etc...
For the kernel parts it's in fact desired.
And with ftrace we are changing some rules, but this is desired too, without
frame pointers we would have nothing anyway. And it's quite easy to
find out a stacktrace is not entirely reliable at a glance.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] x86: make save_stack_address() !CONFIG_FRAME_POINTER friendly
2010-06-03 20:06 ` Frederic Weisbecker
@ 2010-06-03 20:31 ` Oleg Nesterov
2010-06-03 20:50 ` Frederic Weisbecker
0 siblings, 1 reply; 11+ messages in thread
From: Oleg Nesterov @ 2010-06-03 20:31 UTC (permalink / raw)
To: Frederic Weisbecker
Cc: Arjan van de Ven, Andrew Morton, Ingo Molnar, Roland McGrath,
Vegard Nossum, linux-kernel
On 06/03, Frederic Weisbecker wrote:
>
> On Thu, Jun 03, 2010 at 12:53:52PM -0700, Arjan van de Ven wrote:
> > On 6/3/2010 12:32 PM, Oleg Nesterov wrote:
> >> If CONFIG_FRAME_POINTER=n, print_context_stack() shouldn't neglect the
> >> non-reliable addresses on stack, this is all we have if dump_trace(bp)
> >> is called with the wrong or zero bp.
> >>
> >> For example, /proc/pid/stack doesn't work if CONFIG_FRAME_POINTER=n.
> >>
> >> This patch obviously has no effect if CONFIG_FRAME_POINTER=y, otherwise
> >> it reverts 1650743c "x86: don't save unreliable stack trace entries".
> >
> > would be nice if there was a compile time thing to detect if frame
> > pointers are on ratehr than an ifdef.
>
> I wanted to suggest that too, but since only one place got the ifdef
> after the second patch.
>
> But yeah, something like this could be reused:
>
> if (reliable_frame_pointer(reliable))
> return ...;
Do you mean it makes sense to add the helper which depends on
FRAME_POINTER ?
> > you're now also changing the rules; until now, you would ALWAYS get a
> > backtrace without noise....
> > now that's changing quite a bit. How are various tools (like perf and
> > sysprof) going to cope with that?
>
>
>
> perf and sysprof have their own stacktrace ops, so they aren't affected.
> I think the rest is /proc/pid/task, lockdep, latencytop, ftrace, kmemleak,
> etc...
>
> For the kernel parts it's in fact desired.
> And with ftrace we are changing some rules, but this is desired too, without
> frame pointers we would have nothing anyway. And it's quite easy to
> find out a stacktrace is not entirely reliable at a glance.
Frederic, Arjan. Honestly, I have no opinion if this change makes
things better or worse for, say, lockdep.
But note that this only affects the !CONFIG_FRAME_POINTER case.
Looking into Kconfig's I don't even understand how the bug reporters
managed to set CONFIG_STACKTRACE without !CONFIG_FRAME_POINTER.
So, should I redo this patch to fix /proc/pid/stack ? Say, we
can change the meaning of stack_trace-<skip, if it is < 0, then
save_stack_address() ignores reliable. Yes, this is hack.
Oleg.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] x86: make save_stack_address() !CONFIG_FRAME_POINTER friendly
2010-06-03 20:31 ` Oleg Nesterov
@ 2010-06-03 20:50 ` Frederic Weisbecker
2010-06-03 20:59 ` Oleg Nesterov
0 siblings, 1 reply; 11+ messages in thread
From: Frederic Weisbecker @ 2010-06-03 20:50 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Arjan van de Ven, Andrew Morton, Ingo Molnar, Roland McGrath,
Vegard Nossum, linux-kernel
On Thu, Jun 03, 2010 at 10:31:55PM +0200, Oleg Nesterov wrote:
> On 06/03, Frederic Weisbecker wrote:
> >
> > On Thu, Jun 03, 2010 at 12:53:52PM -0700, Arjan van de Ven wrote:
> > > On 6/3/2010 12:32 PM, Oleg Nesterov wrote:
> > >> If CONFIG_FRAME_POINTER=n, print_context_stack() shouldn't neglect the
> > >> non-reliable addresses on stack, this is all we have if dump_trace(bp)
> > >> is called with the wrong or zero bp.
> > >>
> > >> For example, /proc/pid/stack doesn't work if CONFIG_FRAME_POINTER=n.
> > >>
> > >> This patch obviously has no effect if CONFIG_FRAME_POINTER=y, otherwise
> > >> it reverts 1650743c "x86: don't save unreliable stack trace entries".
> > >
> > > would be nice if there was a compile time thing to detect if frame
> > > pointers are on ratehr than an ifdef.
> >
> > I wanted to suggest that too, but since only one place got the ifdef
> > after the second patch.
> >
> > But yeah, something like this could be reused:
> >
> > if (reliable_frame_pointer(reliable))
> > return ...;
>
> Do you mean it makes sense to add the helper which depends on
> FRAME_POINTER ?
Having in asm/stacktrace.h:
#ifdef CONFIG_FRAME_POINTER
static inline int reliable_frame_pointer(int reliable)
{
return reliable;
}
#else
static inline int reliable_frame_pointer(int reliable)
{
return 1;
}
#endif
But if we have only one user I'm not sure it's worth it.
> > > you're now also changing the rules; until now, you would ALWAYS get a
> > > backtrace without noise....
> > > now that's changing quite a bit. How are various tools (like perf and
> > > sysprof) going to cope with that?
> >
> >
> >
> > perf and sysprof have their own stacktrace ops, so they aren't affected.
> > I think the rest is /proc/pid/task, lockdep, latencytop, ftrace, kmemleak,
> > etc...
> >
> > For the kernel parts it's in fact desired.
> > And with ftrace we are changing some rules, but this is desired too, without
> > frame pointers we would have nothing anyway. And it's quite easy to
> > find out a stacktrace is not entirely reliable at a glance.
>
> Frederic, Arjan. Honestly, I have no opinion if this change makes
> things better or worse for, say, lockdep.
>
> But note that this only affects the !CONFIG_FRAME_POINTER case.
> Looking into Kconfig's I don't even understand how the bug reporters
> managed to set CONFIG_STACKTRACE without !CONFIG_FRAME_POINTER.
>
> So, should I redo this patch to fix /proc/pid/stack ? Say, we
> can change the meaning of stack_trace-<skip, if it is < 0, then
> save_stack_address() ignores reliable. Yes, this is hack.
No, people may want to ignore reliable and also to skip
entries.
I think your patches as is are the right way to go: by default provide
as much information as we can.
And those who care about reliability can use their own stack ops, which
is what perf does for example. If needed we can still add a new
save_stack_trace_reliable() in the future.
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 1/2] x86: make save_stack_address() !CONFIG_FRAME_POINTER friendly
2010-06-03 20:50 ` Frederic Weisbecker
@ 2010-06-03 20:59 ` Oleg Nesterov
0 siblings, 0 replies; 11+ messages in thread
From: Oleg Nesterov @ 2010-06-03 20:59 UTC (permalink / raw)
To: Frederic Weisbecker
Cc: Arjan van de Ven, Andrew Morton, Ingo Molnar, Roland McGrath,
Vegard Nossum, linux-kernel
On 06/03, Frederic Weisbecker wrote:
>
> On Thu, Jun 03, 2010 at 10:31:55PM +0200, Oleg Nesterov wrote:
> >
> > Do you mean it makes sense to add the helper which depends on
> > FRAME_POINTER ?
>
> Having in asm/stacktrace.h:
>
> #ifdef CONFIG_FRAME_POINTER
> static inline int reliable_frame_pointer(int reliable)
> {
> return reliable;
> }
> #else
> static inline int reliable_frame_pointer(int reliable)
> {
> return 1;
> }
> #endif
>
> But if we have only one user I'm not sure it's worth it.
Me too ;) let's ignore this.
> > Frederic, Arjan. Honestly, I have no opinion if this change makes
> > things better or worse for, say, lockdep.
> >
> > But note that this only affects the !CONFIG_FRAME_POINTER case.
> > Looking into Kconfig's I don't even understand how the bug reporters
> > managed to set CONFIG_STACKTRACE without !CONFIG_FRAME_POINTER.
> >
> > So, should I redo this patch to fix /proc/pid/stack ? Say, we
> > can change the meaning of stack_trace-<skip, if it is < 0, then
> > save_stack_address() ignores reliable. Yes, this is hack.
>
>
> No, people may want to ignore reliable and also to skip
> entries.
Yes, but currently stack_trace->skip is always >= 0. So I think
this should work
skip > 0 - skip that much entries, consider reliable
skip == 0 - don't skip, consider reliable
skip < 0 - skip nothing
But yes, I do not like this idea too much. I was going to use this
hack if this patch is nacked.
> I think your patches as is are the right way to go: by default provide
> as much information as we can.
>
> And those who care about reliability can use their own stack ops, which
> is what perf does for example. If needed we can still add a new
> save_stack_trace_reliable() in the future.
Great. Thanks!
Oleg.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2010-06-08 21:13 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-06-08 20:13 [GIT PULL] perf updates Frederic Weisbecker
2010-06-08 20:13 ` [PATCH 1/2] x86: Make save_stack_address() !CONFIG_FRAME_POINTER friendly Frederic Weisbecker
2010-06-08 20:13 ` [PATCH 2/2] x86: Unify save_stack_address() and save_stack_address_nosched() Frederic Weisbecker
2010-06-08 21:13 ` [GIT PULL] perf updates Ingo Molnar
-- strict thread matches above, loose matches on Subject: below --
2010-06-03 19:32 [PATCH 1/2] x86: make save_stack_address() !CONFIG_FRAME_POINTER friendly Oleg Nesterov
2010-06-03 19:52 ` Frederic Weisbecker
2010-06-03 19:53 ` Arjan van de Ven
2010-06-03 20:06 ` Frederic Weisbecker
2010-06-03 20:31 ` Oleg Nesterov
2010-06-03 20:50 ` Frederic Weisbecker
2010-06-03 20:59 ` Oleg Nesterov
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).