linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] mm/page_alloc: Fix pageblock_order with HUGETLB_PAGE_SIZE_VARIABLE
@ 2021-02-11  6:22 Anshuman Khandual
  2021-02-11  6:22 ` [PATCH 1/3] mm/page_alloc: Fix pageblock_order when HUGETLB_PAGE_ORDER >= MAX_ORDER Anshuman Khandual
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Anshuman Khandual @ 2021-02-11  6:22 UTC (permalink / raw)
  To: linux-mm, linux-arm-kernel, catalin.marinas, akpm, will
  Cc: Anshuman Khandual, Robin Murphy, Marek Szyprowski,
	Christoph Hellwig, David Hildenbrand, Mark Rutland, iommu,
	linux-kernel

The following warning gets triggered while trying to boot a 64K page size
without THP config kernel on arm64 platform.

WARNING: CPU: 5 PID: 124 at mm/vmstat.c:1080 __fragmentation_index+0xa4/0xc0
Modules linked in:
CPU: 5 PID: 124 Comm: kswapd0 Not tainted 5.11.0-rc6-00004-ga0ea7d62002 #159
Hardware name: linux,dummy-virt (DT)
[    8.810673] pstate: 20400005 (nzCv daif +PAN -UAO -TCO BTYPE=--)
[    8.811732] pc : __fragmentation_index+0xa4/0xc0
[    8.812555] lr : fragmentation_index+0xf8/0x138
[    8.813360] sp : ffff0000864079b0
[    8.813958] x29: ffff0000864079b0 x28: 0000000000000372
[    8.814901] x27: 0000000000007682 x26: ffff8000135b3948
[    8.815847] x25: 1fffe00010c80f48 x24: 0000000000000000
[    8.816805] x23: 0000000000000000 x22: 000000000000000d
[    8.817764] x21: 0000000000000030 x20: ffff0005ffcb4d58
[    8.818712] x19: 000000000000000b x18: 0000000000000000
[    8.819656] x17: 0000000000000000 x16: 0000000000000000
[    8.820613] x15: 0000000000000000 x14: ffff8000114c6258
[    8.821560] x13: ffff6000bff969ba x12: 1fffe000bff969b9
[    8.822514] x11: 1fffe000bff969b9 x10: ffff6000bff969b9
[    8.823461] x9 : dfff800000000000 x8 : ffff0005ffcb4dcf
[    8.824415] x7 : 0000000000000001 x6 : 0000000041b58ab3
[    8.825359] x5 : ffff600010c80f48 x4 : dfff800000000000
[    8.826313] x3 : ffff8000102be670 x2 : 0000000000000007
[    8.827259] x1 : ffff000086407a60 x0 : 000000000000000d
[    8.828218] Call trace:
[    8.828667]  __fragmentation_index+0xa4/0xc0
[    8.829436]  fragmentation_index+0xf8/0x138
[    8.830194]  compaction_suitable+0x98/0xb8
[    8.830934]  wakeup_kcompactd+0xdc/0x128
[    8.831640]  balance_pgdat+0x71c/0x7a0
[    8.832327]  kswapd+0x31c/0x520
[    8.832902]  kthread+0x224/0x230
[    8.833491]  ret_from_fork+0x10/0x30
[    8.834150] ---[ end trace 472836f79c15516b ]---

This warning comes from __fragmentation_index() when the requested order
is greater than MAX_ORDER.

static int __fragmentation_index(unsigned int order,
				 struct contig_page_info *info)
{
        unsigned long requested = 1UL << order;

        if (WARN_ON_ONCE(order >= MAX_ORDER)) <===== Triggered here
                return 0;

Digging it further reveals that pageblock_order has been assigned a value
which is greater than MAX_ORDER failing the above check. But why this
happened ? Because HUGETLB_PAGE_ORDER for the given config on arm64 is
greater than MAX_ORDER.

The solution involves enabling HUGETLB_PAGE_SIZE_VARIABLE which would make
pageblock_order a variable instead of constant HUGETLB_PAGE_ORDER. But that
change alone also did not really work as pageblock_order still got assigned
as HUGETLB_PAGE_ORDER in set_pageblock_order(). HUGETLB_PAGE_ORDER needs to
be less than MAX_ORDER for its appropriateness as pageblock_order otherwise
just fallback to MAX_ORDER - 1 as before. While here it also fixes a build
problem via type casting MAX_ORDER in rmem_cma_setup().

This series applies on v5.11-rc7 and has been slightly tested on arm64. But
looking for some early feedbacks particularly with respect to concerns in
subscribing HUGETLB_PAGE_SIZE_VARIABLE on a platform where the hugetlb page
size is config dependent but not really a runtime variable. Even though it
appears that HUGETLB_PAGE_SIZE_VARIABLE is used only while computing the
pageblock_order, could there be other implications ?

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: David Hildenbrand <david@redhat.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: iommu@lists.linux-foundation.org
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org

Changes in V1:

- Rebased on 5.11-rc7
- Dropped the RFC

Changes in RFC:

https://lore.kernel.org/linux-mm/1612422084-30429-1-git-send-email-anshuman.khandual@arm.com/

Anshuman Khandual (3):
  mm/page_alloc: Fix pageblock_order when HUGETLB_PAGE_ORDER >= MAX_ORDER
  arm64/hugetlb: Enable HUGETLB_PAGE_SIZE_VARIABLE
  dma-contiguous: Type cast MAX_ORDER as unsigned int

 arch/arm64/Kconfig      | 4 ++++
 kernel/dma/contiguous.c | 2 +-
 mm/page_alloc.c         | 2 +-
 3 files changed, 6 insertions(+), 2 deletions(-)

-- 
2.20.1



^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/3] mm/page_alloc: Fix pageblock_order when HUGETLB_PAGE_ORDER >= MAX_ORDER
  2021-02-11  6:22 [PATCH 0/3] mm/page_alloc: Fix pageblock_order with HUGETLB_PAGE_SIZE_VARIABLE Anshuman Khandual
@ 2021-02-11  6:22 ` Anshuman Khandual
  2021-02-11  8:00   ` Christoph Hellwig
  2021-02-11  6:22 ` [PATCH 2/3] arm64/hugetlb: Enable HUGETLB_PAGE_SIZE_VARIABLE Anshuman Khandual
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 15+ messages in thread
From: Anshuman Khandual @ 2021-02-11  6:22 UTC (permalink / raw)
  To: linux-mm, linux-arm-kernel, catalin.marinas, akpm, will
  Cc: Anshuman Khandual, Robin Murphy, Marek Szyprowski,
	Christoph Hellwig, David Hildenbrand, Mark Rutland, iommu,
	linux-kernel

With HUGETLB_PAGE_SIZE_VARIABLE enabled, pageblock_order cannot be assigned
as HUGETLB_PAGE_ORDER when it is greater than or equal to MAX_ORDER during
set_pageblock_order(). Otherwise  the following warning is triggered during
boot as detected on an arm64 platform.

