2023-04-28 15:38:43

by Justin Forbes

[permalink] [raw]
Subject: [PATCH] Revert arm64: drop ranges in definition of ARCH_FORCE_MAX_ORDER

While the ARCH_FORCE_MAX_ORDER changes clarified the descriptions quite
a bit, the aarch64 specific change moved this config to sit behind
CONFIG_EXPERT. This becomes problematic when distros are setting this to
a non default value already. Pushing it behind EXPERT where it was not
before will silently change the configuration for users building with
oldconfig. If distros patch out if EXPERT downstream, it still creates
problems for users testing out upstream patches, or trying to bisect to
find the root of problem, as the configuration will change unexpectedly,
possibly leading to different behavior and false results.

Whem I asked about reverting the EXPERT, dependency, I was asked to add
the ranges back.

This essentially reverts commit 34affcd7577a232803f729d1870ba475f294e4ea

Signed-off-by: Justin M. Forbes <[email protected]>
Cc: Catalin Marinas <[email protected]>
---
arch/arm64/Kconfig | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index b1201d25a8a4..dae18ac01e94 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1516,9 +1516,11 @@ 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" if ARM64_4K_PAGES || ARM64_16K_PAGES
default "13" if ARM64_64K_PAGES
+ range 11 13 if ARM64_16K_PAGES
default "11" if ARM64_16K_PAGES
+ range 10 15 if ARM64_4K_PAGES
default "10"
help
The kernel page allocator limits the size of maximal physically
--
2.39.2


2023-04-28 17:09:25

by Catalin Marinas

[permalink] [raw]
Subject: Re: [PATCH] Revert arm64: drop ranges in definition of ARCH_FORCE_MAX_ORDER

+ Mike and Andrew

On Fri, Apr 28, 2023 at 10:36:45AM -0500, Justin M. Forbes wrote:
> While the ARCH_FORCE_MAX_ORDER changes clarified the descriptions quite
> a bit, the aarch64 specific change moved this config to sit behind
> CONFIG_EXPERT. This becomes problematic when distros are setting this to
> a non default value already. Pushing it behind EXPERT where it was not
> before will silently change the configuration for users building with
> oldconfig. If distros patch out if EXPERT downstream, it still creates
> problems for users testing out upstream patches, or trying to bisect to
> find the root of problem, as the configuration will change unexpectedly,
> possibly leading to different behavior and false results.
>
> Whem I asked about reverting the EXPERT, dependency, I was asked to add
> the ranges back.
>
> This essentially reverts commit 34affcd7577a232803f729d1870ba475f294e4ea
>
> Signed-off-by: Justin M. Forbes <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> ---
> arch/arm64/Kconfig | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index b1201d25a8a4..dae18ac01e94 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1516,9 +1516,11 @@ 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" if ARM64_4K_PAGES || ARM64_16K_PAGES
> default "13" if ARM64_64K_PAGES
> + range 11 13 if ARM64_16K_PAGES
> default "11" if ARM64_16K_PAGES
> + range 10 15 if ARM64_4K_PAGES
> default "10"
> help
> The kernel page allocator limits the size of maximal physically

The revert looks fine to me:

Acked-by: Catalin Marinas <[email protected]>

For the record, the original discussion:

Link: https://lore.kernel.org/r/CAFxkdAr5C7ggZ+WdvDbsfmwuXujT_z_x3qcUnhnCn-WrAurvgA@mail.gmail.com

2023-04-29 19:15:28

by Mike Rapoport

[permalink] [raw]
Subject: Re: [PATCH] Revert arm64: drop ranges in definition of ARCH_FORCE_MAX_ORDER

On Fri, Apr 28, 2023 at 06:01:30PM +0100, Catalin Marinas wrote:
> + Mike and Andrew
>
> On Fri, Apr 28, 2023 at 10:36:45AM -0500, Justin M. Forbes wrote:
> > While the ARCH_FORCE_MAX_ORDER changes clarified the descriptions quite
> > a bit, the aarch64 specific change moved this config to sit behind
> > CONFIG_EXPERT. This becomes problematic when distros are setting this to
> > a non default value already. Pushing it behind EXPERT where it was not
> > before will silently change the configuration for users building with
> > oldconfig. If distros patch out if EXPERT downstream, it still creates
> > problems for users testing out upstream patches, or trying to bisect to
> > find the root of problem, as the configuration will change unexpectedly,
> > possibly leading to different behavior and false results.
> >
> > Whem I asked about reverting the EXPERT, dependency, I was asked to add

