[PATCH v2 0/3] powerpc/mm: movable hotplug memory nodes

[PATCH v2 0/3] powerpc/mm: movable hotplug memory nodes

[Reza Arbab]

These changes enable onlining memory into ZONE_MOVABLE on power, and the 
creation of discrete nodes of movable memory.

We provide a way to describe the extents and numa associativity of such 
a node in the device tree, yet still defer the memory addition to take 
place post-boot through hotplug.

In v1, this patchset introduced a new dt compatible id to explicitly 
create a memoryless node at boot. Here, things have been simplified to 
be applicable regardless of the status of node hotplug on power. We 
still intend to enable hotadding a pgdat, but that's now untangled as a 
separate topic.

v2:
* Use the "status" property of standard dt memory nodes instead of 
  introducing a new "ibm,hotplug-aperture" compatible id.

* Remove the patch which explicitly creates a memoryless node. This set 
  no longer has any bearing on whether the pgdat is created at boot or 
  at the time of memory addition.

v1:
* http://lkml.kernel.org/r/1470680843-28702-1-git-send-email-arbab@linux.vnet.ibm.com

Reza Arbab (3):
  drivers/of: recognize status property of dt memory nodes
  powerpc/mm: allow memory hotplug into a memoryless node
  mm: enable CONFIG_MOVABLE_NODE on powerpc

 Documentation/kernel-parameters.txt |  2 +-
 arch/powerpc/mm/numa.c              | 13 +------------
 drivers/of/fdt.c                    |  8 ++++++++
 mm/Kconfig                          |  2 +-
 4 files changed, 11 insertions(+), 14 deletions(-)

-- 
1.8.3.1

[PATCH v2 1/3] drivers/of: recognize status property of dt memory nodes

[Reza Arbab]

Respect the standard dt "status" property when scanning memory nodes in
early_init_dt_scan_memory(), so that if the property is present and not
"okay", no memory will be added.

The use case at hand is accelerator or device memory, which may be
unusable until post-boot initialization of the memory link. Such a node
can be described in the dt as any other, given its status is "disabled".
Per the device tree specification,

"disabled"
	Indicates that the device is not presently operational, but it
	might become operational in the future (for example, something
	is not plugged in, or switched off).

Once such memory is made operational, it can then be hotplugged.

