Messages in this thread | | | From | Linus Torvalds <> | Date | Fri, 28 Apr 2023 14:40:12 -0700 | Subject | Re: [GIT PULL] locking changes for v6.4 |
| |
On Thu, Apr 27, 2023 at 12:58 PM Ingo Molnar <mingo@kernel.org> wrote: > > - Add non-atomic __xchg() variant, use it in a couple of places
Guys, this is insane, and completely unacceptable.
I pulled this, but I'm going to unpull it, because the code is actively wrong and ugly.
It not only randomly decides to re-use a name that has existing users that now need to be fixed up.
It then *also* decides to start "preferring" this absolutely disgusting new name over a much more legible one in the i915 driver, which had this same functionality except it used a prettier name:
fetch_and_zero()
But what then takes the cake for me is that this horribly ugly feature then didn't even get that right, and only randomly converted *some* of the users, with most of them remaining:
git grep fetch_and_zero drivers/gpu/drm/i915/ | wc 58 187 5534 git grep -w __xchg drivers/gpu/drm/i915/ | wc 22 109 1899
and it looks like the only "logic" to this is that the converted ones were in the "gt/" subdirectory. What a random choice, but happily it caused a trivial conflict, and as a result I noticed how bad things were.
Anyway, I really find this all offensively ugly and pointless. I'm not going to pull some "fixed" version of this. This needs to go away and never come back.
What was so magically great about the name "__xchg" that it needed to be taken over by this function? And why was that legibly named version of it replaced so randomly?
The *whole* point of two underscores is to say "don't use this - it's an internal implementation". That's the historical meaning, and it's the meaning we have in the kernel too. Two underscores means "this is special and doesn't do everything required" (it might need locking around it, for example).
So then making a new interface with two underscores and thinking "we should now make random drivers use this" is fundamentally bogus.
Look, just grep for "__xchg" in the main tree (ie the one *without* this change). It all makes sense. It's all clearly an internal helper - as marked by that double underscore - and it's not used by any driver or filesystem code.
Exactly like K&R and God intended.
Linus
| |