Nit: When

> > the ranges back.
> >
> > This essentially reverts commit 34affcd7577a232803f729d1870ba475f294e4ea
> >
> > Signed-off-by: Justin M. Forbes <[email protected]>
> > Cc: Catalin Marinas <[email protected]>
> > ---
> > arch/arm64/Kconfig | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > index b1201d25a8a4..dae18ac01e94 100644
> > --- a/arch/arm64/Kconfig
> > +++ b/arch/arm64/Kconfig
> > @@ -1516,9 +1516,11 @@ 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" if ARM64_4K_PAGES || ARM64_16K_PAGES
> > default "13" if ARM64_64K_PAGES
> > + range 11 13 if ARM64_16K_PAGES
> > default "11" if ARM64_16K_PAGES
> > + range 10 15 if ARM64_4K_PAGES
> > default "10"
> > help
> > The kernel page allocator limits the size of maximal physically
>
> The revert looks fine to me:
>
> Acked-by: Catalin Marinas <[email protected]>
>
> For the record, the original discussion:
>
> Link: https://lore.kernel.org/r/CAFxkdAr5C7ggZ+WdvDbsfmwuXujT_z_x3qcUnhnCn-WrAurvgA@mail.gmail.com

I'm not really happy about this revert because MAX_ORDER is not something
that should be changed easily.
But since hiding it behind EXPERT would silently change lots of existing
builds, I won't object.

Still, I never got the answer _why_ Fedora/RHEL configs use non-default
value. Quite possible something else needs to be fixed rather than having
overgrown MAX_ORDER.

--
Sincerely yours,
Mike.

2023-04-29 22:44:58

by Justin Forbes

[permalink] [raw]
Subject: Re: [PATCH] Revert arm64: drop ranges in definition of ARCH_FORCE_MAX_ORDER

On Sat, Apr 29, 2023 at 2:01 PM Mike Rapoport <[email protected]> wrote:
>
> On Fri, Apr 28, 2023 at 06:01:30PM +0100, Catalin Marinas wrote:
> > + Mike and Andrew
> >
> > On Fri, Apr 28, 2023 at 10:36:45AM -0500, Justin M. Forbes wrote:
> > > While the ARCH_FORCE_MAX_ORDER changes clarified the descriptions quite
> > > a bit, the aarch64 specific change moved this config to sit behind
> > > CONFIG_EXPERT. This becomes problematic when distros are setting this to
> > > a non default value already. Pushing it behind EXPERT where it was not
> > > before will silently change the configuration for users building with
> > > oldconfig. If distros patch out if EXPERT downstream, it still creates
> > > problems for users testing out upstream patches, or trying to bisect to
> > > find the root of problem, as the configuration will change unexpectedly,
> > > possibly leading to different behavior and false results.
> > >
> > > Whem I asked about reverting the EXPERT, dependency, I was asked to add
>
> Nit: When
>
> > > the ranges back.
> > >
> > > This essentially reverts commit 34affcd7577a232803f729d1870ba475f294e4ea
> > >
> > > Signed-off-by: Justin M. Forbes <[email protected]>
> > > Cc: Catalin Marinas <[email protected]>
> > > ---
> > > arch/arm64/Kconfig | 4 +++-
> > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > > index b1201d25a8a4..dae18ac01e94 100644
> > > --- a/arch/arm64/Kconfig
> > > +++ b/arch/arm64/Kconfig
> > > @@ -1516,9 +1516,11 @@ 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" if ARM64_4K_PAGES || ARM64_16K_PAGES
> > > default "13" if ARM64_64K_PAGES
> > > + range 11 13 if ARM64_16K_PAGES
> > > default "11" if ARM64_16K_PAGES
> > > + range 10 15 if ARM64_4K_PAGES
> > > default "10"
> > > help
> > > The kernel page allocator limits the size of maximal physically
> >
> > The revert looks fine to me:
> >
> > Acked-by: Catalin Marinas <[email protected]>
> >
> > For the record, the original discussion:
> >
> > Link: https://lore.kernel.org/r/CAFxkdAr5C7ggZ+WdvDbsfmwuXujT_z_x3qcUnhnCn-WrAurvgA@mail.gmail.com
>
> I'm not really happy about this revert because MAX_ORDER is not something
> that should be changed easily.
> But since hiding it behind EXPERT would silently change lots of existing
> builds, I won't object.
>
> Still, I never got the answer _why_ Fedora/RHEL configs use non-default
> value. Quite possible something else needs to be fixed rather than having
> overgrown MAX_ORDER.