Signed-off-by: Reza Arbab <arbab@linux.vnet.ibm.com>
---
 drivers/of/fdt.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 085c638..fc19590 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -1022,8 +1022,10 @@ int __init early_init_dt_scan_memory(unsigned long node, const char *uname,
 				     int depth, void *data)
 {
 	const char *type = of_get_flat_dt_prop(node, "device_type", NULL);
+	const char *status;
 	const __be32 *reg, *endp;
 	int l;
+	bool add_memory;
 
 	/* We are scanning "memory" nodes only */
 	if (type == NULL) {
@@ -1044,6 +1046,9 @@ int __init early_init_dt_scan_memory(unsigned long node, const char *uname,
 
 	endp = reg + (l / sizeof(__be32));
 
+	status = of_get_flat_dt_prop(node, "status", NULL);
+	add_memory = !status || !strcmp(status, "okay");
+
 	pr_debug("memory scan node %s, reg size %d,\n", uname, l);
 
 	while ((endp - reg) >= (dt_root_addr_cells + dt_root_size_cells)) {
@@ -1057,6 +1062,9 @@ int __init early_init_dt_scan_memory(unsigned long node, const char *uname,
 		pr_debug(" - %llx ,  %llx\n", (unsigned long long)base,
 		    (unsigned long long)size);
 
+		if (!add_memory)
+			continue;
+
 		early_init_dt_add_memory_arch(base, size);
 	}
 
-- 
1.8.3.1

Re: [PATCH v2 1/3] drivers/of: recognize status property of dt memory nodes

[Rob Herring]

On Wed, Sep 14, 2016 at 3:06 PM, Reza Arbab <arbab@linux.vnet.ibm.com> wrote:
> Respect the standard dt "status" property when scanning memory nodes in
> early_init_dt_scan_memory(), so that if the property is present and not
> "okay", no memory will be added.
>
> The use case at hand is accelerator or device memory, which may be
> unusable until post-boot initialization of the memory link. Such a node
> can be described in the dt as any other, given its status is "disabled".
> Per the device tree specification,
>
> "disabled"
>         Indicates that the device is not presently operational, but it
>         might become operational in the future (for example, something
>         is not plugged in, or switched off).
>
> Once such memory is made operational, it can then be hotplugged.
>
> Signed-off-by: Reza Arbab <arbab@linux.vnet.ibm.com>
> ---
>  drivers/of/fdt.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index 085c638..fc19590 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -1022,8 +1022,10 @@ int __init early_init_dt_scan_memory(unsigned long node, const char *uname,
>                                      int depth, void *data)
>  {
>         const char *type = of_get_flat_dt_prop(node, "device_type", NULL);
> +       const char *status;
>         const __be32 *reg, *endp;
>         int l;
> +       bool add_memory;
>
>         /* We are scanning "memory" nodes only */
>         if (type == NULL) {
> @@ -1044,6 +1046,9 @@ int __init early_init_dt_scan_memory(unsigned long node, const char *uname,
>
>         endp = reg + (l / sizeof(__be32));
>
> +       status = of_get_flat_dt_prop(node, "status", NULL);
> +       add_memory = !status || !strcmp(status, "okay");

Move this into it's own function to mirror the unflattened version
(of_device_is_available). Also, make sure the logic is the same. IIRC,
"ok" is also allowed.

> +
>         pr_debug("memory scan node %s, reg size %d,\n", uname, l);
>
>         while ((endp - reg) >= (dt_root_addr_cells + dt_root_size_cells)) {
> @@ -1057,6 +1062,9 @@ int __init early_init_dt_scan_memory(unsigned long node, const char *uname,
>                 pr_debug(" - %llx ,  %llx\n", (unsigned long long)base,
>                     (unsigned long long)size);
>
> +               if (!add_memory)
> +                       continue;

There's no point in checking this in the loop. status applies to the
whole node. Just return up above.

Rob

Re: [PATCH v2 1/3] drivers/of: recognize status property of dt memory nodes

[Reza Arbab]

On Thu, Sep 15, 2016 at 08:43:08AM -0500, Rob Herring wrote:
>On Wed, Sep 14, 2016 at 3:06 PM, Reza Arbab <arbab@linux.vnet.ibm.com> wrote:
>> +       status = of_get_flat_dt_prop(node, "status", NULL);
>> +       add_memory = !status || !strcmp(status, "okay");
>
>Move this into it's own function to mirror the unflattened version
>(of_device_is_available). Also, make sure the logic is the same. IIRC,
>"ok" is also allowed.

Will do. 

>> @@ -1057,6 +1062,9 @@ int __init early_init_dt_scan_memory(unsigned long node, const char *uname,
>>                 pr_debug(" - %llx ,  %llx\n", (unsigned long long)base,
>>                     (unsigned long long)size);
>>
>> +               if (!add_memory)
>> +                       continue;
>
>There's no point in checking this in the loop. status applies to the
>whole node. Just return up above.

I was trying to preserve that pr_debug output for these nodes, but I'm 
also fine with skipping it.

Thanks for your feedback! I'll spin a v3 of this patchset soon.

-- 
Reza Arbab

Re: [PATCH v2 1/3] drivers/of: recognize status property of dt memory nodes

[Balbir Singh]

On 15/09/16 06:06, Reza Arbab wrote:
> Respect the standard dt "status" property when scanning memory nodes in
> early_init_dt_scan_memory(), so that if the property is present and not
> "okay", no memory will be added.
> 
> The use case at hand is accelerator or device memory, which may be
> unusable until post-boot initialization of the memory link. Such a node
> can be described in the dt as any other, given its status is "disabled".
> Per the device tree specification,
> 
> "disabled"
> 	Indicates that the device is not presently operational, but it
> 	might become operational in the future (for example, something
> 	is not plugged in, or switched off).
> 
> Once such memory is made operational, it can then be hotplugged.
> 
> Signed-off-by: Reza Arbab <arbab@linux.vnet.ibm.com>

Makes sense, so basically a /memory@  with missing status or status = "okay"
are added, others are skipped. No memblocks corresponding to those nodes
are created either.

Balbir Singh

[PATCH v2 2/3] powerpc/mm: allow memory hotplug into a memoryless node

[Reza Arbab]

Remove the check which prevents us from hotplugging into an empty node.

This limitation has been questioned before [1], and judging by the
response, there doesn't seem to be a reason we can't remove it. No issues
have been found in light testing.

[1] http://lkml.kernel.org/r/CAGZKiBrmkSa1yyhbf5hwGxubcjsE5SmkSMY4tpANERMe2UG4bg@mail.gmail.com
    http://lkml.kernel.org/r/20160511215051.GF22115@arbab-laptop.austin.ibm.com

Signed-off-by: Reza Arbab <arbab@linux.vnet.ibm.com>
Acked-by: Balbir Singh <bsingharora@gmail.com>
Cc: Nathan Fontenot <nfont@linux.vnet.ibm.com>
Cc: Bharata B Rao <bharata@linux.vnet.ibm.com>
---
 arch/powerpc/mm/numa.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index 75b9cd6..d7ac419 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -1121,7 +1121,7 @@ static int hot_add_node_scn_to_nid(unsigned long scn_addr)
 int hot_add_scn_to_nid(unsigned long scn_addr)
 {
 	struct device_node *memory = NULL;
-	int nid, found = 0;
+	int nid;
 
 	if (!numa_enabled || (min_common_depth < 0))
 		return first_online_node;
@@ -1137,17 +1137,6 @@ int hot_add_scn_to_nid(unsigned long scn_addr)
 	if (nid < 0 || !node_online(nid))
 		nid = first_online_node;
 
-	if (NODE_DATA(nid)->node_spanned_pages)
-		return nid;
-
-	for_each_online_node(nid) {
-		if (NODE_DATA(nid)->node_spanned_pages) {
-			found = 1;
-			break;
-		}
-	}
-
-	BUG_ON(!found);
 	return nid;
 }
 
-- 
1.8.3.1

Re: [PATCH v2 2/3] powerpc/mm: allow memory hotplug into a memoryless node

[Balbir Singh]

On 15/09/16 06:06, Reza Arbab wrote:
> Remove the check which prevents us from hotplugging into an empty node.
> 
> This limitation has been questioned before [1], and judging by the
> response, there doesn't seem to be a reason we can't remove it. No issues
> have been found in light testing.
> 
> [1] http://lkml.kernel.org/r/CAGZKiBrmkSa1yyhbf5hwGxubcjsE5SmkSMY4tpANERMe2UG4bg@mail.gmail.com
>     http://lkml.kernel.org/r/20160511215051.GF22115@arbab-laptop.austin.ibm.com
> 
> Signed-off-by: Reza Arbab <arbab@linux.vnet.ibm.com>
> Acked-by: Balbir Singh <bsingharora@gmail.com>
> Cc: Nathan Fontenot <nfont@linux.vnet.ibm.com>
> Cc: Bharata B Rao <bharata@linux.vnet.ibm.com>
> ---
>  arch/powerpc/mm/numa.c | 13 +------------
>  1 file changed, 1 insertion(+), 12 deletions(-)
> 

I presume you've tested with CONFIG_NODES_SHIFT of 8 (255 nodes?)

Balbir Singh.

Re: [PATCH v2 2/3] powerpc/mm: allow memory hotplug into a memoryless node

[Reza Arbab]

On Mon, Sep 19, 2016 at 09:53:49PM +1000, Balbir Singh wrote:
>I presume you've tested with CONFIG_NODES_SHIFT of 8 (255 nodes?)

Oh yes, definitely.

The large number of possible nodes does not come into play here.

-- 
Reza Arbab

[PATCH v2 3/3] mm: enable CONFIG_MOVABLE_NODE on powerpc

[Reza Arbab]

Onlining memory into ZONE_MOVABLE requires CONFIG_MOVABLE_NODE. Enable
the use of this config option on PPC64 platforms.

Signed-off-by: Reza Arbab <arbab@linux.vnet.ibm.com>
---
 Documentation/kernel-parameters.txt | 2 +-
 mm/Kconfig                          | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index a4f4d69..3d8460d 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -2344,7 +2344,7 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			that the amount of memory usable for all allocations
 			is not too small.
 
-	movable_node	[KNL,X86] Boot-time switch to enable the effects
+	movable_node	[KNL,X86,PPC] Boot-time switch to enable the effects
 			of CONFIG_MOVABLE_NODE=y. See mm/Kconfig for details.
 
 	MTD_Partition=	[MTD]
diff --git a/mm/Kconfig b/mm/Kconfig
index be0ee11..4b19cd3 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -153,7 +153,7 @@ 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
+	depends on X86_64 || PPC64
 	depends on NUMA
 	default n
 	help
-- 
1.8.3.1

Re: [PATCH v2 3/3] mm: enable CONFIG_MOVABLE_NODE on powerpc

[Aneesh Kumar K.V]

Reza Arbab <arbab@linux.vnet.ibm.com> writes:

> Onlining memory into ZONE_MOVABLE requires CONFIG_MOVABLE_NODE. Enable
> the use of this config option on PPC64 platforms.
>
> Signed-off-by: Reza Arbab <arbab@linux.vnet.ibm.com>
> ---
>  Documentation/kernel-parameters.txt | 2 +-
>  mm/Kconfig                          | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index a4f4d69..3d8460d 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -2344,7 +2344,7 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>  			that the amount of memory usable for all allocations
>  			is not too small.
>  
> -	movable_node	[KNL,X86] Boot-time switch to enable the effects
> +	movable_node	[KNL,X86,PPC] Boot-time switch to enable the effects
>  			of CONFIG_MOVABLE_NODE=y. See mm/Kconfig for details.
>  

Movable node also does.
	memblock_set_bottom_up(true);
What is the impact of that. Do we need changes equivalent to that ? Also
where are we marking the nodes which can be hotplugged, ie where do we
do memblock_mark_hotplug() ?
       
>  	MTD_Partition=	[MTD]
> diff --git a/mm/Kconfig b/mm/Kconfig
> index be0ee11..4b19cd3 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -153,7 +153,7 @@ 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
> +	depends on X86_64 || PPC64
>  	depends on NUMA
>  	default n
>  	help

-aneesh

Re: [PATCH v2 3/3] mm: enable CONFIG_MOVABLE_NODE on powerpc

[Reza Arbab]

On Mon, Sep 19, 2016 at 11:59:35AM +0530, Aneesh Kumar K.V wrote:
>Movable node also does.
>	memblock_set_bottom_up(true);
>What is the impact of that. Do we need changes equivalent to that ? Also
>where are we marking the nodes which can be hotplugged, ie where do we
>do memblock_mark_hotplug() ?

These are related to the mechanism x86 uses to create movable nodes at 
boot. The SRAT is parsed to mark any hotplug memory. That marking is 
used later when initializing ZONE_MOVABLE for each node. [1]

The bottom-up allocation is due to a timing issue [2]. There is a window 
where kernel memory may be allocated before the SRAT is parsed. Any 
bottom-up allocations done during that time will likely be in the same 
(nonmovable) node as the kernel image.

On power, I don't think we have a heuristic equivalent to that SRAT 
memory hotplug info. So, we'll be limited to dynamically adding movable 
nodes after boot.

1. http://events.linuxfoundation.org/sites/events/files/lcjp13_chen.pdf
2. commit 79442ed189ac ("mm/memblock.c: introduce bottom-up allocation 
mode")

-- 
Reza Arbab

Re: [PATCH v2 3/3] mm: enable CONFIG_MOVABLE_NODE on powerpc

[Aneesh Kumar K.V]

Reza Arbab <arbab@linux.vnet.ibm.com> writes:

> On Mon, Sep 19, 2016 at 11:59:35AM +0530, Aneesh Kumar K.V wrote:
>>Movable node also does.
>>	memblock_set_bottom_up(true);
>>What is the impact of that. Do we need changes equivalent to that ? Also
>>where are we marking the nodes which can be hotplugged, ie where do we
>>do memblock_mark_hotplug() ?
>
> These are related to the mechanism x86 uses to create movable nodes at 
> boot. The SRAT is parsed to mark any hotplug memory. That marking is 
> used later when initializing ZONE_MOVABLE for each node. [1]
>
> The bottom-up allocation is due to a timing issue [2]. There is a window 
> where kernel memory may be allocated before the SRAT is parsed. Any 
> bottom-up allocations done during that time will likely be in the same 
> (nonmovable) node as the kernel image.
>
> On power, I don't think we have a heuristic equivalent to that SRAT 
> memory hotplug info. So, we'll be limited to dynamically adding movable 
> nodes after boot.
>
> 1. http://events.linuxfoundation.org/sites/events/files/lcjp13_chen.pdf
> 2. commit 79442ed189ac ("mm/memblock.c: introduce bottom-up allocation 
> mode")
>

What I was checking was how will one mark a node movable in ppc64 ? I
don't see ppc64 code doing the equivalent of memblock_mark_hotplug().

So when you say "Onlining memory into ZONE_MOVABLE requires
CONFIG_MOVABLE_NODE" where is that restriction ?. IIUC,
should_add_memory_movable() will only return ZONE_MOVABLE only if it is
non empty and MOVABLE_NODE will create a ZONE_MOVABLE zone by default
only if it finds a memblock marked hotpluggable. So wondering if we
are not calling memblock_mark_hotplug() how is it working. Or am I
missing something ?

-aneesh

    

Re: [PATCH v2 3/3] mm: enable CONFIG_MOVABLE_NODE on powerpc

[Reza Arbab]

On Wed, Sep 21, 2016 at 12:39:51PM +0530, Aneesh Kumar K.V wrote:
>What I was checking was how will one mark a node movable in ppc64 ? I
>don't see ppc64 code doing the equivalent of memblock_mark_hotplug().

Post boot, the marking mechanism is not necessary. You can create a 
movable node by putting all of the node's memory into ZONE_MOVABLE 
during the hotplug.

>So when you say "Onlining memory into ZONE_MOVABLE requires
>CONFIG_MOVABLE_NODE" where is that restriction ?. IIUC,
>should_add_memory_movable() will only return ZONE_MOVABLE only if it is
>non empty and MOVABLE_NODE will create a ZONE_MOVABLE zone by default
>only if it finds a memblock marked hotpluggable. So wondering if we
>are not calling memblock_mark_hotplug() how is it working. Or am I
>missing something ?

You are looking at the addition step of hotplug. You're correct there, 
the memory is added to the default zone, not ZONE_MOVABLE. The 
transition to ZONE_MOVABLE takes place during the onlining step. In 
online_pages():

	zone = move_pfn_range(zone_shift, pfn, pfn + nr_pages);

The reason we need CONFIG_MOVABLE_NODE is right before that:

	if ((zone_idx(zone) > ZONE_NORMAL ||
	    online_type == MMOP_ONLINE_MOVABLE) &&
	    !can_online_high_movable(zone))
		return -EINVAL;

where can_online_high_movable() is defined like this:

	#ifdef CONFIG_MOVABLE_NODE
	/*
	 * When CONFIG_MOVABLE_NODE, we permit onlining of a node which doesn't have
	 * normal memory.
	 */
	static bool can_online_high_movable(struct zone *zone)
	{
		return true;
	}
	#else /* CONFIG_MOVABLE_NODE */
	/* ensure every online node has NORMAL memory */
	static bool can_online_high_movable(struct zone *zone)
	{
		return node_state(zone_to_nid(zone), N_NORMAL_MEMORY);
	}
	#endif /* CONFIG_MOVABLE_NODE */

To be more clear, I can change the commit log to say "Onlining all of a 
node's memory into ZONE_MOVABLE requires CONFIG_MOVABLE_NODE".

-- 
Reza Arbab

Re: [PATCH v2 3/3] mm: enable CONFIG_MOVABLE_NODE on powerpc

[Aneesh Kumar K.V]

Reza Arbab <arbab@linux.vnet.ibm.com> writes:

> On Wed, Sep 21, 2016 at 12:39:51PM +0530, Aneesh Kumar K.V wrote:
>>What I was checking was how will one mark a node movable in ppc64 ? I
>>don't see ppc64 code doing the equivalent of memblock_mark_hotplug().
>
> Post boot, the marking mechanism is not necessary. You can create a 
> movable node by putting all of the node's memory into ZONE_MOVABLE 
> during the hotplug.
>
>>So when you say "Onlining memory into ZONE_MOVABLE requires
>>CONFIG_MOVABLE_NODE" where is that restriction ?. IIUC,
>>should_add_memory_movable() will only return ZONE_MOVABLE only if it is
>>non empty and MOVABLE_NODE will create a ZONE_MOVABLE zone by default
>>only if it finds a memblock marked hotpluggable. So wondering if we
>>are not calling memblock_mark_hotplug() how is it working. Or am I
>>missing something ?
>
> You are looking at the addition step of hotplug. You're correct there, 
> the memory is added to the default zone, not ZONE_MOVABLE. The 
> transition to ZONE_MOVABLE takes place during the onlining step. In 
> online_pages():
>
> 	zone = move_pfn_range(zone_shift, pfn, pfn + nr_pages);
>
> The reason we need CONFIG_MOVABLE_NODE is right before that:
>
> 	if ((zone_idx(zone) > ZONE_NORMAL ||
> 	    online_type == MMOP_ONLINE_MOVABLE) &&
> 	    !can_online_high_movable(zone))
> 		return -EINVAL;
>

So we are looking at two step online process here. The above explained
the details nicely. Can you capture these details in the commit message. ie,
to say that when using 'echo online-movable > state' we allow the move from
normal to movable only if movable node is set. Also you may want to
mention that we still don't support the auto-online to movable.


> where can_online_high_movable() is defined like this:
>
> 	#ifdef CONFIG_MOVABLE_NODE
> 	/*
> 	 * When CONFIG_MOVABLE_NODE, we permit onlining of a node which doesn't have
> 	 * normal memory.
> 	 */
> 	static bool can_online_high_movable(struct zone *zone)
> 	{
> 		return true;
> 	}
> 	#else /* CONFIG_MOVABLE_NODE */
> 	/* ensure every online node has NORMAL memory */
> 	static bool can_online_high_movable(struct zone *zone)
> 	{
> 		return node_state(zone_to_nid(zone), N_NORMAL_MEMORY);
> 	}
> 	#endif /* CONFIG_MOVABLE_NODE */
>
> To be more clear, I can change the commit log to say "Onlining all of a 
> node's memory into ZONE_MOVABLE requires CONFIG_MOVABLE_NODE".
>
> -- 
> Reza Arbab

-aneesh

Re: [PATCH v2 3/3] mm: enable CONFIG_MOVABLE_NODE on powerpc

[Reza Arbab]

On Wed, Sep 21, 2016 at 08:13:37PM +0530, Aneesh Kumar K.V wrote:
>So we are looking at two step online process here. The above explained
>the details nicely. Can you capture these details in the commit message. ie,
>to say that when using 'echo online-movable > state' we allow the move from
>normal to movable only if movable node is set. Also you may want to
>mention that we still don't support the auto-online to movable.

Sure, no problem. I'll use a more verbose commit message in v3.

-- 
Reza Arbab