On 23/05/2022, 17:18:10, Christophe Leroy wrote:
> 
> 
> Le 23/05/2022 à 14:27, Laurent Dufour a écrit :
>> On 09/04/2022, 19:17:34, Christophe Leroy wrote:
>>> hugetlb_get_unmapped_area() is now identical to the
>>> generic version if only RADIX is enabled, so move it
>>> to slice.c and let it fallback on the generic one
>>> when HASH MMU is not compiled in.
>>>
>>> Do the same with arch_get_unmapped_area() and
>>> arch_get_unmapped_area_topdown().
>>
>> This breaks the build when CONFIG_PPC_64S_HASH_MMU is not set.
>>
>> The root cause is that arch/powerpc/mm/book3s64/slice.c is not built if
>> !CONFIG_PPC_64S_HASH_MMU.
>> The commit f693d38d9468 (powerpc/mm: Remove CONFIG_PPC_MM_SLICES,
>> 2022-04-09) builds slice.c only if CONFIG_PPC_64S_HASH_MMU.
> 
> I think the root cause is slice.h is being included allthough 
> !CONFIG_PPC_64S_HASH_MMU, which leads to HAVE_ARCH_UNMAPPED_AREA and 
> HAVE_ARCH_UNMAPPED_AREA_TOPDOWN being defined while they shouldn't
> 
> Wondering why I didn't see that before.
> 
> slice.h gets included via asm/book3s/64/mmu-hash.h
> 
> I was able to build microwatt_defconfig with the following changes:

That works for me too.

