NUMA Hotplug Emulator (v9)

arch/x86/mm/numa_64.c void __init initmem_init(unsigned long start_pfn, unsigned long last_pfn, · line 650

Quoted 5 lines

arch/x86/mm/numa_64.c +1 -0

@@ -646,6 +647,15 @@ void __init initmem_init(unsigned long start_pfn, unsigned long last_pfn,
 		numa_set_node(i, 0);
 	memblock_x86_register_active_regions(0, start_pfn, last_pfn);
 	setup_node_bootmem(0, start_pfn << PAGE_SHIFT, last_pfn << PAGE_SHIFT);
+out: __maybe_unused

hm, I didn't know you could do that with labels.

Does it work?

Quoted 11 lines

arch/x86/mm/numa_64.c +8 -0

+	for (i = 0; i < numa_possible_nodes; i++) {
+		int nid;
+
+		nid = first_unset_node(node_possible_map);
+		if (nid == MAX_NUMNODES)
+			break;
+		node_set(nid, node_possible_map);
+	}
 }
 
 unsigned long __init numa_free_all_bootmem(void)

From: David Rientjes <[email protected]>

Add an interface to allow new nodes to be added when performing memory hot-add. This provides a convenient interface to test memory hotplug notifier callbacks and surrounding hotplug code when new nodes are onlined without actually having a machine with such hotpluggable SRAT entries.

This adds a new debugfs interface at /sys/kernel/debug/mem_hotplug/add_node that behaves in a similar way to the memory hot-add "probe" interface. Its format is size@start, where "size" is the size of the new node to be added and "start" is the physical address of the new memory.

The new node id is a currently offline, but possible, node. The bit must be set in node_possible_map so that nr_node_ids is sized appropriately.

For emulation on x86, for example, it would be possible to set aside memory for hotplugged nodes (say, anything above 2G) and to add an additional four nodes as being possible on boot with

mem=2G numa=possible=4

and then creating a new 128M node at runtime:

	# echo 128M@0x80000000 > /sys/kernel/debug/mem_hotplug/add_node
	On node 1 totalpages: 0
	init_memory_mapping: 0000000080000000-0000000088000000
	 0080000000 - 0088000000 page 2M
Once the new node has been added, its memory can be onlined.  If this
memory represents memory section 16, for example:

	# echo online > /sys/devices/system/memory/memory16/state
	Built 2 zonelists in Node order, mobility grouping on.  Total pages: 514846
	Policy zone: Normal
 [ The memory section(s) mapped to a particular node are visible via
   /sys/kernel/debug/mem_hotplug/node1, in this example. ]

The new node is now hotplugged and ready for testing.

CC Haicheng Li <[email protected]>

CC Greg KH <[email protected]>

Signed-off-by David Rientjes <[email protected]>

Signed-off-by Shaohui Zheng <[email protected]>

---
 Documentation/memory-hotplug.txt |   24 +++++++++++++++
 mm/memory_hotplug.c              |   59 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 83 insertions(+), 0 deletions(-)

Index: linux-hpe4/Documentation/memory-hotplug.txt
===================================================================

Documentation/memory-hotplug.txt +24 -0

@@ -18,6 +18,7 @@
 4. Physical memory hot-add phase
   4.1 Hardware(Firmware) Support
   4.2 Notify memory hot-add event by hand
+  4.3 Node hotplug emulation
 5. Logical Memory hot-add phase
   5.1. State of memory
   5.2. How to online memory

@@ -215,6 +216,29 @@
 Please see "How to online memory" in this text.
 
 
+4.3 Node hotplug emulation
+------------
+With debugfs, it is possible to test node hotplug by assigning the newly
+added memory to a new node id when using a different interface with a similar
+behavior to "probe" described in section 4.2.  If a node id is possible
+(there are bits in /sys/devices/system/memory/possible that are not online),
+then it may be used to emulate a newly added node as the result of memory
+hotplug by using the debugfs "add_node" interface.
+
+The add_node interface is located at "mem_hotplug/add_node" at the debugfs
+mount point.
+
+You can create a new node of a specified size starting at the physical
+address of new memory by
+
+% echo size@start_address_of_new_memory > /sys/kernel/debug/mem_hotplug/add_node
+
+Where "size" can be represented in megabytes or gigabytes (for example,
+"128M" or "1G").  The minumum size is that of a memory section.
+
+Once the new node has been added, it is possible to online the memory by
+toggling the "state" of its memory section(s) as described in section 5.1.
+
 
 ------------------------------
 5. Logical Memory hot-add phase

Index: linux-hpe4/mm/memory_hotplug.c
===================================================================

mm/memory_hotplug.c +0 -0

@@ -924,3 +924,63 @@
 }
 #endif /* CONFIG_MEMORY_HOTREMOVE */
 EXPORT_SYMBOL_GPL(remove_memory);

mm/memory_hotplug.c +20 -0

+	nodes_andnot(mask, node_possible_map, node_online_map);
+	nid = first_node(mask);
+	if (nid == MAX_NUMNODES)
+		return -ENOMEM;
+
+	ret = add_memory(nid, start, size);
+	return ret ? ret : count;
+}
+
+static const struct file_operations add_node_file_ops = {
+	.write		= add_node_store,
+	.llseek		= generic_file_llseek,
+};
+
+static int __init node_debug_init(void)
+{
+	if (!memhp_debug_root)
+		memhp_debug_root = debugfs_create_dir("mem_hotplug", NULL);
+	if (!memhp_debug_root)
+		return -ENOMEM;

Quoted 12 lines

Index: linux-hpe4/mm/memory_hotplug.c
===================================================================

mm/memory_hotplug.c +4 -0

@@ -924,3 +924,63 @@
 }
 #endif /* CONFIG_MEMORY_HOTREMOVE */
 EXPORT_SYMBOL_GPL(remove_memory);
+
+#ifdef CONFIG_DEBUG_FS
+#include <linux/debugfs.h>
+

mm/memory_hotplug.c line 936

Quoted 6 lines

mm/memory_hotplug.c +6 -0

+static struct dentry *memhp_debug_root;
+
+static ssize_t add_node_store(struct file *file, const char __user *buf,
+				size_t count, loff_t *ppos)
+{
+	nodemask_t mask;

NODEMASK_ALLOC()?

Quoted 12 lines · Andrew Morton

Quoted 12 lines

Index: linux-hpe4/mm/memory_hotplug.c
===================================================================

mm/memory_hotplug.c +4 -0

@@ -924,3 +924,63 @@
 }
 #endif /* CONFIG_MEMORY_HOTREMOVE */
 EXPORT_SYMBOL_GPL(remove_memory);
+
+#ifdef CONFIG_DEBUG_FS
+#include <linux/debugfs.h>
+

Quoted 9 lines

mm/memory_hotplug.c line 936

Quoted 6 lines

mm/memory_hotplug.c +6 -0

