Re: [RFC PATCH 2/2] mm, memory_hotplug: drop CONFIG_MOVABLE_NODE

From: Vlastimil Babka
Date: Wed May 24 2017 - 08:56:37 EST


On 05/24/2017 02:24 PM, Michal Hocko wrote:
> From: Michal Hocko <mhocko@xxxxxxxx>
>
> 20b2f52b73fe ("numa: add CONFIG_MOVABLE_NODE for movable-dedicated
> node") has introduced CONFIG_MOVABLE_NODE without a good explanation on
> why it is actually useful. It makes a lot of sense to make movable node
> semantic opt in but we already have that because the feature has to be
> explicitly enabled on the kernel command line. A config option on top
> only makes the configuration space larger without a good reason. It also
> adds an additional ifdefery that pollutes the code. Just drop the config
> option and make it de-facto always enabled. This shouldn't introduce any
> change to the semantic.
>
> Signed-off-by: Michal Hocko <mhocko@xxxxxxxx>

I agree with the intention.

> ---
> Documentation/admin-guide/kernel-parameters.txt | 7 +++++--
> drivers/base/node.c | 4 ----
> include/linux/memblock.h | 18 -----------------
> include/linux/nodemask.h | 4 ----
> mm/Kconfig | 26 -------------------------
> mm/memblock.c | 2 --
> mm/memory_hotplug.c | 4 ----
> mm/page_alloc.c | 2 --
> 8 files changed, 5 insertions(+), 62 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index facc20a3f962..ec7d6ae01c96 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -2246,8 +2246,11 @@
> that the amount of memory usable for all allocations
> is not too small.
>
> - movable_node [KNL] Boot-time switch to enable the effects
> - of CONFIG_MOVABLE_NODE=y. See mm/Kconfig for details.
> + movable_node [KNL] Boot-time switch to make hotplugable to be

hotplugable what, memory? nodes?

> + movable. This means that the memory of such nodes
> + will be usable only for movable allocations which
> + rules out almost all kernel allocations. Use with
> + caution!
>
> MTD_Partition= [MTD]
> Format: <name>,<region-number>,<size>,<offset>

...

> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -149,32 +149,6 @@ config NO_BOOTMEM
> config MEMORY_ISOLATION
> bool
>
> -config MOVABLE_NODE
> - bool "Enable to assign a node which has only movable memory"
> - depends on HAVE_MEMBLOCK
> - depends on NO_BOOTMEM
> - depends on X86_64 || OF_EARLY_FLATTREE || MEMORY_HOTPLUG
> - depends on NUMA

That's a lot of depends. What happens if some of them are not met and
the movable_node bootparam is used?

> - default n
> - help
> - Allow a node to have only movable memory. Pages used by the kernel,
> - such as direct mapping pages cannot be migrated. So the corresponding
> - memory device cannot be hotplugged. This option allows the following
> - two things:
> - - When the system is booting, node full of hotpluggable memory can
> - be arranged to have only movable memory so that the whole node can
> - be hot-removed. (need movable_node boot option specified).

> - - After the system is up, the option allows users to online all the
> - memory of a node as movable memory so that the whole node can be
> - hot-removed.

Strictly speaking this part is already gone with patch 1/2. Only matters
in case this one is rejected for some reason.

> - Users who don't use the memory hotplug feature are fine with this
> - option on since they don't specify movable_node boot option or they
> - don't online memory as movable.
> -
> - Say Y here if you want to hotplug a whole node.
> - Say N here if you want kernel to use memory on all nodes evenly.
> -
> #
> # Only be set on architectures that have completely implemented memory hotplug
> # feature. If you are not sure, don't touch it.