On 2/2/22 11:50 AM, Christophe Leroy wrote:
>
> Le 02/02/2022 à 06:38, Anshuman Khandual a écrit :
>> Each call into pte_mkhuge() is invariably followed by arch_make_huge_pte().
>> Instead arch_make_huge_pte() can accommodate pte_mkhuge() at the beginning.
>> This updates generic fallback stub for arch_make_huge_pte() and available
>> platforms definitions. This makes huge pte creation much cleaner and easier
>> to follow.
> I think it is a good cleanup. I always wonder why commit d9ed9faac283
> ("mm: add new arch_make_huge_pte() method for tile support") didn't move
> the pte_mkhuge() into arch_make_huge_pte().
+1
>
> When I implemented arch_make_huge_pte() for powerpc 8xx, in one case
> arch_make_huge_pte() have to undo the things done by pte_mkhuge(), see below
>
> As a second step we could probably try to get rid of pte_mkhuge()
> completely, at least in the core.
Sure.
>
>> Cc: Catalin Marinas <catalin.marinas@xxxxxxx>
>> Cc: Will Deacon <will@xxxxxxxxxx>
>> Cc: Michael Ellerman <mpe@xxxxxxxxxxxxxx>
>> Cc: Paul Mackerras <paulus@xxxxxxxxx>
>> Cc: "David S. Miller" <davem@xxxxxxxxxxxxx>
>> Cc: Mike Kravetz <mike.kravetz@xxxxxxxxxx>
>> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
>> Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
>> Cc: linuxppc-dev@xxxxxxxxxxxxxxxx
>> Cc: sparclinux@xxxxxxxxxxxxxxx
>> Cc: linux-mm@xxxxxxxxx
>> Cc: linux-kernel@xxxxxxxxxxxxxxx
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@xxxxxxx>
> Reviewed-by: Christophe Leroy <christophe.leroy@xxxxxxxxxx>
>
>> ---
>> arch/arm64/mm/hugetlbpage.c | 1 +
>> arch/powerpc/include/asm/nohash/32/hugetlb-8xx.h | 1 +
>> arch/sparc/mm/hugetlbpage.c | 1 +
>> include/linux/hugetlb.h | 2 +-
>> mm/hugetlb.c | 3 +--
>> mm/vmalloc.c | 1 -
>> 6 files changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
>> index ffb9c229610a..228226c5fa80 100644
>> --- a/arch/arm64/mm/hugetlbpage.c
>> +++ b/arch/arm64/mm/hugetlbpage.c
>> @@ -347,6 +347,7 @@ pte_t arch_make_huge_pte(pte_t entry, unsigned int shift, vm_flags_t flags)
>> {
>> size_t pagesize = 1UL << shift;
>>
>> + entry = pte_mkhuge(entry);
>> if (pagesize == CONT_PTE_SIZE) {
>> entry = pte_mkcont(entry);
>> } else if (pagesize == CONT_PMD_SIZE) {
>> diff --git a/arch/powerpc/include/asm/nohash/32/hugetlb-8xx.h b/arch/powerpc/include/asm/nohash/32/hugetlb-8xx.h
>> index 64b6c608eca4..e41e095158c7 100644
>> --- a/arch/powerpc/include/asm/nohash/32/hugetlb-8xx.h
>> +++ b/arch/powerpc/include/asm/nohash/32/hugetlb-8xx.h
>> @@ -70,6 +70,7 @@ static inline pte_t arch_make_huge_pte(pte_t entry, unsigned int shift, vm_flags
>> {
>> size_t size = 1UL << shift;
>>
>> + entry = pte_mkhuge(entry);
> Could drop that and replace the below by:
>
> if (size == SZ_16K)
> return __pte(pte_val(entry) | _PAGE_SPS);
> else
> return __pte(pte_val(entry) | _PAGE_SPS | _PAGE_HUGE);
>
>
Sure, will change as stated above.
