[RFT PATCH] MIPS: Octeon: drop dependency on CONFIG_HOLES_IN_ZONE

[RFT PATCH] MIPS: Octeon: drop dependency on CONFIG_HOLES_IN_ZONE

[Mike Rapoport]

From: Mike Rapoport <rppt@linux.ibm.com>

CAVIUM_OCTEON_SOC configuration selects HOLES_IN_ZONE option to cope with
memory crashes that were happening in 2011.

This option effectively aliases pfn_valid_within() to pfn_valid() when
enabled and hardwires it to 1 when disabled. The check for
pfn_valid_within() is only relevant in case the memory map may have holes
or undefined struct page instances inside MAX_ORDER chunks.

Since 2011 memory management initialization in general and memory map
initialization particularly became much more robust so the check for
pfn_valid_within() is not required on Octeon even despite its, hmm, unusual
memory setup.

Remove the selection of HOLES_IN_ZONE by CAVIUM_OCTEON_SOC and drop the
HOLES_IN_ZONE configuration option entirely as Octeon was the only MIPS
platform to use it.

Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
---

Hi,

I've tried to find why Octeon needed CONFIG_HOLES_IN_ZONE in the first
place, but there is nothing except the changelog in commit 465aaed0030b
("MIPS: Octeon: Select CONFIG_HOLES_IN_ZONE"):

  Current Octeon systems do in fact have holes in their memory zones.
  We need to select HOLES_IN_ZONE. If we do not, some memory configurations
  will result in crashes at boot time

Since then there were too many changes around memory management
initialization both in the generic mm and on the MIPS side to track what
exactly could case the crashes.

I'm pretty confident that HOLES_IN_ZONE is not required for Octeon systems
anymore and can be removed.

I'd really appreciate if somebody with access to an Octeon system could
test this patch.

 arch/mips/Kconfig | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index d89efba3d8a4..96b08cd3b442 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -990,7 +990,6 @@ config CAVIUM_OCTEON_SOC
 	select HAVE_PLAT_FW_INIT_CMDLINE
 	select HAVE_PLAT_MEMCPY
 	select ZONE_DMA32
-	select HOLES_IN_ZONE
 	select GPIOLIB
 	select USE_OF
 	select ARCH_SPARSEMEM_ENABLE
@@ -1226,9 +1225,6 @@ config HAVE_PLAT_MEMCPY
 config ISA_DMA_API
 	bool
 
-config HOLES_IN_ZONE
-	bool
-
 config SYS_SUPPORTS_RELOCATABLE
 	bool
 	help
-- 
2.28.0

Re: [RFT PATCH] MIPS: Octeon: drop dependency on CONFIG_HOLES_IN_ZONE

[Thomas Bogendoerfer]

On Sun, Apr 18, 2021 at 12:35:12PM +0300, Mike Rapoport wrote:
> I'd really appreciate if somebody with access to an Octeon system could
> test this patch.

Tested on an Ubiquiti edgerouter 12. Works with problem and I haven't
even seen a change in memory output.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

Re: [RFT PATCH] MIPS: Octeon: drop dependency on CONFIG_HOLES_IN_ZONE

[Mike Rapoport]

Hi Thomas,

On Thu, Apr 29, 2021 at 11:06:32PM +0200, Thomas Bogendoerfer wrote:
> On Sun, Apr 18, 2021 at 12:35:12PM +0300, Mike Rapoport wrote:
> > I'd really appreciate if somebody with access to an Octeon system could
> > test this patch.
> 
> Tested on an Ubiquiti edgerouter 12. Works with problem and I haven't
> even seen a change in memory output.

Is "works with problem" a misprint or something went wrong?
 
> Thomas.
> 
> -- 
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea.                                                [ RFC1925, 2.3 ]

-- 
Sincerely yours,
Mike.

Re: [RFT PATCH] MIPS: Octeon: drop dependency on CONFIG_HOLES_IN_ZONE

[Thomas Bogendoerfer]

