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

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

>>
>> 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)
> 
> You mean to use ptep_get_and_clear() ?
> Makes sense to me.
Yes, using also an atomic to replace it.

I recall that ptep_get_and_clear() might not be atomic, but I guess it is on the
architectures where ptep_try_install() is currently 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)
> 
> I suggested using the ptep_try_set() name too :)
> 
>> 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
> 
> ok.
> I guess __HAVE_ARCH_PTEP_GET_AND_CLEAR is legacy ?

Yeah ... and we have plenty of legacy around :)

-- 
Cheers,

David