| Message ID | 20230428153646.823736-1-jforbes@fedoraproject.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Revert arm64: drop ranges in definition of ARCH_FORCE_MAX_ORDER | expand |
+ 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 <jforbes@fedoraproject.org> > Cc: Catalin Marinas <catalin.marinas@arm.com> > --- > 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 <catalin.marinas@arm.com> For the record, the original discussion: Link: https://lore.kernel.org/r/CAFxkdAr5C7ggZ+WdvDbsfmwuXujT_z_x3qcUnhnCn-WrAurvgA@mail.gmail.com
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 <jforbes@fedoraproject.org> > > Cc: Catalin Marinas <catalin.marinas@arm.com> > > --- > > 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 <catalin.marinas@arm.com> > > 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.
On Sat, Apr 29, 2023 at 2:01 PM Mike Rapoport <rppt@kernel.org> 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 <jforbes@fedoraproject.org> > > > Cc: Catalin Marinas <catalin.marinas@arm.com> > > > --- > > > 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 <catalin.marinas@arm.com> > > > > 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. >
On Sat, Apr 29, 2023 at 05:42:11PM -0500, Justin Forbes wrote: > On Sat, Apr 29, 2023 at 2:01 PM Mike Rapoport <rppt@kernel.org> 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 <jforbes@fedoraproject.org> > > > > Cc: Catalin Marinas <catalin.marinas@arm.com> > > > > --- > > > > 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 <catalin.marinas@arm.com> > > > > > > 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. > >
On Sat, Apr 29, 2023 at 11:02 PM Mike Rapoport <rppt@kernel.org> 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 <rppt@kernel.org> 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 <jforbes@fedoraproject.org> > > > > > Cc: Catalin Marinas <catalin.marinas@arm.com> > > > > > --- > > > > > 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 <catalin.marinas@arm.com> > > > > > > > > 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. >
On Mon, May 01, 2023 at 04:24:38PM -0500, Justin Forbes wrote: > On Sat, Apr 29, 2023 at 11:02 PM Mike Rapoport <rppt@kernel.org> 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.
On Tue, 02 May 2023 15:07:41 +0100, Catalin Marinas <catalin.marinas@arm.com> 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 <rppt@kernel.org> 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.
On Tue, May 02, 2023 at 03:21:17PM +0100, Marc Zyngier wrote: > On Tue, 02 May 2023 15:07:41 +0100, > Catalin Marinas <catalin.marinas@arm.com> 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 <rppt@kernel.org> 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.
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
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/202303232149.Chh6KhiI-lkp@intel.com with the randconfig here: https://download.01.org/0day-ci/archive/20230323/202303232149.Chh6KhiI-lkp@intel.com/config That said, it would fail on other architectures as well, maybe they are just not wired up in the build machines.
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/202303232149.Chh6KhiI-lkp@intel.com > > with the randconfig here: > > https://download.01.org/0day-ci/archive/20230323/202303232149.Chh6KhiI-lkp@intel.com/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 lkp@intel.com). 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"
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/202303232149.Chh6KhiI-lkp@intel.com > > > > with the randconfig here: > > > > https://download.01.org/0day-ci/archive/20230323/202303232149.Chh6KhiI-lkp@intel.com/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 lkp@intel.com). 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
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
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 <jforbes@fedoraproject.org> Cc: Catalin Marinas <catalin.marinas@arm.com> --- arch/arm64/Kconfig | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)