linux-kernel - Re: [GIT PULL] Introduce try_alloc

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAHk-=whVcfPyL3PhmSoQyRQZpYUDaKTFA+MOR9w8HCXDdQX8Uw@mail.gmail.com>
Date: Sun, 30 Mar 2025 13:56:19 -0700
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc: bpf@...r.kernel.org, daniel@...earbox.net, andrii@...nel.org, 
	martin.lau@...nel.org, akpm@...ux-foundation.org, peterz@...radead.org, 
	vbabka@...e.cz, bigeasy@...utronix.de, rostedt@...dmis.org, mhocko@...e.com, 
	shakeel.butt@...ux.dev, linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [GIT PULL] Introduce try_alloc_pages for 6.15

On Sun, 30 Mar 2025 at 13:42, Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> The one reaction I had is that when you basically change

Oh, actually, two reactions now that I fixed up the merge build issue
which forced me to look at the function naming.

That 'localtry_lock_irqsave()' naming is horrendous.

The "try" part in it makes me think it's a trylock. But no, the
"localtry" just comes from the lock naming, and the trylock version is
localtry_trylock_irqsave.

That's horrible.

I missed that on the first read-though, and I'm not unpulling it
because the code generally makes sense.

But I do think that the lock name needs fixing.

"localtry_lock_t" is not a good name, and spreading that odd
"localtry" into the actual (non-try) locking functions makes the
naming actively insane.

If the *only* operation you could do on the lock was "trylock", then
"localtry" would be fine. Then the lock literally is a "only try"
thing. But as it is, the naming now ends up actively broken.

Honestly, the lock name should probably reflect the fact that it can
be used from any context (with a "trylock"), not about the trylock
part itself.

So maybe "nmisafe_local_lock_t" or something in that vein?

Please fix this up, There aren't *that* many users of
"localtry_xyzzy", let's get this fixed before there are more of them.

             Linus