[v1,4/5] mm/memory_hotplug: convert memhp_auto_online to store an online_type
diff mbox series

Message ID 20200311123026.16071-5-david@redhat.com
State In Next
Commit f01d2667aadf3139e6eaf144c8162f726d47d11b
Headers show
Series
  • [v1,1/5] drivers/base/memory: rename MMOP_ONLINE_KEEP to MMOP_ONLINE
Related show

Commit Message

David Hildenbrand March 11, 2020, 12:30 p.m. UTC
... and rename it to memhp_default_online_type. This is a preparation
for more detailed default online behavior.

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Baoquan He <bhe@redhat.com>
Cc: Wei Yang <richard.weiyang@gmail.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: "K. Y. Srinivasan" <kys@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Stephen Hemminger <sthemmin@microsoft.com>
Cc: Wei Liu <wei.liu@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-hyperv@vger.kernel.org
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 arch/powerpc/platforms/powernv/memtrace.c |  2 +-
 drivers/base/memory.c                     | 10 ++++------
 drivers/hv/hv_balloon.c                   |  2 +-
 include/linux/memory_hotplug.h            |  3 ++-
 mm/memory_hotplug.c                       | 13 +++++++------
 5 files changed, 15 insertions(+), 15 deletions(-)

Comments

Wei Yang March 11, 2020, 2:25 p.m. UTC | #1
On Wed, Mar 11, 2020 at 01:30:25PM +0100, David Hildenbrand wrote:
>... and rename it to memhp_default_online_type. This is a preparation
>for more detailed default online behavior.
>
>Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>Cc: Andrew Morton <akpm@linux-foundation.org>
>Cc: Michal Hocko <mhocko@kernel.org>
>Cc: Oscar Salvador <osalvador@suse.de>
>Cc: "Rafael J. Wysocki" <rafael@kernel.org>
>Cc: Baoquan He <bhe@redhat.com>
>Cc: Wei Yang <richard.weiyang@gmail.com>
>Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>Cc: Paul Mackerras <paulus@samba.org>
>Cc: Michael Ellerman <mpe@ellerman.id.au>
>Cc: "K. Y. Srinivasan" <kys@microsoft.com>
>Cc: Haiyang Zhang <haiyangz@microsoft.com>
>Cc: Stephen Hemminger <sthemmin@microsoft.com>
>Cc: Wei Liu <wei.liu@kernel.org>
>Cc: Thomas Gleixner <tglx@linutronix.de>
>Cc: linuxppc-dev@lists.ozlabs.org
>Cc: linux-hyperv@vger.kernel.org
>Signed-off-by: David Hildenbrand <david@redhat.com>

Reviewed-by: Wei Yang <richard.weiyang@gmail.com>