I get that, but I also looked at the rest of the patch set. Nowhere
else was "if EXPERT" added. Why wasn't it added to other
architectures? Not that I am complaining, but aarch64 in particular is
the one arch where, as a distro, we are trying to accommodate both
Raspberry Pi, and server class machines.
It is the practicality of building a single kernel image that works
along a large number of machines. The defaults are fine for smaller
boards, and honestly the majority of aarch64 hardware in circulation.
They are not acceptable for server class machines running those types
of workloads. For a long time, it was just Fedora running with 4K
pages, and carrying a patch to allow MAX_ORDER to hit 13. With RHEL
9, the 4K page size became the default there as well, and they did the
same. Eventually using a MAX_ORDER of 13 no longer required a patch of
any kind. In an ideal world, we would be building a 4k kernel, a 64k
kernel, and even a 16k kernel for users running Apple hardware, but
logistically that is a whole lot more than just a kernel rebuild. It
has QA cycles and support requirements that go along with it, etc.

Justin

> --
> Sincerely yours,
> Mike.
>

2023-04-30 04:12:00

by Mike Rapoport

[permalink] [raw]
Subject: Re: [PATCH] Revert arm64: drop ranges in definition of ARCH_FORCE_MAX_ORDER

On Sat, Apr 29, 2023 at 05:42:11PM -0500, Justin Forbes wrote:
> On Sat, Apr 29, 2023 at 2:01 PM Mike Rapoport <[email protected]> wrote:
> >
> > On Fri, Apr 28, 2023 at 06:01:30PM +0100, Catalin Marinas wrote:
> > > + Mike and Andrew
> > >
> > > On Fri, Apr 28, 2023 at 10:36:45AM -0500, Justin M. Forbes wrote:
> > > > While the ARCH_FORCE_MAX_ORDER changes clarified the descriptions quite
> > > > a bit, the aarch64 specific change moved this config to sit behind
> > > > CONFIG_EXPERT. This becomes problematic when distros are setting this to
> > > > a non default value already. Pushing it behind EXPERT where it was not
> > > > before will silently change the configuration for users building with
> > > > oldconfig. If distros patch out if EXPERT downstream, it still creates
> > > > problems for users testing out upstream patches, or trying to bisect to
> > > > find the root of problem, as the configuration will change unexpectedly,
> > > > possibly leading to different behavior and false results.
> > > >
> > > > Whem I asked about reverting the EXPERT, dependency, I was asked to add
> >
> > Nit: When
> >
> > > > the ranges back.
> > > >
> > > > This essentially reverts commit 34affcd7577a232803f729d1870ba475f294e4ea
> > > >
> > > > Signed-off-by: Justin M. Forbes <[email protected]>
> > > > Cc: Catalin Marinas <[email protected]>
> > > > ---
> > > > arch/arm64/Kconfig | 4 +++-
> > > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > > > index b1201d25a8a4..dae18ac01e94 100644
> > > > --- a/arch/arm64/Kconfig
> > > > +++ b/arch/arm64/Kconfig
> > > > @@ -1516,9 +1516,11 @@ 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" if ARM64_4K_PAGES || ARM64_16K_PAGES
> > > > default "13" if ARM64_64K_PAGES
> > > > + range 11 13 if ARM64_16K_PAGES
> > > > default "11" if ARM64_16K_PAGES
> > > > + range 10 15 if ARM64_4K_PAGES
> > > > default "10"
> > > > help
> > > > The kernel page allocator limits the size of maximal physically
> > >
> > > The revert looks fine to me:
> > >
> > > Acked-by: Catalin Marinas <[email protected]>
> > >
> > > For the record, the original discussion:
> > >
> > > Link: https://lore.kernel.org/r/CAFxkdAr5C7ggZ+WdvDbsfmwuXujT_z_x3qcUnhnCn-WrAurvgA@mail.gmail.com
> >
> > I'm not really happy about this revert because MAX_ORDER is not something
> > that should be changed easily.
> > But since hiding it behind EXPERT would silently change lots of existing
> > builds, I won't object.
> >
> > Still, I never got the answer _why_ Fedora/RHEL configs use non-default
> > value. Quite possible something else needs to be fixed rather than having
> > overgrown MAX_ORDER.
>
> I get that, but I also looked at the rest of the patch set. Nowhere
> else was "if EXPERT" added. Why wasn't it added to other
> architectures? Not that I am complaining, but aarch64 in particular is
> the one arch where, as a distro, we are trying to accommodate both
> Raspberry Pi, and server class machines.

