linux-kernel - Re: [GIT PULL] locking changes for v6.4

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 28 Apr 2023 14:40:12 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Ingo Molnar <mingo@...nel.org>,
        Andrzej Hajda <andrzej.hajda@...el.com>
Cc:     linux-kernel@...r.kernel.org,
        Peter Zijlstra <peterz@...radead.org>,
        Will Deacon <will@...nel.org>,
        Waiman Long <longman@...hat.com>,
        Boqun Feng <boqun.feng@...il.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [GIT PULL] locking changes for v6.4

On Thu, Apr 27, 2023 at 12:58 PM Ingo Molnar <mingo@...nel.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