+static struct dentry *memhp_debug_root;
+
+static ssize_t add_node_store(struct file *file, const char __user *buf,
+				size_t count, loff_t *ppos)
+{
+	nodemask_t mask;

NODEMASK_ALLOC()?

We traditionally haven't been using NODEMASK_ALLOC() in sysfs (or, in this case, debugfs) functions because they're never deep in a call chain. Even for 4K node support, which isn't a supported config on any arch that allows CONFIG_MEMORY_HOTPLUG, this would only be 512 bytes on the short stack.

Quoted 8 lines

Index: linux-hpe4/mm/memory_hotplug.c
===================================================================

mm/memory_hotplug.c +0 -0

@@ -924,3 +924,63 @@
 }
 #endif /* CONFIG_MEMORY_HOTREMOVE */
 EXPORT_SYMBOL_GPL(remove_memory);

mm/memory_hotplug.c line 936

Quoted 10 lines

mm/memory_hotplug.c +10 -0

+
+#ifdef CONFIG_DEBUG_FS
+#include <linux/debugfs.h>
+
+static struct dentry *memhp_debug_root;
+
+static ssize_t add_node_store(struct file *file, const char __user *buf,
+				size_t count, loff_t *ppos)
+{
+	nodemask_t mask;

NODEMASK_ALLOC()?

mm/memory_hotplug.c line 945

Quoted 9 lines

mm/memory_hotplug.c +9 -0

+	u64 start, size;
+	char buffer[64];
+	char *p;
+	int nid;
+	int ret;
+
+	memset(buffer, 0, sizeof(buffer));
+	if (count > sizeof(buffer) - 1)
+		count = sizeof(buffer) - 1;

This will cause the write to return a smaller number than `count': a short write. Some userspace code may then decide to write the remainder of the data (whcih is the correct way to use the write() syscall).

Could be a bit dangerous, and perhaps simply declaring an error if too much data was written would be a better approach.

mm/memory_hotplug.c line 950

Quoted 5 lines

mm/memory_hotplug.c +5 -0

+	if (copy_from_user(buffer, buf, count))
+		return -EFAULT;
+
+	size = memparse(buffer, &p);
+	if (size < (PAGES_PER_SECTION << PAGE_SHIFT))

PAGES_PER_SECTION has type unsigned long, so the rhs of this comparison might overflow on 32-bit, should anyone ever try to use this code on 32-bit.

otoh the compiler might do it as 64-bit because the lhs is 64-bit. Not sure.

mm/memory_hotplug.c line 955

Quoted 5 lines

mm/memory_hotplug.c +5 -0

+		return -EINVAL;
+	if (*p != '@')
+		return -EINVAL;
+
+	start = simple_strtoull(p + 1, NULL, 0);

You disagreed with checkpatch?

Quoted 22 lines

mm/memory_hotplug.c +22 -0

+	nodes_andnot(mask, node_possible_map, node_online_map);
+	nid = first_node(mask);
+	if (nid == MAX_NUMNODES)
+		return -ENOMEM;
+
+	ret = add_memory(nid, start, size);
+	return ret ? ret : count;
+}
+
+static const struct file_operations add_node_file_ops = {
+	.write		= add_node_store,
+	.llseek		= generic_file_llseek,
+};
+
+static int __init node_debug_init(void)
+{
+	if (!memhp_debug_root)
+		memhp_debug_root = debugfs_create_dir("mem_hotplug", NULL);
+	if (!memhp_debug_root)
+		return -ENOMEM;
+
+	if (!debugfs_create_file("add_node", S_IWUSR, memhp_debug_root,

mm/memory_hotplug.c line 986

Quoted 8 lines

mm/memory_hotplug.c +8 -0

+			NULL, &add_node_file_ops))
+		return -ENOMEM;
+
+	return 0;
+}
+
+module_init(node_debug_init);
+#endif /* CONFIG_DEBUG_FS */

-----Original Message-----
From: David Rientjes [mailto:[email protected]]
Sent: Tuesday, December 28, 2010 3:35 PM
To: Zheng, Shaohui; Andrew Morton
Cc: [email protected]; [email protected]; [email protected]; [email protected]; Andi Kleen; [email protected]; Greg Kroah-Hartman; Li, Haicheng
Subject: Re: [3/7, v9] NUMA Hotplug Emulator: Add node hotplug emulation

Shaohui, I'll reply to this message with an updated version of this patch to address Andrew's comments. You can merge it into your series or Andrew can take it seperately (although it doesn't do much good without "x86: add numa=possible command line option" unless you have hotpluggable SRAT entries and CONFIG_ACPI_NUMA).

From: Shaohui Zheng <[email protected]>

CPU physical hot-add/hot-remove are supported on some hardwares, and it was already supported in current linux kernel. NUMA Hotplug Emulator provides a mechanism to emulate the process with software method. It can be used for testing or debuging purpose.

CPU physical hotplug is different with logical CPU online/offline. Logical online/offline is controled by interface /sys/device/cpu/cpuX/online. CPU hotplug emulator uses probe/release interface. It becomes possible to do cpu hotplug automation and stress

Add cpu interface probe/release under sysfs for x86_64. User can use this interface to emulate the cpu hot-add and hot-remove process.

Directive:
*) Reserve CPU thru grub parameter like:
maxcpus=4

the rest CPUs will not be initiliazed.

*) Probe CPU
we can use the probe interface to hot-add new CPUs:
echo nid > /sys/devices/system/cpu/probe

*) Release a CPU
echo cpu > /sys/devices/system/cpu/release

A reserved CPU will be hot-added to the specified node.
1) nid == 0, the CPU will be added to the real node which the CPU should be in
2) nid != 0, add the CPU to node nid even through it is a fake node.

CC Ingo Molnar <[email protected]>

CC Len Brown <[email protected]>

CC Yinghai Lu <[email protected]>

CC Tejun Heo <[email protected]>

Signed-off-by Shaohui Zheng <[email protected]>

Signed-off-by Haicheng Li <[email protected]>

---
This patch is based on Tejun's unification of the 32 and 64 bit NUMA boot paths,
specifically the patch at http://marc.info/?l=linux-kernel&m=129087151912379.
Index: linux-hpe4/arch/x86/kernel/acpi/boot.c
===================================================================

arch/x86/kernel/acpi/boot.c +31 -0

@@ -668,8 +668,39 @@
 }
 EXPORT_SYMBOL(acpi_map_lsapic);
 
+#ifdef CONFIG_ARCH_CPU_PROBE_RELEASE
+static void acpi_map_cpu2node_emu(int cpu, int physid, int nid)
+{
+#ifdef CONFIG_ACPI_NUMA
+	set_apicid_to_node(physid, nid);
+	numa_set_node(cpu, nid);
+#endif
+}
+
+static u16 cpu_to_apicid_saved[CONFIG_NR_CPUS];
+int __ref acpi_map_lsapic_emu(int pcpu, int nid)
+{
+	/* backup cpu apicid to array cpu_to_apicid_saved */
+	if (cpu_to_apicid_saved[pcpu] == 0 &&
+		per_cpu(x86_cpu_to_apicid, pcpu) != BAD_APICID)
+		cpu_to_apicid_saved[pcpu] = per_cpu(x86_cpu_to_apicid, pcpu);
+
+	per_cpu(x86_cpu_to_apicid, pcpu) = cpu_to_apicid_saved[pcpu];
+	acpi_map_cpu2node_emu(pcpu, per_cpu(x86_cpu_to_apicid, pcpu), nid);
+
+	return pcpu;
+}
+EXPORT_SYMBOL(acpi_map_lsapic_emu);
+#endif
+
 int acpi_unmap_lsapic(int cpu)
 {
+#ifdef CONFIG_ARCH_CPU_PROBE_RELEASE
+	/* backup cpu apicid to array cpu_to_apicid_saved */
+	if (cpu_to_apicid_saved[cpu] == 0 &&
+		per_cpu(x86_cpu_to_apicid, cpu) != BAD_APICID)
+		cpu_to_apicid_saved[cpu] = per_cpu(x86_cpu_to_apicid, cpu);
+#endif
 	per_cpu(x86_cpu_to_apicid, cpu) = -1;
 	set_cpu_present(cpu, false);
 	num_processors--;

Index: linux-hpe4/arch/x86/kernel/smpboot.c
===================================================================

arch/x86/kernel/smpboot.c +0 -2

@@ -103,8 +103,6 @@
         mutex_unlock(&x86_cpu_hotplug_driver_mutex);
 }
 
-ssize_t arch_cpu_probe(const char *buf, size_t count) { return -1; }
-ssize_t arch_cpu_release(const char *buf, size_t count) { return -1; }
 #else
 static struct task_struct *idle_thread_array[NR_CPUS] __cpuinitdata ;
 #define get_idle_for_cpu(x)      (idle_thread_array[(x)])

Index: linux-hpe4/arch/x86/kernel/topology.c
===================================================================

arch/x86/kernel/topology.c +83 -2

@@ -30,6 +30,9 @@
 #include <linux/init.h>
 #include <linux/smp.h>
 #include <asm/cpu.h>
+#include <linux/cpu.h>
+#include <linux/topology.h>
+#include <linux/acpi.h>
 
 static DEFINE_PER_CPU(struct x86_cpu, cpu_devices);
 

@@ -66,6 +69,78 @@
 	unregister_cpu(&per_cpu(cpu_devices, num).cpu);
 }
 EXPORT_SYMBOL(arch_unregister_cpu);
+
+ssize_t arch_cpu_probe(const char *buf, size_t count)
+{
+	int nid = 0;
+	int num = 0, selected = 0;
+
+	/* check parameters */
+	if (!buf || count < 2)
+		return -EPERM;
+
+	nid = simple_strtoul(buf, NULL, 0);
+	printk(KERN_DEBUG "Add a cpu to node : %d\n", nid);
+
+	if (nid < 0 || nid > nr_node_ids - 1) {
+		printk(KERN_ERR "Invalid NUMA node id: %d (0 <= nid < %d).\n",
+			nid, nr_node_ids);
+		return -EPERM;
+	}
+
+	if (!node_online(nid)) {
+		printk(KERN_ERR "NUMA node %d is not online, give up.\n", nid);
+		return -EPERM;
+	}
+
+	/* find first uninitialized cpu */
+	for_each_present_cpu(num) {
+		if (per_cpu(cpu_sys_devices, num) == NULL) {
+			selected = num;
+			break;
+		}
+	}
+
+	if (selected >= num_possible_cpus()) {
+		printk(KERN_ERR "No free cpu, give up cpu probing.\n");
+		return -EPERM;
+	}
+
+	/* register cpu */
+	arch_register_cpu_node(selected, nid);
+	acpi_map_lsapic_emu(selected, nid);
+
+	return count;
+}
+EXPORT_SYMBOL(arch_cpu_probe);
+
+ssize_t arch_cpu_release(const char *buf, size_t count)
+{
+	int cpu = 0;
+
+	cpu =  simple_strtoul(buf, NULL, 0);
+	/* cpu 0 is not hotplugable */
+	if (cpu == 0) {
+		printk(KERN_ERR "can not release cpu 0.\n");
+		return -EPERM;
+	}
+
+	if (cpu_online(cpu)) {
+		printk(KERN_DEBUG "offline cpu %d.\n", cpu);
+		if (!cpu_down(cpu)) {
+			printk(KERN_ERR "fail to offline cpu %d, give up.\n", cpu);
+			return -EPERM;
+		}
+
+	}
+
+	arch_unregister_cpu(cpu);
+	acpi_unmap_lsapic(cpu);
+
+	return count;
+}
+EXPORT_SYMBOL(arch_cpu_release);
+
 #else /* CONFIG_HOTPLUG_CPU */
 
 static int __init arch_register_cpu(int num)

@@ -83,8 +158,14 @@
 		register_one_node(i);
 #endif
 
-	for_each_present_cpu(i)
-		arch_register_cpu(i);
+	/*
+	 * when cpu hotplug emulation enabled, register the online cpu only,
+	 * the rests are reserved for cpu probe.
+	 */
+	for_each_present_cpu(i) {
+		if ((cpu_hpe_on && cpu_online(i)) || !cpu_hpe_on)
+			arch_register_cpu(i);
+	}
 
 	return 0;
 }

Index: linux-hpe4/arch/x86/mm/numa_64.c
===================================================================

arch/x86/mm/numa_64.c +15 -0

@@ -13,6 +13,7 @@
 #include <linux/module.h>
 #include <linux/nodemask.h>
 #include <linux/sched.h>
+#include <linux/cpu.h>
 
 #include <asm/e820.h>
 #include <asm/proto.h>

@@ -667,3 +668,17 @@
 		return __apicid_to_node[apicid];
 	return NUMA_NO_NODE;
 }
+
+#ifdef CONFIG_ARCH_CPU_PROBE_RELEASE
+static __init int cpu_hpe_setup(char *opt)
+{
+	if (!opt)
+		return -EINVAL;
+
+	if (!strncmp(opt, "on", 2) || !strncmp(opt, "1", 1))
+		cpu_hpe_on = 1;
+
+	return 0;
+}
+early_param("cpu_hpe", cpu_hpe_setup);
+#endif  /* CONFIG_ARCH_CPU_PROBE_RELEASE */

Index: linux-hpe4/drivers/acpi/processor_driver.c
===================================================================

drivers/acpi/processor_driver.c +9 -0

@@ -542,6 +542,14 @@
 		goto err_free_cpumask;
 
 	sysdev = get_cpu_sysdev(pr->id);
+	/*
+	 * Reserve cpu for hotplug emulation, the reserved cpu can be hot-added
+	 * throu the cpu probe interface. Return directly.
+	 */
+	if (sysdev == NULL) {
+		goto out;
+	}
+
 	if (sysfs_create_link(&device->dev.kobj, &sysdev->kobj, "sysdev")) {
 		result = -EFAULT;
 		goto err_remove_fs;

@@ -582,6 +590,7 @@
 		goto err_remove_sysfs;
 	}
 
+out:
 	return 0;
 
 err_remove_sysfs:

Index: linux-hpe4/drivers/base/cpu.c
===================================================================

drivers/base/cpu.c +7 -1

@@ -22,9 +22,15 @@
 };
 EXPORT_SYMBOL(cpu_sysdev_class);
 
-static DEFINE_PER_CPU(struct sys_device *, cpu_sys_devices);
+DEFINE_PER_CPU(struct sys_device *, cpu_sys_devices);
 
 #ifdef CONFIG_HOTPLUG_CPU
+/*
+ * cpu_hpe_on is a switch to enable/disable cpu hotplug emulation. it is
+ * disabled in default, we can enable it throu grub parameter cpu_hpe=on
+ */
+int cpu_hpe_on;
+
 static ssize_t show_online(struct sys_device *dev, struct sysdev_attribute *attr,
 			   char *buf)
 {

Index: linux-hpe4/include/linux/acpi.h
===================================================================

include/linux/acpi.h +1 -0

@@ -102,6 +102,7 @@
 #ifdef CONFIG_ACPI_HOTPLUG_CPU
 /* Arch dependent functions for cpu hotplug support */
 int acpi_map_lsapic(acpi_handle handle, int *pcpu);
+int acpi_map_lsapic_emu(int pcpu, int nid);
 int acpi_unmap_lsapic(int cpu);
 #endif /* CONFIG_ACPI_HOTPLUG_CPU */
 

Index: linux-hpe4/include/linux/cpu.h
===================================================================

include/linux/cpu.h +4 -0

@@ -25,6 +25,8 @@
 	struct sys_device sysdev;
 };
 
+DECLARE_PER_CPU(struct sys_device *, cpu_sys_devices);
+
 extern int register_cpu_node(struct cpu *cpu, int num, int nid);
 
 static inline int register_cpu(struct cpu *cpu, int num)

@@ -144,6 +146,7 @@
 #define register_hotcpu_notifier(nb)	register_cpu_notifier(nb)
 #define unregister_hotcpu_notifier(nb)	unregister_cpu_notifier(nb)
 int cpu_down(unsigned int cpu);
+extern int cpu_hpe_on;
 
 #ifdef CONFIG_ARCH_CPU_PROBE_RELEASE
 extern void cpu_hotplug_driver_lock(void);

@@ -166,6 +169,7 @@
 /* These aren't inline functions due to a GCC bug. */
 #define register_hotcpu_notifier(nb)	({ (void)(nb); 0; })
 #define unregister_hotcpu_notifier(nb)	({ (void)(nb); })
+static int cpu_hpe_on;
 #endif		/* CONFIG_HOTPLUG_CPU */
 
 #ifdef CONFIG_PM_SLEEP_SMP

Index: linux-hpe4/Documentation/x86/x86_64/boot-options.txt
===================================================================

Documentation/x86/x86_64/boot-options.txt +5 -0

@@ -320,3 +320,8 @@
 		Do not use GB pages for kernel direct mappings.
 	gbpages
 		Use GB pages for kernel direct mappings.
+	cpu_hpe=on/off
+		Enable/disable CPU hotplug emulation with software method. When cpu_hpe=on,
+		sysfs provides probe/release interface to hot add/remove CPUs dynamically.
+		We can use maxcpus=<N> to reserve CPUs.
+		This option is disabled by default.

Signature · 11 lines

-- 
Thanks & Regards,
Shaohui


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Shaohui,

What kernel is this series based on? I cannot get it to build when applied to mainline. I seem to be missing a definition for set_apicid_to_node.

Eric

From: Shaohui Zheng <[email protected]>

CPU physical hot-add/hot-remove are supported on some hardwares, and it was already supported in current linux kernel. NUMA Hotplug Emulator provides a mechanism to emulate the process with software method. It can be used for testing or debuging purpose.

CPU physical hotplug is different with logical CPU online/offline. Logical online/offline is controled by interface /sys/device/cpu/cpuX/online. CPU hotplug emulator uses probe/release interface. It becomes possible to do cpu hotplug automation and stress

Add cpu interface probe/release under sysfs for x86_64. User can use this interface to emulate the cpu hot-add and hot-remove process.

Directive:
*) Reserve CPU thru grub parameter like:
maxcpus=4

the rest CPUs will not be initiliazed.

*) Probe CPU
we can use the probe interface to hot-add new CPUs:
echo nid > /sys/devices/system/cpu/probe

*) Release a CPU
echo cpu > /sys/devices/system/cpu/release

A reserved CPU will be hot-added to the specified node.
1) nid == 0, the CPU will be added to the real node which the CPU should be in
2) nid != 0, add the CPU to node nid even through it is a fake node.

...

arch/x86/kernel/topology.c +3 -0

@@ -30,6 +30,9 @@
 #include <linux/init.h>
 #include <linux/smp.h>
 #include <asm/cpu.h>
+#include <linux/cpu.h>
+#include <linux/topology.h>
+#include <linux/acpi.h>
 
 static DEFINE_PER_CPU(struct x86_cpu, cpu_devices);
 

arch/x86/kernel/topology.c +6 -0

+			break;
+		}
+	}
+
+	if (selected >= num_possible_cpus()) {
+		printk(KERN_ERR "No free cpu, give up cpu probing.\n");

arch/x86/kernel/topology.c +4 -0

+	for_each_present_cpu(i) {
+		if ((cpu_hpe_on && cpu_online(i)) || !cpu_hpe_on)
+			arch_register_cpu(i);
+	}
 
 	return 0;
 }
 
...

drivers/acpi/processor_driver.c +1 -0

 	if (sysfs_create_link(&device->dev.kobj, &sysdev->kobj, "sysdev")) {
 		result = -EFAULT;
 		goto err_remove_fs;

@@ -582,6 +590,7 @@
 		goto err_remove_sysfs;
 	}
 
+out:
 	return 0;
 
 
...

On Fri, 10 Dec 2010 15:31:24 +0800

arch/x86/kernel/topology.c line 76

Quoted 5 lines

arch/x86/kernel/topology.c +5 -0

+
+ssize_t arch_cpu_probe(const char *buf, size_t count)
+{
+	int nid = 0;
+	int num = 0, selected = 0;

One definition per line make for more maintainable code.

Two of these initialisations are unnecessary.

arch/x86/kernel/topology.c line 82

Quoted 5 lines

arch/x86/kernel/topology.c +5 -0

+	/* check parameters */
+	if (!buf || count < 2)
+		return -EPERM;
+
+	nid = simple_strtoul(buf, NULL, 0);

checkpatch?

arch/x86/kernel/topology.c line 83

Quoted 2 lines

arch/x86/kernel/topology.c +1 -0

+	printk(KERN_DEBUG "Add a cpu to node : %d\n", nid);

"Add a CPU to node %d" would make more sense.

arch/x86/kernel/topology.c line 92

Quoted 8 lines

arch/x86/kernel/topology.c +8 -0

+	if (nid < 0 || nid > nr_node_ids - 1) {
+		printk(KERN_ERR "Invalid NUMA node id: %d (0 <= nid < %d).\n",
+			nid, nr_node_ids);
+		return -EPERM;
+	}
+
+	if (!node_online(nid)) {
+		printk(KERN_ERR "NUMA node %d is not online, give up.\n", nid);

"giving"

arch/x86/kernel/topology.c line 97

Quoted 5 lines

arch/x86/kernel/topology.c +5 -0

+		return -EPERM;
+	}
+
+	/* find first uninitialized cpu */
+	for_each_present_cpu(num) {

s/num/cpu/ would be conventional. "num" is a pretty poor identifier in general - it fails to identify what it is counting.

arch/x86/kernel/topology.c line 99

Quoted 2 lines

arch/x86/kernel/topology.c +2 -0

+		if (per_cpu(cpu_sys_devices, num) == NULL) {
+			selected = num;

Similarly, I'd have used "selected_cpu".

Quoted 6 lines

arch/x86/kernel/topology.c +6 -0

+			break;
+		}
+	}
+
+	if (selected >= num_possible_cpus()) {
+		printk(KERN_ERR "No free cpu, give up cpu probing.\n");

arch/x86/kernel/topology.c line 115

Quoted 10 lines

arch/x86/kernel/topology.c +10 -0

+		return -EPERM;
+	}
+
+	/* register cpu */
+	arch_register_cpu_node(selected, nid);
+	acpi_map_lsapic_emu(selected, nid);
+
+	return count;
+}
+EXPORT_SYMBOL(arch_cpu_probe);

arch_cpu_probe() is global and exported to modules, but is undocumented.

If it had been documented, I might have been able to work out why arg `count' is checked, but never used.

arch/x86/kernel/topology.c line 121

Quoted 5 lines

arch/x86/kernel/topology.c +5 -0

+ssize_t arch_cpu_release(const char *buf, size_t count)
+{
+	int cpu = 0;
+
+	cpu =  simple_strtoul(buf, NULL, 0);

unneeded initialisation, spurious whitespace, checkpatch.

arch/x86/kernel/topology.c line 124

Quoted 3 lines

arch/x86/kernel/topology.c +3 -0

+	/* cpu 0 is not hotplugable */
+	if (cpu == 0) {
+		printk(KERN_ERR "can not release cpu 0.\n");

It's generally better to make kernel messages self-identifying. Especially error messages. If someone comes along and sees "can not release cpu 0" in their logs, they don't have a clue what caused it unless they download the kernel sources and go grepping.

arch/x86/kernel/topology.c line 131

Quoted 7 lines

arch/x86/kernel/topology.c +7 -0

+		return -EPERM;
+	}
+
+	if (cpu_online(cpu)) {
+		printk(KERN_DEBUG "offline cpu %d.\n", cpu);
+		if (!cpu_down(cpu)) {
+			printk(KERN_ERR "fail to offline cpu %d, give up.\n", cpu);

"failed", "giving".

Quoted 5 lines

arch/x86/kernel/topology.c +5 -0

+			return -EPERM;
+		}
+
+	}
+

arch/x86/kernel/topology.c line 142

Quoted 6 lines

arch/x86/kernel/topology.c +6 -0

+	arch_unregister_cpu(cpu);
+	acpi_unmap_lsapic(cpu);
+
+	return count;
+}
+EXPORT_SYMBOL(arch_cpu_release);

No documentation.

Quoted 7 lines

#else /* CONFIG_HOTPLUG_CPU */

static int __init arch_register_cpu(int num)

arch/x86/kernel/topology.c +0 -0

@@ -83,8 +158,14 @@
 		register_one_node(i);
 #endif
 

arch/x86/kernel/topology.c line 164

Quoted 6 lines

arch/x86/kernel/topology.c +4 -2

-	for_each_present_cpu(i)
-		arch_register_cpu(i);
+	/*
+	 * when cpu hotplug emulation enabled, register the online cpu only,
+	 * the rests are reserved for cpu probe.
+	 */

Something like "When cpu hotplug emulation is enabled, register only the online cpu. The remainder are reserved for cpu probing.".

Quoted 13 lines

arch/x86/kernel/topology.c +4 -0

+	for_each_present_cpu(i) {
+		if ((cpu_hpe_on && cpu_online(i)) || !cpu_hpe_on)
+			arch_register_cpu(i);
+	}
 
 	return 0;
 }
 
