Re: [PATCH v3 14/15] mm/mempolicy: Add alloc_frozen_pages()

[David Hildenbrand <david@redhat.com>]

On 04.12.24 12:34, Vlastimil Babka wrote:
> On 11/29/24 18:18, David Hildenbrand wrote:
>> On 29.11.24 17:29, Zi Yan wrote:
>>> On 29 Nov 2024, at 9:44, David Hildenbrand wrote:
>>>
>>>> On 25.11.24 22:01, Matthew Wilcox (Oracle) wrote:
>>>>
>>>> What I don't quite like is that we now have a bit of an inconsistency, that makes it harder to understand what gives you frozen and what doesn't give you frozen pages.
>>>>
>>>> alloc_pages(): non-frozen/refcounted
>>>> alloc_frozen_pages(): frozen
>>>> folio_alloc_mpol_noprof(): non-frozen/refcounted
>>>> alloc_pages_mpol(): ... frozen pages?
>>>>
>>>>
>>>> Ideally, it would be "alloc_pages": non-frozen, "alloc_frozen_pages": frozen.
> 
> I don't mind it that much as long as it concerns only static functions. We
> didn't add _frozen to all those in mm/page_alloc.c either.
 > > The exported ones seem consistent, AFAICS?

alloc_frozen / free_frozen even only exists in mm/internal.h

So at least to the outside world it is consistent.

> 
>>>> The same concern applies to other internal functions, like "__alloc_pages_cpuset_fallback".
>>>
>>> Or all internal functions (with "__" prefix) give frozen pages and external
>>> ones give non-frozen ones, unless with frozen in its name, i.e., alloc_frozen_pages()?
>>
>> If we go that path, we should make it consistent for __free_pages() as
>> well I am afraid.
> 
> That one seems to have many callers so not worth the churn.

Agreed.

-- 
Cheers,

David / dhildenb