On Fri, Apr 30, 2021 at 12:35:32PM +0300, Mike Rapoport wrote:
> Hi Thomas,
> 
> On Thu, Apr 29, 2021 at 11:06:32PM +0200, Thomas Bogendoerfer wrote:
> > On Sun, Apr 18, 2021 at 12:35:12PM +0300, Mike Rapoport wrote:
> > > I'd really appreciate if somebody with access to an Octeon system could
> > > test this patch.
> > 
> > Tested on an Ubiquiti edgerouter 12. Works with problem and I haven't
> > even seen a change in memory output.
> 
> Is "works with problem" a misprint or something went wrong?

that should have been "without problem". All good in my test, but that
was more or less booting and checking memory log messages. 

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

Re: [RFT PATCH] MIPS: Octeon: drop dependency on CONFIG_HOLES_IN_ZONE

[Mike Rapoport]

On Fri, Apr 30, 2021 at 12:42:56PM +0200, Thomas Bogendoerfer wrote:
> On Fri, Apr 30, 2021 at 12:35:32PM +0300, Mike Rapoport wrote:
> > Hi Thomas,
> > 
> > On Thu, Apr 29, 2021 at 11:06:32PM +0200, Thomas Bogendoerfer wrote:
> > > On Sun, Apr 18, 2021 at 12:35:12PM +0300, Mike Rapoport wrote:
> > > > I'd really appreciate if somebody with access to an Octeon system could
> > > > test this patch.
> > > 
> > > Tested on an Ubiquiti edgerouter 12. Works with problem and I haven't
> > > even seen a change in memory output.
> > 
> > Is "works with problem" a misprint or something went wrong?
> 
> that should have been "without problem".

Thanks for the clarification :)

> All good in my test, but that was more or less booting and checking
> memory log messages. 

Well, this is way better than build testing I did. The commit 465aaed0030b
("MIPS: Octeon: Select CONFIG_HOLES_IN_ZONE") that added
CONFIG_HOLES_IN_ZONE for Octeons talked about crashes at boot time, so
boot testing seems appropriate.

The only concern, is peculiar memory configuration Octeon may have with
Linux and non-Linux "partitions" that will be different on different
systems, but nevertheless I think the generic mm nowadays is robust enough
to cope with those without CONFIG_HOLES_IN_ZONE.

I presumed the patch would go via MIPS tree, so it's you call Thomas.

-- 
Sincerely yours,
Mike.

Re: [RFT PATCH] MIPS: Octeon: drop dependency on CONFIG_HOLES_IN_ZONE

[Thomas Bogendoerfer]

On Sun, Apr 18, 2021 at 12:35:12PM +0300, Mike Rapoport wrote:
> From: Mike Rapoport <rppt@linux.ibm.com>
> 
> CAVIUM_OCTEON_SOC configuration selects HOLES_IN_ZONE option to cope with
> memory crashes that were happening in 2011.
> 
> This option effectively aliases pfn_valid_within() to pfn_valid() when
> enabled and hardwires it to 1 when disabled. The check for
> pfn_valid_within() is only relevant in case the memory map may have holes
> or undefined struct page instances inside MAX_ORDER chunks.
> 
> Since 2011 memory management initialization in general and memory map
> initialization particularly became much more robust so the check for
> pfn_valid_within() is not required on Octeon even despite its, hmm, unusual
> memory setup.
> 
> Remove the selection of HOLES_IN_ZONE by CAVIUM_OCTEON_SOC and drop the
> HOLES_IN_ZONE configuration option entirely as Octeon was the only MIPS
> platform to use it.
> 
> Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
> ---
> 
> Hi,
> 
> I've tried to find why Octeon needed CONFIG_HOLES_IN_ZONE in the first
> place, but there is nothing except the changelog in commit 465aaed0030b
> ("MIPS: Octeon: Select CONFIG_HOLES_IN_ZONE"):
> 
>   Current Octeon systems do in fact have holes in their memory zones.
>   We need to select HOLES_IN_ZONE. If we do not, some memory configurations
>   will result in crashes at boot time
> 
> Since then there were too many changes around memory management
> initialization both in the generic mm and on the MIPS side to track what
> exactly could case the crashes.
> 
> I'm pretty confident that HOLES_IN_ZONE is not required for Octeon systems
> anymore and can be removed.
> 
> I'd really appreciate if somebody with access to an Octeon system could
> test this patch.
> 
>  arch/mips/Kconfig | 4 ----
>  1 file changed, 4 deletions(-)

applied to mips-next.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]