...

drivers/acpi/processor_driver.c line 547

Quoted 6 lines

drivers/acpi/processor_driver.c +3 -0

@@ -542,6 +542,14 @@
 		goto err_free_cpumask;
 
 	sysdev = get_cpu_sysdev(pr->id);
+	/*
+	 * Reserve cpu for hotplug emulation, the reserved cpu can be hot-added
+	 * throu the cpu probe interface. Return directly.

s/emulation, the/emulation. The/
s/throu/through/

drivers/acpi/processor_driver.c line 551

Quoted 4 lines

drivers/acpi/processor_driver.c +4 -0

+	 */
+	if (sysdev == NULL) {
+		goto out;
+	}

Unneeded braces.

Quoted 16 lines

drivers/acpi/processor_driver.c +1 -0

 	if (sysfs_create_link(&device->dev.kobj, &sysdev->kobj, "sysdev")) {
 		result = -EFAULT;
 		goto err_remove_fs;

@@ -582,6 +590,7 @@
 		goto err_remove_sysfs;
 	}
 
+out:
 	return 0;
 
 
...

drivers/base/cpu.c line 29

Quoted 9 lines

drivers/base/cpu.c +3 -1

@@ -22,9 +22,15 @@
 };
 EXPORT_SYMBOL(cpu_sysdev_class);
 