WARNING: CPU: 5 PID: 124 at mm/vmstat.c:1080 __fragmentation_index+0xa4/0xc0
Modules linked in:
CPU: 5 PID: 124 Comm: kswapd0 Not tainted 5.11.0-rc6-00004-ga0ea7d62002 #159
Hardware name: linux,dummy-virt (DT)
[    8.810673] pstate: 20400005 (nzCv daif +PAN -UAO -TCO BTYPE=--)
[    8.811732] pc : __fragmentation_index+0xa4/0xc0
[    8.812555] lr : fragmentation_index+0xf8/0x138
[    8.813360] sp : ffff0000864079b0
[    8.813958] x29: ffff0000864079b0 x28: 0000000000000372
[    8.814901] x27: 0000000000007682 x26: ffff8000135b3948
[    8.815847] x25: 1fffe00010c80f48 x24: 0000000000000000
[    8.816805] x23: 0000000000000000 x22: 000000000000000d
[    8.817764] x21: 0000000000000030 x20: ffff0005ffcb4d58
[    8.818712] x19: 000000000000000b x18: 0000000000000000
[    8.819656] x17: 0000000000000000 x16: 0000000000000000
[    8.820613] x15: 0000000000000000 x14: ffff8000114c6258
[    8.821560] x13: ffff6000bff969ba x12: 1fffe000bff969b9
[    8.822514] x11: 1fffe000bff969b9 x10: ffff6000bff969b9
[    8.823461] x9 : dfff800000000000 x8 : ffff0005ffcb4dcf
[    8.824415] x7 : 0000000000000001 x6 : 0000000041b58ab3
[    8.825359] x5 : ffff600010c80f48 x4 : dfff800000000000
[    8.826313] x3 : ffff8000102be670 x2 : 0000000000000007
[    8.827259] x1 : ffff000086407a60 x0 : 000000000000000d
[    8.828218] Call trace:
[    8.828667]  __fragmentation_index+0xa4/0xc0
[    8.829436]  fragmentation_index+0xf8/0x138
[    8.830194]  compaction_suitable+0x98/0xb8
[    8.830934]  wakeup_kcompactd+0xdc/0x128
[    8.831640]  balance_pgdat+0x71c/0x7a0
[    8.832327]  kswapd+0x31c/0x520
[    8.832902]  kthread+0x224/0x230
[    8.833491]  ret_from_fork+0x10/0x30
[    8.834150] ---[ end trace 472836f79c15516b ]---

The above warning happens because pageblock_order exceeds MAX_ORDER, caused
by large HUGETLB_PAGE_ORDER on certain platforms like arm64. Lets prevent
the scenario by first checking HUGETLB_PAGE_ORDER against MAX_ORDER, before
its assignment as pageblock_order.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 mm/page_alloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 519a60d5b6f7..36473f2fa683 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6798,7 +6798,7 @@ void __init set_pageblock_order(void)
 	if (pageblock_order)
 		return;
 
-	if (HPAGE_SHIFT > PAGE_SHIFT)
+	if ((HPAGE_SHIFT > PAGE_SHIFT) && (HUGETLB_PAGE_ORDER < MAX_ORDER))
 		order = HUGETLB_PAGE_ORDER;
 	else
 		order = MAX_ORDER - 1;
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/3] arm64/hugetlb: Enable HUGETLB_PAGE_SIZE_VARIABLE
  2021-02-11  6:22 [PATCH 0/3] mm/page_alloc: Fix pageblock_order with HUGETLB_PAGE_SIZE_VARIABLE Anshuman Khandual
  2021-02-11  6:22 ` [PATCH 1/3] mm/page_alloc: Fix pageblock_order when HUGETLB_PAGE_ORDER >= MAX_ORDER Anshuman Khandual
@ 2021-02-11  6:22 ` Anshuman Khandual
  2021-02-11  8:01   ` Christoph Hellwig
  2021-02-11  6:22 ` [PATCH 3/3] dma-contiguous: Type cast MAX_ORDER as unsigned int Anshuman Khandual
  2021-02-11  8:37 ` [PATCH 0/3] mm/page_alloc: Fix pageblock_order with HUGETLB_PAGE_SIZE_VARIABLE David Hildenbrand
  3 siblings, 1 reply; 15+ messages in thread
From: Anshuman Khandual @ 2021-02-11  6:22 UTC (permalink / raw)
  To: linux-mm, linux-arm-kernel, catalin.marinas, akpm, will
  Cc: Anshuman Khandual, Robin Murphy, Marek Szyprowski,
	Christoph Hellwig, David Hildenbrand, Mark Rutland, iommu,
	linux-kernel

MAX_ORDER which invariably depends on FORCE_MAX_ZONEORDER can be a variable
for a given page size, depending on whether TRANSPARENT_HUGEPAGE is enabled
or not. In certain page size and THP combinations HUGETLB_PAGE_ORDER can be
greater than MAX_ORDER, making it unusable as pageblock_order.

This enables HUGETLB_PAGE_SIZE_VARIABLE making pageblock_order a variable
rather than the compile time constant HUGETLB_PAGE_ORDER which could break
MAX_ORDER rule for certain configurations.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 arch/arm64/Kconfig | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index f39568b28ec1..8e3a5578f663 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1909,6 +1909,10 @@ config ARCH_ENABLE_THP_MIGRATION
 	def_bool y
 	depends on TRANSPARENT_HUGEPAGE
 
+config HUGETLB_PAGE_SIZE_VARIABLE
+	def_bool y
+	depends on HUGETLB_PAGE
+
 menu "Power management options"
 
 source "kernel/power/Kconfig"
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 3/3] dma-contiguous: Type cast MAX_ORDER as unsigned int
  2021-02-11  6:22 [PATCH 0/3] mm/page_alloc: Fix pageblock_order with HUGETLB_PAGE_SIZE_VARIABLE Anshuman Khandual
  2021-02-11  6:22 ` [PATCH 1/3] mm/page_alloc: Fix pageblock_order when HUGETLB_PAGE_ORDER >= MAX_ORDER Anshuman Khandual
  2021-02-11  6:22 ` [PATCH 2/3] arm64/hugetlb: Enable HUGETLB_PAGE_SIZE_VARIABLE Anshuman Khandual
@ 2021-02-11  6:22 ` Anshuman Khandual
  2021-02-11  8:04   ` Christoph Hellwig
  2021-02-11  8:37 ` [PATCH 0/3] mm/page_alloc: Fix pageblock_order with HUGETLB_PAGE_SIZE_VARIABLE David Hildenbrand
  3 siblings, 1 reply; 15+ messages in thread
From: Anshuman Khandual @ 2021-02-11  6:22 UTC (permalink / raw)
  To: linux-mm, linux-arm-kernel, catalin.marinas, akpm, will
  Cc: Anshuman Khandual, Robin Murphy, Marek Szyprowski,
	Christoph Hellwig, David Hildenbrand, Mark Rutland, iommu,
	linux-kernel

Type cast MAX_ORDER as unsigned int to fix the following build warning.

In file included from ./include/linux/kernel.h:14,
                 from ./include/asm-generic/bug.h:20,
                 from ./arch/arm64/include/asm/bug.h:26,
                 from ./include/linux/bug.h:5,
                 from ./include/linux/mmdebug.h:5,
                 from ./arch/arm64/include/asm/memory.h:166,
                 from ./arch/arm64/include/asm/page.h:42,
                 from kernel/dma/contiguous.c:46:
kernel/dma/contiguous.c: In function ‘rmem_cma_setup’:
./include/linux/minmax.h:18:28: warning: comparison of distinct pointer
types lacks a cast
  (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
                            ^~
./include/linux/minmax.h:32:4: note: in expansion of macro ‘__typecheck’
   (__typecheck(x, y) && __no_side_effects(x, y))
    ^~~~~~~~~~~
./include/linux/minmax.h:42:24: note: in expansion of macro ‘__safe_cmp’
  __builtin_choose_expr(__safe_cmp(x, y), \
                        ^~~~~~~~~~
./include/linux/minmax.h:58:19: note: in expansion of macro
‘__careful_cmp’
 #define max(x, y) __careful_cmp(x, y, >)
                   ^~~~~~~~~~~~~
kernel/dma/contiguous.c:402:35: note: in expansion of macro ‘max’
  phys_addr_t align = PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order);

Cc: Christoph Hellwig <hch@lst.de>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: iommu@lists.linux-foundation.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 kernel/dma/contiguous.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/dma/contiguous.c b/kernel/dma/contiguous.c
index 3d63d91cba5c..1c2782349d71 100644
--- a/kernel/dma/contiguous.c
+++ b/kernel/dma/contiguous.c
@@ -399,7 +399,7 @@ static const struct reserved_mem_ops rmem_cma_ops = {
 
 static int __init rmem_cma_setup(struct reserved_mem *rmem)
 {
-	phys_addr_t align = PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order);
+	phys_addr_t align = PAGE_SIZE << max((unsigned int)MAX_ORDER - 1, pageblock_order);
 	phys_addr_t mask = align - 1;
 	unsigned long node = rmem->fdt_node;
 	bool default_cma = of_get_flat_dt_prop(node, "linux,cma-default", NULL);
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/3] mm/page_alloc: Fix pageblock_order when HUGETLB_PAGE_ORDER >= MAX_ORDER
  2021-02-11  6:22 ` [PATCH 1/3] mm/page_alloc: Fix pageblock_order when HUGETLB_PAGE_ORDER >= MAX_ORDER Anshuman Khandual
@ 2021-02-11  8:00   ` Christoph Hellwig
  2021-02-12  7:12     ` Anshuman Khandual
  0 siblings, 1 reply; 15+ messages in thread
From: Christoph Hellwig @ 2021-02-11  8:00 UTC (permalink / raw)
  To: Anshuman Khandual
  Cc: linux-mm, linux-arm-kernel, catalin.marinas, akpm, will,
	Robin Murphy, Marek Szyprowski, Christoph Hellwig,
	David Hildenbrand, Mark Rutland, iommu, linux-kernel

> -	if (HPAGE_SHIFT > PAGE_SHIFT)
> +	if ((HPAGE_SHIFT > PAGE_SHIFT) && (HUGETLB_PAGE_ORDER < MAX_ORDER))

No need for the braces.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/3] arm64/hugetlb: Enable HUGETLB_PAGE_SIZE_VARIABLE
  2021-02-11  6:22 ` [PATCH 2/3] arm64/hugetlb: Enable HUGETLB_PAGE_SIZE_VARIABLE Anshuman Khandual
@ 2021-02-11  8:01   ` Christoph Hellwig
  2021-02-12  7:13     ` Anshuman Khandual
  0 siblings, 1 reply; 15+ messages in thread
From: Christoph Hellwig @ 2021-02-11  8:01 UTC (permalink / raw)
  To: Anshuman Khandual
  Cc: linux-mm, linux-arm-kernel, catalin.marinas, akpm, will,
	Robin Murphy, Marek Szyprowski, Christoph Hellwig,
	David Hildenbrand, Mark Rutland, iommu, linux-kernel

On Thu, Feb 11, 2021 at 11:52:10AM +0530, Anshuman Khandual wrote:
> MAX_ORDER which invariably depends on FORCE_MAX_ZONEORDER can be a variable
> for a given page size, depending on whether TRANSPARENT_HUGEPAGE is enabled
> or not. In certain page size and THP combinations HUGETLB_PAGE_ORDER can be
> greater than MAX_ORDER, making it unusable as pageblock_order.
> 
> This enables HUGETLB_PAGE_SIZE_VARIABLE making pageblock_order a variable
> rather than the compile time constant HUGETLB_PAGE_ORDER which could break
> MAX_ORDER rule for certain configurations.
> 
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
>  arch/arm64/Kconfig | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index f39568b28ec1..8e3a5578f663 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1909,6 +1909,10 @@ config ARCH_ENABLE_THP_MIGRATION
>  	def_bool y
>  	depends on TRANSPARENT_HUGEPAGE
>  
> +config HUGETLB_PAGE_SIZE_VARIABLE

Please move the definition of HUGETLB_PAGE_SIZE_VARIABLE to
mm/Kconfig and select it from the arch Kconfigfs instead of duplicating
the definition.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/3] dma-contiguous: Type cast MAX_ORDER as unsigned int
  2021-02-11  6:22 ` [PATCH 3/3] dma-contiguous: Type cast MAX_ORDER as unsigned int Anshuman Khandual
@ 2021-02-11  8:04   ` Christoph Hellwig
  2021-02-12  7:19     ` Anshuman Khandual
  0 siblings, 1 reply; 15+ messages in thread
From: Christoph Hellwig @ 2021-02-11  8:04 UTC (permalink / raw)
  To: Anshuman Khandual
  Cc: linux-mm, linux-arm-kernel, catalin.marinas, akpm, will,
	Robin Murphy, Marek Szyprowski, Christoph Hellwig,
	David Hildenbrand, Mark Rutland, iommu, linux-kernel

On Thu, Feb 11, 2021 at 11:52:11AM +0530, Anshuman Khandual wrote:
> Type cast MAX_ORDER as unsigned int to fix the following build warning.
> 
> In file included from ./include/linux/kernel.h:14,
>                  from ./include/asm-generic/bug.h:20,
>                  from ./arch/arm64/include/asm/bug.h:26,
>                  from ./include/linux/bug.h:5,
>                  from ./include/linux/mmdebug.h:5,
>                  from ./arch/arm64/include/asm/memory.h:166,
>                  from ./arch/arm64/include/asm/page.h:42,
>                  from kernel/dma/contiguous.c:46:
> kernel/dma/contiguous.c: In function ‘rmem_cma_setup’:
> ./include/linux/minmax.h:18:28: warning: comparison of distinct pointer
> types lacks a cast
>   (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
>                             ^~
> ./include/linux/minmax.h:32:4: note: in expansion of macro ‘__typecheck’
>    (__typecheck(x, y) && __no_side_effects(x, y))
>     ^~~~~~~~~~~
> ./include/linux/minmax.h:42:24: note: in expansion of macro ‘__safe_cmp’
>   __builtin_choose_expr(__safe_cmp(x, y), \
>                         ^~~~~~~~~~
> ./include/linux/minmax.h:58:19: note: in expansion of macro
> ‘__careful_cmp’
>  #define max(x, y) __careful_cmp(x, y, >)
>                    ^~~~~~~~~~~~~
> kernel/dma/contiguous.c:402:35: note: in expansion of macro ‘max’
>   phys_addr_t align = PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order);
> 
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: iommu@lists.linux-foundation.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
>  kernel/dma/contiguous.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/dma/contiguous.c b/kernel/dma/contiguous.c
> index 3d63d91cba5c..1c2782349d71 100644
> --- a/kernel/dma/contiguous.c
> +++ b/kernel/dma/contiguous.c
> @@ -399,7 +399,7 @@ static const struct reserved_mem_ops rmem_cma_ops = {
>  
>  static int __init rmem_cma_setup(struct reserved_mem *rmem)
>  {
> -	phys_addr_t align = PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order);
> +	phys_addr_t align = PAGE_SIZE << max((unsigned int)MAX_ORDER - 1, pageblock_order);

MAX_ORDER and pageblock_order should be the same type.  So either fix
MAX_ORDER to be an unsigned constant, which would be fundamentally
the right thing to do but might cause some fallout, or turn
pageblock_order into an int, which is probably much either as the stub
define of it already has an integer type derived from MAX_ORDER as well.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/3] mm/page_alloc: Fix pageblock_order with HUGETLB_PAGE_SIZE_VARIABLE
  2021-02-11  6:22 [PATCH 0/3] mm/page_alloc: Fix pageblock_order with HUGETLB_PAGE_SIZE_VARIABLE Anshuman Khandual
                   ` (2 preceding siblings ...)
  2021-02-11  6:22 ` [PATCH 3/3] dma-contiguous: Type cast MAX_ORDER as unsigned int Anshuman Khandual
@ 2021-02-11  8:37 ` David Hildenbrand
  2021-02-12  7:02   ` Anshuman Khandual
  3 siblings, 1 reply; 15+ messages in thread
From: David Hildenbrand @ 2021-02-11  8:37 UTC (permalink / raw)
  To: Anshuman Khandual, linux-mm, linux-arm-kernel, catalin.marinas,
	akpm, will
  Cc: Robin Murphy, Marek Szyprowski, Christoph Hellwig, Mark Rutland,
	iommu, linux-kernel

On 11.02.21 07:22, Anshuman Khandual wrote:
> The following warning gets triggered while trying to boot a 64K page size
> without THP config kernel on arm64 platform.
> 
> WARNING: CPU: 5 PID: 124 at mm/vmstat.c:1080 __fragmentation_index+0xa4/0xc0
> Modules linked in:
> CPU: 5 PID: 124 Comm: kswapd0 Not tainted 5.11.0-rc6-00004-ga0ea7d62002 #159
> Hardware name: linux,dummy-virt (DT)
> [    8.810673] pstate: 20400005 (nzCv daif +PAN -UAO -TCO BTYPE=--)
> [    8.811732] pc : __fragmentation_index+0xa4/0xc0
> [    8.812555] lr : fragmentation_index+0xf8/0x138
> [    8.813360] sp : ffff0000864079b0
> [    8.813958] x29: ffff0000864079b0 x28: 0000000000000372
> [    8.814901] x27: 0000000000007682 x26: ffff8000135b3948
> [    8.815847] x25: 1fffe00010c80f48 x24: 0000000000000000
> [    8.816805] x23: 0000000000000000 x22: 000000000000000d
> [    8.817764] x21: 0000000000000030 x20: ffff0005ffcb4d58
> [    8.818712] x19: 000000000000000b x18: 0000000000000000
> [    8.819656] x17: 0000000000000000 x16: 0000000000000000
> [    8.820613] x15: 0000000000000000 x14: ffff8000114c6258
> [    8.821560] x13: ffff6000bff969ba x12: 1fffe000bff969b9
> [    8.822514] x11: 1fffe000bff969b9 x10: ffff6000bff969b9
> [    8.823461] x9 : dfff800000000000 x8 : ffff0005ffcb4dcf
> [    8.824415] x7 : 0000000000000001 x6 : 0000000041b58ab3
> [    8.825359] x5 : ffff600010c80f48 x4 : dfff800000000000
> [    8.826313] x3 : ffff8000102be670 x2 : 0000000000000007
> [    8.827259] x1 : ffff000086407a60 x0 : 000000000000000d
> [    8.828218] Call trace:
> [    8.828667]  __fragmentation_index+0xa4/0xc0
> [    8.829436]  fragmentation_index+0xf8/0x138
> [    8.830194]  compaction_suitable+0x98/0xb8
> [    8.830934]  wakeup_kcompactd+0xdc/0x128
> [    8.831640]  balance_pgdat+0x71c/0x7a0
> [    8.832327]  kswapd+0x31c/0x520
> [    8.832902]  kthread+0x224/0x230
> [    8.833491]  ret_from_fork+0x10/0x30
> [    8.834150] ---[ end trace 472836f79c15516b ]---
> 
> This warning comes from __fragmentation_index() when the requested order
> is greater than MAX_ORDER.
> 
> static int __fragmentation_index(unsigned int order,
> 				 struct contig_page_info *info)
> {
>          unsigned long requested = 1UL << order;
> 
>          if (WARN_ON_ONCE(order >= MAX_ORDER)) <===== Triggered here
>                  return 0;
> 
> Digging it further reveals that pageblock_order has been assigned a value
> which is greater than MAX_ORDER failing the above check. But why this
> happened ? Because HUGETLB_PAGE_ORDER for the given config on arm64 is
> greater than MAX_ORDER.
> 
> The solution involves enabling HUGETLB_PAGE_SIZE_VARIABLE which would make
> pageblock_order a variable instead of constant HUGETLB_PAGE_ORDER. But that
> change alone also did not really work as pageblock_order still got assigned
> as HUGETLB_PAGE_ORDER in set_pageblock_order(). HUGETLB_PAGE_ORDER needs to
> be less than MAX_ORDER for its appropriateness as pageblock_order otherwise
> just fallback to MAX_ORDER - 1 as before. While here it also fixes a build
> problem via type casting MAX_ORDER in rmem_cma_setup().

I'm wondering, is there any real value in allowing FORCE_MAX_ZONEORDER 
to be "11" with ARM64_64K_PAGES/ARM64_16K_PAGES?

Meaning: are there any real use cases that actually build a kernel 
without TRANSPARENT_HUGEPAGE and with ARM64_64K_PAGES/ARM64_16K_PAGES?

As builds are essentially broken, I assume this is not that relevant? Or 
how long has it been broken?

It might be easier to just drop the "TRANSPARENT_HUGEPAGE" part from the 
FORCE_MAX_ZONEORDER config.

-- 
Thanks,

David / dhildenb



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/3] mm/page_alloc: Fix pageblock_order with HUGETLB_PAGE_SIZE_VARIABLE
  2021-02-11  8:37 ` [PATCH 0/3] mm/page_alloc: Fix pageblock_order with HUGETLB_PAGE_SIZE_VARIABLE David Hildenbrand