> 
> diff --git a/arch/powerpc/include/asm/book3s/64/slice.h 
> b/arch/powerpc/include/asm/book3s/64/slice.h
> index b8eb4ad271b9..5fbe18544cbd 100644
> --- a/arch/powerpc/include/asm/book3s/64/slice.h
> +++ b/arch/powerpc/include/asm/book3s/64/slice.h
> @@ -4,11 +4,13 @@
> 
>   #ifndef __ASSEMBLY__
> 
> +#ifdef CONFIG_PPC_64S_HASH_MMU
>   #ifdef CONFIG_HUGETLB_PAGE
>   #define HAVE_ARCH_HUGETLB_UNMAPPED_AREA
>   #endif
>   #define HAVE_ARCH_UNMAPPED_AREA
>   #define HAVE_ARCH_UNMAPPED_AREA_TOPDOWN
> +#endif
> 
>   #define SLICE_LOW_SHIFT             28
>   #define SLICE_LOW_TOP               (0x100000000ul)
> diff --git a/arch/powerpc/sysdev/xics/ics-native.c 
> b/arch/powerpc/sysdev/xics/ics-native.c
> index e33b77da861e..112c8a1e8159 100644
> --- a/arch/powerpc/sysdev/xics/ics-native.c
> +++ b/arch/powerpc/sysdev/xics/ics-native.c
> @@ -15,6 +15,7 @@
>   #include <linux/init.h>
>   #include <linux/cpu.h>
>   #include <linux/of.h>
> +#include <linux/of_address.h>
>   #include <linux/spinlock.h>
>   #include <linux/msi.h>
>   #include <linux/list.h>
> 
> 
>>
>> I'm wondering if these functions have to be moved in slice.c
>>
>> Attached is the config file I used.
>>
>>> Signed-off-by: Christophe Leroy <christophe.le...@csgroup.eu>
>>> ---
>>>   arch/powerpc/include/asm/book3s/64/mmu.h   |  6 ----
>>>   arch/powerpc/include/asm/book3s/64/slice.h |  6 ++++
>>>   arch/powerpc/mm/book3s64/slice.c           | 42 ++++++++++++++++++++++
>>>   arch/powerpc/mm/hugetlbpage.c              | 21 -----------
>>>   arch/powerpc/mm/mmap.c                     | 36 -------------------
>>>   5 files changed, 48 insertions(+), 63 deletions(-)
>>>
>>> diff --git a/arch/powerpc/include/asm/book3s/64/mmu.h 
>>> b/arch/powerpc/include/asm/book3s/64/mmu.h
>>> index 006cbec70ffe..570a4960cf17 100644
>>> --- a/arch/powerpc/include/asm/book3s/64/mmu.h
>>> +++ b/arch/powerpc/include/asm/book3s/64/mmu.h
>>> @@ -4,12 +4,6 @@
>>>   
>>>   #include <asm/page.h>
>>>   
>>> -#ifdef CONFIG_HUGETLB_PAGE
>>> -#define HAVE_ARCH_HUGETLB_UNMAPPED_AREA
>>> -#endif
>>> -#define HAVE_ARCH_UNMAPPED_AREA
>>> -#define HAVE_ARCH_UNMAPPED_AREA_TOPDOWN
>>> -
>>>   #ifndef __ASSEMBLY__
>>>   /*
>>>    * Page size definition
>>> diff --git a/arch/powerpc/include/asm/book3s/64/slice.h 
>>> b/arch/powerpc/include/asm/book3s/64/slice.h
>>> index 5b0f7105bc8b..b8eb4ad271b9 100644
>>> --- a/arch/powerpc/include/asm/book3s/64/slice.h
>>> +++ b/arch/powerpc/include/asm/book3s/64/slice.h
>>> @@ -4,6 +4,12 @@
>>>   
>>>   #ifndef __ASSEMBLY__
>>>   
>>> +#ifdef CONFIG_HUGETLB_PAGE
>>> +#define HAVE_ARCH_HUGETLB_UNMAPPED_AREA
>>> +#endif
>>> +#define HAVE_ARCH_UNMAPPED_AREA
>>> +#define HAVE_ARCH_UNMAPPED_AREA_TOPDOWN
>>> +
>>>   #define SLICE_LOW_SHIFT           28
>>>   #define SLICE_LOW_TOP             (0x100000000ul)
>>>   #define SLICE_NUM_LOW             (SLICE_LOW_TOP >> SLICE_LOW_SHIFT)
>>> diff --git a/arch/powerpc/mm/book3s64/slice.c 
>>> b/arch/powerpc/mm/book3s64/slice.c
>>> index e4382713746d..03681042b807 100644
>>> --- a/arch/powerpc/mm/book3s64/slice.c
>>> +++ b/arch/powerpc/mm/book3s64/slice.c
>>> @@ -639,6 +639,32 @@ unsigned long slice_get_unmapped_area(unsigned long 
>>> addr, unsigned long len,
>>>   }
>>>   EXPORT_SYMBOL_GPL(slice_get_unmapped_area);
>>>   
>>> +unsigned long arch_get_unmapped_area(struct file *filp,
>>> +                                unsigned long addr,
>>> +                                unsigned long len,
>>> +                                unsigned long pgoff,
>>> +                                unsigned long flags)
>>> +{
>>> +   if (radix_enabled())
>>> +           return generic_get_unmapped_area(filp, addr, len, pgoff, flags);
>>> +
>>> +   return slice_get_unmapped_area(addr, len, flags,
>>> +                                  
>>> mm_ctx_user_psize(&current->mm->context), 0);
>>> +}
>>> +
>>> +unsigned long arch_get_unmapped_area_topdown(struct file *filp,
>>> +                                        const unsigned long addr0,
>>> +                                        const unsigned long len,
>>> +                                        const unsigned long pgoff,
>>> +                                        const unsigned long flags)
>>> +{
>>> +   if (radix_enabled())
>>> +           return generic_get_unmapped_area_topdown(filp, addr0, len, 
>>> pgoff, flags);
>>> +
>>> +   return slice_get_unmapped_area(addr0, len, flags,
>>> +                                  
>>> mm_ctx_user_psize(&current->mm->context), 1);
>>> +}
>>> +
>>>   unsigned int notrace get_slice_psize(struct mm_struct *mm, unsigned long 
>>> addr)
>>>   {
>>>     unsigned char *psizes;
>>> @@ -766,4 +792,20 @@ unsigned long vma_mmu_pagesize(struct vm_area_struct 
>>> *vma)
>>>   
>>>     return 1UL << mmu_psize_to_shift(get_slice_psize(vma->vm_mm, 
>>> vma->vm_start));
>>>   }
>>> +
>>> +static int file_to_psize(struct file *file)
>>> +{
>>> +   struct hstate *hstate = hstate_file(file);
>>> +   return shift_to_mmu_psize(huge_page_shift(hstate));
>>> +}
>>> +
>>> +unsigned long hugetlb_get_unmapped_area(struct file *file, unsigned long 
>>> addr,
>>> +                                   unsigned long len, unsigned long pgoff,
>>> +                                   unsigned long flags)
>>> +{
>>> +   if (radix_enabled())
>>> +           return generic_hugetlb_get_unmapped_area(file, addr, len, 
>>> pgoff, flags);
>>> +
>>> +   return slice_get_unmapped_area(addr, len, flags, file_to_psize(file), 
>>> 1);
>>> +}
>>>   #endif
>>> diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
>>> index a87c886042e9..b282af39fcf6 100644
>>> --- a/arch/powerpc/mm/hugetlbpage.c
>>> +++ b/arch/powerpc/mm/hugetlbpage.c
>>> @@ -542,27 +542,6 @@ struct page *follow_huge_pd(struct vm_area_struct *vma,
>>>     return page;
>>>   }
>>>   
>>> -#ifdef HAVE_ARCH_HUGETLB_UNMAPPED_AREA
>>> -static inline int file_to_psize(struct file *file)
>>> -{
>>> -   struct hstate *hstate = hstate_file(file);
>>> -   return shift_to_mmu_psize(huge_page_shift(hstate));
>>> -}
>>> -
>>> -unsigned long hugetlb_get_unmapped_area(struct file *file, unsigned long 
>>> addr,
>>> -                                   unsigned long len, unsigned long pgoff,
>>> -                                   unsigned long flags)
>>> -{
>>> -   if (radix_enabled())
>>> -           return generic_hugetlb_get_unmapped_area(file, addr, len,
>>> -                                                  pgoff, flags);
>>> -#ifdef CONFIG_PPC_64S_HASH_MMU
>>> -   return slice_get_unmapped_area(addr, len, flags, file_to_psize(file), 
>>> 1);
>>> -#endif
>>> -   BUG();
>>> -}
>>> -#endif
>>> -
>>>   bool __init arch_hugetlb_valid_size(unsigned long size)
>>>   {
>>>     int shift = __ffs(size);
>>> diff --git a/arch/powerpc/mm/mmap.c b/arch/powerpc/mm/mmap.c
>>> index 46781d0103d1..5972d619d274 100644
>>> --- a/arch/powerpc/mm/mmap.c
>>> +++ b/arch/powerpc/mm/mmap.c
>>> @@ -80,42 +80,6 @@ static inline unsigned long mmap_base(unsigned long rnd,
>>>     return PAGE_ALIGN(DEFAULT_MAP_WINDOW - gap - rnd);
>>>   }
>>>   
>>> -#ifdef HAVE_ARCH_UNMAPPED_AREA
>>> -unsigned long arch_get_unmapped_area(struct file *filp,
>>> -                                unsigned long addr,
>>> -                                unsigned long len,
>>> -                                unsigned long pgoff,
>>> -                                unsigned long flags)
>>> -{
>>> -   if (radix_enabled())
>>> -           return generic_get_unmapped_area(filp, addr, len, pgoff, flags);
>>> -
>>> -#ifdef CONFIG_PPC_64S_HASH_MMU
>>> -   return slice_get_unmapped_area(addr, len, flags,
>>> -                                  
>>> mm_ctx_user_psize(&current->mm->context), 0);
>>> -#else
>>> -   BUG();
>>> -#endif
>>> -}
>>> -
>>> -unsigned long arch_get_unmapped_area_topdown(struct file *filp,
>>> -                                        const unsigned long addr0,
>>> -                                        const unsigned long len,
>>> -                                        const unsigned long pgoff,
>>> -                                        const unsigned long flags)
>>> -{
>>> -   if (radix_enabled())
>>> -           return generic_get_unmapped_area_topdown(filp, addr0, len, 
>>> pgoff, flags);
>>> -
>>> -#ifdef CONFIG_PPC_64S_HASH_MMU
>>> -   return slice_get_unmapped_area(addr0, len, flags,
>>> -                                  
>>> mm_ctx_user_psize(&current->mm->context), 1);
>>> -#else
>>> -   BUG();
>>> -#endif
>>> -}
>>> -#endif /* HAVE_ARCH_UNMAPPED_AREA */
>>> -
>>>   /*
>>>    * This function, called very early during the creation of a new
>>>    * process VM image, sets up which VM layout function to use:

Reply via email to