Re: [PATCH] mm: set hugepage to false when anon mthp allocation

[David Hildenbrand <david@redhat.com>]

On 09.10.24 12:44, Ryan Roberts wrote:
> On 09/10/2024 10:15, Kefeng Wang wrote:
>>
>> On 2024/9/13 18:36, Kefeng Wang wrote:
>>> Hi All,
>>>
>>> On 2024/9/10 22:18, Kefeng Wang wrote:
>>>>
>>>>
>>>> On 2024/9/10 22:06, Kefeng Wang wrote:
>>>>> When the hugepage parameter is true in vma_alloc_folio(), it indicates
>>>>> that we only try allocation on preferred node if possible for PMD_ORDER,
>>>>
>>>> Should remove "for PMD_ORDER", I mean that it was used for PMD_ORDER, but for
>>>> other high-order, it will reduce the success rate of allocation if without
>>>> ddc1a5cbc05d.
>>>>
>>>>
>>>>> but it could lead to lots of failures for large folio allocation,
>>>>> luckily the hugepage parameter was deprecated since commit ddc1a5cbc05d
>>>>> ("mempolicy: alloc_pages_mpol() for NUMA policy without vma"), so no
>>>>> effect on runtime behavior.
>>>>>
>>>>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>>>>> ---
>>>>>
>>>>> Found the issue when backport mthp to inner kernel without ddc1a5cbc05d,
>>>>> but for mainline, there is no issue, no clue why hugepage parameter was
>>>>> retained, maybe just kill the parameter for mainline?
>>>
>>>
>>> Any comments, fix in alloc_anon_folio() or remove hugepage parameter in
>>> vma_alloc_folio(), thanks.
>>
>> * vma_alloc_folio - Allocate a folio for a VMA.
>> @hugepage: Unused (was: For hugepages try only preferred node if possible).
>>
>> Since hugepage won't be used in vma_alloc_folio(), maybe just delete this
>> parameter?
> 
> Sorry for the radio silence. Given the param is no longer used, I think it would
> be cleaner to just remove it.

Agreed, no dead code.

> 
> It was set to true here on purpose though; the aim was to follow the pattern set
> by PMD-sized THP, which also sets it to true. And the aargument was that the
> benefit of having a huge page would be outstripped by having to access it on a
> remote node.
> 
> Now that the parameter is deprecated, do you know if the policy is still
> enforced by other means?

Right, it might indicate a bug. So figuring out why there are no users 
left would be interesting. Maybe it was all on purpose.

-- 
Cheers,

David / dhildenb