-static DEFINE_PER_CPU(struct sys_device *, cpu_sys_devices);
+DEFINE_PER_CPU(struct sys_device *, cpu_sys_devices);
 
 #ifdef CONFIG_HOTPLUG_CPU
+/*
+ * cpu_hpe_on is a switch to enable/disable cpu hotplug emulation. it is

s/it/It/.

drivers/base/cpu.c line 30

Quoted 1 line

drivers/base/cpu.c +1 -0

+ * disabled in default, we can enable it throu grub parameter cpu_hpe=on

"through".

drivers/base/cpu.c line 32

Quoted 2 lines

drivers/base/cpu.c +2 -0

+ */
+int cpu_hpe_on;

__read_mostly, perhaps.

Quoted 6 lines

drivers/base/cpu.c +0 -0

 static ssize_t show_online(struct sys_device *dev, struct sysdev_attribute *attr,
 			   char *buf)
 {
 
...

Quoted 11 lines · Andrew Morton

On Fri, 10 Dec 2010 15:31:24 +0800

arch/x86/kernel/topology.c line 76

Quoted 5 lines

arch/x86/kernel/topology.c +5 -0

+
+ssize_t arch_cpu_probe(const char *buf, size_t count)
+{
+	int nid = 0;
+	int num = 0, selected = 0;

One definition per line make for more maintainable code.

Two of these initialisations are unnecessary.

Agree, I will put them into 2 lines, and remove the initialisations.
I always try to initialize them when we define it, it seems that it is a bad habit.

Quoted 7 lines

arch/x86/kernel/topology.c line 82

Quoted 5 lines

arch/x86/kernel/topology.c +5 -0

+	/* check parameters */
+	if (!buf || count < 2)
+		return -EPERM;
+
+	nid = simple_strtoul(buf, NULL, 0);

checkpatch?

it is a warning, so I ignore it.

Quoted 19 lines

Quoted 6 lines

arch/x86/kernel/topology.c +6 -0

+			break;
+		}
+	}
+
+	if (selected >= num_possible_cpus()) {
+		printk(KERN_ERR "No free cpu, give up cpu probing.\n");

arch/x86/kernel/topology.c line 115

Quoted 10 lines

arch/x86/kernel/topology.c +10 -0

+		return -EPERM;
+	}
+
+	/* register cpu */
+	arch_register_cpu_node(selected, nid);
+	acpi_map_lsapic_emu(selected, nid);
+
+	return count;
+}
+EXPORT_SYMBOL(arch_cpu_probe);