@ 2021-02-12  7:02   ` Anshuman Khandual
  2021-02-12  9:39     ` David Hildenbrand
  0 siblings, 1 reply; 15+ messages in thread
From: Anshuman Khandual @ 2021-02-12  7:02 UTC (permalink / raw)
  To: David Hildenbrand, linux-mm, linux-arm-kernel, catalin.marinas,
	akpm, will
  Cc: Robin Murphy, Marek Szyprowski, Christoph Hellwig, Mark Rutland,
	iommu, linux-kernel


On 2/11/21 2:07 PM, David Hildenbrand wrote:
> On 11.02.21 07:22, Anshuman Khandual wrote:
>> The following warning gets triggered while trying to boot a 64K page size
>> without THP config kernel on arm64 platform.
>>
>> WARNING: CPU: 5 PID: 124 at mm/vmstat.c:1080 __fragmentation_index+0xa4/0xc0
>> Modules linked in:
>> CPU: 5 PID: 124 Comm: kswapd0 Not tainted 5.11.0-rc6-00004-ga0ea7d62002 #159
>> Hardware name: linux,dummy-virt (DT)
>> [    8.810673] pstate: 20400005 (nzCv daif +PAN -UAO -TCO BTYPE=--)
>> [    8.811732] pc : __fragmentation_index+0xa4/0xc0
>> [    8.812555] lr : fragmentation_index+0xf8/0x138
>> [    8.813360] sp : ffff0000864079b0
>> [    8.813958] x29: ffff0000864079b0 x28: 0000000000000372
>> [    8.814901] x27: 0000000000007682 x26: ffff8000135b3948
>> [    8.815847] x25: 1fffe00010c80f48 x24: 0000000000000000
>> [    8.816805] x23: 0000000000000000 x22: 000000000000000d
>> [    8.817764] x21: 0000000000000030 x20: ffff0005ffcb4d58
>> [    8.818712] x19: 000000000000000b x18: 0000000000000000
>> [    8.819656] x17: 0000000000000000 x16: 0000000000000000
>> [    8.820613] x15: 0000000000000000 x14: ffff8000114c6258
>> [    8.821560] x13: ffff6000bff969ba x12: 1fffe000bff969b9
>> [    8.822514] x11: 1fffe000bff969b9 x10: ffff6000bff969b9
>> [    8.823461] x9 : dfff800000000000 x8 : ffff0005ffcb4dcf
>> [    8.824415] x7 : 0000000000000001 x6 : 0000000041b58ab3
>> [    8.825359] x5 : ffff600010c80f48 x4 : dfff800000000000
>> [    8.826313] x3 : ffff8000102be670 x2 : 0000000000000007
>> [    8.827259] x1 : ffff000086407a60 x0 : 000000000000000d
>> [    8.828218] Call trace:
>> [    8.828667]  __fragmentation_index+0xa4/0xc0
>> [    8.829436]  fragmentation_index+0xf8/0x138
>> [    8.830194]  compaction_suitable+0x98/0xb8
>> [    8.830934]  wakeup_kcompactd+0xdc/0x128
>> [    8.831640]  balance_pgdat+0x71c/0x7a0
>> [    8.832327]  kswapd+0x31c/0x520
>> [    8.832902]  kthread+0x224/0x230
>> [    8.833491]  ret_from_fork+0x10/0x30
>> [    8.834150] ---[ end trace 472836f79c15516b ]---
>>
>> This warning comes from __fragmentation_index() when the requested order
>> is greater than MAX_ORDER.
>>
>> static int __fragmentation_index(unsigned int order,
>>                  struct contig_page_info *info)
>> {
>>          unsigned long requested = 1UL << order;
>>
>>          if (WARN_ON_ONCE(order >= MAX_ORDER)) <===== Triggered here
>>                  return 0;
>>
>> Digging it further reveals that pageblock_order has been assigned a value
>> which is greater than MAX_ORDER failing the above check. But why this
>> happened ? Because HUGETLB_PAGE_ORDER for the given config on arm64 is
>> greater than MAX_ORDER.
>>
>> The solution involves enabling HUGETLB_PAGE_SIZE_VARIABLE which would make
>> pageblock_order a variable instead of constant HUGETLB_PAGE_ORDER. But that
>> change alone also did not really work as pageblock_order still got assigned
>> as HUGETLB_PAGE_ORDER in set_pageblock_order(). HUGETLB_PAGE_ORDER needs to
>> be less than MAX_ORDER for its appropriateness as pageblock_order otherwise
>> just fallback to MAX_ORDER - 1 as before. While here it also fixes a build
>> problem via type casting MAX_ORDER in rmem_cma_setup().
> 
> I'm wondering, is there any real value in allowing FORCE_MAX_ZONEORDER to be "11" with ARM64_64K_PAGES/ARM64_16K_PAGES?

MAX_ORDER should be as high as would be required for the current config.
Unless THP is enabled, there is no need for it to be any higher than 11.
But I might be missing historical reasons around this as well. Probably
others from arm64 could help here.

> 
> Meaning: are there any real use cases that actually build a kernel without TRANSPARENT_HUGEPAGE and with ARM64_64K_PAGES/ARM64_16K_PAGES?

THP is always optional. Besides kernel builds without THP should always
be supported. Assuming that all builds will have THP enabled, might not
be accurate.

> 
> As builds are essentially broken, I assume this is not that relevant? Or how long has it been broken?

Git blame shows that it's been there for some time now. But how does
that make this irrelevant ? A problem should be fixed nonetheless.

> 
> It might be easier to just drop the "TRANSPARENT_HUGEPAGE" part from the FORCE_MAX_ZONEORDER config.
> 

Not sure if it would be a good idea to unnecessarily have larger MAX_ORDER
value for a given config. But I might be missing other contexts here.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/3] mm/page_alloc: Fix pageblock_order when HUGETLB_PAGE_ORDER >= MAX_ORDER
  2021-02-11  8:00   ` Christoph Hellwig
@ 2021-02-12  7:12     ` Anshuman Khandual
  0 siblings, 0 replies; 15+ messages in thread
