Re: [PATCH 1/2] x86: make save_stack_address()!CONFIG_FRAME_POINTER friendly

[Oleg Nesterov]

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.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/