>---
> arch/powerpc/platforms/powernv/memtrace.c |  2 +-
> drivers/base/memory.c                     | 10 ++++------
> drivers/hv/hv_balloon.c                   |  2 +-
> include/linux/memory_hotplug.h            |  3 ++-
> mm/memory_hotplug.c                       | 13 +++++++------
> 5 files changed, 15 insertions(+), 15 deletions(-)
>
>diff --git a/arch/powerpc/platforms/powernv/memtrace.c b/arch/powerpc/platforms/powernv/memtrace.c
>index d6d64f8718e6..e15a600cfa4d 100644
>--- a/arch/powerpc/platforms/powernv/memtrace.c
>+++ b/arch/powerpc/platforms/powernv/memtrace.c
>@@ -235,7 +235,7 @@ static int memtrace_online(void)
> 		 * If kernel isn't compiled with the auto online option
> 		 * we need to online the memory ourselves.
> 		 */
>-		if (!memhp_auto_online) {
>+		if (memhp_default_online_type == MMOP_OFFLINE) {
> 			lock_device_hotplug();
> 			walk_memory_blocks(ent->start, ent->size, NULL,
> 					   online_mem_block);
>diff --git a/drivers/base/memory.c b/drivers/base/memory.c
>index 8a7f29c0bf97..8d3e16dab69f 100644
>--- a/drivers/base/memory.c
>+++ b/drivers/base/memory.c
>@@ -386,10 +386,8 @@ static DEVICE_ATTR_RO(block_size_bytes);
> static ssize_t auto_online_blocks_show(struct device *dev,
> 				       struct device_attribute *attr, char *buf)
> {
>-	if (memhp_auto_online)
>-		return sprintf(buf, "online\n");
>-	else
>-		return sprintf(buf, "offline\n");
>+	return sprintf(buf, "%s\n",
>+		       online_type_to_str[memhp_default_online_type]);
> }
> 
> static ssize_t auto_online_blocks_store(struct device *dev,
>@@ -397,9 +395,9 @@ static ssize_t auto_online_blocks_store(struct device *dev,
> 					const char *buf, size_t count)
> {
> 	if (sysfs_streq(buf, "online"))
>-		memhp_auto_online = true;
>+		memhp_default_online_type = MMOP_ONLINE;
> 	else if (sysfs_streq(buf, "offline"))
>-		memhp_auto_online = false;
>+		memhp_default_online_type = MMOP_OFFLINE;
> 	else
> 		return -EINVAL;
> 
>diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
>index a02ce43d778d..3b90fd12e0c5 100644
>--- a/drivers/hv/hv_balloon.c
>+++ b/drivers/hv/hv_balloon.c
>@@ -727,7 +727,7 @@ static void hv_mem_hot_add(unsigned long start, unsigned long size,
> 		spin_unlock_irqrestore(&dm_device.ha_lock, flags);
> 
> 		init_completion(&dm_device.ol_waitevent);
>-		dm_device.ha_waiting = !memhp_auto_online;
>+		dm_device.ha_waiting = memhp_default_online_type == MMOP_OFFLINE;
> 
> 		nid = memory_add_physaddr_to_nid(PFN_PHYS(start_pfn));
> 		ret = add_memory(nid, PFN_PHYS((start_pfn)),
>diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
>index c2e06ed5e0e9..c6e090b34c4b 100644
>--- a/include/linux/memory_hotplug.h
>+++ b/include/linux/memory_hotplug.h
>@@ -117,7 +117,8 @@ extern int arch_add_memory(int nid, u64 start, u64 size,
> 			struct mhp_restrictions *restrictions);
> extern u64 max_mem_size;
> 
>-extern bool memhp_auto_online;
>+/* Default online_type (MMOP_*) when new memory blocks are added. */
>+extern int memhp_default_online_type;
> /* If movable_node boot option specified */
> extern bool movable_node_enabled;
> static inline bool movable_node_is_enabled(void)
>diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
>index 1a00b5a37ef6..01443c70aa27 100644
>--- a/mm/memory_hotplug.c
>+++ b/mm/memory_hotplug.c
>@@ -67,18 +67,18 @@ void put_online_mems(void)
> bool movable_node_enabled = false;
> 
> #ifndef CONFIG_MEMORY_HOTPLUG_DEFAULT_ONLINE
>-bool memhp_auto_online;
>+int memhp_default_online_type = MMOP_OFFLINE;
> #else
>-bool memhp_auto_online = true;
>+int memhp_default_online_type = MMOP_ONLINE;
> #endif
>-EXPORT_SYMBOL_GPL(memhp_auto_online);
>+EXPORT_SYMBOL_GPL(memhp_default_online_type);
> 
> static int __init setup_memhp_default_state(char *str)
> {
> 	if (!strcmp(str, "online"))
>-		memhp_auto_online = true;
>+		memhp_default_online_type = MMOP_ONLINE;
> 	else if (!strcmp(str, "offline"))
>-		memhp_auto_online = false;
>+		memhp_default_online_type = MMOP_OFFLINE;
> 
> 	return 1;
> }
>@@ -991,6 +991,7 @@ static int check_hotplug_memory_range(u64 start, u64 size)
> 
> static int online_memory_block(struct memory_block *mem, void *arg)
> {
>+	mem->online_type = memhp_default_online_type;
> 	return device_online(&mem->dev);
> }
> 
>@@ -1063,7 +1064,7 @@ int __ref add_memory_resource(int nid, struct resource *res)
> 	mem_hotplug_done();
> 
> 	/* online pages if requested */
>-	if (memhp_auto_online)
>+	if (memhp_default_online_type != MMOP_OFFLINE)
> 		walk_memory_blocks(start, size, NULL, online_memory_block);
> 
> 	return ret;
>-- 
>2.24.1
Michal Hocko March 16, 2020, 3:24 p.m. UTC | #2
On Wed 11-03-20 13:30:25, David Hildenbrand wrote:
[...]
> diff --git a/arch/powerpc/platforms/powernv/memtrace.c b/arch/powerpc/platforms/powernv/memtrace.c
> index d6d64f8718e6..e15a600cfa4d 100644
> --- a/arch/powerpc/platforms/powernv/memtrace.c
> +++ b/arch/powerpc/platforms/powernv/memtrace.c
> @@ -235,7 +235,7 @@ static int memtrace_online(void)
>  		 * If kernel isn't compiled with the auto online option
>  		 * we need to online the memory ourselves.
>  		 */
> -		if (!memhp_auto_online) {
> +		if (memhp_default_online_type == MMOP_OFFLINE) {
>  			lock_device_hotplug();
>  			walk_memory_blocks(ent->start, ent->size, NULL,
>  					   online_mem_block);

Whut? This stinks, doesn't it. For your defense, the original code is
fishy already but this just makes it even more ugly.
David Hildenbrand March 16, 2020, 3:34 p.m. UTC | #3
On 16.03.20 16:24, Michal Hocko wrote:
> On Wed 11-03-20 13:30:25, David Hildenbrand wrote:
> [...]
>> diff --git a/arch/powerpc/platforms/powernv/memtrace.c b/arch/powerpc/platforms/powernv/memtrace.c
>> index d6d64f8718e6..e15a600cfa4d 100644
>> --- a/arch/powerpc/platforms/powernv/memtrace.c
>> +++ b/arch/powerpc/platforms/powernv/memtrace.c
>> @@ -235,7 +235,7 @@ static int memtrace_online(void)
>>  		 * If kernel isn't compiled with the auto online option
>>  		 * we need to online the memory ourselves.
>>  		 */
>> -		if (!memhp_auto_online) {
>> +		if (memhp_default_online_type == MMOP_OFFLINE) {
>>  			lock_device_hotplug();
>>  			walk_memory_blocks(ent->start, ent->size, NULL,
>>  					   online_mem_block);
> 
> Whut? This stinks, doesn't it. For your defense, the original code is
> fishy already but this just makes it even more ugly.

PPC64 onlines all memory directly from the kernel, and not triggered by
user space (I think that's ugly and not desired, but it is what it is
and I am not going to touch that).

See
arch/powerpc/platforms/pseries/hotplug-memory.c:dlpar_change_lmb_state().

Best I can do here is to also always online all memory.

What are your suggestions?
Michal Hocko March 16, 2020, 3:46 p.m. UTC | #4
On Mon 16-03-20 16:34:06, David Hildenbrand wrote:
[...]
> Best I can do here is to also always online all memory.

Yes that sounds like a cleaner solution than having a condition that
doesn't make much sense at first glance.

Patch
diff mbox series

diff --git a/arch/powerpc/platforms/powernv/memtrace.c b/arch/powerpc/platforms/powernv/memtrace.c
index d6d64f8718e6..e15a600cfa4d 100644
--- a/arch/powerpc/platforms/powernv/memtrace.c
+++ b/arch/powerpc/platforms/powernv/memtrace.c
@@ -235,7 +235,7 @@  static int memtrace_online(void)
 		 * If kernel isn't compiled with the auto online option
 		 * we need to online the memory ourselves.
 		 */
-		if (!memhp_auto_online) {
+		if (memhp_default_online_type == MMOP_OFFLINE) {
 			lock_device_hotplug();
 			walk_memory_blocks(ent->start, ent->size, NULL,
 					   online_mem_block);
diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index 8a7f29c0bf97..8d3e16dab69f 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -386,10 +386,8 @@  static DEVICE_ATTR_RO(block_size_bytes);
 static ssize_t auto_online_blocks_show(struct device *dev,
 				       struct device_attribute *attr, char *buf)
 {
-	if (memhp_auto_online)
-		return sprintf(buf, "online\n");
-	else
-		return sprintf(buf, "offline\n");
+	return sprintf(buf, "%s\n",
+		       online_type_to_str[memhp_default_online_type]);
 }
 
 static ssize_t auto_online_blocks_store(struct device *dev,
@@ -397,9 +395,9 @@  static ssize_t auto_online_blocks_store(struct device *dev,
 					const char *buf, size_t count)
 {
 	if (sysfs_streq(buf, "online"))
-		memhp_auto_online = true;
+		memhp_default_online_type = MMOP_ONLINE;
 	else if (sysfs_streq(buf, "offline"))
-		memhp_auto_online = false;
+		memhp_default_online_type = MMOP_OFFLINE;
 	else
 		return -EINVAL;
 
diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index a02ce43d778d..3b90fd12e0c5 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -727,7 +727,7 @@  static void hv_mem_hot_add(unsigned long start, unsigned long size,
 		spin_unlock_irqrestore(&dm_device.ha_lock, flags);
 
 		init_completion(&dm_device.ol_waitevent);
-		dm_device.ha_waiting = !memhp_auto_online;
+		dm_device.ha_waiting = memhp_default_online_type == MMOP_OFFLINE;
 
 		nid = memory_add_physaddr_to_nid(PFN_PHYS(start_pfn));
 		ret = add_memory(nid, PFN_PHYS((start_pfn)),
diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index c2e06ed5e0e9..c6e090b34c4b 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -117,7 +117,8 @@  extern int arch_add_memory(int nid, u64 start, u64 size,
 			struct mhp_restrictions *restrictions);
 extern u64 max_mem_size;
 
-extern bool memhp_auto_online;
+/* Default online_type (MMOP_*) when new memory blocks are added. */
+extern int memhp_default_online_type;
 /* If movable_node boot option specified */
 extern bool movable_node_enabled;
 static inline bool movable_node_is_enabled(void)
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 1a00b5a37ef6..01443c70aa27 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -67,18 +67,18 @@  void put_online_mems(void)
 bool movable_node_enabled = false;
 
 #ifndef CONFIG_MEMORY_HOTPLUG_DEFAULT_ONLINE
-bool memhp_auto_online;
+int memhp_default_online_type = MMOP_OFFLINE;
 #else
-bool memhp_auto_online = true;
+int memhp_default_online_type = MMOP_ONLINE;
 #endif
-EXPORT_SYMBOL_GPL(memhp_auto_online);
+EXPORT_SYMBOL_GPL(memhp_default_online_type);
 
 static int __init setup_memhp_default_state(char *str)
 {
 	if (!strcmp(str, "online"))
-		memhp_auto_online = true;
+		memhp_default_online_type = MMOP_ONLINE;
 	else if (!strcmp(str, "offline"))
-		memhp_auto_online = false;
+		memhp_default_online_type = MMOP_OFFLINE;
 
 	return 1;
 }
@@ -991,6 +991,7 @@  static int check_hotplug_memory_range(u64 start, u64 size)
 
 static int online_memory_block(struct memory_block *mem, void *arg)
 {
+	mem->online_type = memhp_default_online_type;
 	return device_online(&mem->dev);
 }
 
@@ -1063,7 +1064,7 @@  int __ref add_memory_resource(int nid, struct resource *res)
 	mem_hotplug_done();
 
 	/* online pages if requested */
-	if (memhp_auto_online)
+	if (memhp_default_online_type != MMOP_OFFLINE)
 		walk_memory_blocks(start, size, NULL, online_memory_block);
 
 	return ret;