From: Anshuman Khandual @ 2021-02-12  7:12 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-mm, linux-arm-kernel, catalin.marinas, akpm, will,
	Robin Murphy, Marek Szyprowski, David Hildenbrand, Mark Rutland,
	iommu, linux-kernel



On 2/11/21 1:30 PM, Christoph Hellwig wrote:
>> -	if (HPAGE_SHIFT > PAGE_SHIFT)
>> +	if ((HPAGE_SHIFT > PAGE_SHIFT) && (HUGETLB_PAGE_ORDER < MAX_ORDER))
> 
> No need for the braces.

Will drop them.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/3] arm64/hugetlb: Enable HUGETLB_PAGE_SIZE_VARIABLE
  2021-02-11  8:01   ` Christoph Hellwig
@ 2021-02-12  7:13     ` Anshuman Khandual
  0 siblings, 0 replies; 15+ messages in thread
From: Anshuman Khandual @ 2021-02-12  7:13 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-mm, linux-arm-kernel, catalin.marinas, akpm, will,
	Robin Murphy, Marek Szyprowski, David Hildenbrand, Mark Rutland,
	iommu, linux-kernel



On 2/11/21 1:31 PM, Christoph Hellwig wrote:
> On Thu, Feb 11, 2021 at 11:52:10AM +0530, Anshuman Khandual wrote:
>> MAX_ORDER which invariably depends on FORCE_MAX_ZONEORDER can be a variable
>> for a given page size, depending on whether TRANSPARENT_HUGEPAGE is enabled
>> or not. In certain page size and THP combinations HUGETLB_PAGE_ORDER can be
>> greater than MAX_ORDER, making it unusable as pageblock_order.
>>
>> This enables HUGETLB_PAGE_SIZE_VARIABLE making pageblock_order a variable
>> rather than the compile time constant HUGETLB_PAGE_ORDER which could break
>> MAX_ORDER rule for certain configurations.
>>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Will Deacon <will@kernel.org>
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>>  arch/arm64/Kconfig | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index f39568b28ec1..8e3a5578f663 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -1909,6 +1909,10 @@ config ARCH_ENABLE_THP_MIGRATION
>>  	def_bool y
>>  	depends on TRANSPARENT_HUGEPAGE
>>  
>> +config HUGETLB_PAGE_SIZE_VARIABLE
> 
> Please move the definition of HUGETLB_PAGE_SIZE_VARIABLE to
> mm/Kconfig and select it from the arch Kconfigfs instead of duplicating
> the definition.

Sure, will do.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/3] dma-contiguous: Type cast MAX_ORDER as unsigned int
  2021-02-11  8:04   ` Christoph Hellwig
@ 2021-02-12  7:19     ` Anshuman Khandual
  0 siblings, 0 replies; 15+ messages in thread
From: Anshuman Khandual @ 2021-02-12  7:19 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-mm, linux-arm-kernel, catalin.marinas, akpm, will,
	Robin Murphy, Marek Szyprowski, David Hildenbrand, Mark Rutland,
	iommu, linux-kernel