The patch was about dropping the ranges, not about adding EXPERT. So on
arm64 it was added because Catalin requested it, other arch maintainers
didn't.

> It is the practicality of building a single kernel image that works
> along a large number of machines. The defaults are fine for smaller
> boards, and honestly the majority of aarch64 hardware in circulation.
> They are not acceptable for server class machines running those types
> of workloads.

Why the default MAX_ORDER was not acceptable on arm64 server machines but
it is fine on, say, x86 and s390?
I'm not asking how you made it possible in Fedora and RHEL, I'm asking why
did you switch from the default order at all.

> Justin
>
> > --
> > Sincerely yours,
> > Mike.
> >

--
Sincerely yours,
Mike.

2023-05-01 21:26:24

by Justin Forbes

[permalink] [raw]
Subject: Re: [PATCH] Revert arm64: drop ranges in definition of ARCH_FORCE_MAX_ORDER

On Sat, Apr 29, 2023 at 11:02 PM Mike Rapoport <[email protected]> wrote:
>
> On Sat, Apr 29, 2023 at 05:42:11PM -0500, Justin Forbes wrote:
> > On Sat, Apr 29, 2023 at 2:01 PM Mike Rapoport <[email protected]> wrote:
> > >
> > > On Fri, Apr 28, 2023 at 06:01:30PM +0100, Catalin Marinas wrote:
> > > > + Mike and Andrew
> > > >
> > > > On Fri, Apr 28, 2023 at 10:36:45AM -0500, Justin M. Forbes wrote:
> > > > > While the ARCH_FORCE_MAX_ORDER changes clarified the descriptions quite
> > > > > a bit, the aarch64 specific change moved this config to sit behind
> > > > > CONFIG_EXPERT. This becomes problematic when distros are setting this to
> > > > > a non default value already. Pushing it behind EXPERT where it was not
> > > > > before will silently change the configuration for users building with
> > > > > oldconfig. If distros patch out if EXPERT downstream, it still creates
> > > > > problems for users testing out upstream patches, or trying to bisect to
> > > > > find the root of problem, as the configuration will change unexpectedly,
> > > > > possibly leading to different behavior and false results.
> > > > >
> > > > > Whem I asked about reverting the EXPERT, dependency, I was asked to add
> > >
> > > Nit: When
> > >
> > > > > the ranges back.
> > > > >
> > > > > This essentially reverts commit 34affcd7577a232803f729d1870ba475f294e4ea
> > > > >
> > > > > Signed-off-by: Justin M. Forbes <[email protected]>
> > > > > Cc: Catalin Marinas <[email protected]>
> > > > > ---
> > > > > arch/arm64/Kconfig | 4 +++-
> > > > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > > > > index b1201d25a8a4..dae18ac01e94 100644
> > > > > --- a/arch/arm64/Kconfig
> > > > > +++ b/arch/arm64/Kconfig
> > > > > @@ -1516,9 +1516,11 @@ 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" if ARM64_4K_PAGES || ARM64_16K_PAGES
> > > > > default "13" if ARM64_64K_PAGES
> > > > > + range 11 13 if ARM64_16K_PAGES
> > > > > default "11" if ARM64_16K_PAGES
> > > > > + range 10 15 if ARM64_4K_PAGES
> > > > > default "10"
> > > > > help
> > > > > The kernel page allocator limits the size of maximal physically
> > > >
> > > > The revert looks fine to me:
> > > >
> > > > Acked-by: Catalin Marinas <[email protected]>
> > > >
> > > > For the record, the original discussion:
> > > >
> > > > Link: https://lore.kernel.org/r/CAFxkdAr5C7ggZ+WdvDbsfmwuXujT_z_x3qcUnhnCn-WrAurvgA@mail.gmail.com
> > >
> > > I'm not really happy about this revert because MAX_ORDER is not something
> > > that should be changed easily.
> > > But since hiding it behind EXPERT would silently change lots of existing
> > > builds, I won't object.
> > >
> > > Still, I never got the answer _why_ Fedora/RHEL configs use non-default
> > > value. Quite possible something else needs to be fixed rather than having
> > > overgrown MAX_ORDER.
> >
> > I get that, but I also looked at the rest of the patch set. Nowhere
> > else was "if EXPERT" added. Why wasn't it added to other
> > architectures? Not that I am complaining, but aarch64 in particular is
> > the one arch where, as a distro, we are trying to accommodate both
> > Raspberry Pi, and server class machines.
>
> The patch was about dropping the ranges, not about adding EXPERT. So on
> arm64 it was added because Catalin requested it, other arch maintainers
> didn't.
>
> > It is the practicality of building a single kernel image that works
> > along a large number of machines. The defaults are fine for smaller
> > boards, and honestly the majority of aarch64 hardware in circulation.
> > They are not acceptable for server class machines running those types
> > of workloads.
>
> Why the default MAX_ORDER was not acceptable on arm64 server machines but
> it is fine on, say, x86 and s390?
> I'm not asking how you made it possible in Fedora and RHEL, I'm asking why
> did you switch from the default order at all.

