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: <39586553-6185-4b83-b18a-3716caf2f3cf@suse.cz>
Date: Mon, 31 Mar 2025 11:59:05 +0200
From: Vlastimil Babka <vbabka@...e.cz>
To: Sebastian Sewior <bigeasy@...utronix.de>,
 Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>, bpf
 <bpf@...r.kernel.org>, Daniel Borkmann <daniel@...earbox.net>,
 Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau
 <martin.lau@...nel.org>, Andrew Morton <akpm@...ux-foundation.org>,
 Peter Zijlstra <peterz@...radead.org>, Steven Rostedt <rostedt@...dmis.org>,
 Michal Hocko <mhocko@...e.com>, Shakeel Butt <shakeel.butt@...ux.dev>,
 linux-mm <linux-mm@...ck.org>, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [GIT PULL] Introduce try_alloc_pages for 6.15

On 3/31/25 09:14, Sebastian Sewior wrote:
> On 2025-03-30 14:49:25 [-0700], Alexei Starovoitov wrote:
>> On Sun, Mar 30, 2025 at 1:56 PM Linus Torvalds
>> <torvalds@...ux-foundation.org> wrote:
>> > 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.
>> 
>> Ok. Agree with the reasoning that the name doesn't quite fit.
>> 
>> nmisafe_local_lock_t name works for me,
>> though nmisafe_local_lock_irqsave() is a bit verbose.
>> 
>> Don't have better name suggestions at the moment.
>> 
>> Sebastian, Vlastimil,
>> what do you prefer ?
> 
> nmisafe_local_lock_t sounds okay assuming the "nmisafe" part does not
> make it look like it can be used without the trylock part in NMI context.

Yes I was going to point out that e.g. "nmisafe_local_lock_irqsave()" seems
rather misleading to me as this operation is not a nmisafe one?

I think it comes back to what we discussed in previous versions of the
patchset. IMHO conceptually it's still a local_lock, it just supports the
new trylock operations. However, to make them possible (on non-RT) if a
particular lock instance is to be used with the trylock anywhere, it needs
the new "acquired" field, and for the non-trylock operations to work with
the field too.

So (to inform Linus) the earlier attempt [1] was to just change the existing
local_lock_t, but that adds the overhead of the field and manipulating it
for instances that don't use trylock.

The following attempt [2] meant there would be only a new local_trylock_t
type, but the existing locking operations would remain the same, relying on
_Generic() parts inside them.

It was preferred to make the implementation more obvious, but then we have
the naming issue for the operations in addition to the type...

[1]
https://lore.kernel.org/bpf/20241218030720.1602449-4-alexei.starovoitov@gmail.com/
[2]
https://lore.kernel.org/bpf/20250124035655.78899-4-alexei.starovoitov@gmail.com/

> But yeah, it sounds better than the previous one.
> 
> Sebastian