[PATCH v2 0/9] mm: Use hotplug_memory_notifier() instead of register_hotmemory_notifier()

[PATCH v2 0/9] mm: Use hotplug_memory_notifier() instead of register_hotmemory_notifier()

[Liu Shixin]

Patch f02c69680088 introduced register_hotmemory_notifier() to avoid a
compile problem with gcc-4.4.4:

    When CONFIG_MEMORY_HOTPLUG=n, we don't want the memory-hotplug notifier
    handlers to be included in the .o files, for space reasons.
    
    The existing hotplug_memory_notifier() tries to handle this but testing
    with gcc-4.4.4 shows that it doesn't work - the hotplug functions are
    still present in the .o files.

Since patch 316346243be6 has already updated the minimum gcc version to 5.1.
The previous problem mentioned in patch f02c69680088 is not existed. So we
can now revert to use hotplug_memory_notifier() directly rather than
register_hotmemory_notifier().

Patch [1-6] replace register_hotmemory_notifier() with hotplug_memory_notifier()
for every caller.
Patch [7] remove unused register_hotmemory_notifier().
Patch [8] replace IPC_CALLBACK_PRI with MM_BATCH_CALLBACK_PRI.
Patch [9] collect all priority of hotplug memory callback into include/linux/memory.h
for easy reading.

v1->v2: fix compile error of patch [4,5].

Liu Shixin (9):
  cgroup/cpuset: use hotplug_memory_notifier() directly
  fs/proc/kcore.c: use hotplug_memory_notifier() directly
  mm/slub.c: use hotplug_memory_notifier() directly
  mm/mmap: use hotplug_memory_notifier() directly
  mm/mm_init.c: use hotplug_memory_notifier() directly
  ACPI: HMAT: use hotplug_memory_notifier() directly
  memory: remove unused register_hotmemory_notifier()
  memory: replace IPC_CALLBACK_PRI with MM_BATCH_CALLBACK_PRI
  memory: clean up hotplug memory callback priority

 drivers/acpi/numa/hmat.c     |  7 +------
 fs/proc/kcore.c              |  7 +------
 include/linux/memory-tiers.h |  1 -
 include/linux/memory.h       | 15 +++++++--------
 kernel/cgroup/cpuset.c       |  7 +------
 mm/kasan/shadow.c            |  2 +-
 mm/ksm.c                     |  2 +-
 mm/mm_init.c                 |  8 +-------
 mm/mmap.c                    |  6 +-----
 mm/page_ext.c                |  2 +-
 mm/slub.c                    |  7 +------
 11 files changed, 16 insertions(+), 48 deletions(-)

-- 
2.25.1

[PATCH v2 1/9] cgroup/cpuset: use hotplug_memory_notifier() directly

[Liu Shixin]

Since patch 316346243be6 has already updated the minimum gcc version to 5.1.
The previous problem mentioned in patch f02c69680088 is not existed. So we
can now revert to use hotplug_memory_notifier() directly rather than
register_hotmemory_notifier().

Signed-off-by: Liu Shixin <liushixin2@huawei.com>
---
 kernel/cgroup/cpuset.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index b474289c15b8..0c6db6a4f427 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -3630,11 +3630,6 @@ static int cpuset_track_online_nodes(struct notifier_block *self,
 	return NOTIFY_OK;
 }
 