Because the MAX_ORDER on aarch64 with 4K pages is more tuned to the
needs of the average edge client, not so much those of a server class
machine. And I get it, I would say well over 90% of the Fedora users
running aarch64 are indeed running on a rPi or similar with a small
memory footprint, and workloads which match that. But we do support
and run a 4K page size aarch64 kernel on proper server class hardware,
running typical server workloads, and RHEL has a lot more users in the
server class than edge clients. RHEL could probably default to 64K
pages, and most users would be happy with that. Fedora certainly could
not. At some point, we may consider adding another build so that we
offer both 4K and 64K pages, but for now, this is where we are, and
where we have been for years.

Justin

> > Justin
> >
> > > --
> > > Sincerely yours,
> > > Mike.
> > >
>
> --
> Sincerely yours,
> Mike.
>

2023-05-02 14:15:08

by Catalin Marinas

[permalink] [raw]
Subject: Re: [PATCH] Revert arm64: drop ranges in definition of ARCH_FORCE_MAX_ORDER

On Mon, May 01, 2023 at 04:24:38PM -0500, Justin Forbes wrote:
> On Sat, Apr 29, 2023 at 11:02 PM Mike Rapoport <[email protected]> wrote:
> > Why the default MAX_ORDER was not acceptable on arm64 server machines but
> > it is fine on, say, x86 and s390?
> > I'm not asking how you made it possible in Fedora and RHEL, I'm asking why
> > did you switch from the default order at all.
>
> Because the MAX_ORDER on aarch64 with 4K pages is more tuned to the
> needs of the average edge client, not so much those of a server class
> machine. And I get it, I would say well over 90% of the Fedora users
> running aarch64 are indeed running on a rPi or similar with a small
> memory footprint, and workloads which match that. But we do support
> and run a 4K page size aarch64 kernel on proper server class hardware,
> running typical server workloads, and RHEL has a lot more users in the
> server class than edge clients. RHEL could probably default to 64K
> pages, and most users would be happy with that. Fedora certainly could
> not.

I was talking to Marc Zyngier earlier and he reckons the need for a
higher MAX_ORDER is the GIC driver ITS allocation for Thunder-X. I'm
happy to make ARCH_MAX_ORDER higher in defconfig (12, 13?) if
CONFIG_ARCH_THUNDER. Mobile vendors won't enable this platform.

Regarding EXPERT, we could drop it and do like the other architectures
but we'll have randconfig occasionally hitting weird values that won't
build (like -1). Not sure EXPERT helps here.

--
Catalin

2023-05-02 14:27:45

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH] Revert arm64: drop ranges in definition of ARCH_FORCE_MAX_ORDER