arch_cpu_probe() is global and exported to modules, but is undocumented.

Quoted 3 lines

If it had been documented, I might have been able to work out why arg `count' is checked, but never used.

Sorry, Andrew, I did not catch it. Do you mean to add the document before
the definition of the function arch_cpu_probe?

Quoted 10 lines

arch/x86/kernel/topology.c line 124

Quoted 4 lines

arch/x86/kernel/topology.c +3 -0

+	/* cpu 0 is not hotplugable */
+	if (cpu == 0) {
+		printk(KERN_ERR "can not release cpu 0.\n");

It's generally better to make kernel messages self-identifying. Especially error messages. If someone comes along and sees "can not release cpu 0" in their logs, they don't have a clue what caused it unless they download the kernel sources and go grepping.

How about "arch_cpu_release: can not release cpu 0.\n"?

Quoted 4 lines

Quoted 2 lines

checkpatch?

it is a warning, so I ignore it.

Don't ignore warnings! At least, not until you've understood the reason for them and have a *reason* to ignore them.

simple_strtoul() will silently accept input of the form "42foo", treating it as "42". That's a userspace bug and the kernel should report it. This means that the code should be changed to handle error returns from strict_strtoul(). And those error paths should be tested.

Quoted 19 lines

Quoted 19 lines

Quoted 6 lines

arch/x86/kernel/topology.c +6 -0

+			break;
+		}
+	}
+
+	if (selected >= num_possible_cpus()) {
+		printk(KERN_ERR "No free cpu, give up cpu probing.\n");

arch/x86/kernel/topology.c line 115

Quoted 10 lines

arch/x86/kernel/topology.c +10 -0

+		return -EPERM;
+	}
+
+	/* register cpu */
+	arch_register_cpu_node(selected, nid);
+	acpi_map_lsapic_emu(selected, nid);
+
+	return count;
+}
+EXPORT_SYMBOL(arch_cpu_probe);

arch_cpu_probe() is global and exported to modules, but is undocumented.

Quoted 6 lines

Quoted 3 lines

If it had been documented, I might have been able to work out why arg `count' is checked, but never used.

