Re: [PATCH 1/8] mm: Add ptep_try_install() for lockless empty-slot installs

["David Hildenbrand (Arm)" <david@kernel.org>]

On 5/19/26 21:04, Alexei Starovoitov wrote:
> On Tue, May 19, 2026 at 10:11 AM Tejun Heo <tj@kernel.org> wrote:
>>
>> On Tue, May 19, 2026 at 11:40:48AM +0200, David Hildenbrand (Arm) wrote:
>> ...
>>
>> Wouldn't it still be either or with both cases being okay?
>>
>>>
>>> ... or can we run into similar problems with kprobes? (I am obviously no bpf
>>> expert ...)
>>
>> Yeah, I mean, that was just the first TP I found scanning the code. Any
>> kprobes or other TPs in the path would behave the same.
>>
>> When this fault triggers, the BPF program has already malfunctioned, so it's
>> not going to be a high frequency path and performance isn't a primary
>> consideration. So, anything that can ensure that the kernel doesn't crash or
>> lock up would be fine. Any better ideas?
> 
> As you guys already figured out the trylock is not an option.

And that should be carefully documented.

> The fault handler has to install _some_ page and let kernel continue.
> Scratch page or arena page doesn't matter. Potentially different CPUs
> will see different page. It's not a concern at all.
> bpf prog is buggy, but the kernel will continue to work without a glitch.
> bpf runtime will disable and unload misbehaving prog.

Having one page table walker overwrite a scratch page on race is just rather ...
questionable locking design, that just screams for problems long-term, really.

At least in apply_range_clear_cb() one could similarly switch to
ptep_try_install() to at least have both these paths handle races in a
reasonable way. (having to handle when ptep_try_install() is not really implemented)


Anyhow, the documentation of ptep_try_install() must clearly spell out that this
must be used very carefully, and only in special kernel page tables, never user
page tables. There are likely other scenarios we should document (caller must
prevent concurrent page table teardown somehow, and must be prepared to handle
races if other code is not using atomics).

To highlight that, we should likely consider adding a "kernel" in the name, like
"ptep_try_install_kernel()".

I am also not sure if "install" is the right terminology and whether it should
instead be "ptep_try_set()". (set_pte_at is the non-atomic interface right now)


Further note that last time I talked to Linus about arch helpers, he preferred

#define ptep_try_install ptep_try_install

over __HAVE_ARCH_PTEP_TRY_INSTALL

-- 
Cheers,

David