Re: [PATCH] mm/huge_memory: let special huge VMAs bypass the THP policy check

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

>>
>> +static bool file_vma_honors_thp_toggles(struct vm_area_struct *vma,> +       
>> enum tva_type type)
>> +{
>> +    const bool forced_collapse = type == TVA_FORCED_COLLAPSE;
>> +
>> +    if (forced_collapse)
>> +        return false;
>> +    /* Huge PFN mappings allocate no folios so the policy doesn't apply. */
> 
> So, may be rename the routine to file_vma_thp_policy_applies() ?

Also a possibility. I'll let Lorenzo handle this.

> 
>> +    return !(vma_test(vma, VMA_PFNMAP_BIT) && vma->vm_ops->huge_fault);
>> +}
>> +
>> +static bool vma_thp_toggles_enabled(struct vm_area_struct *vma,
> 
> This routine could use the same 'file_vma_' prefix.

It's true even for non-file vmas.

> 
>> +        vm_flags_t vm_flags)
>> +{
>> +    /* THP=always? */
>> +    if (hugepage_global_always())
>> +        return true;
>> +    /* THP=madvise and actually advised? */
>> +    return hugepage_global_enabled() && vm_flags & VM_HUGEPAGE;
> 
> I would add extra parentheses around 'vm_flags & VM_HUGEPAGE'

It's even required and the compiler might complain :)

> 
>> +}
>> +
>> +static bool file_vma_forces_order_0(struct vm_area_struct *vma,
>> +        vm_flags_t vm_flags, enum tva_type type)
>> +{
>> +    const bool in_pf = type == TVA_PAGEFAULT;
>> +    const bool smaps = type == TVA_SMAPS;
>> +
>> +    /*
>> +     * Enforce THP collapse requirements as necessary. Anonymous vmas
>> +     * were already handled in thp_vma_allowable_orders().
>> +     */
>> +
>> +    if (file_vma_honors_thp_toggles(vma, type) &&
>> +        !vma_thp_toggles_enabled(vma, vm_flags))
>> +        return true;
>> +
>> +    /*
>> +     * Trust that ->huge_fault() handlers know what they are doing
>> +     * in fault path.
>> +     */
>> +    if (((in_pf || smaps)) && vma->vm_ops->huge_fault)
> 
> and there remove the extra parentheses.

Agreed, that was already there in existing code.

Anyhow, it was mostly for inspiration for Lorenzo. I should start spelling out
more clearly in the future that this is just a quick prototype, because recently
people assumed that some quick stuff I post inline would be complete or even
tested :)

-- 
Cheers,

David