Sorry, Andrew, I did not catch it. Do you mean to add the document before
the definition of the function arch_cpu_probe?

Sure, add a comment documenting the function.

Why *does* it check `count' and then not use it?

Quoted 11 lines

Quoted 10 lines

arch/x86/kernel/topology.c line 124

Quoted 4 lines

arch/x86/kernel/topology.c +3 -0

+	/* cpu 0 is not hotplugable */
+	if (cpu == 0) {
+		printk(KERN_ERR "can not release cpu 0.\n");

It's generally better to make kernel messages self-identifying. Especially error messages. If someone comes along and sees "can not release cpu 0" in their logs, they don't have a clue what caused it unless they download the kernel sources and go grepping.

Quoted 1 line

How about "arch_cpu_release: can not release cpu 0.\n"?

Better, although "arch_cpu_release" isn't very meaningful to an administrator. "NUMA hotplug remove" or something like that would be more useful.

All these messages should be looked at from the point of view of the people who they are to serve. Although in this special case, that's most likely to be a kernel developer so I guess such clarity isn't needed.

Quoted 3 lines

Why *does* it check `count' and then not use it?

it is a tricky thing. When I debug it under a Virtual Machine, If I do a cpu probe via sysfs cpu/probe interface, The function arch_cpu_probe will be called __three__ times, but only one call is valid, so I add a check on count to ignore the invalid calls.

Quoted 4 lines · On Thu, 23 Dec 2010 10:24:28 +0800 Shaohui Zheng…

Quoted 3 lines