-static struct notifier_block cpuset_track_online_nodes_nb = {
-	.notifier_call = cpuset_track_online_nodes,
-	.priority = 10,		/* ??! */
-};
-
 /**
  * cpuset_init_smp - initialize cpus_allowed
  *
@@ -3652,7 +3647,7 @@ void __init cpuset_init_smp(void)
 	cpumask_copy(top_cpuset.effective_cpus, cpu_active_mask);
 	top_cpuset.effective_mems = node_states[N_MEMORY];
 
-	register_hotmemory_notifier(&cpuset_track_online_nodes_nb);
+	hotplug_memory_notifier(cpuset_track_online_nodes, 10);
 
 	cpuset_migrate_mm_wq = alloc_ordered_workqueue("cpuset_migrate_mm", 0);
 	BUG_ON(!cpuset_migrate_mm_wq);
-- 
2.25.1

[PATCH v2 2/9] fs/proc/kcore.c: use hotplug_memory_notifier() directly

[Liu Shixin]

Since patch 316346243be6 has already updated the minimum gcc version to 5.1.
The previous problem mentioned in patch f02c69680088 is not existed. So we
can now revert to use hotplug_memory_notifier() directly rather than
register_hotmemory_notifier().

Signed-off-by: Liu Shixin <liushixin2@huawei.com>
---
 fs/proc/kcore.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/fs/proc/kcore.c b/fs/proc/kcore.c
index dff921f7ca33..7692a360972d 100644
--- a/fs/proc/kcore.c
+++ b/fs/proc/kcore.c
@@ -18,7 +18,6 @@
 #include <linux/capability.h>
 #include <linux/elf.h>
 #include <linux/elfcore.h>
-#include <linux/notifier.h>
 #include <linux/vmalloc.h>
 #include <linux/highmem.h>
 #include <linux/printk.h>
@@ -638,10 +637,6 @@ static int __meminit kcore_callback(struct notifier_block *self,
 	return NOTIFY_OK;
 }
 
-static struct notifier_block kcore_callback_nb __meminitdata = {
-	.notifier_call = kcore_callback,
-	.priority = 0,
-};
 
 static struct kcore_list kcore_vmalloc;
 
@@ -694,7 +689,7 @@ static int __init proc_kcore_init(void)
 	add_modules_range();
 	/* Store direct-map area from physical memory map */
 	kcore_update_ram();
-	register_hotmemory_notifier(&kcore_callback_nb);
+	hotplug_memory_notifier(kcore_callback, 0);
 
 	return 0;
 }
-- 
2.25.1

[PATCH v2 3/9] mm/slub.c: use hotplug_memory_notifier() directly

[Liu Shixin]

Since patch 316346243be6 has already updated the minimum gcc version to 5.1.
The previous problem mentioned in patch f02c69680088 is not existed. So we
can now revert to use hotplug_memory_notifier() directly rather than
register_hotmemory_notifier().

Signed-off-by: Liu Shixin <liushixin2@huawei.com>
---
 mm/slub.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 234bf0e44262..9a0853e01fd7 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -4698,11 +4698,6 @@ static int slab_memory_callback(struct notifier_block *self,
 	return ret;
 }
 
-static struct notifier_block slab_memory_callback_nb = {
-	.notifier_call = slab_memory_callback,
-	.priority = SLAB_CALLBACK_PRI,
-};
-
 /********************************************************************
  *			Basic setup of slabs
  *******************************************************************/
@@ -4768,7 +4763,7 @@ void __init kmem_cache_init(void)
 	create_boot_cache(kmem_cache_node, "kmem_cache_node",
 		sizeof(struct kmem_cache_node), SLAB_HWCACHE_ALIGN, 0, 0);
 
-	register_hotmemory_notifier(&slab_memory_callback_nb);
+	hotplug_memory_notifier(slab_memory_callback, SLAB_CALLBACK_PRI);
 
 	/* Able to allocate the per node structures */
 	slab_state = PARTIAL;
-- 
2.25.1

[PATCH v2 4/9] mm/mmap: use hotplug_memory_notifier() directly

[Liu Shixin]

Since patch 316346243be6 has already updated the minimum gcc version to 5.1.
The previous problem mentioned in patch f02c69680088 is not existed. So we
can now revert to use hotplug_memory_notifier() directly rather than
register_hotmemory_notifier().

Signed-off-by: Liu Shixin <liushixin2@huawei.com>
---
 mm/mmap.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/mm/mmap.c b/mm/mmap.c
index 6445fd386f04..5a217378a62a 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -3745,13 +3745,9 @@ static int reserve_mem_notifier(struct notifier_block *nb,
 	return NOTIFY_OK;
 }
 
