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

"Vlastimil Babka (SUSE)" <[email protected]>
Newsgroups dev.linux.lists.linux-rt-devel,org.kernel.vger.linux-kernel,org.kvack.linux-mm
Message-ID <[email protected]>
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/[email protected]/
>>>> Signed-off-by: Brendan Jackman <[email protected]>
>>>
>>> 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/[email protected]/
> 
> 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.
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.