From: Laurent Dufour <ldufour@linux.ibm.com>
To: Christophe Leroy <christophe.leroy@csgroup.eu>,
Benjamin Herrenschmidt <benh@kernel.crashing.org>,
Paul Mackerras <paulus@samba.org>,
Michael Ellerman <mpe@ellerman.id.au>,
"akpm@linux-foundation.org" <akpm@linux-foundation.org>
Cc: "linux-mm@kvack.org" <linux-mm@kvack.org>,
"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v10 10/13] powerpc/mm: Move get_unmapped_area functions to slice.c
Date: Mon, 23 May 2022 18:44:25 +0200 [thread overview]
Message-ID: <d33ac27a-ac7f-4ffb-c22c-08ff9a29bbd5@linux.ibm.com> (raw)
In-Reply-To: <78b3a7f4-56e2-e11f-005c-2a1dd563b5b3@csgroup.eu>
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.leroy@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(¤t->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(¤t->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(¤t->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(¤t->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:
next prev parent reply other threads:[~2022-05-23 16:44 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-09 17:17 [PATCH v10 00/13] Convert powerpc to default topdown mmap layout (v10) Christophe Leroy
2022-04-09 17:17 ` [PATCH v10 01/13] mm, hugetlbfs: Allow for "high" userspace addresses Christophe Leroy
2022-05-03 16:29 ` Christophe Leroy
2022-04-09 17:17 ` [PATCH v10 02/13] mm: Allow arch specific arch_randomize_brk() with CONFIG_ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT Christophe Leroy
2022-04-09 17:17 ` [PATCH v10 03/13] mm, hugetlbfs: Allow an arch to always use generic versions of get_unmapped_area functions Christophe Leroy
2022-04-09 17:17 ` [PATCH v10 04/13] mm: Add len and flags parameters to arch_get_mmap_end() Christophe Leroy
2022-04-09 17:17 ` [PATCH v10 05/13] powerpc/mm: Move vma_mmu_pagesize() Christophe Leroy
2022-04-09 17:17 ` [PATCH v10 06/13] powerpc/mm: Make slice specific to book3s/64 Christophe Leroy
2022-04-09 17:17 ` [PATCH v10 07/13] powerpc/mm: Remove CONFIG_PPC_MM_SLICES Christophe Leroy
2022-04-09 17:17 ` [PATCH v10 08/13] powerpc/mm: Use generic_get_unmapped_area() and call it from arch_get_unmapped_area() Christophe Leroy
2022-04-09 17:17 ` [PATCH v10 09/13] powerpc/mm: Use generic_hugetlb_get_unmapped_area() Christophe Leroy
2022-04-09 17:17 ` [PATCH v10 10/13] powerpc/mm: Move get_unmapped_area functions to slice.c Christophe Leroy
2022-05-23 12:27 ` Laurent Dufour
2022-05-23 15:18 ` Christophe Leroy
2022-05-23 16:44 ` Laurent Dufour [this message]
2022-04-09 17:17 ` [PATCH v10 11/13] powerpc/mm: Enable full randomisation of memory mappings Christophe Leroy
2022-04-09 17:17 ` [PATCH v10 12/13] powerpc/mm: Convert to default topdown mmap layout Christophe Leroy
2022-04-09 17:17 ` [PATCH v10 13/13] powerpc: Simplify and move arch_randomize_brk() Christophe Leroy
2022-05-15 10:28 ` [PATCH v10 00/13] Convert powerpc to default topdown mmap layout (v10) Michael Ellerman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d33ac27a-ac7f-4ffb-c22c-08ff9a29bbd5@linux.ibm.com \
--to=ldufour@linux.ibm.com \
--cc=akpm@linux-foundation.org \
--cc=benh@kernel.crashing.org \
--cc=christophe.leroy@csgroup.eu \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mpe@ellerman.id.au \
--cc=paulus@samba.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).