-static struct notifier_block reserve_mem_nb = {
-	.notifier_call = reserve_mem_notifier,
-};
-
 static int __meminit init_reserve_notifier(void)
 {
-	if (register_hotmemory_notifier(&reserve_mem_nb))
+	if (hotplug_memory_notifier(reserve_mem_notifier, 0))
 		pr_err("Failed registering memory add/remove notifier for admin reserve\n");
 
 	return 0;
-- 
2.25.1

[PATCH v2 5/9] mm/mm_init.c: use hotplug_memory_notifier() directly

[Liu Shixin]

Since patch 316346243be6 has already updated the minimum gcc version to 5.1.
The previous problem mentioned in patch f02c69680088 is not existed. So we
can now revert to use hotplug_memory_notifier() directly rather than
register_hotmemory_notifier().

Signed-off-by: Liu Shixin <liushixin2@huawei.com>
---
 mm/mm_init.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/mm/mm_init.c b/mm/mm_init.c
index 0d7b2bd2454a..44aadc162d1f 100644
--- a/mm/mm_init.c
+++ b/mm/mm_init.c
@@ -178,16 +178,10 @@ static int __meminit mm_compute_batch_notifier(struct notifier_block *self,
 	return NOTIFY_OK;
 }
 
-static struct notifier_block compute_batch_nb __meminitdata = {
-	.notifier_call = mm_compute_batch_notifier,
-	.priority = IPC_CALLBACK_PRI, /* use lowest priority */
-};
-
 static int __init mm_compute_batch_init(void)
 {
 	mm_compute_batch(sysctl_overcommit_memory);
-	register_hotmemory_notifier(&compute_batch_nb);
-
+	hotplug_memory_notifier(mm_compute_batch_notifier, IPC_CALLBACK_PRI);
 	return 0;
 }
 
-- 
2.25.1

[PATCH v2 6/9] ACPI: HMAT: use hotplug_memory_notifier() directly

[Liu Shixin]

Since patch 316346243be6 has already updated the minimum gcc version to 5.1.
The previous problem mentioned in patch f02c69680088 is not existed. So we
can now revert to use hotplug_memory_notifier() directly rather than
register_hotmemory_notifier().

Signed-off-by: Liu Shixin <liushixin2@huawei.com>
---
 drivers/acpi/numa/hmat.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/acpi/numa/hmat.c b/drivers/acpi/numa/hmat.c
index c3d783aca196..84dffb3dc30e 100644
--- a/drivers/acpi/numa/hmat.c
+++ b/drivers/acpi/numa/hmat.c
@@ -768,11 +768,6 @@ static int hmat_callback(struct notifier_block *self,
 	return NOTIFY_OK;
 }
 
-static struct notifier_block hmat_callback_nb = {
-	.notifier_call = hmat_callback,
-	.priority = 2,
-};
-
 static __init void hmat_free_structures(void)
 {
 	struct memory_target *target, *tnext;
@@ -855,7 +850,7 @@ static __init int hmat_init(void)
 	hmat_register_targets();
 
 	/* Keep the table and structures if the notifier may use them */
-	if (!register_hotmemory_notifier(&hmat_callback_nb))
+	if (!hotplug_memory_notifier(hmat_callback, 2))
 		return 0;
 out_put:
 	hmat_free_structures();
-- 
2.25.1

[PATCH v2 7/9] memory: remove unused register_hotmemory_notifier()

[Liu Shixin]

Remove unused register_hotmemory_notifier().

Signed-off-by: Liu Shixin <liushixin2@huawei.com>
---
 include/linux/memory.h | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/include/linux/memory.h b/include/linux/memory.h
index aa619464a1df..98d2a2ebcc10 100644
--- a/include/linux/memory.h
+++ b/include/linux/memory.h
@@ -19,7 +19,6 @@
 #include <linux/node.h>
 #include <linux/compiler.h>
 #include <linux/mutex.h>
-#include <linux/notifier.h>
 
 #define MIN_MEMORY_BLOCK_SIZE     (1UL << SECTION_SIZE_BITS)
 
@@ -136,9 +135,6 @@ static inline int hotplug_memory_notifier(notifier_fn_t fn, int pri)
 {
 	return 0;
 }
-/* These aren't inline functions due to a GCC bug. */
-#define register_hotmemory_notifier(nb)    ({ (void)(nb); 0; })
-#define unregister_hotmemory_notifier(nb)  ({ (void)(nb); })
 #else /* CONFIG_MEMORY_HOTPLUG */
 extern int register_memory_notifier(struct notifier_block *nb);
 extern void unregister_memory_notifier(struct notifier_block *nb);
@@ -166,8 +162,6 @@ int walk_dynamic_memory_groups(int nid, walk_memory_groups_func_t func,
 		{ .notifier_call = fn, .priority = pri };\
 	register_memory_notifier(&fn##_mem_nb);			\
 })
-#define register_hotmemory_notifier(nb)		register_memory_notifier(nb)
-#define unregister_hotmemory_notifier(nb) 	unregister_memory_notifier(nb)
 
 #ifdef CONFIG_NUMA
 void memory_block_add_nid(struct memory_block *mem, int nid,
-- 
2.25.1

[PATCH v2 8/9] memory: replace IPC_CALLBACK_PRI with MM_BATCH_CALLBACK_PRI

[Liu Shixin]

Since there is nothing to do with ipc, it is strange to use IPC_CALLBACK_PRI
here. Replace IPC_CALLBACK_PRI with MM_BATCH_CALLBACK_PRI for easy reading.

Signed-off-by: Liu Shixin <liushixin2@huawei.com>
---
 include/linux/memory.h | 4 ++--
 mm/mm_init.c           | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/memory.h b/include/linux/memory.h
index 98d2a2ebcc10..e5f71d3d9af8 100644
--- a/include/linux/memory.h
+++ b/include/linux/memory.h
@@ -112,8 +112,8 @@ struct mem_section;
  * Priorities for the hotplug memory callback routines (stored in decreasing
  * order in the callback chain)
  */
-#define SLAB_CALLBACK_PRI       1
-#define IPC_CALLBACK_PRI        10
+#define SLAB_CALLBACK_PRI	1
+#define MM_BATCH_CALLBACK_PRI	10
 
 #ifndef CONFIG_MEMORY_HOTPLUG
 static inline void memory_dev_init(void)
diff --git a/mm/mm_init.c b/mm/mm_init.c
index 44aadc162d1f..1dd9257e2204 100644
--- a/mm/mm_init.c
+++ b/mm/mm_init.c
@@ -181,7 +181,7 @@ static int __meminit mm_compute_batch_notifier(struct notifier_block *self,
 static int __init mm_compute_batch_init(void)
 {
 	mm_compute_batch(sysctl_overcommit_memory);
-	hotplug_memory_notifier(mm_compute_batch_notifier, IPC_CALLBACK_PRI);
+	hotplug_memory_notifier(mm_compute_batch_notifier, MM_BATCH_CALLBACK_PRI);
 	return 0;
 }
 
-- 
2.25.1

[PATCH v2 9/9] memory: clean up hotplug memory callback priority

[Liu Shixin]

The priority of hotplug memory callback is defined in different file.
And there are some caller using number directly. Collect them together
into include/linux/memory.h for easy reading.

Signed-off-by: Liu Shixin <liushixin2@huawei.com>
---
 drivers/acpi/numa/hmat.c     | 2 +-
 fs/proc/kcore.c              | 2 +-
 include/linux/memory-tiers.h | 1 -
 include/linux/memory.h       | 5 +++++
 kernel/cgroup/cpuset.c       | 2 +-
 mm/kasan/shadow.c            | 2 +-
 mm/ksm.c                     | 2 +-
 mm/mmap.c                    | 2 +-
 mm/page_ext.c                | 2 +-
 9 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/acpi/numa/hmat.c b/drivers/acpi/numa/hmat.c
index 84dffb3dc30e..47c7fae8b6de 100644
--- a/drivers/acpi/numa/hmat.c
+++ b/drivers/acpi/numa/hmat.c
@@ -850,7 +850,7 @@ static __init int hmat_init(void)
 	hmat_register_targets();
 
 	/* Keep the table and structures if the notifier may use them */
-	if (!hotplug_memory_notifier(hmat_callback, 2))
+	if (!hotplug_memory_notifier(hmat_callback, HMAT_CALLBACK_PRI))
 		return 0;
 out_put:
 	hmat_free_structures();
diff --git a/fs/proc/kcore.c b/fs/proc/kcore.c
index 7692a360972d..98f3289556e4 100644
--- a/fs/proc/kcore.c
+++ b/fs/proc/kcore.c
@@ -689,7 +689,7 @@ static int __init proc_kcore_init(void)
 	add_modules_range();
 	/* Store direct-map area from physical memory map */
 	kcore_update_ram();
-	hotplug_memory_notifier(kcore_callback, 0);
+	hotplug_memory_notifier(kcore_callback, DEFAULT_CALLBACK_PRI);
 
 	return 0;
 }
diff --git a/include/linux/memory-tiers.h b/include/linux/memory-tiers.h
index 965009aa01d7..fc9647b1b4f9 100644
--- a/include/linux/memory-tiers.h
+++ b/include/linux/memory-tiers.h
@@ -18,7 +18,6 @@
  * the same memory tier.
  */
 #define MEMTIER_ADISTANCE_DRAM	((4 * MEMTIER_CHUNK_SIZE) + (MEMTIER_CHUNK_SIZE >> 1))
-#define MEMTIER_HOTPLUG_PRIO	100
 
 struct memory_tier;
 struct memory_dev_type {
diff --git a/include/linux/memory.h b/include/linux/memory.h
index e5f71d3d9af8..e62d6d2b9c39 100644
--- a/include/linux/memory.h
+++ b/include/linux/memory.h
@@ -112,8 +112,13 @@ struct mem_section;
  * Priorities for the hotplug memory callback routines (stored in decreasing
  * order in the callback chain)
  */
+#define DEFAULT_CALLBACK_PRI	0
 #define SLAB_CALLBACK_PRI	1
+#define HMAT_CALLBACK_PRI	2
 #define MM_BATCH_CALLBACK_PRI	10
+#define CPUSET_CALLBACK_PRI	10
+#define MEMTIER_HOTPLUG_PRIO	100
+#define KSM_CALLBACK_PRI	100
 
 #ifndef CONFIG_MEMORY_HOTPLUG
 static inline void memory_dev_init(void)
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 0c6db6a4f427..3ea2e836e93e 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -3647,7 +3647,7 @@ void __init cpuset_init_smp(void)
 	cpumask_copy(top_cpuset.effective_cpus, cpu_active_mask);
 	top_cpuset.effective_mems = node_states[N_MEMORY];
 
-	hotplug_memory_notifier(cpuset_track_online_nodes, 10);
+	hotplug_memory_notifier(cpuset_track_online_nodes, CPUSET_CALLBACK_PRI);
 
 	cpuset_migrate_mm_wq = alloc_ordered_workqueue("cpuset_migrate_mm", 0);
 	BUG_ON(!cpuset_migrate_mm_wq);
diff --git a/mm/kasan/shadow.c b/mm/kasan/shadow.c
index 0e3648b603a6..2fba1f51f042 100644
--- a/mm/kasan/shadow.c
+++ b/mm/kasan/shadow.c
@@ -244,7 +244,7 @@ static int __meminit kasan_mem_notifier(struct notifier_block *nb,
 
 static int __init kasan_memhotplug_init(void)
 {
-	hotplug_memory_notifier(kasan_mem_notifier, 0);
+	hotplug_memory_notifier(kasan_mem_notifier, DEFAULT_CALLBACK_PRI);
 
 	return 0;
 }
diff --git a/mm/ksm.c b/mm/ksm.c
index c19fcca9bc03..7ba97f86d831 100644
--- a/mm/ksm.c
+++ b/mm/ksm.c
@@ -3211,7 +3211,7 @@ static int __init ksm_init(void)
 
 #ifdef CONFIG_MEMORY_HOTREMOVE
 	/* There is no significance to this priority 100 */
-	hotplug_memory_notifier(ksm_memory_callback, 100);
+	hotplug_memory_notifier(ksm_memory_callback, KSM_CALLBACK_PRI);
 #endif
 	return 0;
 
diff --git a/mm/mmap.c b/mm/mmap.c
index 5a217378a62a..0fde397963f8 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -3747,7 +3747,7 @@ static int reserve_mem_notifier(struct notifier_block *nb,
 
 static int __meminit init_reserve_notifier(void)
 {
-	if (hotplug_memory_notifier(reserve_mem_notifier, 0))
+	if (hotplug_memory_notifier(reserve_mem_notifier, DEFAULT_CALLBACK_PRI))
 		pr_err("Failed registering memory add/remove notifier for admin reserve\n");
 
 	return 0;
diff --git a/mm/page_ext.c b/mm/page_ext.c
index affe80243b6d..b2ff5c9129f4 100644
--- a/mm/page_ext.c
+++ b/mm/page_ext.c
@@ -513,7 +513,7 @@ void __init page_ext_init(void)
 			cond_resched();
 		}
 	}
-	hotplug_memory_notifier(page_ext_callback, 0);
+	hotplug_memory_notifier(page_ext_callback, DEFAULT_CALLBACK_PRI);
 	pr_info("allocated %ld bytes of page_ext\n", total_usage);
 	invoke_init_callbacks();
 	return;
-- 
2.25.1

Re: [PATCH v2 1/9] cgroup/cpuset: use hotplug_memory_notifier() directly

[David Hildenbrand]

On 20.09.22 04:51, Liu Shixin wrote:
> Since patch 316346243be6 has already updated the minimum gcc version to 5.1.
> The previous problem mentioned in patch f02c69680088 is not existed. So we
> can now revert to use hotplug_memory_notifier() directly rather than
> register_hotmemory_notifier().
> 
> Signed-off-by: Liu Shixin <liushixin2@huawei.com>

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb

Re: [PATCH v2 2/9] fs/proc/kcore.c: use hotplug_memory_notifier() directly

[David Hildenbrand]

On 20.09.22 04:51, Liu Shixin wrote:
> Since patch 316346243be6 has already updated the minimum gcc version to 5.1.
> The previous problem mentioned in patch f02c69680088 is not existed. So we
> can now revert to use hotplug_memory_notifier() directly rather than
> register_hotmemory_notifier().
> 
> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
> ---

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb

Re: [PATCH v2 3/9] mm/slub.c: use hotplug_memory_notifier() directly

[David Hildenbrand]

On 20.09.22 04:51, Liu Shixin wrote:
> Since patch 316346243be6 has already updated the minimum gcc version to 5.1.
> The previous problem mentioned in patch f02c69680088 is not existed. So we
> can now revert to use hotplug_memory_notifier() directly rather than
> register_hotmemory_notifier().
> 
> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
> ---

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb

Re: [PATCH v2 4/9] mm/mmap: use hotplug_memory_notifier() directly

[David Hildenbrand]

On 20.09.22 04:51, Liu Shixin wrote:
> Since patch 316346243be6 has already updated the minimum gcc version to 5.1.
> The previous problem mentioned in patch f02c69680088 is not existed. So we
> can now revert to use hotplug_memory_notifier() directly rather than
> register_hotmemory_notifier().
> 
> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
> ---

Reviewed-by: David Hildenbrand <david@redhat.com>
-- 
Thanks,

David / dhildenb

Re: [PATCH v2 5/9] mm/mm_init.c: use hotplug_memory_notifier() directly

[David Hildenbrand]

On 20.09.22 04:51, Liu Shixin wrote:
> Since patch 316346243be6 has already updated the minimum gcc version to 5.1.
> The previous problem mentioned in patch f02c69680088 is not existed. So we
> can now revert to use hotplug_memory_notifier() directly rather than
> register_hotmemory_notifier().
> 
> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
> ---

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb

Re: [PATCH v2 6/9] ACPI: HMAT: use hotplug_memory_notifier() directly

[David Hildenbrand]

On 20.09.22 04:51, Liu Shixin wrote:
> Since patch 316346243be6 has already updated the minimum gcc version to 5.1.
> The previous problem mentioned in patch f02c69680088 is not existed. So we
> can now revert to use hotplug_memory_notifier() directly rather than
> register_hotmemory_notifier().
> 
> Signed-off-by: Liu Shixin <liushixin2@huawei.com>

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb

Re: [PATCH v2 7/9] memory: remove unused register_hotmemory_notifier()

[David Hildenbrand]

On 20.09.22 04:51, Liu Shixin wrote:
> Remove unused register_hotmemory_notifier().
> 
> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
> ---
>   include/linux/memory.h | 6 ------
>   1 file changed, 6 deletions(-)
> 
> diff --git a/include/linux/memory.h b/include/linux/memory.h
> index aa619464a1df..98d2a2ebcc10 100644
> --- a/include/linux/memory.h
> +++ b/include/linux/memory.h
> @@ -19,7 +19,6 @@
>   #include <linux/node.h>
>   #include <linux/compiler.h>
>   #include <linux/mutex.h>
> -#include <linux/notifier.h>

^ why remove that?

register_memory_notifier/unregister_memory_notifier still uses that.

-- 
Thanks,

David / dhildenb

Re: [PATCH v2 8/9] memory: replace IPC_CALLBACK_PRI with MM_BATCH_CALLBACK_PRI

[David Hildenbrand]

On 20.09.22 04:51, Liu Shixin wrote:
> Since there is nothing to do with ipc, it is strange to use IPC_CALLBACK_PRI
> here. Replace IPC_CALLBACK_PRI with MM_BATCH_CALLBACK_PRI for easy reading.

Should this better be called MM_COMPUTE_BATCH_CALLBACK_PRI

Not so sure if "CALLBACK" is really the right term to use here in 
general ... we're talking about the memory notifier priority.

-- 
Thanks,

David / dhildenb

Re: [PATCH v2 9/9] memory: clean up hotplug memory callback priority

[David Hildenbrand]

On 20.09.22 04:51, Liu Shixin wrote:
> The priority of hotplug memory callback is defined in different file.
> And there are some caller using number directly. Collect them together
> into include/linux/memory.h for easy reading.
> 
> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
> ---

Please squash that into the previous patch.

-- 
Thanks,

David / dhildenb

Re: [PATCH v2 0/9] mm: Use hotplug_memory_notifier() instead of register_hotmemory_notifier()

[Waiman Long]

On 9/19/22 22:51, Liu Shixin wrote:
> Patch f02c69680088 introduced register_hotmemory_notifier() to avoid a
> compile problem with gcc-4.4.4:
>
>      When CONFIG_MEMORY_HOTPLUG=n, we don't want the memory-hotplug notifier
>      handlers to be included in the .o files, for space reasons.
>      
>      The existing hotplug_memory_notifier() tries to handle this but testing
>      with gcc-4.4.4 shows that it doesn't work - the hotplug functions are
>      still present in the .o files.
>
> Since patch 316346243be6 has already updated the minimum gcc version to 5.1.
> The previous problem mentioned in patch f02c69680088 is not existed. So we
> can now revert to use hotplug_memory_notifier() directly rather than
> register_hotmemory_notifier().
>
> Patch [1-6] replace register_hotmemory_notifier() with hotplug_memory_notifier()
> for every caller.
> Patch [7] remove unused register_hotmemory_notifier().
> Patch [8] replace IPC_CALLBACK_PRI with MM_BATCH_CALLBACK_PRI.
> Patch [9] collect all priority of hotplug memory callback into include/linux/memory.h
> for easy reading.
>
> v1->v2: fix compile error of patch [4,5].

I agreed with David's comments about patch 8 and 9. My understanding is 
that this patch series makes it easy to see all the priorities of the 
various memory hotplug notifier callbacks and hence their calling order. 
It is kind of being stated when mentioning about patch 9 above. However, 
it should be more prominently mentioned as the first thing that I look 
for is the benefit provided by this patch series.

Cheers,
Longman

Re: [PATCH v2 7/9] memory: remove unused register_hotmemory_notifier()

[Liu Shixin]

On 2022/9/20 20:06, David Hildenbrand wrote:
> On 20.09.22 04:51, Liu Shixin wrote:
>> Remove unused register_hotmemory_notifier().
>>
>> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
>> ---
>>   include/linux/memory.h | 6 ------
>>   1 file changed, 6 deletions(-)
>>
>> diff --git a/include/linux/memory.h b/include/linux/memory.h
>> index aa619464a1df..98d2a2ebcc10 100644
>> --- a/include/linux/memory.h
>> +++ b/include/linux/memory.h
>> @@ -19,7 +19,6 @@
>>   #include <linux/node.h>
>>   #include <linux/compiler.h>
>>   #include <linux/mutex.h>
>> -#include <linux/notifier.h>
>
> ^ why remove that?
>
> register_memory_notifier/unregister_memory_notifier still uses that.
Yes, you're right. I don't notice this point since there is no compile warning.
I will add it back in v2.

Thanks,

Re: [PATCH v2 7/9] memory: remove unused register_hotmemory_notifier()

[David Hildenbrand]

On 22.09.22 14:54, Liu Shixin wrote:
> On 2022/9/20 20:06, David Hildenbrand wrote:
>> On 20.09.22 04:51, Liu Shixin wrote:
>>> Remove unused register_hotmemory_notifier().
>>>
>>> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
>>> ---
>>>    include/linux/memory.h | 6 ------
>>>    1 file changed, 6 deletions(-)
>>>
>>> diff --git a/include/linux/memory.h b/include/linux/memory.h
>>> index aa619464a1df..98d2a2ebcc10 100644
>>> --- a/include/linux/memory.h
>>> +++ b/include/linux/memory.h
>>> @@ -19,7 +19,6 @@
>>>    #include <linux/node.h>
>>>    #include <linux/compiler.h>
>>>    #include <linux/mutex.h>
>>> -#include <linux/notifier.h>
>>
>> ^ why remove that?
>>
>> register_memory_notifier/unregister_memory_notifier still uses that.
> Yes, you're right. I don't notice this point since there is no compile warning.
> I will add it back in v2.

If there is no warning/error, maybe we can just leave it out.

-- 
Thanks,

David / dhildenb

Re: [PATCH v2 8/9] memory: replace IPC_CALLBACK_PRI with MM_BATCH_CALLBACK_PRI

[Liu Shixin]

On 2022/9/20 20:07, David Hildenbrand wrote:
> On 20.09.22 04:51, Liu Shixin wrote:
>> Since there is nothing to do with ipc, it is strange to use IPC_CALLBACK_PRI
>> here. Replace IPC_CALLBACK_PRI with MM_BATCH_CALLBACK_PRI for easy reading.
>
> Should this better be called MM_COMPUTE_BATCH_CALLBACK_PRI
>
> Not so sure if "CALLBACK" is really the right term to use here in general ... we're talking about the memory notifier priority.
>
"CALLBACK" is not mandatory. Just for the sake of being consistent with other macros. MM_COMPUTE_BATCH_CALLBACK_PRI
is too long, maybe reduce to MM_COMPUTE_BATCH_PRI is better.

Thanks,

Re: [PATCH v2 0/9] mm: Use hotplug_memory_notifier() instead of register_hotmemory_notifier()

[Liu Shixin]

On 2022/9/20 22:22, Waiman Long wrote:
> On 9/19/22 22:51, Liu Shixin wrote:
>> Patch f02c69680088 introduced register_hotmemory_notifier() to avoid a
>> compile problem with gcc-4.4.4:
>>
>>      When CONFIG_MEMORY_HOTPLUG=n, we don't want the memory-hotplug notifier
>>      handlers to be included in the .o files, for space reasons.
>>           The existing hotplug_memory_notifier() tries to handle this but testing
>>      with gcc-4.4.4 shows that it doesn't work - the hotplug functions are
>>      still present in the .o files.
>>
>> Since patch 316346243be6 has already updated the minimum gcc version to 5.1.
>> The previous problem mentioned in patch f02c69680088 is not existed. So we
>> can now revert to use hotplug_memory_notifier() directly rather than
>> register_hotmemory_notifier().
>>
>> Patch [1-6] replace register_hotmemory_notifier() with hotplug_memory_notifier()
>> for every caller.
>> Patch [7] remove unused register_hotmemory_notifier().
>> Patch [8] replace IPC_CALLBACK_PRI with MM_BATCH_CALLBACK_PRI.
>> Patch [9] collect all priority of hotplug memory callback into include/linux/memory.h
>> for easy reading.
>>
>> v1->v2: fix compile error of patch [4,5].
>
> I agreed with David's comments about patch 8 and 9. My understanding is that this patch series makes it easy to see all the priorities of the various memory hotplug notifier callbacks and hence their calling order. It is kind of being stated when mentioning about patch 9 above. However, it should be more prominently mentioned as the first thing that I look for is the benefit provided by this patch series.
>
> Cheers,
> Longman
>
> .
I'll rewrite the commit log.

Thanks,