2006-05-18 05:51:41

by Yasunori Goto

[permalink] [raw]
Subject: [PATCH] Register sysfs file for hotpluged new node take 2.

Hello.

This is the last part of patches for new nodes addition v4.
It is to create sysfs file for new node when new node is enabled.

This version makes generic function register_one_node() instead of
arch_register_node().
I386's old arch_register_node() is removed.
(This means include/asm-i386/node.h is not necessary.)
Powerpc's same code is consolidated into driver/base/node.c

This patch is for 2.6.17-rc4-mm1 too.

Please apply.

-------------------------------------
Change log from v1 of register_node()
- make generic function register_one_node() from arch_register_node().

Change log from v4 of node hot-add.
- update for 2.6.17-rc4-mm1.
- define arch_register_node() for not only ia64 but also powerpc and x86.

V4 of post is here.
<description>
http://marc.theaimsgroup.com/?l=linux-mm&m=114258404023573&w=2
<patches>
http://marc.theaimsgroup.com/?l=linux-mm&w=2&r=1&s=memory+hotplug+node+v.4.&q=b


Signed-off-by: Keiichiro Tokunaga <[email protected]>
Signed-off-by: Yasunori Goto <[email protected]>


include/asm-i386/node.h | 29 -----------------------------
arch/i386/kernel/topology.c | 9 +++------
arch/powerpc/kernel/sysfs.c | 15 ++-------------
drivers/base/node.c | 25 +++++++++++++++++++++++++
include/asm-i386/cpu.h | 2 --
include/linux/node.h | 4 ++++
mm/memory_hotplug.c | 12 +++++++++++-
7 files changed, 45 insertions(+), 51 deletions(-)