On Tue, 02 May 2023 15:07:41 +0100,
Catalin Marinas <[email protected]> wrote:
>
> On Mon, May 01, 2023 at 04:24:38PM -0500, Justin Forbes wrote:
> > On Sat, Apr 29, 2023 at 11:02 PM Mike Rapoport <[email protected]> wrote:
> > > Why the default MAX_ORDER was not acceptable on arm64 server machines but
> > > it is fine on, say, x86 and s390?
> > > I'm not asking how you made it possible in Fedora and RHEL, I'm asking why
> > > did you switch from the default order at all.
> >
> > Because the MAX_ORDER on aarch64 with 4K pages is more tuned to the
> > needs of the average edge client, not so much those of a server class
> > machine. And I get it, I would say well over 90% of the Fedora users
> > running aarch64 are indeed running on a rPi or similar with a small
> > memory footprint, and workloads which match that. But we do support
> > and run a 4K page size aarch64 kernel on proper server class hardware,
> > running typical server workloads, and RHEL has a lot more users in the
> > server class than edge clients. RHEL could probably default to 64K
> > pages, and most users would be happy with that. Fedora certainly could
> > not.
>
> I was talking to Marc Zyngier earlier and he reckons the need for a
> higher MAX_ORDER is the GIC driver ITS allocation for Thunder-X. I'm
> happy to make ARCH_MAX_ORDER higher in defconfig (12, 13?) if
> CONFIG_ARCH_THUNDER. Mobile vendors won't enable this platform.

In any case, I'd like to know exactly *what* requires it. The only
platform I know would benefit from this is the old TX1, but this
machine is more a boat anchor than a real server.

M.

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

2023-05-02 16:23:04

by Mike Rapoport

[permalink] [raw]
Subject: Re: [PATCH] Revert arm64: drop ranges in definition of ARCH_FORCE_MAX_ORDER

On Tue, May 02, 2023 at 03:07:41PM +0100, Catalin Marinas wrote:
> On Mon, May 01, 2023 at 04:24:38PM -0500, Justin Forbes wrote:
>
> Regarding EXPERT, we could drop it and do like the other architectures
> but we'll have randconfig occasionally hitting weird values that won't
> build (like -1). Not sure EXPERT helps here.

AFAIU, randconfig does not randomize int values, it's probably random
people that do ;-)

> --
> Catalin

--
Sincerely yours,
Mike.

2023-05-02 16:33:48

by Mike Rapoport

[permalink] [raw]
Subject: Re: [PATCH] Revert arm64: drop ranges in definition of ARCH_FORCE_MAX_ORDER

On Tue, May 02, 2023 at 03:21:17PM +0100, Marc Zyngier wrote:
> On Tue, 02 May 2023 15:07:41 +0100,
> Catalin Marinas <[email protected]> wrote:
> >
> > On Mon, May 01, 2023 at 04:24:38PM -0500, Justin Forbes wrote:
> > > On Sat, Apr 29, 2023 at 11:02 PM Mike Rapoport <[email protected]> wrote:
> > > > Why the default MAX_ORDER was not acceptable on arm64 server machines but
> > > > it is fine on, say, x86 and s390?
> > > > I'm not asking how you made it possible in Fedora and RHEL, I'm asking why
> > > > did you switch from the default order at all.
> > >
> > > Because the MAX_ORDER on aarch64 with 4K pages is more tuned to the
> > > needs of the average edge client, not so much those of a server class
> > > machine. And I get it, I would say well over 90% of the Fedora users
> > > running aarch64 are indeed running on a rPi or similar with a small
> > > memory footprint, and workloads which match that. But we do support
> > > and run a 4K page size aarch64 kernel on proper server class hardware,
> > > running typical server workloads, and RHEL has a lot more users in the
> > > server class than edge clients. RHEL could probably default to 64K
> > > pages, and most users would be happy with that. Fedora certainly could
> > > not.

The memory size of the machine or how heavy the workloads it runs have
nothing to do with MAX_ORDER. Again, x86 and s390 are perfectly fine with
MAX_ORDER == 10 ...

> > I was talking to Marc Zyngier earlier and he reckons the need for a
> > higher MAX_ORDER is the GIC driver ITS allocation for Thunder-X.

... but this indeed could be the reason to increase MAX_ORDER.

> > I'm happy to make ARCH_MAX_ORDER higher in defconfig (12, 13?) if
> > CONFIG_ARCH_THUNDER. Mobile vendors won't enable this platform.
>
> In any case, I'd like to know exactly *what* requires it. The only
> platform I know would benefit from this is the old TX1, but this
> machine is more a boat anchor than a real server.

Yeah, if we'd knew what exactly requires such huge contiguous allocation,
we probably could fix that and leave Kconfig alone.

--
Sincerely yours,
Mike.

2023-05-02 17:58:08

by Catalin Marinas

[permalink] [raw]
Subject: Re: [PATCH] Revert arm64: drop ranges in definition of ARCH_FORCE_MAX_ORDER

