| Message ID | 20230503123342.90538-1-catalin.marinas@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: Make the ARCH_FORCE_MAX_ORDER config input prompt unconditional | expand |
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"
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" >
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.
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.
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/
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?
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?
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.
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,
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.
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).
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
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"
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(-)