Cc: Andrew Morton <akpm@...>, Christoph Lameter <clameter@...>, Jack Steiner <steiner@...>, Robin Holt <holt@...>, Nick Piggin <npiggin@...>, Peter Zijlstra <a.p.zijlstra@...>, <kvm-devel@...>, Kanoj Sarcar <kanojsarcar@...>, Roland Dreier <rdreier@...>, Steve Wise <swise@...>, <linux-kernel@...>, Avi Kivity <avi@...>, <linux-mm@...>, <general@...>, Hugh Dickins <hugh@...>, Rusty Russell <rusty@...>, Anthony Liguori <aliguori@...>, Chris Wright <chrisw@...>, Marcelo Tosatti <marcelo@...>, Eric Dumazet <dada1@...>, Paul E. McKenney <paulmck@...>
I really suspect we don't really have to, and that it would be better to
just fix the code that does that.
The thing is, I didn't like that one *either*. I thought it was the
biggest turd in the series (and by "biggest", I literally mean "most lines
of turd-ness" rather than necessarily "ugliest per se").
I literally think that mm_lock() is an unbelievable piece of utter and
horrible CRAP.
There's simply no excuse for code like that.
If you want to avoid the deadlock from taking multiple locks in order, but
there is really just a single operation that needs it, there's a really
really simple solution.
And that solution is *not* to sort the whole damn f*cking list in a
vmalloc'ed data structure prior to locking!
Damn.
No, the simple solution is to just make up a whole new upper-level lock,
and get that lock *first*. You can then take all the multiple locks at a
lower level in any order you damn well please.
And yes, it's one more lock, and yes, it serializes stuff, but:
- that code had better not be critical anyway, because if it was, then
the whole "vmalloc+sort+lock+vunmap" sh*t was wrong _anyway_
- parallelism is overrated: it doesn't matter one effing _whit_ if
something is a hundred times more parallel, if it's also a hundred
times *SLOWER*.
So dang it, flush the whole damn series down the toilet and either forget
the thing entirely, or re-do it sanely.
And here's an admission that I lied: it wasn't *all* clearly crap. I did
like one part, namely list_del_init_rcu(), but that one should have been
in a separate patch. I'll happily apply that one.
Linus
--