On Tue, May 02, 2023 at 07:15:20PM +0300, Mike Rapoport wrote:
> On Tue, May 02, 2023 at 03:07:41PM +0100, Catalin Marinas wrote:
> > On Mon, May 01, 2023 at 04:24:38PM -0500, Justin Forbes wrote:
> >
> > Regarding EXPERT, we could drop it and do like the other architectures
> > but we'll have randconfig occasionally hitting weird values that won't
> > build (like -1). Not sure EXPERT helps here.
>
> AFAIU, randconfig does not randomize int values, it's probably random
> people that do ;-)

https://lore.kernel.org/r/[email protected]

with the randconfig here:

https://download.01.org/0day-ci/archive/20230323/[email protected]/config

That said, it would fail on other architectures as well, maybe they are
just not wired up in the build machines.

--
Catalin

2023-05-03 10:29:03

by Catalin Marinas

[permalink] [raw]
Subject: Re: [PATCH] Revert arm64: drop ranges in definition of ARCH_FORCE_MAX_ORDER

On Tue, May 02, 2023 at 06:40:14PM +0100, Catalin Marinas wrote:
> On Tue, May 02, 2023 at 07:15:20PM +0300, Mike Rapoport wrote:
> > On Tue, May 02, 2023 at 03:07:41PM +0100, Catalin Marinas wrote:
> > > On Mon, May 01, 2023 at 04:24:38PM -0500, Justin Forbes wrote:
> > >
> > > Regarding EXPERT, we could drop it and do like the other architectures
> > > but we'll have randconfig occasionally hitting weird values that won't
> > > build (like -1). Not sure EXPERT helps here.
> >
> > AFAIU, randconfig does not randomize int values, it's probably random
> > people that do ;-)
>
> https://lore.kernel.org/r/[email protected]
>
> with the randconfig here:
>
> https://download.01.org/0day-ci/archive/20230323/[email protected]/config

You may be right, I can't get my randconfig to set ARCH_FORCE_MAX_ORDER
to anything other than the default. Maybe the kernel test robot has its
own config randomisation (cc'ing [email protected]).

If we don't care about about this randconfig, I'm fine do drop EXPERT
from current mainline, together with the 4K/16K pages condition. The
condition only made sense if we kept the ranges in since these were
configurable (no range for 64K).

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"

--
Catalin

2023-05-03 12:12:42

by Li, Philip

[permalink] [raw]
Subject: Re: [PATCH] Revert arm64: drop ranges in definition of ARCH_FORCE_MAX_ORDER

On Wed, May 03, 2023 at 11:20:40AM +0100, Catalin Marinas wrote:
> On Tue, May 02, 2023 at 06:40:14PM +0100, Catalin Marinas wrote:
> > On Tue, May 02, 2023 at 07:15:20PM +0300, Mike Rapoport wrote:
> > > On Tue, May 02, 2023 at 03:07:41PM +0100, Catalin Marinas wrote:
> > > > On Mon, May 01, 2023 at 04:24:38PM -0500, Justin Forbes wrote:
> > > >
> > > > Regarding EXPERT, we could drop it and do like the other architectures
> > > > but we'll have randconfig occasionally hitting weird values that won't
> > > > build (like -1). Not sure EXPERT helps here.
> > >
> > > AFAIU, randconfig does not randomize int values, it's probably random
> > > people that do ;-)
> >
> > https://lore.kernel.org/r/[email protected]
> >
> > with the randconfig here:
> >
> > https://download.01.org/0day-ci/archive/20230323/[email protected]/config
>
> You may be right, I can't get my randconfig to set ARCH_FORCE_MAX_ORDER
> to anything other than the default. Maybe the kernel test robot has its
> own config randomisation (cc'ing [email protected]).

Really appologize here, around Mar 23 time period, there's a bug in kernel test
robot code which wrongly set the value of ARCH_FORCE_MAX_ORDER to -1. We fixed
it after noticing this, and replied to the false positive reports we sent out.
But we missed to reply this report to point out it was invalid report.

Sorry about this again, pls ignore this report.

>
> If we don't care about about this randconfig, I'm fine do drop EXPERT
> from current mainline, together with the 4K/16K pages condition. The
> condition only made sense if we kept the ranges in since these were
> configurable (no range for 64K).
>
> 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"
>
> --
> Catalin