2022-04-11 02:29:23

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v10 10/13] powerpc/mm: Move get_unmapped_area functions to slice.c

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().

Signed-off-by: Christophe Leroy <[email protected]>
---
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:
--
2.35.1


2022-05-23 12:28:38

by Laurent Dufour

[permalink] [raw]
Subject: Re: [PATCH v10 10/13] powerpc/mm: Move get_unmapped_area functions to slice.c

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'm wondering if these functions have to be moved in slice.c

Attached is the config file I used.

> Signed-off-by: Christophe Leroy <[email protected]>
> ---
> 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:


Attachments:
config (104.94 kB)

2022-05-23 15:18:28

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH v10 10/13] powerpc/mm: Move get_unmapped_area functions to slice.c



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:

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 <[email protected]>
>> ---
>> 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:

2022-05-23 16:45:08

by Laurent Dufour

[permalink] [raw]
Subject: Re: [PATCH v10 10/13] powerpc/mm: Move get_unmapped_area functions to slice.c

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 <[email protected]>
>>> ---
>>> 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: