Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Tue, 17 Jul 2018 15:31:09 +0200
From:   Michal Hocko <mhocko@kernel.org>
To:     Baoquan He <bhe@redhat.com>
Cc:     linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        akpm@linux-foundation.org, corbet@lwn.net,
        linux-doc@vger.kernel.org
Subject: Re: [PATCH] mm/page_alloc: Deprecate kernelcore=nn and movable_core=
Message-ID: <20180717133109.GI7193@dhcp22.suse.cz>
References: <20180717131837.18411-1-bhe@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20180717131837.18411-1-bhe@redhat.com>
User-Agent: Mutt/1.10.0 (2018-05-17)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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