lkml.org 
[lkml]   [2014]   [Nov]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH, RFC] x86: also CFI-annotate certain inline asm()s
>>> On 10.11.14 at 18:56, <torvalds@linux-foundation.org> wrote:
> So no. A very strong NACK. The code was too ugly to live, there is no good
> stated reason for it, and the only reason I can even remotely imagine is
> wrong and complete crap anyway (ie making the CFI annotations a correctness
> issue by introducing other infrastructure that depends on it always being
> right).
>
> We've been through this before. The same suspects were involved. If this is
> some crazy "debug info has to be right, because otherwise our total crap
> unwinding goes boom", then the solution is not to add more broken and ugly
> CFI annotations, the solution is to not depend on a broken and fragile
> unwinder.
>
> Any debug infrastructure *must* be robust enough that missing or even
> actively wrong frame information must not break it. If it's too fragile to
> handle missing or bad debug information, then it is too fragile to be used
> for debugging.

And no-one said this not to be the case. The fact here is that the
unwind information can be improved, resulting in better use of
whatever consumes it. And the "whatever" that prompted this was
upstream's perf subsystem (I think I even mentioned this in the
patch description), which - through the way things get connected
together in the kernel - just _happens_ to use the unwinder when
it's present in the kernel.

Nothing crashes with the unwind information being wrong. It is
solely you who was claiming (without proof) years ago that the
unwinder repeatedly caused issues. Yes, we did find a bug or two
over the years in it, but there being bugs in software is pretty
much unavoidable. If you didn't accept that, you shouldn't be
merging any patches introducing new features.

The point of the patch, however, is to make the unwinder do a
better job. And even if you prefer to ignore this fact, people
over here actually prefer looking at stack traces they don't have
to guess or verify for each entry whether it actually belongs to
the call stack - this saves them time, and hence allows them to
be more productive.

> The fact is, debug information had better *never* be so important that it
> has to be right. Seriously. Even gcc historically gets the debug
> information wrong quite regularly, exactly because there are so few things
> that depend on it. We have to assume that frame information is incomplete,
> and that also means that we don't add insane crap to our inline asm code.
>
> So for something like this to be workable, it has better be:

So finally you're getting to at least slightly productive criticism.

> - much cleaner and less hacky

And I asked for suggestions in the patch comment.

> - not have to depend on random gcc code generation

I don't think I can correlate this to any particular aspect of the
change.

Jan



\
 
 \ /
  Last update: 2014-11-11 09:21    [W:0.814 / U:0.124 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site