Why *does* it check `count' and then not use it?

Quoted 4 lines

it is a tricky thing. When I debug it under a Virtual Machine, If I do a cpu probe via sysfs cpu/probe interface, The function arch_cpu_probe will be called __three__ times, but only one call is valid, so I add a check on count to ignore the invalid calls.

hm, why does it get called three times? Is that something which can/should be fixed in callers rather than in the callee?

From: Shaohui Zheng <[email protected]>

When hotplug a CPU with emulator, we are using a logical CPU to emulate the CPU hotplug process. For the CPU supported SMT, some logical CPUs are in the same socket, but it may located in different NUMA node after we have emulator. it misleads the scheduling domain to build the incorrect hierarchy, and it causes the following call trace when rebalance the scheduling domain:

divide error: 0000 [#1] SMP
last sysfs file: /sys/devices/system/cpu/cpu8/online CPU 0

Modules linked in: fbcon tileblit font bitblit softcursor radeon ttm drm_kms_helper e1000e usbhid via_rhine mii drm i2c_algo_bit igb dca
Pid: 0, comm: swapper Not tainted 2.6.32hpe #78 X8DTN
RIP: 0010:[<ffffffff81051da5>]  [<ffffffff81051da5>] find_busiest_group+0x6c5/0xa10
RSP: 0018:ffff880028203c30  EFLAGS: 00010246
RAX: 0000000000000000 RBX: 0000000000015ac0 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffff880277e8cfa0 RDI: 0000000000000000
RBP: ffff880028203dc0 R08: ffff880277e8cfa0 R09: 0000000000000040
R10: 0000000000000000 R11: 0000000000000001 R12: 0000000000000000
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
FS:  0000000000000000(0000) GS:ffff880028200000(0000) knlGS:0000000000000000
CS:  0010 DS: 0018 ES: 0018 CR0: 000000008005003b
CR2: 00007f16cfc85770 CR3: 0000000001001000 CR4: 00000000000006f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Process swapper (pid: 0, threadinfo ffffffff81822000, task ffffffff8184a600)
Stack:
 ffff880028203d60 ffff880028203cd0 ffff8801c204ff08 ffff880028203e38
<0> 0101ffff81018c59 ffff880028203e44 00000001810806bd ffff8801c204fe00
<0> 0000000528200000 ffffffff00000000 0000000000000018 0000000000015ac0
Call Trace:
 <IRQ> 
 [<ffffffff81088ee0>] ? tick_dev_program_event+0x40/0xd0
 [<ffffffff81053b2c>] rebalance_domains+0x17c/0x570
 [<ffffffff81018c89>] ? read_tsc+0x9/0x20
 [<ffffffff81088ee0>] ? tick_dev_program_event+0x40/0xd0
 [<ffffffff810569ed>] run_rebalance_domains+0xbd/0xf0
 [<ffffffff8106471f>] __do_softirq+0xaf/0x1e0
 [<ffffffff810b7d18>] ? handle_IRQ_event+0x58/0x160
 [<ffffffff810130ac>] call_softirq+0x1c/0x30
 [<ffffffff81014a85>] do_softirq+0x65/0xa0
 [<ffffffff810645cd>] irq_exit+0x7d/0x90
 [<ffffffff81013ff0>] do_IRQ+0x70/0xe0
 [<ffffffff810128d3>] ret_from_intr+0x0/0x11
 <EOI> 
 [<ffffffff8133387f>] ? acpi_idle_enter_bm+0x281/0x2b5
 [<ffffffff81333878>] ? acpi_idle_enter_bm+0x27a/0x2b5
 [<ffffffff8145dc8f>] ? cpuidle_idle_call+0x9f/0x130
 [<ffffffff81010e2b>] ? cpu_idle+0xab/0x100
 [<ffffffff8158aee6>] ? rest_init+0x66/0x70
 [<ffffffff81905d90>] ? start_kernel+0x3e3/0x3ef
 [<ffffffff8190533a>] ? x86_64_start_reservations+0x125/0x129
 [<ffffffff81905438>] ? x86_64_start_kernel+0xfa/0x109
Code: 00 00 e9 4c fb ff ff 0f 1f 80 00 00 00 00 48 8b b5 d8 fe ff ff 48 8b 45 a8 4d 29 ef 8b 56 08 48 c1 e0 0a 49 89 f0 48 89 d7 31 d2 <48> f7 f7 31 d2 48 89 45 a0 8b 76 08 4c 89 f0 48 c1 e0 0a 48 f7 
RIP  [<ffffffff81051da5>] find_busiest_group+0x6c5/0xa10
 RSP <ffff880028203c30>

Solution:

We put the logical CPU into a fake CPU socket, and assign it an unique
phys_proc_id. For the fake socket, we put one logical CPU in only. This
method fixes the above bug.

...

arch/x86/include/asm/processor.h +5 -0

+	 * logical cpu into a fake socket, assign a fake physical id to it,
+	 * and create a fake core.
+	 */
+	__u8		cpu_probe_on; /* A flag to enable cpu probe/release */
+#endif
 } __attribute__((__aligned__(SMP_CACHE_BYTES)));
 
 #define X86_VENDOR_INTEL	0

Index: linux-hpe4/arch/x86/kernel/smpboot.c
===================================================================

arch/x86/kernel/smpboot.c +2 -0

@@ -97,6 +97,7 @@
  */
 static DEFINE_MUTEX(x86_cpu_hotplug_driver_mutex);
 
+#ifdef CONFIG_ARCH_CPU_PROBE_RELEASE
 void cpu_hotplug_driver_lock()
 {
         mutex_lock(&x86_cpu_hotplug_driver_mutex);

@@ -106,6 +107,7 @@
 {
         mutex_unlock(&x86_cpu_hotplug_driver_mutex);
 }
+#endif
 
 #else
 static struct task_struct *idle_thread_array[NR_CPUS] __cpuinitdata ;

+ */

arch/x86/kernel/smpboot.c +9 -0

+static void fake_cpu_socket_info(int cpu)
+{
+	struct cpuinfo_x86 *c = &cpu_data(cpu);
+	int i, phys_id = 0;
+
+	/* calculate the max phys_id */
+	for_each_present_cpu(i) {
+		struct cpuinfo_x86 *c = &cpu_data(i);
+		if (phys_id < c->phys_proc_id)

arch/x86/kernel/smpboot.c line 203

Quoted 6 lines

static struct task_struct *idle_thread_array[NR_CPUS] __cpuinitdata ;

arch/x86/kernel/smpboot.c +1 -0

@@ -198,6 +200,8 @@
 {
 	int cpuid, phys_id;
 	unsigned long timeout;
+	u8 cpu_probe_on = 0;

Unneeded initialisation.

Does this cause an unused var warning when
CONFIG_ARCH_CPU_PROBE_RELEASE=n?

Quoted 5 lines

arch/x86/kernel/smpboot.c +1 -0

+	struct cpuinfo_x86 *c;
 
 	/*
 	 * If waken up by an INIT in an 82489DX configuration

arch/x86/kernel/topology.c line 75

Quoted 5 lines

...

arch/x86/kernel/topology.c +3 -0

+#ifdef CONFIG_ARCH_CPU_PROBE_RELEASE
+/*
+ * Put the logical cpu into a new sokect, and encapsule it into core 0.

That comment needs help.

From: Shaohui Zheng <[email protected]>

Add add_memory interface to support to memory hotplug emulation for each online node under debugfs. The reserved memory can be added into desired node with this interface.

The layout on debugfs:

	mem_hotplug/node0/add_memory
	mem_hotplug/node1/add_memory
	mem_hotplug/node2/add_memory
	...

Add a memory section(128M) to node 3(boots with mem=1024m)

	echo 0x40000000 > mem_hotplug/node3/add_memory

...

mm/memory_hotplug.c +11 -0

+	return 0;
+}
+
+static const struct file_operations add_memory_file_ops = {
+	.open		= add_memory_open,
+	.write		= add_memory_store,
+	.llseek		= generic_file_llseek,
+};
+
+/*
+ * Create add_memory debugfs entry under specified node

mm/memory_hotplug.c +16 -0

+	/* the nid information was represented by the offset of pointer(NULL+nid) */
+	if (!debugfs_create_file("add_memory", S_IWUSR, node_debug_root,
+			NULL + nid, &add_memory_file_ops))
+		return -ENOMEM;
+
+	return 0;
+}
+
+static int __init memory_debug_init(void)
+{
+	int nid;
+
+	if (!memhp_debug_root)
+		memhp_debug_root = debugfs_create_dir("mem_hotplug", NULL);
+	if (!memhp_debug_root)
+		return -ENOMEM;

mm/memory_hotplug.c +5 -0

+{
+	return 0;
+}
+#endif /* CONFIG_ARCH_MEMORY_PROBE */
+
 static ssize_t add_node_store(struct file *file, const char __user *buf,
 				size_t count, loff_t *ppos)
 {

Quoted 19 lines · On Fri, 10 Dec 2010 15:31:26 +0800 [email protected] wrote:

From: Shaohui Zheng <[email protected]>

Add add_memory interface to support to memory hotplug emulation for each online node under debugfs. The reserved memory can be added into desired node with this interface.

The layout on debugfs:

	mem_hotplug/node0/add_memory
	mem_hotplug/node1/add_memory
	mem_hotplug/node2/add_memory
	...

Add a memory section(128M) to node 3(boots with mem=1024m)

	echo 0x40000000 > mem_hotplug/node3/add_memory

...

mm/memory_hotplug.c line 941

Quoted 6 lines

mm/memory_hotplug.c +6 -0

+#ifdef CONFIG_ARCH_MEMORY_PROBE
+
+static ssize_t add_memory_store(struct file *file, const char __user *buf,
+				size_t count, loff_t *ppos)
+{
+	u64 phys_addr = 0;

Even more unneeded initalisation.

Please check the whole patchset for this. It's bad because it can sometimes generate more code and because it can sometimes hide bugs by suppressing used-uninitialsied warnings.

mm/memory_hotplug.c line 942

Quoted 1 line

mm/memory_hotplug.c +1 -0

+	int nid = file->private_data - NULL;

Well that was sneaky.

It would be more conventional to just use the typecast:

	int nid = (long)file->private_data;

mm/memory_hotplug.c line 946

Quoted 4 lines

mm/memory_hotplug.c +4 -0

+	int ret;
+
+	printk(KERN_INFO "Add a memory section to node: %d.\n", nid);
+	phys_addr = simple_strtoull(buf, NULL, 0);

checkpatch

Quoted 5 lines

mm/memory_hotplug.c +5 -0

+	ret = add_memory(nid, phys_addr, PAGES_PER_SECTION << PAGE_SHIFT);
+	if (ret)
+		count = ret;
+
+	return count;

mm/memory_hotplug.c line 957

Quoted 5 lines

mm/memory_hotplug.c +5 -0

+}
+
+static int add_memory_open(struct inode *inode, struct file *file)
+{
+	file->private_data = inode->i_private;

Was this usage of i_private and private_data documented in comments somewhere?

Quoted 16 lines

mm/memory_hotplug.c +16 -0

+	return 0;
+}
+
+static const struct file_operations add_memory_file_ops = {
+	.open		= add_memory_open,
+	.write		= add_memory_store,
+	.llseek		= generic_file_llseek,
+};
+
+/*
+ * Create add_memory debugfs entry under specified node
+ */
+static int debugfs_create_add_memory_entry(int nid)
+{
+	char buf[32];
+	static struct dentry *node_debug_root;

mm/memory_hotplug.c line 978

Quoted 5 lines

mm/memory_hotplug.c +5 -0

+
+	snprintf(buf, sizeof(buf), "node%d", nid);
+	node_debug_root = debugfs_create_dir(buf, memhp_debug_root);
+	if (!node_debug_root)
+		return -ENOMEM;

hm, debugfs_create_dir() was poorly designed - it should return an ERR_PTR() so callers don't need to assume ENOMEM, which may be incorrect.

Quoted 20 lines

mm/memory_hotplug.c +20 -0

+	/* the nid information was represented by the offset of pointer(NULL+nid) */
+	if (!debugfs_create_file("add_memory", S_IWUSR, node_debug_root,
+			NULL + nid, &add_memory_file_ops))
+		return -ENOMEM;
+
+	return 0;
+}
+
+static int __init memory_debug_init(void)
+{
+	int nid;
+
+	if (!memhp_debug_root)
+		memhp_debug_root = debugfs_create_dir("mem_hotplug", NULL);
+	if (!memhp_debug_root)
+		return -ENOMEM;
+
+	for_each_online_node(nid)
+		 debugfs_create_add_memory_entry(nid);
+

mm/memory_hotplug.c line 1005

Quoted 6 lines

mm/memory_hotplug.c +6 -0

+	return 0;
+}
+
+module_init(memory_debug_init);
+#else
+static debugfs_create_add_memory_entry(int nid)

"static int".

Quoted 20 lines

mm/memory_hotplug.c +7 -0

+{
+	return 0;
+}
+#endif /* CONFIG_ARCH_MEMORY_PROBE */
+
 static ssize_t add_node_store(struct file *file, const char __user *buf,
 				size_t count, loff_t *ppos)
 {

@@ -963,6 +1038,8 @@
 		return -ENOMEM;
 
 	ret = add_memory(nid, start, size);
+
+	debugfs_create_add_memory_entry(nid);
 	return ret ? ret : count;
 }
 
 
...

v9:

Solve the bug reported by Eric B Munson, check the return value of cpu_down when do
CPU release.

Solve the conflicts with Tejun Heo' Unificaton NUMA code, re-work patch 5 based on his patch.

Some small changes on debugfs per-node add_memory interface.

Quoted 10 lines · On Fri, 10 Dec 2010, [email protected] wrote:

v9:

Solve the bug reported by Eric B Munson, check the return value of cpu_down when do
CPU release.

Solve the conflicts with Tejun Heo' Unificaton NUMA code, re-work patch 5 based on his patch.

Some small changes on debugfs per-node add_memory interface.

Hi Shaohui,

Tejun's NUMA unification work has been merged into x86/mm, so I think it would possible to rebase your hotplug emulator patchset on top of it without too many conflicts now.

It should probably be based on x86/mm from http://git.kernel.org/?p=linux/kernel/git/mingo/linux-2.6-x86.git
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

-----Original Message-----
From: Haicheng Li [mailto:[email protected]]
Sent: Wednesday, February 23, 2011 11:30 AM
To: David Rientjes
Cc: Zheng, Shaohui; Andrew Morton; [email protected]; [email protected]; [email protected]; Andi Kleen; [email protected]; Greg Kroah-Hartman; Zhang, Yang Z; You, Yongkang
Subject: Re: [0/7, v9] NUMA Hotplug Emulator (v9)

Shaohui is out of position recently. Include Yang Zhang and Yongkang You in this loop, who are Shaohui's backup.

Quoted 2 lines · David Rientjes

On Fri, 10 Dec 2010, [email protected] wrote:

Quoted 3 lines

Quoted 3 lines

v9:

Solve the bug reported by Eric B Munson, check the return value of

cpu_down when do

Quoted 3 lines

Quoted 3 lines

CPU release.

Solve the conflicts with Tejun Heo' Unificaton NUMA code, re-work patch 5

based on his

Quoted 19 lines

Quoted 4 lines

patch.

Some small changes on debugfs per-node add_memory interface.

Hi Shaohui,

Tejun's NUMA unification work has been merged into x86/mm, so I think it would possible to rebase your hotplug emulator patchset on top of it without too many conflicts now.

It should probably be based on x86/mm from
http://git.kernel.org/?p=linux/kernel/git/mingo/linux-2.6-x86.git
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/