On 2/11/21 1:34 PM, Christoph Hellwig wrote:
> On Thu, Feb 11, 2021 at 11:52:11AM +0530, Anshuman Khandual wrote:
>> Type cast MAX_ORDER as unsigned int to fix the following build warning.
>>
>> In file included from ./include/linux/kernel.h:14,
>>                  from ./include/asm-generic/bug.h:20,
>>                  from ./arch/arm64/include/asm/bug.h:26,
>>                  from ./include/linux/bug.h:5,
>>                  from ./include/linux/mmdebug.h:5,
>>                  from ./arch/arm64/include/asm/memory.h:166,
>>                  from ./arch/arm64/include/asm/page.h:42,
>>                  from kernel/dma/contiguous.c:46:
>> kernel/dma/contiguous.c: In function ‘rmem_cma_setup’:
>> ./include/linux/minmax.h:18:28: warning: comparison of distinct pointer
>> types lacks a cast
>>   (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
>>                             ^~
>> ./include/linux/minmax.h:32:4: note: in expansion of macro ‘__typecheck’
>>    (__typecheck(x, y) && __no_side_effects(x, y))
>>     ^~~~~~~~~~~
>> ./include/linux/minmax.h:42:24: note: in expansion of macro ‘__safe_cmp’
>>   __builtin_choose_expr(__safe_cmp(x, y), \
>>                         ^~~~~~~~~~
>> ./include/linux/minmax.h:58:19: note: in expansion of macro
>> ‘__careful_cmp’
>>  #define max(x, y) __careful_cmp(x, y, >)
>>                    ^~~~~~~~~~~~~
>> kernel/dma/contiguous.c:402:35: note: in expansion of macro ‘max’
>>   phys_addr_t align = PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order);
>>
>> Cc: Christoph Hellwig <hch@lst.de>
>> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
>> Cc: Robin Murphy <robin.murphy@arm.com>
>> Cc: iommu@lists.linux-foundation.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>>  kernel/dma/contiguous.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/kernel/dma/contiguous.c b/kernel/dma/contiguous.c
>> index 3d63d91cba5c..1c2782349d71 100644
>> --- a/kernel/dma/contiguous.c
>> +++ b/kernel/dma/contiguous.c
>> @@ -399,7 +399,7 @@ static const struct reserved_mem_ops rmem_cma_ops = {
>>  
>>  static int __init rmem_cma_setup(struct reserved_mem *rmem)
>>  {
>> -	phys_addr_t align = PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order);
>> +	phys_addr_t align = PAGE_SIZE << max((unsigned int)MAX_ORDER - 1, pageblock_order);
> 
> MAX_ORDER and pageblock_order should be the same type.  So either fix

Right.

> MAX_ORDER to be an unsigned constant, which would be fundamentally
> the right thing to do but might cause some fallout, or turn
> pageblock_order into an int, which is probably much either as the stub
> define of it already has an integer type derived from MAX_ORDER as well.

Right, will change pageblock_order as 'int' which would be easier.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/3] mm/page_alloc: Fix pageblock_order with HUGETLB_PAGE_SIZE_VARIABLE
  2021-02-12  7:02   ` Anshuman Khandual
@ 2021-02-12  9:39     ` David Hildenbrand
  2021-02-16  9:02       ` Anshuman Khandual
  0 siblings, 1 reply; 15+ messages in thread
From: David Hildenbrand @ 2021-02-12  9:39 UTC (permalink / raw)
  To: Anshuman Khandual, linux-mm, linux-arm-kernel, catalin.marinas,
	akpm, will
  Cc: Robin Murphy, Marek Szyprowski, Christoph Hellwig, Mark Rutland,
	iommu, linux-kernel

On 12.02.21 08:02, Anshuman Khandual wrote:
> 
> On 2/11/21 2:07 PM, David Hildenbrand wrote:
>> On 11.02.21 07:22, Anshuman Khandual wrote:
>>> The following warning gets triggered while trying to boot a 64K page size
>>> without THP config kernel on arm64 platform.
>>>
>>> WARNING: CPU: 5 PID: 124 at mm/vmstat.c:1080 __fragmentation_index+0xa4/0xc0
>>> Modules linked in:
>>> CPU: 5 PID: 124 Comm: kswapd0 Not tainted 5.11.0-rc6-00004-ga0ea7d62002 #159
>>> Hardware name: linux,dummy-virt (DT)
>>> [    8.810673] pstate: 20400005 (nzCv daif +PAN -UAO -TCO BTYPE=--)
>>> [    8.811732] pc : __fragmentation_index+0xa4/0xc0
>>> [    8.812555] lr : fragmentation_index+0xf8/0x138
>>> [    8.813360] sp : ffff0000864079b0
>>> [    8.813958] x29: ffff0000864079b0 x28: 0000000000000372
>>> [    8.814901] x27: 0000000000007682 x26: ffff8000135b3948
>>> [    8.815847] x25: 1fffe00010c80f48 x24: 0000000000000000
>>> [    8.816805] x23: 0000000000000000 x22: 000000000000000d
>>> [    8.817764] x21: 0000000000000030 x20: ffff0005ffcb4d58
>>> [    8.818712] x19: 000000000000000b x18: 0000000000000000
>>> [    8.819656] x17: 0000000000000000 x16: 0000000000000000
>>> [    8.820613] x15: 0000000000000000 x14: ffff8000114c6258
>>> [    8.821560] x13: ffff6000bff969ba x12: 1fffe000bff969b9
>>> [    8.822514] x11: 1fffe000bff969b9 x10: ffff6000bff969b9
>>> [    8.823461] x9 : dfff800000000000 x8 : ffff0005ffcb4dcf
>>> [    8.824415] x7 : 0000000000000001 x6 : 0000000041b58ab3
>>> [    8.825359] x5 : ffff600010c80f48 x4 : dfff800000000000
>>> [    8.826313] x3 : ffff8000102be670 x2 : 0000000000000007
>>> [    8.827259] x1 : ffff000086407a60 x0 : 000000000000000d
>>> [    8.828218] Call trace:
>>> [    8.828667]  __fragmentation_index+0xa4/0xc0
>>> [    8.829436]  fragmentation_index+0xf8/0x138
>>> [    8.830194]  compaction_suitable+0x98/0xb8
>>> [    8.830934]  wakeup_kcompactd+0xdc/0x128
>>> [    8.831640]  balance_pgdat+0x71c/0x7a0
>>> [    8.832327]  kswapd+0x31c/0x520
>>> [    8.832902]  kthread+0x224/0x230
>>> [    8.833491]  ret_from_fork+0x10/0x30
>>> [    8.834150] ---[ end trace 472836f79c15516b ]---
>>>
>>> This warning comes from __fragmentation_index() when the requested order
>>> is greater than MAX_ORDER.
>>>
>>> static int __fragmentation_index(unsigned int order,
>>>                   struct contig_page_info *info)
>>> {
>>>           unsigned long requested = 1UL << order;
>>>
>>>           if (WARN_ON_ONCE(order >= MAX_ORDER)) <===== Triggered here
>>>                   return 0;
>>>
>>> Digging it further reveals that pageblock_order has been assigned a value
>>> which is greater than MAX_ORDER failing the above check. But why this
>>> happened ? Because HUGETLB_PAGE_ORDER for the given config on arm64 is
>>> greater than MAX_ORDER.
>>>
>>> The solution involves enabling HUGETLB_PAGE_SIZE_VARIABLE which would make
>>> pageblock_order a variable instead of constant HUGETLB_PAGE_ORDER. But that
>>> change alone also did not really work as pageblock_order still got assigned
>>> as HUGETLB_PAGE_ORDER in set_pageblock_order(). HUGETLB_PAGE_ORDER needs to
>>> be less than MAX_ORDER for its appropriateness as pageblock_order otherwise
>>> just fallback to MAX_ORDER - 1 as before. While here it also fixes a build
>>> problem via type casting MAX_ORDER in rmem_cma_setup().
>>
>> I'm wondering, is there any real value in allowing FORCE_MAX_ZONEORDER to be "11" with ARM64_64K_PAGES/ARM64_16K_PAGES?
> 
> MAX_ORDER should be as high as would be required for the current config.
> Unless THP is enabled, there is no need for it to be any higher than 11.
> But I might be missing historical reasons around this as well. Probably
> others from arm64 could help here.

Theoretically yes, practically no. If nobody cares about a 
configuration, no need to make the code more complicated for that 
configuration.

> 
>>
>> Meaning: are there any real use cases that actually build a kernel without TRANSPARENT_HUGEPAGE and with ARM64_64K_PAGES/ARM64_16K_PAGES?
> 
> THP is always optional. Besides kernel builds without THP should always
> be supported. Assuming that all builds will have THP enabled, might not
> be accurate.
> 
>>
>> As builds are essentially broken, I assume this is not that relevant? Or how long has it been broken?
> 
> Git blame shows that it's been there for some time now. But how does
> that make this irrelevant ? A problem should be fixed nonetheless.

When exactly did I say not to fix it? I'm saying if nobody uses it, we 
might be able to simplify.

> 
>>
>> It might be easier to just drop the "TRANSPARENT_HUGEPAGE" part from the FORCE_MAX_ZONEORDER config.
>>
> 
> Not sure if it would be a good idea to unnecessarily have larger MAX_ORDER
> value for a given config. But I might be missing other contexts here.

My point is: keep it simple if there is no need to make it complicated. 
If these arm64 variants are the only cases where we run into that issue 
and nobody uses them ("hat it's been there for some time now"), why make 
stuff complicated?

The current code seems to assume that HUGETLB_PAGE_ORDER <= MAX_ORDER. 
Instead of changing that for optimizing an unused use case (it is 
broken), just simplify the arm64 conditions. I'd even say add a

/*
  * Some code assumes that HUGETLB_PAGE_ORDER <= MAX_ORDER. For now, only
  * !TRANSPARENT_HUGEPAGE could lead in archs creating such setups.
  * Forbid it for now; anybody that has a valid use case has to sanitize
  * the code.
  */
BUILD_BUG_ON(HUGETLB_PAGE_ORDER <= MAX_ORDER);


But again, if there are valid use cases then sure, let's make the code 
fully compatible with HUGETLB_PAGE_ORDER > MAX_ORDER.


BTW: can we be sure that you caught all issues? For example:

#define COMPACTION_HPAGE_ORDER  HUGETLB_PAGE_ORDER

I'm not sure if it will work as expected if HUGETLB_PAGE_ORDER > 
MAX_ORDER ... fill_contig_page_info() will never find any suitable free 
blocks ...

-- 
Thanks,

David / dhildenb



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/3] mm/page_alloc: Fix pageblock_order with HUGETLB_PAGE_SIZE_VARIABLE
  2021-02-12  9:39     ` David Hildenbrand
@ 2021-02-16  9:02       ` Anshuman Khandual
  2021-02-16  9:04         ` David Hildenbrand
  0 siblings, 1 reply; 15+ messages in thread
From: Anshuman Khandual @ 2021-02-16  9:02 UTC (permalink / raw)
  To: David Hildenbrand, linux-mm, linux-arm-kernel, catalin.marinas,
	akpm, will
  Cc: Robin Murphy, Marek Szyprowski, Christoph Hellwig, Mark Rutland,
	iommu, linux-kernel


On 2/12/21 3:09 PM, David Hildenbrand wrote:
> On 12.02.21 08:02, Anshuman Khandual wrote:
>>
>> On 2/11/21 2:07 PM, David Hildenbrand wrote:
>>> On 11.02.21 07:22, Anshuman Khandual wrote:
>>>> The following warning gets triggered while trying to boot a 64K page size
>>>> without THP config kernel on arm64 platform.
>>>>
>>>> WARNING: CPU: 5 PID: 124 at mm/vmstat.c:1080 __fragmentation_index+0xa4/0xc0
>>>> Modules linked in:
>>>> CPU: 5 PID: 124 Comm: kswapd0 Not tainted 5.11.0-rc6-00004-ga0ea7d62002 #159
>>>> Hardware name: linux,dummy-virt (DT)
>>>> [    8.810673] pstate: 20400005 (nzCv daif +PAN -UAO -TCO BTYPE=--)
>>>> [    8.811732] pc : __fragmentation_index+0xa4/0xc0
>>>> [    8.812555] lr : fragmentation_index+0xf8/0x138
>>>> [    8.813360] sp : ffff0000864079b0
>>>> [    8.813958] x29: ffff0000864079b0 x28: 0000000000000372
>>>> [    8.814901] x27: 0000000000007682 x26: ffff8000135b3948
>>>> [    8.815847] x25: 1fffe00010c80f48 x24: 0000000000000000
>>>> [    8.816805] x23: 0000000000000000 x22: 000000000000000d
>>>> [    8.817764] x21: 0000000000000030 x20: ffff0005ffcb4d58
>>>> [    8.818712] x19: 000000000000000b x18: 0000000000000000
>>>> [    8.819656] x17: 0000000000000000 x16: 0000000000000000
>>>> [    8.820613] x15: 0000000000000000 x14: ffff8000114c6258
>>>> [    8.821560] x13: ffff6000bff969ba x12: 1fffe000bff969b9
>>>> [    8.822514] x11: 1fffe000bff969b9 x10: ffff6000bff969b9
>>>> [    8.823461] x9 : dfff800000000000 x8 : ffff0005ffcb4dcf
>>>> [    8.824415] x7 : 0000000000000001 x6 : 0000000041b58ab3
>>>> [    8.825359] x5 : ffff600010c80f48 x4 : dfff800000000000
>>>> [    8.826313] x3 : ffff8000102be670 x2 : 0000000000000007
>>>> [    8.827259] x1 : ffff000086407a60 x0 : 000000000000000d
>>>> [    8.828218] Call trace:
>>>> [    8.828667]  __fragmentation_index+0xa4/0xc0
>>>> [    8.829436]  fragmentation_index+0xf8/0x138
>>>> [    8.830194]  compaction_suitable+0x98/0xb8
>>>> [    8.830934]  wakeup_kcompactd+0xdc/0x128
>>>> [    8.831640]  balance_pgdat+0x71c/0x7a0
>>>> [    8.832327]  kswapd+0x31c/0x520
>>>> [    8.832902]  kthread+0x224/0x230
>>>> [    8.833491]  ret_from_fork+0x10/0x30
>>>> [    8.834150] ---[ end trace 472836f79c15516b ]---
>>>>
>>>> This warning comes from __fragmentation_index() when the requested order
>>>> is greater than MAX_ORDER.
>>>>
>>>> static int __fragmentation_index(unsigned int order,
>>>>                   struct contig_page_info *info)
>>>> {
>>>>           unsigned long requested = 1UL << order;
>>>>
>>>>           if (WARN_ON_ONCE(order >= MAX_ORDER)) <===== Triggered here
>>>>                   return 0;
>>>>
>>>> Digging it further reveals that pageblock_order has been assigned a value
>>>> which is greater than MAX_ORDER failing the above check. But why this
>>>> happened ? Because HUGETLB_PAGE_ORDER for the given config on arm64 is
>>>> greater than MAX_ORDER.
>>>>
>>>> The solution involves enabling HUGETLB_PAGE_SIZE_VARIABLE which would make
>>>> pageblock_order a variable instead of constant HUGETLB_PAGE_ORDER. But that
>>>> change alone also did not really work as pageblock_order still got assigned
>>>> as HUGETLB_PAGE_ORDER in set_pageblock_order(). HUGETLB_PAGE_ORDER needs to
>>>> be less than MAX_ORDER for its appropriateness as pageblock_order otherwise
>>>> just fallback to MAX_ORDER - 1 as before. While here it also fixes a build
>>>> problem via type casting MAX_ORDER in rmem_cma_setup().
>>>
>>> I'm wondering, is there any real value in allowing FORCE_MAX_ZONEORDER to be "11" with ARM64_64K_PAGES/ARM64_16K_PAGES?
>>
>> MAX_ORDER should be as high as would be required for the current config.
>> Unless THP is enabled, there is no need for it to be any higher than 11.
>> But I might be missing historical reasons around this as well. Probably
>> others from arm64 could help here.
> 
> Theoretically yes, practically no. If nobody cares about a configuration, no need to make the code more complicated for that configuration.
> 
>>
>>>
>>> Meaning: are there any real use cases that actually build a kernel without TRANSPARENT_HUGEPAGE and with ARM64_64K_PAGES/ARM64_16K_PAGES?
>>
>> THP is always optional. Besides kernel builds without THP should always
>> be supported. Assuming that all builds will have THP enabled, might not
>> be accurate.
>>
>>>
>>> As builds are essentially broken, I assume this is not that relevant? Or how long has it been broken?
>>
>> Git blame shows that it's been there for some time now. But how does
>> that make this irrelevant ? A problem should be fixed nonetheless.
> 
> When exactly did I say not to fix it? I'm saying if nobody uses it, we might be able to simplify.

I might have misunderstood the relevance of a problematic configuration
wrt the need for it to be fixed. My bad, never mind :)

> 
>>
>>>
>>> It might be easier to just drop the "TRANSPARENT_HUGEPAGE" part from the FORCE_MAX_ZONEORDER config.
>>>
>>
>> Not sure if it would be a good idea to unnecessarily have larger MAX_ORDER
>> value for a given config. But I might be missing other contexts here.
> 
> My point is: keep it simple if there is no need to make it complicated. If these arm64 variants are the only cases where we run into that issue and nobody uses them ("hat it's been there for some time now"), why make stuff complicated?
> 
> The current code seems to assume that HUGETLB_PAGE_ORDER <= MAX_ORDER. Instead of changing that for optimizing an unused use case (it is broken), just simplify the arm64 conditions. I'd even say add a

Will try and change MAX_ORDER on arm64.

> 
> /*
>  * Some code assumes that HUGETLB_PAGE_ORDER <= MAX_ORDER. For now, only
>  * !TRANSPARENT_HUGEPAGE could lead in archs creating such setups.
>  * Forbid it for now; anybody that has a valid use case has to sanitize
>  * the code.
>  */
> BUILD_BUG_ON(HUGETLB_PAGE_ORDER <= MAX_ORDER);

Also add another, might have to find a proper place to put these.

BUILD_BUG_ON(HPAGE_PMD_ORDER <= MAX_ORDER)

> 
> 
> But again, if there are valid use cases then sure, let's make the code fully compatible with HUGETLB_PAGE_ORDER > MAX_ORDER.

Given that gigantic HugeTLB allocation can fallback on alloc_contig_pages()
or CMA if/when available, is there a real need for HUGETLB_PAGE_ORDER to be
upto MAX_ORDER, used as pageblock_order or as COMPACTION_HPAGE_ORDER ? With
gigantic HugeTLB pages being available, HUGETLB_PAGE_ORDER seems to be just
detached from the buddy allocator. But I am not sure, will keep looking.

> 
> 
> BTW: can we be sure that you caught all issues? For example:
> 
> #define COMPACTION_HPAGE_ORDER  HUGETLB_PAGE_ORDER
> 
> I'm not sure if it will work as expected if HUGETLB_PAGE_ORDER > MAX_ORDER ... fill_contig_page_info() will never find any suitable free blocks ...
> 

Right, there is an assumption that COMPACTION_HPAGE_ORDER < MAX_ORDER in
fill_contig_page_info() and will never find any suitable free blocks. This
along with pageblock_order needs to change when HUGETLB_PAGE_ORDER can be
greater than equal to MAX_ORDER.

- Anshuman


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/3] mm/page_alloc: Fix pageblock_order with HUGETLB_PAGE_SIZE_VARIABLE
  2021-02-16  9:02       ` Anshuman Khandual
@ 2021-02-16  9:04         ` David Hildenbrand
  0 siblings, 0 replies; 15+ messages in thread
