Re: [PATCH v3 4/6] mm/hugetlb: Allow architectures to override huge_pte_clear()

[Punit Agrawal]

Arnd Bergmann <arnd@xxxxxxxx> writes:

> On Mon, May 22, 2017 at 3:36 PM, Punit Agrawal <punit.agrawal@xxxxxxx> wrote:
>> diff --git a/include/asm-generic/hugetlb.h b/include/asm-generic/hugetlb.h
>> index 99b490b4d05a..3138e126f43b 100644
>> --- a/include/asm-generic/hugetlb.h
>> +++ b/include/asm-generic/hugetlb.h
>> @@ -31,10 +31,7 @@ static inline pte_t huge_pte_modify(pte_t pte, pgprot_t newprot)
>>         return pte_modify(pte, newprot);
>>  }
>>
>> -static inline void huge_pte_clear(struct mm_struct *mm, unsigned long addr,
>> -                                 pte_t *ptep)
>> -{
>> -       pte_clear(mm, addr, ptep);
>> -}
>> +void huge_pte_clear(struct mm_struct *mm, unsigned long addr,
>> +                   pte_t *ptep, unsigned long sz);
>>
>>  #endif /* _ASM_GENERIC_HUGETLB_H */
>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>> index 0e4d1fb3122f..2b0f6f96f2c1 100644
>> --- a/mm/hugetlb.c
>> +++ b/mm/hugetlb.c
>> @@ -3289,6 +3289,12 @@ int copy_hugetlb_page_range(struct mm_struct *dst, struct mm_struct *src,
>>         return ret;
>>  }
>>
>> +void __weak huge_pte_clear(struct mm_struct *mm, unsigned long addr,
>> +                          pte_t *ptep, unsigned long sz)
>> +{
>> +       pte_clear(mm, addr, ptep);
>> +}
>> +
>>  void __unmap_hugepage_range(struct mmu_gather *tlb, struct vm_area_struct *vma,
>>                             unsigned long start, unsigned long end,
>>                             struct page *ref_page)
>
> I don't really like how this moves the inline version from asm-generic into
> a __weak function here. I think it would be better to either stop
> using asm-generic/hugetlb.h
> on s390, or enclose the generic definition in
>
> #ifndef huge_pte_clear
>
> and then override by defining a macro in s390 as we do in other files
> in asm-generic.

Nice! I wasn't aware asm-generic follows this as a standard pattern.

s390 doesn't use asm-generic, but I needed to update the prototype with
an additional parameter (size) and needlessly moved the function. I'll
update the patch.

The changes is needed to enable contiguous pte hugepage support on arm64
[0].

Thanks for taking a look.

Punit

[0] https://www.spinics.net/lists/arm-kernel/msg582758.html