2017-04-17 17:14:41

by Aneesh Kumar K.V

[permalink] [raw]
Subject: [PATCH] powerpc/mm/hugetlb: Add support for 1G huge pages

POWER9 supports hugepages of size 2M and 1G in radix MMU mode. This patch
enables the usage of 1G page size for hugetlbfs. This also update the helper
such we can do 1G page allocation at runtime.

Since we can do this only when radix translation mode is enabled, we can't use
the generic gigantic_page_supported helper. Hence provide a way for architecture
to override gigantic_page_supported helper.

We still don't enable 1G page size on DD1 version. This is to avoid doing
workaround mentioned in commit: 6d3a0379ebdc8 (powerpc/mm: Add
radix__tlb_flush_pte_p9_dd1()

Signed-off-by: Aneesh Kumar K.V <[email protected]>
---
arch/powerpc/include/asm/book3s/64/hugetlb.h | 13 +++++++++++++
arch/powerpc/mm/hugetlbpage.c | 7 +++++--
arch/powerpc/platforms/Kconfig.cputype | 1 +
mm/hugetlb.c | 4 ++++
4 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/book3s/64/hugetlb.h b/arch/powerpc/include/asm/book3s/64/hugetlb.h
index 6666cd366596..86f27cc8ec61 100644
--- a/arch/powerpc/include/asm/book3s/64/hugetlb.h
+++ b/arch/powerpc/include/asm/book3s/64/hugetlb.h
@@ -50,4 +50,17 @@ static inline pte_t arch_make_huge_pte(pte_t entry, struct vm_area_struct *vma,
else
return entry;
}
+
+#if defined(CONFIG_ARCH_HAS_GIGANTIC_PAGE) && \
+ ((defined(CONFIG_MEMORY_ISOLATION) && defined(CONFIG_COMPACTION)) || \
+ defined(CONFIG_CMA))
+#define gigantic_page_supported gigantic_page_supported
+static inline bool gigantic_page_supported(void)
+{
+ if (radix_enabled())
+ return true;
+ return false;
+}
+#endif
+
#endif
diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
index a4f33de4008e..80f6d2ed551a 100644
--- a/arch/powerpc/mm/hugetlbpage.c
+++ b/arch/powerpc/mm/hugetlbpage.c
@@ -763,8 +763,11 @@ static int __init add_huge_page_size(unsigned long long size)
* Hash: 16M and 16G
*/
if (radix_enabled()) {
- if (mmu_psize != MMU_PAGE_2M)
- return -EINVAL;
+ if (mmu_psize != MMU_PAGE_2M) {
+ if (cpu_has_feature(CPU_FTR_POWER9_DD1) ||
+ (mmu_psize != MMU_PAGE_1G))
+ return -EINVAL;
+ }
} else {
if (mmu_psize != MMU_PAGE_16M && mmu_psize != MMU_PAGE_16G)
return -EINVAL;
diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
index ef4c4b8fc547..f4ba4bf0d762 100644
--- a/arch/powerpc/platforms/Kconfig.cputype
+++ b/arch/powerpc/platforms/Kconfig.cputype
@@ -343,6 +343,7 @@ config PPC_STD_MMU_64
config PPC_RADIX_MMU
bool "Radix MMU Support"
depends on PPC_BOOK3S_64
+ select ARCH_HAS_GIGANTIC_PAGE
default y
help
Enable support for the Power ISA 3.0 Radix style MMU. Currently this
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 3d0aab9ee80d..2c090189f314 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -1158,7 +1158,11 @@ static int alloc_fresh_gigantic_page(struct hstate *h,
return 0;
}

+#ifndef gigantic_page_supported
static inline bool gigantic_page_supported(void) { return true; }
+#define gigantic_page_supported gigantic_page_supported
+#endif
+
#else
static inline bool gigantic_page_supported(void) { return false; }
static inline void free_gigantic_page(struct page *page, unsigned int order) { }
--
2.7.4


2017-04-26 13:50:04

by Anshuman Khandual

[permalink] [raw]
Subject: Re: [PATCH] powerpc/mm/hugetlb: Add support for 1G huge pages

On 04/17/2017 10:44 PM, Aneesh Kumar K.V wrote:
> POWER9 supports hugepages of size 2M and 1G in radix MMU mode. This patch
> enables the usage of 1G page size for hugetlbfs. This also update the helper
> such we can do 1G page allocation at runtime.
>
> Since we can do this only when radix translation mode is enabled, we can't use
> the generic gigantic_page_supported helper. Hence provide a way for architecture
> to override gigantic_page_supported helper.
>
> We still don't enable 1G page size on DD1 version. This is to avoid doing
> workaround mentioned in commit: 6d3a0379ebdc8 (powerpc/mm: Add
> radix__tlb_flush_pte_p9_dd1()
>
> Signed-off-by: Aneesh Kumar K.V <[email protected]>
> ---
> arch/powerpc/include/asm/book3s/64/hugetlb.h | 13 +++++++++++++
> arch/powerpc/mm/hugetlbpage.c | 7 +++++--
> arch/powerpc/platforms/Kconfig.cputype | 1 +
> mm/hugetlb.c | 4 ++++
> 4 files changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/book3s/64/hugetlb.h b/arch/powerpc/include/asm/book3s/64/hugetlb.h
> index 6666cd366596..86f27cc8ec61 100644
> --- a/arch/powerpc/include/asm/book3s/64/hugetlb.h
> +++ b/arch/powerpc/include/asm/book3s/64/hugetlb.h
> @@ -50,4 +50,17 @@ static inline pte_t arch_make_huge_pte(pte_t entry, struct vm_area_struct *vma,
> else
> return entry;
> }
> +
> +#if defined(CONFIG_ARCH_HAS_GIGANTIC_PAGE) && \
> + ((defined(CONFIG_MEMORY_ISOLATION) && defined(CONFIG_COMPACTION)) || \
> + defined(CONFIG_CMA))
> +#define gigantic_page_supported gigantic_page_supported

As I have mentioned in later part of the reply, it does not really
make sense to have both arch call back as well as generic config
option checking to decide on whether a feature is enabled or not.

> +static inline bool gigantic_page_supported(void)
> +{
> + if (radix_enabled())
> + return true;
> + return false;
> +}
> +#endif
> +
> #endif
> diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
> index a4f33de4008e..80f6d2ed551a 100644
> --- a/arch/powerpc/mm/hugetlbpage.c
> +++ b/arch/powerpc/mm/hugetlbpage.c
> @@ -763,8 +763,11 @@ static int __init add_huge_page_size(unsigned long long size)
> * Hash: 16M and 16G
> */
> if (radix_enabled()) {
> - if (mmu_psize != MMU_PAGE_2M)
> - return -EINVAL;
> + if (mmu_psize != MMU_PAGE_2M) {
> + if (cpu_has_feature(CPU_FTR_POWER9_DD1) ||
> + (mmu_psize != MMU_PAGE_1G))
> + return -EINVAL;
> + }
> } else {
> if (mmu_psize != MMU_PAGE_16M && mmu_psize != MMU_PAGE_16G)
> return -EINVAL;
> diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
> index ef4c4b8fc547..f4ba4bf0d762 100644
> --- a/arch/powerpc/platforms/Kconfig.cputype
> +++ b/arch/powerpc/platforms/Kconfig.cputype
> @@ -343,6 +343,7 @@ config PPC_STD_MMU_64
> config PPC_RADIX_MMU
> bool "Radix MMU Support"
> depends on PPC_BOOK3S_64
> + select ARCH_HAS_GIGANTIC_PAGE
> default y
> help

As we are already checking for radix_enabled() test inside function
gigantic_page_supported(), do we still need to conditionally enable
this on Radix based platforms only ?


> Enable support for the Power ISA 3.0 Radix style MMU. Currently this
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 3d0aab9ee80d..2c090189f314 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -1158,7 +1158,11 @@ static int alloc_fresh_gigantic_page(struct hstate *h,
> return 0;
> }
>
> +#ifndef gigantic_page_supported
> static inline bool gigantic_page_supported(void) { return true; }
> +#define gigantic_page_supported gigantic_page_supported
> +#endif

As seen above, now that arch's decision to support this feature is not
based solely on ARCH_HAS_GIGANTIC_PAGE config option but also on the
availability of platform features like radix, is it a good time to have
an arch call back deciding on gigantic_page_supported() test instead of
just checking presence of config options like

#if defined(CONFIG_ARCH_HAS_GIGANTIC_PAGE) && \
((defined(CONFIG_MEMORY_ISOLATION) && defined(CONFIG_COMPACTION)) || \
defined(CONFIG_CMA))

We should not have both as proposed. I mean CONFIG_ARCH_HAS_GIGANTIC_PAGE
should not be enabled unless we have MEMORY_ISOLATION && COMPACTION && CMA
and once enabled we should have arch_gigantic_page_supported() deciding for
gigantic_page_supported().

2017-04-27 02:52:43

by Aneesh Kumar K.V

[permalink] [raw]
Subject: Re: [PATCH] powerpc/mm/hugetlb: Add support for 1G huge pages

Anshuman Khandual <[email protected]> writes:

> On 04/17/2017 10:44 PM, Aneesh Kumar K.V wrote:
>> POWER9 supports hugepages of size 2M and 1G in radix MMU mode. This patch
>> enables the usage of 1G page size for hugetlbfs. This also update the helper
>> such we can do 1G page allocation at runtime.
>>
>> Since we can do this only when radix translation mode is enabled, we can't use
>> the generic gigantic_page_supported helper. Hence provide a way for architecture
>> to override gigantic_page_supported helper.
>>
>> We still don't enable 1G page size on DD1 version. This is to avoid doing
>> workaround mentioned in commit: 6d3a0379ebdc8 (powerpc/mm: Add
>> radix__tlb_flush_pte_p9_dd1()
>>
>> Signed-off-by: Aneesh Kumar K.V <[email protected]>
>> ---
>> arch/powerpc/include/asm/book3s/64/hugetlb.h | 13 +++++++++++++
>> arch/powerpc/mm/hugetlbpage.c | 7 +++++--
>> arch/powerpc/platforms/Kconfig.cputype | 1 +
>> mm/hugetlb.c | 4 ++++
>> 4 files changed, 23 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/book3s/64/hugetlb.h b/arch/powerpc/include/asm/book3s/64/hugetlb.h
>> index 6666cd366596..86f27cc8ec61 100644
>> --- a/arch/powerpc/include/asm/book3s/64/hugetlb.h
>> +++ b/arch/powerpc/include/asm/book3s/64/hugetlb.h
>> @@ -50,4 +50,17 @@ static inline pte_t arch_make_huge_pte(pte_t entry, struct vm_area_struct *vma,
>> else
>> return entry;
>> }
>> +
>> +#if defined(CONFIG_ARCH_HAS_GIGANTIC_PAGE) && \
>> + ((defined(CONFIG_MEMORY_ISOLATION) && defined(CONFIG_COMPACTION)) || \
>> + defined(CONFIG_CMA))
>> +#define gigantic_page_supported gigantic_page_supported
>
> As I have mentioned in later part of the reply, it does not really
> make sense to have both arch call back as well as generic config
> option checking to decide on whether a feature is enabled or not.
>
>> +static inline bool gigantic_page_supported(void)
>> +{
>> + if (radix_enabled())
>> + return true;
>> + return false;
>> +}
>> +#endif
>> +
>> #endif
>> diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
>> index a4f33de4008e..80f6d2ed551a 100644
>> --- a/arch/powerpc/mm/hugetlbpage.c
>> +++ b/arch/powerpc/mm/hugetlbpage.c
>> @@ -763,8 +763,11 @@ static int __init add_huge_page_size(unsigned long long size)
>> * Hash: 16M and 16G
>> */
>> if (radix_enabled()) {
>> - if (mmu_psize != MMU_PAGE_2M)
>> - return -EINVAL;
>> + if (mmu_psize != MMU_PAGE_2M) {
>> + if (cpu_has_feature(CPU_FTR_POWER9_DD1) ||
>> + (mmu_psize != MMU_PAGE_1G))
>> + return -EINVAL;
>> + }
>> } else {
>> if (mmu_psize != MMU_PAGE_16M && mmu_psize != MMU_PAGE_16G)
>> return -EINVAL;
>> diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
>> index ef4c4b8fc547..f4ba4bf0d762 100644
>> --- a/arch/powerpc/platforms/Kconfig.cputype
>> +++ b/arch/powerpc/platforms/Kconfig.cputype
>> @@ -343,6 +343,7 @@ config PPC_STD_MMU_64
>> config PPC_RADIX_MMU
>> bool "Radix MMU Support"
>> depends on PPC_BOOK3S_64
>> + select ARCH_HAS_GIGANTIC_PAGE
>> default y
>> help
>
> As we are already checking for radix_enabled() test inside function
> gigantic_page_supported(), do we still need to conditionally enable
> this on Radix based platforms only ?
>
>
>> Enable support for the Power ISA 3.0 Radix style MMU. Currently this
>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>> index 3d0aab9ee80d..2c090189f314 100644
>> --- a/mm/hugetlb.c
>> +++ b/mm/hugetlb.c
>> @@ -1158,7 +1158,11 @@ static int alloc_fresh_gigantic_page(struct hstate *h,
>> return 0;
>> }
>>
>> +#ifndef gigantic_page_supported
>> static inline bool gigantic_page_supported(void) { return true; }
>> +#define gigantic_page_supported gigantic_page_supported
>> +#endif
>
> As seen above, now that arch's decision to support this feature is not
> based solely on ARCH_HAS_GIGANTIC_PAGE config option but also on the
> availability of platform features like radix, is it a good time to have
> an arch call back deciding on gigantic_page_supported() test instead of
> just checking presence of config options like
>
> #if defined(CONFIG_ARCH_HAS_GIGANTIC_PAGE) && \
> ((defined(CONFIG_MEMORY_ISOLATION) && defined(CONFIG_COMPACTION)) || \
> defined(CONFIG_CMA))
>
> We should not have both as proposed. I mean CONFIG_ARCH_HAS_GIGANTIC_PAGE
> should not be enabled unless we have MEMORY_ISOLATION && COMPACTION && CMA
> and once enabled we should have arch_gigantic_page_supported() deciding for
> gigantic_page_supported().

I will update the patch. I guess I can also fixup other arch that enable
GIGANTIC_PAGE accordingly.

-aneesh