Index: pgdat14/arch/i386/kernel/topology.c
===================================================================
--- pgdat14.orig/arch/i386/kernel/topology.c 2006-05-18 10:38:50.000000000 +0900
+++ pgdat14/arch/i386/kernel/topology.c 2006-05-18 10:38:54.000000000 +0900
@@ -38,7 +38,7 @@ int arch_register_cpu(int num){
#ifdef CONFIG_NUMA
int node = cpu_to_node(num);
if (node_online(node))
- parent = &node_devices[node].node;
+ parent = &node_devices[parent_node(node)];
#endif /* CONFIG_NUMA */

/*
@@ -61,7 +61,7 @@ void arch_unregister_cpu(int num) {
#ifdef CONFIG_NUMA
int node = cpu_to_node(num);
if (node_online(node))
- parent = &node_devices[node].node;
+ parent = &node_devices[parent_node(node)];
#endif /* CONFIG_NUMA */

return unregister_cpu(&cpu_devices[num].cpu, parent);
@@ -74,16 +74,13 @@ EXPORT_SYMBOL(arch_unregister_cpu);

#ifdef CONFIG_NUMA
#include <linux/mmzone.h>
-#include <asm/node.h>
-
-struct i386_node node_devices[MAX_NUMNODES];

static int __init topology_init(void)
{
int i;

for_each_online_node(i)
- arch_register_node(i);
+ register_one_node(i);

for_each_present_cpu(i)
arch_register_cpu(i);
Index: pgdat14/arch/powerpc/kernel/sysfs.c
===================================================================
--- pgdat14.orig/arch/powerpc/kernel/sysfs.c 2006-05-18 10:38:50.000000000 +0900
+++ pgdat14/arch/powerpc/kernel/sysfs.c 2006-05-18 10:38:54.000000000 +0900
@@ -304,23 +304,12 @@ static struct notifier_block sysfs_cpu_n
/* NUMA stuff */

#ifdef CONFIG_NUMA
-static struct node node_devices[MAX_NUMNODES];
-
static void register_nodes(void)
{
int i;

- for (i = 0; i < MAX_NUMNODES; i++) {
- if (node_online(i)) {
- int p_node = parent_node(i);
- struct node *parent = NULL;
-
- if (p_node != i)
- parent = &node_devices[p_node];
-
- register_node(&node_devices[i], i, parent);
- }
- }
+ for (i = 0; i < MAX_NUMNODES; i++)
+ register_one_node(i);
}

int sysfs_add_device_to_node(struct sys_device *dev, int nid)
Index: pgdat14/drivers/base/node.c
===================================================================
--- pgdat14.orig/drivers/base/node.c 2006-05-18 10:38:50.000000000 +0900
+++ pgdat14/drivers/base/node.c 2006-05-18 10:38:54.000000000 +0900
@@ -190,6 +190,31 @@ void unregister_node(struct node *node)
sysdev_unregister(&node->sysdev);
}

+struct node node_devices[MAX_NUMNODES];
+
+int register_one_node(int nid)
+{
+ int error = 0;
+
+ if (node_online(nid)) {
+ int p_node = parent_node(nid);
+ struct node *parent = NULL;
+
+ if (p_node != nid)
+ parent = &node_devices[p_node];
+
+ error = register_node(&node_devices[nid], nid, parent);
+ }
+
+ return error;
+
+}
+
+void unregister_one_node(int nid)
+{
+ unregister_node(&node_devices[nid]);
+}
+
static int __init register_node_type(void)
{
return sysdev_class_register(&node_class);
Index: pgdat14/include/asm-i386/node.h
===================================================================
--- pgdat14.orig/include/asm-i386/node.h 2006-05-18 10:38:50.000000000 +0900
+++ /dev/null 1970-01-01 00:00:00.000000000 +0000
@@ -1,29 +0,0 @@
-#ifndef _ASM_I386_NODE_H_
-#define _ASM_I386_NODE_H_
-
-#include <linux/device.h>
-#include <linux/mmzone.h>
-#include <linux/node.h>
-#include <linux/topology.h>
-#include <linux/nodemask.h>
-
-struct i386_node {
- struct node node;
-};
-extern struct i386_node node_devices[MAX_NUMNODES];
-
-static inline int arch_register_node(int num){
- int p_node;
- struct node *parent = NULL;
-
- if (!node_online(num))
- return 0;
- p_node = parent_node(num);
-
- if (p_node != num)
- parent = &node_devices[p_node].node;
-
- return register_node(&node_devices[num].node, num, parent);
-}
-
-#endif /* _ASM_I386_NODE_H_ */
Index: pgdat14/mm/memory_hotplug.c
===================================================================
--- pgdat14.orig/mm/memory_hotplug.c 2006-05-18 10:38:50.000000000 +0900
+++ pgdat14/mm/memory_hotplug.c 2006-05-18 12:00:48.000000000 +0900
@@ -256,9 +256,19 @@ int add_memory(int nid, u64 start, u64 s
if (ret < 0)
goto error;

- /* we online node here. we have no error path from here. */
+ /* we online node here. we can't roll back from here. */
node_set_online(nid);

+ if (new_pgdat) {
+ ret = register_one_node(nid);
+ /*
+ * If sysfs file of new node can't create, cpu on the node
+ * can't be hot-added. There is no rollback way now.
+ * So, check by BUG_ON() to catch it reluctantly..
+ */
+ BUG_ON(ret);
+ }
+
/* register this memory as resource */
register_memory_resource(start, size);

Index: pgdat14/include/asm-i386/cpu.h
===================================================================
--- pgdat14.orig/include/asm-i386/cpu.h 2006-05-18 10:38:50.000000000 +0900
+++ pgdat14/include/asm-i386/cpu.h 2006-05-18 10:38:54.000000000 +0900
@@ -7,8 +7,6 @@
#include <linux/nodemask.h>
#include <linux/percpu.h>

-#include <asm/node.h>
-
struct i386_cpu {
struct cpu cpu;
};
Index: pgdat14/include/linux/node.h
===================================================================
--- pgdat14.orig/include/linux/node.h 2006-05-18 10:38:50.000000000 +0900
+++ pgdat14/include/linux/node.h 2006-05-18 10:38:54.000000000 +0900
@@ -26,8 +26,12 @@ struct node {
struct sys_device sysdev;
};

+extern struct node node_devices[];
+
extern int register_node(struct node *, int, struct node *);
extern void unregister_node(struct node *node);
+extern int register_one_node(int nid);
+extern void unregister_one_node(int nid);

#define to_node(sys_device) container_of(sys_device, struct node, sysdev)


--
Yasunori Goto



2006-05-19 10:19:05

by Yasunori Goto

[permalink] [raw]
Subject: Re: [PATCH] Register sysfs file for hotpluged new node take 2.

Andrew-san.

Sorry. I realize that I forgot to remove old sysfs structure of node for ia64
in yesterday's patch. :-(

Please apply this too.

-------------

Creating sysfs file for node is consolidated as generic code
by creating registrer_one_node() and node_devices[].
But, ia64's boot time code remains old sysfs_nodes structure
as an arch dependent code. This is to remove it.

This patch is for 2.6.17-rc4-mm1 with
+ register-sysfs-file-for-hotpluged-new-node.patch

I tested this on Tiger4 box with my multi nodes emulation.

Signed-off-by: Yasunori Goto <[email protected]>

-------------

arch/ia64/kernel/topology.c | 15 +++------------
1 files changed, 3 insertions(+), 12 deletions(-)

Index: pgdat14/arch/ia64/kernel/topology.c
===================================================================
--- pgdat14.orig/arch/ia64/kernel/topology.c 2006-05-19 14:54:37.000000000 +0900
+++ pgdat14/arch/ia64/kernel/topology.c 2006-05-19 15:16:09.000000000 +0900
@@ -26,9 +26,6 @@
#include <asm/numa.h>
#include <asm/cpu.h>

-#ifdef CONFIG_NUMA
-static struct node *sysfs_nodes;
-#endif
static struct ia64_cpu *sysfs_cpus;

int arch_register_cpu(int num)
@@ -36,7 +33,7 @@ int arch_register_cpu(int num)
struct node *parent = NULL;

#ifdef CONFIG_NUMA
- parent = &sysfs_nodes[cpu_to_node(num)];
+ parent = &node_devices[cpu_to_node(num)];
#endif /* CONFIG_NUMA */

#if defined (CONFIG_ACPI) && defined (CONFIG_HOTPLUG_CPU)
@@ -59,7 +56,7 @@ void arch_unregister_cpu(int num)

#ifdef CONFIG_NUMA
int node = cpu_to_node(num);
- parent = &sysfs_nodes[node];
+ parent = &node_devices[node];
#endif /* CONFIG_NUMA */

return unregister_cpu(&sysfs_cpus[num].cpu, parent);
@@ -74,17 +71,11 @@ static int __init topology_init(void)
int i, err = 0;

#ifdef CONFIG_NUMA
- sysfs_nodes = kzalloc(sizeof(struct node) * MAX_NUMNODES, GFP_KERNEL);
- if (!sysfs_nodes) {
- err = -ENOMEM;
- goto out;
- }
-
/*
* MCD - Do we want to register all ONLINE nodes, or all POSSIBLE nodes?
*/
for_each_online_node(i) {
- if ((err = register_node(&sysfs_nodes[i], i, 0)))
+ if ((err = register_one_node(i)))
goto out;
}
#endif

--
Yasunori Goto


2006-05-19 17:03:12

by Dave Hansen

[permalink] [raw]
Subject: Re: [PATCH] Register sysfs file for hotpluged new node take 2.

On Thu, 2006-05-18 at 14:50 +0900, Yasunori Goto wrote:
> + if (new_pgdat) {
> + ret = register_one_node(nid);
> + /*
> + * If sysfs file of new node can't create, cpu on the node
> + * can't be hot-added. There is no rollback way now.
> + * So, check by BUG_ON() to catch it reluctantly..
> + */
> + BUG_ON(ret);
> + }

How about we register the node in sysfs _before_ it is
set_node_online()'d? Effectively an empty node with no memory and no
CPUs. It might be a wee bit confusing to any user tools watching the
NUMA sysfs stuff, but I think it beats a BUG().

-- Dave

2006-05-20 12:19:20

by Yasunori Goto

[permalink] [raw]
Subject: Re: [PATCH] Register sysfs file for hotpluged new node take 2.

> On Thu, 2006-05-18 at 14:50 +0900, Yasunori Goto wrote:
> > + if (new_pgdat) {
> > + ret = register_one_node(nid);
> > + /*
> > + * If sysfs file of new node can't create, cpu on the node
> > + * can't be hot-added. There is no rollback way now.
> > + * So, check by BUG_ON() to catch it reluctantly..
> > + */
> > + BUG_ON(ret);
> > + }
>
> How about we register the node in sysfs _before_ it is
> set_node_online()'d? Effectively an empty node with no memory and no
> CPUs. It might be a wee bit confusing to any user tools watching the
> NUMA sysfs stuff, but I think it beats a BUG().

Hmmm. I'm not sure what will happen when sysfs file is accessed by user
at this time. I think this issue should be going to be solved when
__remove_memory() and pgdat offline will be created.

Thanks for your comment.

--
Yasunori Goto