Messages in this thread | | | Date | Wed, 2 Apr 2014 11:46:38 -0700 | Subject | Re: [PATCH 0/2] x86: Fix perf deadlock caused by dumpstack cleanup | From | Linus Torvalds <> |
| |
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.
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.
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.
Linus
| |