Re: [PATCH 2/2] mm/page_alloc: rename FPI_TRYLOCK -> FPI_NOLOCK

["Vlastimil Babka (SUSE)" <vbabka@kernel.org>]

On 7/13/26 15:30, Brendan Jackman wrote:
> On Fri Jul 10, 2026 at 2:14 PM UTC, Zi Yan wrote:
>> On Fri Jul 10, 2026 at 8:40 AM EDT, Vlastimil Babka (SUSE) wrote:
>>> On 7/10/26 12:42, Brendan Jackman wrote:
>>>> As discussed in the linked patch, the there is some inconsistency between
>>>> "trylock" and "nolock" nomenclature, let's align it. Since "nolock" is
>>>> used in the public API it seems to have more mindshare so do that.
>>>> 
>>>> The linked patch did this for the ALLOC_ flag but forgot about FPI_.
>>>> 
>>>> Link: https://lore.kernel.org/all/20260703-alloc-trylock-v5-1-c87b714e19d3@google.com/
>>>> Signed-off-by: Brendan Jackman <jackmanb@google.com>
>>>
>>> Naming things is hard. Maybe it should have all been called "nospin". I
>>> don't know anymore :)
>>> _nolock() functions and ALLOC_NOLOCK are part of API, FPI_ is internal so
>>> it's not that urgent. Furthermore:
>>
>> I had a similar concern when reading ALLOC_TRYLOCK -> ALLOC_NOLOCK[1],
>> since the name is _NOLOCK, but the comment says spin_trylock.
> 
> Yeah I do actually think "nolock" is a bad name here, it takes a lock.
> But I never cared very much about the _bad_ ame. On the other hand
> _inconsistent_ naming is a concrete problem IMO.
> 
>> I agree that "nospin" is better and less confusing. But whether we want
>> to churn it again, TBD. :)
>>
>> [1] https://lore.kernel.org/all/DJOZN5H048UX.1ZVSFD48QZN05@nvidia.com/
> 
> Yeah I also dunno what's best here. I guess this is a decision for Vlastimil?

Let's keep the FPI_NOLOCK rename then, but separately from the fix.

> 
> IMO can_spin_trylock() is matched with spin_trlock() while FPI_NOLOCK is
> matched with ALLOC_NOLOCK which is matched with alloc_pages_nolock().
> 
> If you like, we could just drop the ALLOC_ and FPI_ renames and just

ALLOC_NOLOCK is better to me than ALLOC_TRYLOCK.

> rename alloc_pages_nolock(). I steered away from that because "the
> latter is public API", but... it's not like it would be a huge treewide
> patch, it only has one user.