From: David Hildenbrand @ 2021-02-16  9:04 UTC (permalink / raw)
  To: Anshuman Khandual, linux-mm, linux-arm-kernel, catalin.marinas,
	akpm, will
  Cc: Robin Murphy, Marek Szyprowski, Christoph Hellwig, Mark Rutland,
	iommu, linux-kernel

>>
>>
>> But again, if there are valid use cases then sure, let's make the code fully compatible with HUGETLB_PAGE_ORDER > MAX_ORDER.
> 
> Given that gigantic HugeTLB allocation can fallback on alloc_contig_pages()
> or CMA if/when available, is there a real need for HUGETLB_PAGE_ORDER to be
> upto MAX_ORDER, used as pageblock_order or as COMPACTION_HPAGE_ORDER ? With
> gigantic HugeTLB pages being available, HUGETLB_PAGE_ORDER seems to be just
> detached from the buddy allocator. But I am not sure, will keep looking.

Having HPAGE_PMD_ORDER >>= MAX_ORDER ("significantly larger") will make 
it very unlikely that you are able to reliably allocate any huge pages 
at all dynamically at runtime without CMA.

Gigantic pages are problematic by nature :)

-- 
Thanks,

David / dhildenb



^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2021-02-16  9:04 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-11  6:22 [PATCH 0/3] mm/page_alloc: Fix pageblock_order with HUGETLB_PAGE_SIZE_VARIABLE Anshuman Khandual
2021-02-11  6:22 ` [PATCH 1/3] mm/page_alloc: Fix pageblock_order when HUGETLB_PAGE_ORDER >= MAX_ORDER Anshuman Khandual
2021-02-11  8:00   ` Christoph Hellwig
2021-02-12  7:12     ` Anshuman Khandual
2021-02-11  6:22 ` [PATCH 2/3] arm64/hugetlb: Enable HUGETLB_PAGE_SIZE_VARIABLE Anshuman Khandual
2021-02-11  8:01   ` Christoph Hellwig
2021-02-12  7:13     ` Anshuman Khandual
2021-02-11  6:22 ` [PATCH 3/3] dma-contiguous: Type cast MAX_ORDER as unsigned int Anshuman Khandual
2021-02-11  8:04   ` Christoph Hellwig
2021-02-12  7:19     ` Anshuman Khandual
2021-02-11  8:37 ` [PATCH 0/3] mm/page_alloc: Fix pageblock_order with HUGETLB_PAGE_SIZE_VARIABLE David Hildenbrand
2021-02-12  7:02   ` Anshuman Khandual
2021-02-12  9:39     ` David Hildenbrand
2021-02-16  9:02       ` Anshuman Khandual
2021-02-16  9:04         ` David Hildenbrand

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