linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] arm64: Make the ARCH_FORCE_MAX_ORDER config input prompt unconditional
@ 2023-05-03 12:33 Catalin Marinas
  2023-05-03 14:58 ` Mike Rapoport
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Catalin Marinas @ 2023-05-03 12:33 UTC (permalink / raw)
  To: Will Deacon
  Cc: linux-arm-kernel, Justin M . Forbes, Mike Rapoport, Andrew Morton

Commit 34affcd7577a ("arm64: drop ranges in definition of
ARCH_FORCE_MAX_ORDER") dropped the ranges from the config entry and
introduced an EXPERT condition on the input prompt instead. This change
may affect some distro kernels that change ARCH_FORCE_MAX_ORDER but do
not want to enable EXPERT.

Drop EXPERT from the input prompt together with the (ARM64_4K_PAGES ||
ARM64_16K_PAGES) condition as the latter no longer makes sense after the
ranges were removed. The latter makes all the page size configurations
consistent w.r.t. ARCH_FORCE_MAX_ORDER.

Fixes: 34affcd7577a ("arm64: drop ranges in definition of ARCH_FORCE_MAX_ORDER")
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
Reported-by: Justin M. Forbes <jforbes@fedoraproject.org>
Cc: Will Deacon <will@kernel.org>
Cc: Mike Rapoport <rppt@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 arch/arm64/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index b1201d25a8a4..1867aba83ba3 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1516,7 +1516,7 @@ config XEN
 # 16K |       27          |      14      |       13        |         11         |
 # 64K |       29          |      16      |       13        |         13         |
 config ARCH_FORCE_MAX_ORDER
-	int "Order of maximal physically contiguous allocations" if EXPERT && (ARM64_4K_PAGES || ARM64_16K_PAGES)
+	int "Order of maximal physically contiguous allocations"
 	default "13" if ARM64_64K_PAGES
 	default "11" if ARM64_16K_PAGES
 	default "10"

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] arm64: Make the ARCH_FORCE_MAX_ORDER config input prompt unconditional
  2023-05-03 12:33 [PATCH] arm64: Make the ARCH_FORCE_MAX_ORDER config input prompt unconditional Catalin Marinas
@ 2023-05-03 14:58 ` Mike Rapoport
  2023-05-03 15:35 ` Justin Forbes
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 13+ messages in thread
From: Mike Rapoport @ 2023-05-03 14:58 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: Will Deacon, linux-arm-kernel, Justin M . Forbes, Andrew Morton

On Wed, May 03, 2023 at 01:33:42PM +0100, Catalin Marinas wrote:
> Commit 34affcd7577a ("arm64: drop ranges in definition of
> ARCH_FORCE_MAX_ORDER") dropped the ranges from the config entry and
> introduced an EXPERT condition on the input prompt instead. This change
> may affect some distro kernels that change ARCH_FORCE_MAX_ORDER but do
> not want to enable EXPERT.
> 
> Drop EXPERT from the input prompt together with the (ARM64_4K_PAGES ||
> ARM64_16K_PAGES) condition as the latter no longer makes sense after the
> ranges were removed. The latter makes all the page size configurations
> consistent w.r.t. ARCH_FORCE_MAX_ORDER.
> 
> Fixes: 34affcd7577a ("arm64: drop ranges in definition of ARCH_FORCE_MAX_ORDER")
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> Reported-by: Justin M. Forbes <jforbes@fedoraproject.org>
> Cc: Will Deacon <will@kernel.org>
> Cc: Mike Rapoport <rppt@kernel.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>

Acked-by: Mike Rapoport (IBM) <rppt@kernel.org>

> ---
>  arch/arm64/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index b1201d25a8a4..1867aba83ba3 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1516,7 +1516,7 @@ config XEN
>  # 16K |       27          |      14      |       13        |         11         |
>  # 64K |       29          |      16      |       13        |         13         |
>  config ARCH_FORCE_MAX_ORDER
> -	int "Order of maximal physically contiguous allocations" if EXPERT && (ARM64_4K_PAGES || ARM64_16K_PAGES)
> +	int "Order of maximal physically contiguous allocations"
>  	default "13" if ARM64_64K_PAGES
>  	default "11" if ARM64_16K_PAGES
>  	default "10"

-- 
Sincerely yours,
Mike.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] arm64: Make the ARCH_FORCE_MAX_ORDER config input prompt unconditional
  2023-05-03 12:33 [PATCH] arm64: Make the ARCH_FORCE_MAX_ORDER config input prompt unconditional Catalin Marinas
  2023-05-03 14:58 ` Mike Rapoport
@ 2023-05-03 15:35 ` Justin Forbes
  2023-05-03 15:41   ` Ard Biesheuvel
  2023-05-16 15:14 ` Will Deacon
  2023-05-18 15:56 ` Marc Zyngier
  3 siblings, 1 reply; 13+ messages in thread
From: Justin Forbes @ 2023-05-03 15:35 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: Will Deacon, linux-arm-kernel, Mike Rapoport, Andrew Morton

On Wed, May 3, 2023 at 7:33 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> Commit 34affcd7577a ("arm64: drop ranges in definition of
> ARCH_FORCE_MAX_ORDER") dropped the ranges from the config entry and
> introduced an EXPERT condition on the input prompt instead. This change
> may affect some distro kernels that change ARCH_FORCE_MAX_ORDER but do
> not want to enable EXPERT.
>
> Drop EXPERT from the input prompt together with the (ARM64_4K_PAGES ||
> ARM64_16K_PAGES) condition as the latter no longer makes sense after the
> ranges were removed. The latter makes all the page size configurations
> consistent w.r.t. ARCH_FORCE_MAX_ORDER.
>
> Fixes: 34affcd7577a ("arm64: drop ranges in definition of ARCH_FORCE_MAX_ORDER")
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> Reported-by: Justin M. Forbes <jforbes@fedoraproject.org>
> Cc: Will Deacon <will@kernel.org>
> Cc: Mike Rapoport <rppt@kernel.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>

This works for me, thanks.

Acked-by: Justin M. Forbes <jforbes@fedoraproject.org>

>  arch/arm64/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index b1201d25a8a4..1867aba83ba3 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1516,7 +1516,7 @@ config XEN
>  # 16K |       27          |      14      |       13        |         11         |
>  # 64K |       29          |      16      |       13        |         13         |
>  config ARCH_FORCE_MAX_ORDER
> -       int "Order of maximal physically contiguous allocations" if EXPERT && (ARM64_4K_PAGES || ARM64_16K_PAGES)
> +       int "Order of maximal physically contiguous allocations"
>         default "13" if ARM64_64K_PAGES
>         default "11" if ARM64_16K_PAGES
>         default "10"
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] arm64: Make the ARCH_FORCE_MAX_ORDER config input prompt unconditional
  2023-05-03 15:35 ` Justin Forbes
@ 2023-05-03 15:41   ` Ard Biesheuvel
  2023-05-05 22:00     ` Mike Rapoport
  0 siblings, 1 reply; 13+ messages in thread
From: Ard Biesheuvel @ 2023-05-03 15:41 UTC (permalink / raw)
  To: Justin Forbes
  Cc: Catalin Marinas, Will Deacon, linux-arm-kernel, Mike Rapoport,
	Andrew Morton

On Wed, 3 May 2023 at 17:36, Justin Forbes <jforbes@fedoraproject.org> wrote:
>
> On Wed, May 3, 2023 at 7:33 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
> >
> > Commit 34affcd7577a ("arm64: drop ranges in definition of
> > ARCH_FORCE_MAX_ORDER") dropped the ranges from the config entry and
> > introduced an EXPERT condition on the input prompt instead. This change
> > may affect some distro kernels that change ARCH_FORCE_MAX_ORDER but do
> > not want to enable EXPERT.
> >
> > Drop EXPERT from the input prompt together with the (ARM64_4K_PAGES ||
> > ARM64_16K_PAGES) condition as the latter no longer makes sense after the
> > ranges were removed. The latter makes all the page size configurations
> > consistent w.r.t. ARCH_FORCE_MAX_ORDER.
> >
> > Fixes: 34affcd7577a ("arm64: drop ranges in definition of ARCH_FORCE_MAX_ORDER")
> > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> > Reported-by: Justin M. Forbes <jforbes@fedoraproject.org>
> > Cc: Will Deacon <will@kernel.org>
> > Cc: Mike Rapoport <rppt@kernel.org>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
>
> This works for me, thanks.
>
> Acked-by: Justin M. Forbes <jforbes@fedoraproject.org>
>

I'd still be interested in gaining a better understanding as to why
Fedora/RHEL think they need to change this value on arm64. In
particular, whether it is to support ThunderX, or whether there are
any good reasons for doing so that we are unaware of.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] arm64: Make the ARCH_FORCE_MAX_ORDER config input prompt unconditional
  2023-05-03 15:41   ` Ard Biesheuvel
@ 2023-05-05 22:00     ` Mike Rapoport
  2023-05-05 22:08       ` Ard Biesheuvel
  0 siblings, 1 reply; 13+ messages in thread
From: Mike Rapoport @ 2023-05-05 22:00 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Justin Forbes, Catalin Marinas, Will Deacon, linux-arm-kernel,
	Andrew Morton

On Wed, May 03, 2023 at 05:41:19PM +0200, Ard Biesheuvel wrote:
> On Wed, 3 May 2023 at 17:36, Justin Forbes <jforbes@fedoraproject.org> wrote:
> >
> > On Wed, May 3, 2023 at 7:33 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > >
> > > Commit 34affcd7577a ("arm64: drop ranges in definition of
> > > ARCH_FORCE_MAX_ORDER") dropped the ranges from the config entry and
> > > introduced an EXPERT condition on the input prompt instead. This change
> > > may affect some distro kernels that change ARCH_FORCE_MAX_ORDER but do
> > > not want to enable EXPERT.
> > >
> > > Drop EXPERT from the input prompt together with the (ARM64_4K_PAGES ||
> > > ARM64_16K_PAGES) condition as the latter no longer makes sense after the
> > > ranges were removed. The latter makes all the page size configurations
> > > consistent w.r.t. ARCH_FORCE_MAX_ORDER.
> > >
> > > Fixes: 34affcd7577a ("arm64: drop ranges in definition of ARCH_FORCE_MAX_ORDER")
> > > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> > > Reported-by: Justin M. Forbes <jforbes@fedoraproject.org>
> > > Cc: Will Deacon <will@kernel.org>
> > > Cc: Mike Rapoport <rppt@kernel.org>
> > > Cc: Andrew Morton <akpm@linux-foundation.org>
> >
> > This works for me, thanks.
> >
> > Acked-by: Justin M. Forbes <jforbes@fedoraproject.org>
> >
> 
> I'd still be interested in gaining a better understanding as to why
> Fedora/RHEL think they need to change this value on arm64. In
> particular, whether it is to support ThunderX, or whether there are
> any good reasons for doing so that we are unaware of.

Yeah, there still was no explanation why Fedora/RHEL had to increase
MAX_ORDER in their configs.

I'm surely missing something, but I also don't understand why ThunderX
would need physically contiguous allocations larger than 4M.

-- 
Sincerely yours,
Mike.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] arm64: Make the ARCH_FORCE_MAX_ORDER config input prompt unconditional
  2023-05-05 22:00     ` Mike Rapoport
@ 2023-05-05 22:08       ` Ard Biesheuvel
  2023-05-05 22:47         ` Mike Rapoport
  0 siblings, 1 reply; 13+ messages in thread
From: Ard Biesheuvel @ 2023-05-05 22:08 UTC (permalink / raw)
  To: Mike Rapoport
  Cc: Justin Forbes, Catalin Marinas, Will Deacon, linux-arm-kernel,
	Andrew Morton

On Sat, 6 May 2023 at 00:01, Mike Rapoport <rppt@kernel.org> wrote:
>
> On Wed, May 03, 2023 at 05:41:19PM +0200, Ard Biesheuvel wrote:
> > On Wed, 3 May 2023 at 17:36, Justin Forbes <jforbes@fedoraproject.org> wrote:
> > >
> > > On Wed, May 3, 2023 at 7:33 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > > >
> > > > Commit 34affcd7577a ("arm64: drop ranges in definition of
> > > > ARCH_FORCE_MAX_ORDER") dropped the ranges from the config entry and
> > > > introduced an EXPERT condition on the input prompt instead. This change
> > > > may affect some distro kernels that change ARCH_FORCE_MAX_ORDER but do
> > > > not want to enable EXPERT.
> > > >
> > > > Drop EXPERT from the input prompt together with the (ARM64_4K_PAGES ||
> > > > ARM64_16K_PAGES) condition as the latter no longer makes sense after the
> > > > ranges were removed. The latter makes all the page size configurations
> > > > consistent w.r.t. ARCH_FORCE_MAX_ORDER.
> > > >
> > > > Fixes: 34affcd7577a ("arm64: drop ranges in definition of ARCH_FORCE_MAX_ORDER")
> > > > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> > > > Reported-by: Justin M. Forbes <jforbes@fedoraproject.org>
> > > > Cc: Will Deacon <will@kernel.org>
> > > > Cc: Mike Rapoport <rppt@kernel.org>
> > > > Cc: Andrew Morton <akpm@linux-foundation.org>
> > >
> > > This works for me, thanks.
> > >
> > > Acked-by: Justin M. Forbes <jforbes@fedoraproject.org>
> > >
> >
> > I'd still be interested in gaining a better understanding as to why
> > Fedora/RHEL think they need to change this value on arm64. In
> > particular, whether it is to support ThunderX, or whether there are
> > any good reasons for doing so that we are unaware of.
>
> Yeah, there still was no explanation why Fedora/RHEL had to increase
> MAX_ORDER in their configs.
>
> I'm surely missing something, but I also don't understand why ThunderX
> would need physically contiguous allocations larger than 4M.
>

https://lore.kernel.org/all/1430686172-18222-5-git-send-email-rric@kernel.org/

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] arm64: Make the ARCH_FORCE_MAX_ORDER config input prompt unconditional
  2023-05-05 22:08       ` Ard Biesheuvel
@ 2023-05-05 22:47         ` Mike Rapoport
  2023-05-05 22:51           ` Ard Biesheuvel
  0 siblings, 1 reply; 13+ messages in thread
From: Mike Rapoport @ 2023-05-05 22:47 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Justin Forbes, Catalin Marinas, Will Deacon, linux-arm-kernel,
	Andrew Morton

On Sat, May 06, 2023 at 12:08:33AM +0200, Ard Biesheuvel wrote:
> On Sat, 6 May 2023 at 00:01, Mike Rapoport <rppt@kernel.org> wrote:
> >
> > On Wed, May 03, 2023 at 05:41:19PM +0200, Ard Biesheuvel wrote:
> > > On Wed, 3 May 2023 at 17:36, Justin Forbes <jforbes@fedoraproject.org> wrote:
> > > >
> > > > On Wed, May 3, 2023 at 7:33 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > > > >
> > > > > Commit 34affcd7577a ("arm64: drop ranges in definition of
> > > > > ARCH_FORCE_MAX_ORDER") dropped the ranges from the config entry and
> > > > > introduced an EXPERT condition on the input prompt instead. This change
> > > > > may affect some distro kernels that change ARCH_FORCE_MAX_ORDER but do
> > > > > not want to enable EXPERT.
> > > > >
> > > > > Drop EXPERT from the input prompt together with the (ARM64_4K_PAGES ||
> > > > > ARM64_16K_PAGES) condition as the latter no longer makes sense after the
> > > > > ranges were removed. The latter makes all the page size configurations
> > > > > consistent w.r.t. ARCH_FORCE_MAX_ORDER.
> > > > >
> > > > > Fixes: 34affcd7577a ("arm64: drop ranges in definition of ARCH_FORCE_MAX_ORDER")
> > > > > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> > > > > Reported-by: Justin M. Forbes <jforbes@fedoraproject.org>
> > > > > Cc: Will Deacon <will@kernel.org>
> > > > > Cc: Mike Rapoport <rppt@kernel.org>
> > > > > Cc: Andrew Morton <akpm@linux-foundation.org>
> > > >
> > > > This works for me, thanks.
> > > >
> > > > Acked-by: Justin M. Forbes <jforbes@fedoraproject.org>
> > > >
> > >
> > > I'd still be interested in gaining a better understanding as to why
> > > Fedora/RHEL think they need to change this value on arm64. In
> > > particular, whether it is to support ThunderX, or whether there are
> > > any good reasons for doing so that we are unaware of.
> >
> > Yeah, there still was no explanation why Fedora/RHEL had to increase
> > MAX_ORDER in their configs.
> >
> > I'm surely missing something, but I also don't understand why ThunderX
> > would need physically contiguous allocations larger than 4M.
> 
> https://lore.kernel.org/all/1430686172-18222-5-git-send-email-rric@kernel.org/

But does not the second patch in that series (now commit 30f2136346ca
("irqchip/gicv3-its: Add range check for number of allocated pages"))
ensures that allocation is not larger than 256 pages?

Or this is another allocation?

-- 
Sincerely yours,
Mike.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] arm64: Make the ARCH_FORCE_MAX_ORDER config input prompt unconditional
  2023-05-05 22:47         ` Mike Rapoport
@ 2023-05-05 22:51           ` Ard Biesheuvel
  2023-05-05 23:23             ` Mike Rapoport
  0 siblings, 1 reply; 13+ messages in thread
From: Ard Biesheuvel @ 2023-05-05 22:51 UTC (permalink / raw)
  To: Mike Rapoport
  Cc: Justin Forbes, Catalin Marinas, Will Deacon, linux-arm-kernel,
	Andrew Morton

On Sat, 6 May 2023 at 00:47, Mike Rapoport <rppt@kernel.org> wrote:
>
> On Sat, May 06, 2023 at 12:08:33AM +0200, Ard Biesheuvel wrote:
> > On Sat, 6 May 2023 at 00:01, Mike Rapoport <rppt@kernel.org> wrote:
> > >
> > > On Wed, May 03, 2023 at 05:41:19PM +0200, Ard Biesheuvel wrote:
> > > > On Wed, 3 May 2023 at 17:36, Justin Forbes <jforbes@fedoraproject.org> wrote:
> > > > >
> > > > > On Wed, May 3, 2023 at 7:33 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > > > > >
> > > > > > Commit 34affcd7577a ("arm64: drop ranges in definition of
> > > > > > ARCH_FORCE_MAX_ORDER") dropped the ranges from the config entry and
> > > > > > introduced an EXPERT condition on the input prompt instead. This change
> > > > > > may affect some distro kernels that change ARCH_FORCE_MAX_ORDER but do
> > > > > > not want to enable EXPERT.
> > > > > >
> > > > > > Drop EXPERT from the input prompt together with the (ARM64_4K_PAGES ||
> > > > > > ARM64_16K_PAGES) condition as the latter no longer makes sense after the
> > > > > > ranges were removed. The latter makes all the page size configurations
> > > > > > consistent w.r.t. ARCH_FORCE_MAX_ORDER.
> > > > > >
> > > > > > Fixes: 34affcd7577a ("arm64: drop ranges in definition of ARCH_FORCE_MAX_ORDER")
> > > > > > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> > > > > > Reported-by: Justin M. Forbes <jforbes@fedoraproject.org>
> > > > > > Cc: Will Deacon <will@kernel.org>
> > > > > > Cc: Mike Rapoport <rppt@kernel.org>
> > > > > > Cc: Andrew Morton <akpm@linux-foundation.org>
> > > > >
> > > > > This works for me, thanks.
> > > > >
> > > > > Acked-by: Justin M. Forbes <jforbes@fedoraproject.org>
> > > > >
> > > >
> > > > I'd still be interested in gaining a better understanding as to why
> > > > Fedora/RHEL think they need to change this value on arm64. In
> > > > particular, whether it is to support ThunderX, or whether there are
> > > > any good reasons for doing so that we are unaware of.
> > >
> > > Yeah, there still was no explanation why Fedora/RHEL had to increase
> > > MAX_ORDER in their configs.
> > >
> > > I'm surely missing something, but I also don't understand why ThunderX
> > > would need physically contiguous allocations larger than 4M.
> >
> > https://lore.kernel.org/all/1430686172-18222-5-git-send-email-rric@kernel.org/
>
> But does not the second patch in that series (now commit 30f2136346ca
> ("irqchip/gicv3-its: Add range check for number of allocated pages"))
> ensures that allocation is not larger than 256 pages?
>
> Or this is another allocation?
>

I have no idea, but that it not really the point.

The point is that ThunderX is obsolete - it was never a very
compelling value proposition in the first place, but today it is just
a waste of electricity. So if it was the only reason for changing the
max order, perhaps it's time to change it back?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] arm64: Make the ARCH_FORCE_MAX_ORDER config input prompt unconditional
  2023-05-05 22:51           ` Ard Biesheuvel
@ 2023-05-05 23:23             ` Mike Rapoport
  0 siblings, 0 replies; 13+ messages in thread
From: Mike Rapoport @ 2023-05-05 23:23 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Justin Forbes, Catalin Marinas, Will Deacon, linux-arm-kernel,
	Andrew Morton

On Sat, May 06, 2023 at 12:51:52AM +0200, Ard Biesheuvel wrote:
> On Sat, 6 May 2023 at 00:47, Mike Rapoport <rppt@kernel.org> wrote:
> >
> > On Sat, May 06, 2023 at 12:08:33AM +0200, Ard Biesheuvel wrote:
> > > On Sat, 6 May 2023 at 00:01, Mike Rapoport <rppt@kernel.org> wrote:
> > > >
> > > > On Wed, May 03, 2023 at 05:41:19PM +0200, Ard Biesheuvel wrote:
> > > > > On Wed, 3 May 2023 at 17:36, Justin Forbes <jforbes@fedoraproject.org> wrote:
> > > > > >
> > > > > > On Wed, May 3, 2023 at 7:33 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > > > > > >
> > > > > > > Commit 34affcd7577a ("arm64: drop ranges in definition of
> > > > > > > ARCH_FORCE_MAX_ORDER") dropped the ranges from the config entry and
> > > > > > > introduced an EXPERT condition on the input prompt instead. This change
> > > > > > > may affect some distro kernels that change ARCH_FORCE_MAX_ORDER but do
> > > > > > > not want to enable EXPERT.
> > > > > > >
> > > > > > > Drop EXPERT from the input prompt together with the (ARM64_4K_PAGES ||
> > > > > > > ARM64_16K_PAGES) condition as the latter no longer makes sense after the
> > > > > > > ranges were removed. The latter makes all the page size configurations
> > > > > > > consistent w.r.t. ARCH_FORCE_MAX_ORDER.
> > > > > > >
> > > > > > > Fixes: 34affcd7577a ("arm64: drop ranges in definition of ARCH_FORCE_MAX_ORDER")
> > > > > > > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> > > > > > > Reported-by: Justin M. Forbes <jforbes@fedoraproject.org>
> > > > > > > Cc: Will Deacon <will@kernel.org>
> > > > > > > Cc: Mike Rapoport <rppt@kernel.org>
> > > > > > > Cc: Andrew Morton <akpm@linux-foundation.org>
> > > > > >
> > > > > > This works for me, thanks.
> > > > > >
> > > > > > Acked-by: Justin M. Forbes <jforbes@fedoraproject.org>
> > > > > >
> > > > >
> > > > > I'd still be interested in gaining a better understanding as to why
> > > > > Fedora/RHEL think they need to change this value on arm64. In
> > > > > particular, whether it is to support ThunderX, or whether there are
> > > > > any good reasons for doing so that we are unaware of.
> > > >
> > > > Yeah, there still was no explanation why Fedora/RHEL had to increase
> > > > MAX_ORDER in their configs.
> > > >
> > > > I'm surely missing something, but I also don't understand why ThunderX
> > > > would need physically contiguous allocations larger than 4M.
> > >
> > > https://lore.kernel.org/all/1430686172-18222-5-git-send-email-rric@kernel.org/
> >
> > But does not the second patch in that series (now commit 30f2136346ca
> > ("irqchip/gicv3-its: Add range check for number of allocated pages"))
> > ensures that allocation is not larger than 256 pages?
> >
> > Or this is another allocation?
> >
> 
> I have no idea, but that it not really the point.
> 
> The point is that ThunderX is obsolete - it was never a very
> compelling value proposition in the first place, but today it is just
> a waste of electricity. So if it was the only reason for changing the
> max order, perhaps it's time to change it back?

Well, that's up to Fedora/RHEL to drop their changes to the configuration.
Since Kirill's changes to MAX_ORDER semantics they will have to update
their config anyway for 6.4 and onwards.

-- 
Sincerely yours,
Mike.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] arm64: Make the ARCH_FORCE_MAX_ORDER config input prompt unconditional
  2023-05-03 12:33 [PATCH] arm64: Make the ARCH_FORCE_MAX_ORDER config input prompt unconditional Catalin Marinas
  2023-05-03 14:58 ` Mike Rapoport
  2023-05-03 15:35 ` Justin Forbes
@ 2023-05-16 15:14 ` Will Deacon
  2023-05-18 15:56 ` Marc Zyngier
  3 siblings, 0 replies; 13+ messages in thread
From: Will Deacon @ 2023-05-16 15:14 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: kernel-team, Will Deacon, Andrew Morton, Justin M . Forbes,
	Mike Rapoport, linux-arm-kernel

On Wed, 3 May 2023 13:33:42 +0100, Catalin Marinas wrote:
> Commit 34affcd7577a ("arm64: drop ranges in definition of
> ARCH_FORCE_MAX_ORDER") dropped the ranges from the config entry and
> introduced an EXPERT condition on the input prompt instead. This change
> may affect some distro kernels that change ARCH_FORCE_MAX_ORDER but do
> not want to enable EXPERT.
> 
> Drop EXPERT from the input prompt together with the (ARM64_4K_PAGES ||
> ARM64_16K_PAGES) condition as the latter no longer makes sense after the
> ranges were removed. The latter makes all the page size configurations
> consistent w.r.t. ARCH_FORCE_MAX_ORDER.
> 
> [...]

Applied to arm64 (for-next/fixes), thanks!

[1/1] arm64: Make the ARCH_FORCE_MAX_ORDER config input prompt unconditional
      https://git.kernel.org/arm64/c/fd2d1cb8c545

Cheers,
-- 
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] arm64: Make the ARCH_FORCE_MAX_ORDER config input prompt unconditional
  2023-05-03 12:33 [PATCH] arm64: Make the ARCH_FORCE_MAX_ORDER config input prompt unconditional Catalin Marinas
                   ` (2 preceding siblings ...)
  2023-05-16 15:14 ` Will Deacon
@ 2023-05-18 15:56 ` Marc Zyngier
  2023-05-18 16:04   ` Catalin Marinas
  3 siblings, 1 reply; 13+ messages in thread
From: Marc Zyngier @ 2023-05-18 15:56 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: Will Deacon, linux-arm-kernel, Justin M . Forbes, Mike Rapoport,
	Andrew Morton

On Wed, 03 May 2023 13:33:42 +0100,
Catalin Marinas <catalin.marinas@arm.com> wrote:
> 
> Commit 34affcd7577a ("arm64: drop ranges in definition of
> ARCH_FORCE_MAX_ORDER") dropped the ranges from the config entry and
> introduced an EXPERT condition on the input prompt instead. This change
> may affect some distro kernels that change ARCH_FORCE_MAX_ORDER but do
> not want to enable EXPERT.
> 
> Drop EXPERT from the input prompt together with the (ARM64_4K_PAGES ||
> ARM64_16K_PAGES) condition as the latter no longer makes sense after the
> ranges were removed. The latter makes all the page size configurations
> consistent w.r.t. ARCH_FORCE_MAX_ORDER.
> 
> Fixes: 34affcd7577a ("arm64: drop ranges in definition of ARCH_FORCE_MAX_ORDER")
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> Reported-by: Justin M. Forbes <jforbes@fedoraproject.org>
> Cc: Will Deacon <will@kernel.org>
> Cc: Mike Rapoport <rppt@kernel.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> ---
>  arch/arm64/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index b1201d25a8a4..1867aba83ba3 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1516,7 +1516,7 @@ config XEN
>  # 16K |       27          |      14      |       13        |         11         |
>  # 64K |       29          |      16      |       13        |         13         |
>  config ARCH_FORCE_MAX_ORDER
> -	int "Order of maximal physically contiguous allocations" if EXPERT && (ARM64_4K_PAGES || ARM64_16K_PAGES)
> +	int "Order of maximal physically contiguous allocations"
>  	default "13" if ARM64_64K_PAGES
>  	default "11" if ARM64_16K_PAGES
>  	default "10"
> 

This patch (and the previous one) has the unfortunate side effect of
completely breaking a change of page size (from 4k to 16k, for
example):

<quote>
maz@valley-girl:~/hot-poop/arm-platforms$ make defconfig
*** Default configuration is based on 'defconfig'
#
# configuration written to .config
#
maz@valley-girl:~/hot-poop/arm-platforms$ egrep 'PAGE_SHIFT|MAX_ORDER' .config
CONFIG_ARM64_PAGE_SHIFT=12
CONFIG_ARCH_FORCE_MAX_ORDER=10
maz@valley-girl:~/hot-poop/arm-platforms$ make menuconfig
configuration written to .config

*** End of the configuration.
*** Execute 'make' to start the build or try 'make help'.

maz@valley-girl:~/hot-poop/arm-platforms$ egrep 'PAGE_SHIFT|MAX_ORDER' .config
CONFIG_ARM64_PAGE_SHIFT=14
CONFIG_ARCH_FORCE_MAX_ORDER=10
</quote>

The build then fails in ways that aren't obvious (BUILD_BUG in the THP
code). It would much better if the result of the configuration tool
would produce something that can actually build.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] arm64: Make the ARCH_FORCE_MAX_ORDER config input prompt unconditional
  2023-05-18 15:56 ` Marc Zyngier
@ 2023-05-18 16:04   ` Catalin Marinas
  2023-05-19 10:35     ` Will Deacon
  0 siblings, 1 reply; 13+ messages in thread
From: Catalin Marinas @ 2023-05-18 16:04 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Will Deacon, linux-arm-kernel, Justin M . Forbes, Mike Rapoport,
	Andrew Morton

On Thu, May 18, 2023 at 04:56:28PM +0100, Marc Zyngier wrote:
> On Wed, 03 May 2023 13:33:42 +0100,
> Catalin Marinas <catalin.marinas@arm.com> wrote:
> > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > index b1201d25a8a4..1867aba83ba3 100644
> > --- a/arch/arm64/Kconfig
> > +++ b/arch/arm64/Kconfig
> > @@ -1516,7 +1516,7 @@ config XEN
> >  # 16K |       27          |      14      |       13        |         11         |
> >  # 64K |       29          |      16      |       13        |         13         |
> >  config ARCH_FORCE_MAX_ORDER
> > -	int "Order of maximal physically contiguous allocations" if EXPERT && (ARM64_4K_PAGES || ARM64_16K_PAGES)
> > +	int "Order of maximal physically contiguous allocations"
> >  	default "13" if ARM64_64K_PAGES
> >  	default "11" if ARM64_16K_PAGES
> >  	default "10"
> 
> This patch (and the previous one) has the unfortunate side effect of
> completely breaking a change of page size (from 4k to 16k, for
> example):
> 
> <quote>
> maz@valley-girl:~/hot-poop/arm-platforms$ make defconfig
> *** Default configuration is based on 'defconfig'
> #
> # configuration written to .config
> #
> maz@valley-girl:~/hot-poop/arm-platforms$ egrep 'PAGE_SHIFT|MAX_ORDER' .config
> CONFIG_ARM64_PAGE_SHIFT=12
> CONFIG_ARCH_FORCE_MAX_ORDER=10
> maz@valley-girl:~/hot-poop/arm-platforms$ make menuconfig
> configuration written to .config
> 
> *** End of the configuration.
> *** Execute 'make' to start the build or try 'make help'.
> 
> maz@valley-girl:~/hot-poop/arm-platforms$ egrep 'PAGE_SHIFT|MAX_ORDER' .config
> CONFIG_ARM64_PAGE_SHIFT=14
> CONFIG_ARCH_FORCE_MAX_ORDER=10
> </quote>
> 
> The build then fails in ways that aren't obvious (BUILD_BUG in the THP
> code). It would much better if the result of the configuration tool
> would produce something that can actually build.

Thanks Marc for reporting this. The main culprit is the removal of the
ranges, so MAX_ORDER stays at 10 when changing the page size, but only
with EXPERT enabled. With this patch, it just generalises the problem
even without EXPERT.

So we either re-introduce the ranges or we drop the menuconfig entry
completely, regardless of EXPERT. I'm inclined to go with the latter,
just don't allow people to redefine this (still unclear to me if we need
a higher default with 4K pages).

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] arm64: Make the ARCH_FORCE_MAX_ORDER config input prompt unconditional
  2023-05-18 16:04   ` Catalin Marinas
@ 2023-05-19 10:35     ` Will Deacon
  0 siblings, 0 replies; 13+ messages in thread
From: Will Deacon @ 2023-05-19 10:35 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: Marc Zyngier, linux-arm-kernel, Justin M . Forbes, Mike Rapoport,
	Andrew Morton

On Thu, May 18, 2023 at 05:04:26PM +0100, Catalin Marinas wrote:
> On Thu, May 18, 2023 at 04:56:28PM +0100, Marc Zyngier wrote:
> > On Wed, 03 May 2023 13:33:42 +0100,
> > Catalin Marinas <catalin.marinas@arm.com> wrote:
> > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > > index b1201d25a8a4..1867aba83ba3 100644
> > > --- a/arch/arm64/Kconfig
> > > +++ b/arch/arm64/Kconfig
> > > @@ -1516,7 +1516,7 @@ config XEN
> > >  # 16K |       27          |      14      |       13        |         11         |
> > >  # 64K |       29          |      16      |       13        |         13         |
> > >  config ARCH_FORCE_MAX_ORDER
> > > -	int "Order of maximal physically contiguous allocations" if EXPERT && (ARM64_4K_PAGES || ARM64_16K_PAGES)
> > > +	int "Order of maximal physically contiguous allocations"
> > >  	default "13" if ARM64_64K_PAGES
> > >  	default "11" if ARM64_16K_PAGES
> > >  	default "10"
> > 
> > This patch (and the previous one) has the unfortunate side effect of
> > completely breaking a change of page size (from 4k to 16k, for
> > example):
> > 
> > <quote>
> > maz@valley-girl:~/hot-poop/arm-platforms$ make defconfig
> > *** Default configuration is based on 'defconfig'
> > #
> > # configuration written to .config
> > #
> > maz@valley-girl:~/hot-poop/arm-platforms$ egrep 'PAGE_SHIFT|MAX_ORDER' .config
> > CONFIG_ARM64_PAGE_SHIFT=12
> > CONFIG_ARCH_FORCE_MAX_ORDER=10
> > maz@valley-girl:~/hot-poop/arm-platforms$ make menuconfig
> > configuration written to .config
> > 
> > *** End of the configuration.
> > *** Execute 'make' to start the build or try 'make help'.
> > 
> > maz@valley-girl:~/hot-poop/arm-platforms$ egrep 'PAGE_SHIFT|MAX_ORDER' .config
> > CONFIG_ARM64_PAGE_SHIFT=14
> > CONFIG_ARCH_FORCE_MAX_ORDER=10
> > </quote>
> > 
> > The build then fails in ways that aren't obvious (BUILD_BUG in the THP
> > code). It would much better if the result of the configuration tool
> > would produce something that can actually build.
> 
> Thanks Marc for reporting this. The main culprit is the removal of the
> ranges, so MAX_ORDER stays at 10 when changing the page size, but only
> with EXPERT enabled. With this patch, it just generalises the problem
> even without EXPERT.
> 
> So we either re-introduce the ranges or we drop the menuconfig entry
> completely, regardless of EXPERT. I'm inclined to go with the latter,
> just don't allow people to redefine this (still unclear to me if we need
> a higher default with 4K pages).

It doesn't solve the issue, but I'll drop this patch for now since it sounds
like we're going to be reworking this further and we may as well avoid the
churn.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

end of thread, other threads:[~2023-05-19 10:36 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-03 12:33 [PATCH] arm64: Make the ARCH_FORCE_MAX_ORDER config input prompt unconditional Catalin Marinas
2023-05-03 14:58 ` Mike Rapoport
2023-05-03 15:35 ` Justin Forbes
2023-05-03 15:41   ` Ard Biesheuvel
2023-05-05 22:00     ` Mike Rapoport
2023-05-05 22:08       ` Ard Biesheuvel
2023-05-05 22:47         ` Mike Rapoport
2023-05-05 22:51           ` Ard Biesheuvel
2023-05-05 23:23             ` Mike Rapoport
2023-05-16 15:14 ` Will Deacon
2023-05-18 15:56 ` Marc Zyngier
2023-05-18 16:04   ` Catalin Marinas
2023-05-19 10:35     ` Will Deacon

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