[PATCH] mm/page_alloc: Deprecate kernelcore=nn and movable_core=

[PATCH] mm/page_alloc: Deprecate kernelcore=nn and movable_core=

[Baoquan He]

We can still use 'kernelcore=mirror' or 'movable_node' for the usage
of hotplug and movable zone. If somebody shows up with a valid usecase
we can reconsider.

Suggested-by: Michal Hocko <mhocko@kernel.org>
Signed-off-by: Baoquan He <bhe@redhat.com>
---
 Documentation/admin-guide/kernel-parameters.txt | 2 ++
 mm/page_alloc.c                                 | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index efc7aa7a0670..1e22c49866a2 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1855,6 +1855,7 @@
 	keepinitrd	[HW,ARM]
 
 	kernelcore=	[KNL,X86,IA-64,PPC]
+			[Usage of kernelcore=nn[KMGTPE] | nn% is deprecated]
 			Format: nn[KMGTPE] | nn% | "mirror"
 			This parameter specifies the amount of memory usable by
 			the kernel for non-movable allocations.  The requested
@@ -2395,6 +2396,7 @@
 			reporting absolute coordinates, such as tablets
 
 	movablecore=	[KNL,X86,IA-64,PPC]
+			[Deprecated]
 			Format: nn[KMGTPE] | nn%
 			This parameter is the complement to kernelcore=, it
 			specifies the amount of memory used for migratable
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 1521100f1e63..86cf05f48b5f 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6899,6 +6899,8 @@ static int __init cmdline_parse_kernelcore(char *p)
 		return 0;
 	}
 
+	pr_warn("Only kernelcore=mirror supported, "
+		"usage of kernelcore=nn[KMGTPE]|nn%% is deprecated.\n");
 	return cmdline_parse_core(p, &required_kernelcore,
 				  &required_kernelcore_percent);
 }
@@ -6909,6 +6911,7 @@ static int __init cmdline_parse_kernelcore(char *p)
  */
 static int __init cmdline_parse_movablecore(char *p)
 {
+	pr_warn("Option movablecore= is deprecated.\n");
 	return cmdline_parse_core(p, &required_movablecore,
 				  &required_movablecore_percent);
 }
-- 
2.13.6

Re: [PATCH] mm/page_alloc: Deprecate kernelcore=nn and movable_core=

[Michal Hocko]

On Tue 17-07-18 21:18:37, Baoquan He wrote:
> We can still use 'kernelcore=mirror' or 'movable_node' for the usage
> of hotplug and movable zone. If somebody shows up with a valid usecase
> we can reconsider.

Well this doesn't really explain why to deprecate this functionality.
It is a rather ugly hack that has been originally introduced for large
order allocations. But we do have compaction these days. Even though the
compaction cannot solve all the fragmentation issues the zone movable is
not a great answer as it introduces other issues (basically highmem kind
of issues we used to have on 32b systems).
The current code doesn't work with KASLR and the code is too subtle to
work properly in other cases as well. E.g. movablecore range might cover
already used memory (e.g. bootmem allocations) and therefore it doesn't
comply with the basic assumption that the memory is movable and that
confuses memory hotplug (e.g. 15c30bc09085 ("mm, memory_hotplug: make
has_unmovable_pages more robust").

There are probably other issues I am not aware of but primarily the code
adds a maintenance burden which would be better to get rid of.

I would also go further and remove all the code the feature is using at
one go. If somebody really needs this functionality we would need to
revert the whole thing anyway.

> Suggested-by: Michal Hocko <mhocko@kernel.org>
> Signed-off-by: Baoquan He <bhe@redhat.com>
> ---
>  Documentation/admin-guide/kernel-parameters.txt | 2 ++
>  mm/page_alloc.c                                 | 3 +++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index efc7aa7a0670..1e22c49866a2 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -1855,6 +1855,7 @@
>  	keepinitrd	[HW,ARM]
>  
>  	kernelcore=	[KNL,X86,IA-64,PPC]
> +			[Usage of kernelcore=nn[KMGTPE] | nn% is deprecated]
>  			Format: nn[KMGTPE] | nn% | "mirror"
>  			This parameter specifies the amount of memory usable by
>  			the kernel for non-movable allocations.  The requested
> @@ -2395,6 +2396,7 @@
>  			reporting absolute coordinates, such as tablets
>  
>  	movablecore=	[KNL,X86,IA-64,PPC]
> +			[Deprecated]
>  			Format: nn[KMGTPE] | nn%
>  			This parameter is the complement to kernelcore=, it
>  			specifies the amount of memory used for migratable
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 1521100f1e63..86cf05f48b5f 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -6899,6 +6899,8 @@ static int __init cmdline_parse_kernelcore(char *p)
>  		return 0;
>  	}
>  
> +	pr_warn("Only kernelcore=mirror supported, "
> +		"usage of kernelcore=nn[KMGTPE]|nn%% is deprecated.\n");
>  	return cmdline_parse_core(p, &required_kernelcore,
>  				  &required_kernelcore_percent);
>  }
> @@ -6909,6 +6911,7 @@ static int __init cmdline_parse_kernelcore(char *p)
>   */
>  static int __init cmdline_parse_movablecore(char *p)
>  {
> +	pr_warn("Option movablecore= is deprecated.\n");
>  	return cmdline_parse_core(p, &required_movablecore,
>  				  &required_movablecore_percent);
>  }
> -- 
> 2.13.6

-- 
Michal Hocko
SUSE Labs

Re: [PATCH] mm/page_alloc: Deprecate kernelcore=nn and movable_core=

[Baoquan He]

Hi Michal,

On 07/17/18 at 03:31pm, Michal Hocko wrote:
> On Tue 17-07-18 21:18:37, Baoquan He wrote:
> > We can still use 'kernelcore=mirror' or 'movable_node' for the usage
> > of hotplug and movable zone. If somebody shows up with a valid usecase
> > we can reconsider.
> 
> Well this doesn't really explain why to deprecate this functionality.
> It is a rather ugly hack that has been originally introduced for large
> order allocations. But we do have compaction these days. Even though the
> compaction cannot solve all the fragmentation issues the zone movable is
> not a great answer as it introduces other issues (basically highmem kind
> of issues we used to have on 32b systems).
> The current code doesn't work with KASLR and the code is too subtle to
> work properly in other cases as well. E.g. movablecore range might cover
> already used memory (e.g. bootmem allocations) and therefore it doesn't
> comply with the basic assumption that the memory is movable and that
> confuses memory hotplug (e.g. 15c30bc09085 ("mm, memory_hotplug: make
> has_unmovable_pages more robust").
> 
> There are probably other issues I am not aware of but primarily the code
> adds a maintenance burden which would be better to get rid of.
> 
> I would also go further and remove all the code the feature is using at
> one go. If somebody really needs this functionality we would need to
> revert the whole thing anyway.

Thanks for these details. I can arrange your above saying and rewrite
patch log. Are you suggesting removing the code "kernelcore=nn" and
"movablecore=" are using? If yes, I can repost with these changes.

Just saw some deprecated codes are still there for future cleaning up.
So posted this v1 patch.

Thanks
Baoquan

> > ---
> >  Documentation/admin-guide/kernel-parameters.txt | 2 ++
> >  mm/page_alloc.c                                 | 3 +++
> >  2 files changed, 5 insertions(+)
> > 
> > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> > index efc7aa7a0670..1e22c49866a2 100644
> > --- a/Documentation/admin-guide/kernel-parameters.txt
> > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > @@ -1855,6 +1855,7 @@
> >  	keepinitrd	[HW,ARM]
> >  
> >  	kernelcore=	[KNL,X86,IA-64,PPC]
> > +			[Usage of kernelcore=nn[KMGTPE] | nn% is deprecated]
> >  			Format: nn[KMGTPE] | nn% | "mirror"
> >  			This parameter specifies the amount of memory usable by
> >  			the kernel for non-movable allocations.  The requested
> > @@ -2395,6 +2396,7 @@
> >  			reporting absolute coordinates, such as tablets
> >  
> >  	movablecore=	[KNL,X86,IA-64,PPC]
> > +			[Deprecated]
> >  			Format: nn[KMGTPE] | nn%
> >  			This parameter is the complement to kernelcore=, it
> >  			specifies the amount of memory used for migratable
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index 1521100f1e63..86cf05f48b5f 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -6899,6 +6899,8 @@ static int __init cmdline_parse_kernelcore(char *p)
> >  		return 0;
> >  	}
> >  
> > +	pr_warn("Only kernelcore=mirror supported, "
> > +		"usage of kernelcore=nn[KMGTPE]|nn%% is deprecated.\n");
> >  	return cmdline_parse_core(p, &required_kernelcore,
> >  				  &required_kernelcore_percent);
> >  }
> > @@ -6909,6 +6911,7 @@ static int __init cmdline_parse_kernelcore(char *p)
> >   */
> >  static int __init cmdline_parse_movablecore(char *p)
> >  {
> > +	pr_warn("Option movablecore= is deprecated.\n");
> >  	return cmdline_parse_core(p, &required_movablecore,
> >  				  &required_movablecore_percent);
> >  }
> > -- 
> > 2.13.6
> 
> -- 
> Michal Hocko
> SUSE Labs

Re: [PATCH] mm/page_alloc: Deprecate kernelcore=nn and movable_core=

[Michal Hocko]

On Tue 17-07-18 22:24:43, Baoquan He wrote:
> Hi Michal,
> 
> On 07/17/18 at 03:31pm, Michal Hocko wrote:
> > On Tue 17-07-18 21:18:37, Baoquan He wrote:
> > > We can still use 'kernelcore=mirror' or 'movable_node' for the usage
> > > of hotplug and movable zone. If somebody shows up with a valid usecase
> > > we can reconsider.
> > 
> > Well this doesn't really explain why to deprecate this functionality.
> > It is a rather ugly hack that has been originally introduced for large
> > order allocations. But we do have compaction these days. Even though the
> > compaction cannot solve all the fragmentation issues the zone movable is
> > not a great answer as it introduces other issues (basically highmem kind
> > of issues we used to have on 32b systems).
> > The current code doesn't work with KASLR and the code is too subtle to
> > work properly in other cases as well. E.g. movablecore range might cover
> > already used memory (e.g. bootmem allocations) and therefore it doesn't
> > comply with the basic assumption that the memory is movable and that
> > confuses memory hotplug (e.g. 15c30bc09085 ("mm, memory_hotplug: make
> > has_unmovable_pages more robust").
> > 
> > There are probably other issues I am not aware of but primarily the code
> > adds a maintenance burden which would be better to get rid of.
> > 
> > I would also go further and remove all the code the feature is using at
> > one go. If somebody really needs this functionality we would need to
> > revert the whole thing anyway.
> 
> Thanks for these details. I can arrange your above saying and rewrite
> patch log. Are you suggesting removing the code "kernelcore=nn" and
> "movablecore=" are using? If yes, I can repost with these changes.

Yes.
-- 
Michal Hocko
SUSE Labs

Re: [PATCH] mm/page_alloc: Deprecate kernelcore=nn and movable_core=

[David Rientjes]

On Tue, 17 Jul 2018, Baoquan He wrote:

> We can still use 'kernelcore=mirror' or 'movable_node' for the usage
> of hotplug and movable zone. If somebody shows up with a valid usecase
> we can reconsider.
> 

We actively use kernelcore=n%, I had recently added support for the option 
in the first place in 4.17.  It's certainly not deprecated.

commit a5c6d6509342785bef53bf9508e1842b303f1878
Author: David Rientjes <rientjes@google.com>
Date:   Thu Apr 5 16:23:09 2018 -0700

    mm, page_alloc: extend kernelcore and movablecore for percent

Re: [PATCH] mm/page_alloc: Deprecate kernelcore=nn and movable_core=

[Baoquan He]

On 07/17/18 at 01:46pm, David Rientjes wrote:
> On Tue, 17 Jul 2018, Baoquan He wrote:
> 
> > We can still use 'kernelcore=mirror' or 'movable_node' for the usage
> > of hotplug and movable zone. If somebody shows up with a valid usecase
> > we can reconsider.
> > 
> 
> We actively use kernelcore=n%, I had recently added support for the option 
> in the first place in 4.17.  It's certainly not deprecated.

Thanks for telling. Just for curiosity, could you tell the scenario you
are using kernelcore=n%? Since it evenly spread movable area on nodes,
we may not be able to physically hot unplug/plug RAM.

Thanks
Baoquan

Re: [PATCH] mm/page_alloc: Deprecate kernelcore=nn and movable_core=

[David Rientjes]

On Wed, 18 Jul 2018, Baoquan He wrote:

> > > We can still use 'kernelcore=mirror' or 'movable_node' for the usage
> > > of hotplug and movable zone. If somebody shows up with a valid usecase
> > > we can reconsider.
> > > 
> > 
> > We actively use kernelcore=n%, I had recently added support for the option 
> > in the first place in 4.17.  It's certainly not deprecated.
> 
> Thanks for telling. Just for curiosity, could you tell the scenario you
> are using kernelcore=n%? Since it evenly spread movable area on nodes,
> we may not be able to physically hot unplug/plug RAM.
> 

To evenly distribute ZONE_MOVABLE over a set of nodes regardless of the 
many memory capacities of systems that we have where individual command 
lines cannot be tuned.  But you want to deprecate kernelcore=nn, not just 
the percent version, so I assume that's not the answer you're looking for. 
We do not enable CONFIG_MEMORY_HOTPLUG so this deprecation would break our 
userspace such that we cannot use ZONE_MOVABLE.

Re: [PATCH] mm/page_alloc: Deprecate kernelcore=nn and movable_core=

[Michal Hocko]

On Tue 17-07-18 13:46:43, David Rientjes wrote:
> On Tue, 17 Jul 2018, Baoquan He wrote:
> 
> > We can still use 'kernelcore=mirror' or 'movable_node' for the usage
> > of hotplug and movable zone. If somebody shows up with a valid usecase
> > we can reconsider.
> > 
> 
> We actively use kernelcore=n%, I had recently added support for the option 
> in the first place in 4.17.  It's certainly not deprecated.
> 
> commit a5c6d6509342785bef53bf9508e1842b303f1878
> Author: David Rientjes <rientjes@google.com>
> Date:   Thu Apr 5 16:23:09 2018 -0700
> 
>     mm, page_alloc: extend kernelcore and movablecore for percent

What kind of functionality do you need to not depend on this knob?
I mean it is a gross hack and you are basically working around
fragmentation issues. ZONE_MOVABLE doesn't seem to be a long term
solution here IMHO.
-- 
Michal Hocko
SUSE Labs