lkml.org 
[lkml]   [2014]   [Apr]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 0/2] x86: Fix perf deadlock caused by dumpstack cleanup
On Wed, 2 Apr 2014 11:46:38 -0700
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Wed, Apr 2, 2014 at 10:26 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > Here are two patches that fix the deadlock that you discovered.
> >
> > The first patch is the real culprit, but as I was looking at the code
> > I realized that the irq stack part was a bit off too. That part didn't
> > cause the lock up, but needs to be fixed regardless.
> >
> > Note, I'm hoping to use these clean ups to make x86_64 and i386 code
> > a bit closer to each other, so I hope the original change does not
> > get reverted.
>
> I'll apply them.
>
> That said, I'm not *AT*ALL* convinced that a "cleanup" that involves
> having helper functions with multiple bugs and now seven (7!)
> arguments passed into it is a "cleanup" at all.
>

Yeah, I didn't like it after the second patch :-(

> The cleanup was claimed to help improve readability. Really? Somebody
> needs to reconsider their goals in life if they think that a
> 7-argument helper functions where the caller needs to pass in
> addresses to variables because the helper function will change them is
> a good idea. Just the *call* is spread out over two lines because
> calling that function is so complex.

The helper can be merged back to the main function as it doesn't seem
to be much of a helper anymore.

>
> I suspect that maybe creating some kind of "struct stack_information"
> structure might help somewhat. Because right now I'm very doubtful
> about the whole "this clarifies/simplifies" argument. The very fact
> that the "cleaned-up" function was a buggy pile of sh*t should make
> you question the cleanup itself.

Well, actually, it proves that there's a clean up needed, as the reason
for the bugs was due to the subtle interactions of the original code.
That said, I agree, the cleanup itself isn't much of a cleanup. But I
intend on continuing to cleanup this code, and remove all the subtle
interactions that is going on here.

This is an ongoing project and what you see here is not the end result.
I am quite ashamed of the bugs though.

-- Steve



\
 
 \ /
  Last update: 2014-04-02 22:41    [W:0.049 / U:0.344 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site