[RFC/PATCH] unregister_node() for hotplug use

[RFC/PATCH] unregister_node() for hotplug use

[Keiichiro Tokunaga]

  This is to add a generic function 'unregister_node()'.
It is used to remove objects of a node going away for
hotplug.  If CONFIG_HOTPLUG=y, it becomes available.
This is against 2.6.12-rc2-mm3.

Thanks,
Keiichiro Tokunaga

Signed-off-by: Keiichiro Tokunaga <tokunaga.keiich@jp.fujitsu.com>
---

 linux-2.6.12-rc2-mm3-kei/drivers/base/node.c  |   29 ++++++++++++++++++++++++--
 linux-2.6.12-rc2-mm3-kei/include/linux/node.h |    6 ++++-
 2 files changed, 32 insertions(+), 3 deletions(-)

diff -puN drivers/base/node.c~numa_hp_base drivers/base/node.c
--- linux-2.6.12-rc2-mm3/drivers/base/node.c~numa_hp_base	2005-04-14 20:49:37.000000000 +0900
+++ linux-2.6.12-rc2-mm3-kei/drivers/base/node.c	2005-04-14 20:49:37.000000000 +0900
@@ -136,7 +136,7 @@ static SYSDEV_ATTR(distance, S_IRUGO, no
  *
  * Initialize and register the node device.
  */
-int __init register_node(struct node *node, int num, struct node *parent)
+int __devinit register_node(struct node *node, int num, struct node *parent)
 {
 	int error;
 
@@ -145,6 +145,9 @@ int __init register_node(struct node *no
 	error = sysdev_register(&node->sysdev);
 
 	if (!error){
+		/*
+		 * If you add new object here, delete it when unregistering.
+		 */
 		sysdev_create_file(&node->sysdev, &attr_cpumap);
 		sysdev_create_file(&node->sysdev, &attr_meminfo);
 		sysdev_create_file(&node->sysdev, &attr_numastat);
@@ -153,8 +156,30 @@ int __init register_node(struct node *no
 	return error;
 }
 
+/*
+ * unregister_node - Remove objects of a node going away from sysfs.
+ * @node - node going away
+ *
+ * This is used only for hotplug.
+ */
+#ifdef CONFIG_HOTPLUG
+void unregister_node(struct node *node)
+{
+	if (node == NULL)
+		return;
+
+	sysdev_remove_file(&node->sysdev, &attr_cpumap);
+	sysdev_remove_file(&node->sysdev, &attr_meminfo);
+	sysdev_remove_file(&node->sysdev, &attr_numastat);
+	sysdev_remove_file(&node->sysdev, &attr_distance);
+
+	sysdev_unregister(&node->sysdev);
+}
+EXPORT_SYMBOL(register_node);
+EXPORT_SYMBOL(unregister_node);
+#endif /* CONFIG_HOTPLUG */
 
-int __init register_node_type(void)
+static int __init register_node_type(void)
 {
 	return sysdev_class_register(&node_class);
 }
diff -puN include/linux/node.h~numa_hp_base include/linux/node.h
--- linux-2.6.12-rc2-mm3/include/linux/node.h~numa_hp_base	2005-04-14 20:49:37.000000000 +0900
+++ linux-2.6.12-rc2-mm3-kei/include/linux/node.h	2005-04-14 20:49:37.000000000 +0900
@@ -21,12 +21,16 @@
 
 #include <linux/sysdev.h>
 #include <linux/cpumask.h>
+#include <linux/module.h>
 
 struct node {
 	struct sys_device	sysdev;
 };
 
-extern int register_node(struct node *, int, struct node *);
+extern int __devinit register_node(struct node *, int, struct node *);
+#ifdef CONFIG_HOTPLUG
+extern void unregister_node(struct node *node);
+#endif
 
 #define to_node(sys_device) container_of(sys_device, struct node, sysdev)
 

_

Re: [RFC/PATCH] unregister_node() for hotplug use

[Arjan van de Ven]

> diff -puN drivers/base/node.c~numa_hp_base drivers/base/node.c
> --- linux-2.6.12-rc2-mm3/drivers/base/node.c~numa_hp_base	2005-04-14 20:49:37.000000000 +0900
> +++ linux-2.6.12-rc2-mm3-kei/drivers/base/node.c	2005-04-14 20:49:37.000000000 +0900
> @@ -136,7 +136,7 @@ static SYSDEV_ATTR(distance, S_IRUGO, no
>   
> +EXPORT_SYMBOL(register_node);
> +EXPORT_SYMBOL(unregister_node);
> +#endif /* CONFIG_HOTPLUG */
>  

please make these EXPORT_SYMBOL_GPL; the rest of sysfs is too and this
is very much deep kernel internals that are linux specific.

Re: [RFC/PATCH] unregister_node() for hotplug use

[Greg KH]

On Wed, Apr 20, 2005 at 09:07:44PM +0900, Keiichiro Tokunaga wrote:
>   This is to add a generic function 'unregister_node()'.
> It is used to remove objects of a node going away for
> hotplug.  If CONFIG_HOTPLUG=y, it becomes available.
> This is against 2.6.12-rc2-mm3.

Please CC: this kind of stuff to the driver core maintainer, otherwise
it can get dropped...

Anyway, comments below:

> diff -puN drivers/base/node.c~numa_hp_base drivers/base/node.c
> --- linux-2.6.12-rc2-mm3/drivers/base/node.c~numa_hp_base	2005-04-14 20:49:37.000000000 +0900
> +++ linux-2.6.12-rc2-mm3-kei/drivers/base/node.c	2005-04-14 20:49:37.000000000 +0900
> @@ -136,7 +136,7 @@ static SYSDEV_ATTR(distance, S_IRUGO, no
>   *
>   * Initialize and register the node device.
>   */
> -int __init register_node(struct node *node, int num, struct node *parent)
> +int __devinit register_node(struct node *node, int num, struct node *parent)
>  {
>  	int error;
>  
> @@ -145,6 +145,9 @@ int __init register_node(struct node *no
>  	error = sysdev_register(&node->sysdev);
>  
>  	if (!error){
> +		/*
> +		 * If you add new object here, delete it when unregistering.
> +		 */

Comment really isn't needed.

> +/*
> + * unregister_node - Remove objects of a node going away from sysfs.
> + * @node - node going away
> + *
> + * This is used only for hotplug.
> + */

If you are going to create function comments, at least use the proper
kerneldoc format.

> +#ifdef CONFIG_HOTPLUG

You don't provide function prototype for when CONFIG_HOTPLUG is not
enabled.

> +void unregister_node(struct node *node)
> +{
> +	if (node == NULL)
> +		return;

How can this happen?

> +
> +	sysdev_remove_file(&node->sysdev, &attr_cpumap);
> +	sysdev_remove_file(&node->sysdev, &attr_meminfo);
> +	sysdev_remove_file(&node->sysdev, &attr_numastat);
> +	sysdev_remove_file(&node->sysdev, &attr_distance);
> +
> +	sysdev_unregister(&node->sysdev);
> +}
> +EXPORT_SYMBOL(register_node);
> +EXPORT_SYMBOL(unregister_node);

All of sysfs and the driver core are EXPORT_SYMBOL_GPL().  Please follow
that convention.

> +#endif /* CONFIG_HOTPLUG */
>  
> -int __init register_node_type(void)
> +static int __init register_node_type(void)

Are you sure no one calls this?

>  {
>  	return sysdev_class_register(&node_class);
>  }
> diff -puN include/linux/node.h~numa_hp_base include/linux/node.h
> --- linux-2.6.12-rc2-mm3/include/linux/node.h~numa_hp_base	2005-04-14 20:49:37.000000000 +0900
> +++ linux-2.6.12-rc2-mm3-kei/include/linux/node.h	2005-04-14 20:49:37.000000000 +0900
> @@ -21,12 +21,16 @@
>  
>  #include <linux/sysdev.h>
>  #include <linux/cpumask.h>
> +#include <linux/module.h>

Why?

>  
>  struct node {
>  	struct sys_device	sysdev;
>  };
>  
> -extern int register_node(struct node *, int, struct node *);
> +extern int __devinit register_node(struct node *, int, struct node *);

__devinit is not needed on a function prototype.

> +#ifdef CONFIG_HOTPLUG
> +extern void unregister_node(struct node *node);
> +#endif

Not needed for a function prototype.

thanks,

greg k-h

Re: [RFC/PATCH] unregister_node() for hotplug use

[Keiichiro Tokunaga]

On Wed, 20 Apr 2005 14:35:10 +0200 Arjan van de Ven wrote:
> 
> > diff -puN drivers/base/node.c~numa_hp_base drivers/base/node.c
> > --- linux-2.6.12-rc2-mm3/drivers/base/node.c~numa_hp_base	2005-04-14 20:49:37.000000000 +0900
> > +++ linux-2.6.12-rc2-mm3-kei/drivers/base/node.c	2005-04-14 20:49:37.000000000 +0900
> > @@ -136,7 +136,7 @@ static SYSDEV_ATTR(distance, S_IRUGO, no
> >   
> > +EXPORT_SYMBOL(register_node);
> > +EXPORT_SYMBOL(unregister_node);
> > +#endif /* CONFIG_HOTPLUG */
> >  
> 
> please make these EXPORT_SYMBOL_GPL; the rest of sysfs is too and this
> is very much deep kernel internals that are linux specific.

  OK, I will make that change.  Thanks for the comments!

Thanks,
Keiichiro Tokunaga

Re: [RFC/PATCH] unregister_node() for hotplug use

[Keiichiro Tokunaga]

On Wed, 20 Apr 2005 10:32:35 -0700 Greg KH wrote:
> On Wed, Apr 20, 2005 at 09:07:44PM +0900, Keiichiro Tokunaga wrote:
> >   This is to add a generic function 'unregister_node()'.
> > It is used to remove objects of a node going away for
> > hotplug.  If CONFIG_HOTPLUG=y, it becomes available.
> > This is against 2.6.12-rc2-mm3.
> 
> Please CC: this kind of stuff to the driver core maintainer, otherwise
> it can get dropped...

  I will for sure CC appropriate maintainers next time...

> Anyway, comments below:
> 
> > diff -puN drivers/base/node.c~numa_hp_base drivers/base/node.c
> > --- linux-2.6.12-rc2-mm3/drivers/base/node.c~numa_hp_base	2005-04-14 20:49:37.000000000 +0900
> > +++ linux-2.6.12-rc2-mm3-kei/drivers/base/node.c	2005-04-14 20:49:37.000000000 +0900
> > @@ -136,7 +136,7 @@ static SYSDEV_ATTR(distance, S_IRUGO, no
> >   *
> >   * Initialize and register the node device.
> >   */
> > -int __init register_node(struct node *node, int num, struct node *parent)
> > +int __devinit register_node(struct node *node, int num, struct node *parent)
> >  {
> >  	int error;
> >  
> > @@ -145,6 +145,9 @@ int __init register_node(struct node *no
> >  	error = sysdev_register(&node->sysdev);
> >  
> >  	if (!error){
> > +		/*
> > +		 * If you add new object here, delete it when unregistering.
> > +		 */
> 
> Comment really isn't needed.

  I removed the comments.

> > +/*
> > + * unregister_node - Remove objects of a node going away from sysfs.
> > + * @node - node going away
> > + *
> > + * This is used only for hotplug.
> > + */
> 
> If you are going to create function comments, at least use the proper
> kerneldoc format.

  I removed it.  I thought about its necessity again and
it seems that the comments isn't really necessary because
the purpose of the function is obvious anyway.

> > +#ifdef CONFIG_HOTPLUG
> 
> You don't provide function prototype for when CONFIG_HOTPLUG is not
> enabled.
>
> > +void unregister_node(struct node *node)
> > +{
> > +	if (node == NULL)
> > +		return;
> 
> How can this happen?

  It's a redundant check, so I removed it.

> > +
> > +	sysdev_remove_file(&node->sysdev, &attr_cpumap);
> > +	sysdev_remove_file(&node->sysdev, &attr_meminfo);
> > +	sysdev_remove_file(&node->sysdev, &attr_numastat);
> > +	sysdev_remove_file(&node->sysdev, &attr_distance);
> > +
> > +	sysdev_unregister(&node->sysdev);
> > +}
> > +EXPORT_SYMBOL(register_node);
> > +EXPORT_SYMBOL(unregister_node);
> 
> All of sysfs and the driver core are EXPORT_SYMBOL_GPL().  Please follow
> that convention.

  OK, I made that change.

> > +#endif /* CONFIG_HOTPLUG */
> >  
> > -int __init register_node_type(void)
> > +static int __init register_node_type(void)
> 
> Are you sure no one calls this?

  Yes, no one calls this today.

> >  {
> >  	return sysdev_class_register(&node_class);
> >  }
> > diff -puN include/linux/node.h~numa_hp_base include/linux/node.h
> > --- linux-2.6.12-rc2-mm3/include/linux/node.h~numa_hp_base	2005-04-14 20:49:37.000000000 +0900
> > +++ linux-2.6.12-rc2-mm3-kei/include/linux/node.h	2005-04-14 20:49:37.000000000 +0900
> > @@ -21,12 +21,16 @@
> >  
> >  #include <linux/sysdev.h>
> >  #include <linux/cpumask.h>
> > +#include <linux/module.h>
> 
> Why?
> 
> >  
> >  struct node {
> >  	struct sys_device	sysdev;
> >  };
> >  
> > -extern int register_node(struct node *, int, struct node *);
> > +extern int __devinit register_node(struct node *, int, struct node *);
> 
> __devinit is not needed on a function prototype.
> 
> > +#ifdef CONFIG_HOTPLUG
> > +extern void unregister_node(struct node *node);
> > +#endif
> 
> Not needed for a function prototype.

  I corrected them.  Thanks for all the comments!
  I am attaching the updated patch.  Please take a look.

Thanks,
Keiichiro Tokunaga


Signed-off-by: Keiichiro Tokunaga <tokunaga.keiich@jp.fujitsu.com>
---

 linux-2.6.12-rc2-mm3-kei/drivers/base/node.c  |   21 +++++++++++++++++++--
 linux-2.6.12-rc2-mm3-kei/include/linux/node.h |    1 +
 2 files changed, 20 insertions(+), 2 deletions(-)

diff -puN drivers/base/node.c~numa_hp_base drivers/base/node.c
--- linux-2.6.12-rc2-mm3/drivers/base/node.c~numa_hp_base	2005-04-14 20:49:37.000000000 +0900
+++ linux-2.6.12-rc2-mm3-kei/drivers/base/node.c	2005-04-21 19:20:41.000000000 +0900
@@ -136,7 +136,7 @@ static SYSDEV_ATTR(distance, S_IRUGO, no
  *
  * Initialize and register the node device.
  */
-int __init register_node(struct node *node, int num, struct node *parent)
+int __devinit register_node(struct node *node, int num, struct node *parent)
 {
 	int error;
 
@@ -153,8 +153,25 @@ int __init register_node(struct node *no
 	return error;
 }
 
+#ifdef CONFIG_HOTPLUG
+void unregister_node(struct node *node)
+{
+	sysdev_remove_file(&node->sysdev, &attr_cpumap);
+	sysdev_remove_file(&node->sysdev, &attr_meminfo);
+	sysdev_remove_file(&node->sysdev, &attr_numastat);
+	sysdev_remove_file(&node->sysdev, &attr_distance);
+
+	sysdev_unregister(&node->sysdev);
+}
+EXPORT_SYMBOL_GPL(register_node);
+EXPORT_SYMBOL_GPL(unregister_node);
+#else /* !CONFIG_HOTPLUG */
+void unregister_node(struct node *node)
+{
+}
+#endif /* !CONFIG_HOTPLUG */
 
-int __init register_node_type(void)
+static int __init register_node_type(void)
 {
 	return sysdev_class_register(&node_class);
 }
diff -puN include/linux/node.h~numa_hp_base include/linux/node.h
--- linux-2.6.12-rc2-mm3/include/linux/node.h~numa_hp_base	2005-04-14 20:49:37.000000000 +0900
+++ linux-2.6.12-rc2-mm3-kei/include/linux/node.h	2005-04-21 13:48:58.000000000 +0900
@@ -27,6 +27,7 @@ struct node {
 };
 
 extern int register_node(struct node *, int, struct node *);
+extern void unregister_node(struct node *node);
 
 #define to_node(sys_device) container_of(sys_device, struct node, sysdev)
 

_

Re: [RFC/PATCH] unregister_node() for hotplug use

[Greg KH]

On Fri, Apr 22, 2005 at 12:30:09AM +0900, Keiichiro Tokunaga wrote:
> +#ifdef CONFIG_HOTPLUG
> +void unregister_node(struct node *node)
> +{
> +	sysdev_remove_file(&node->sysdev, &attr_cpumap);
> +	sysdev_remove_file(&node->sysdev, &attr_meminfo);
> +	sysdev_remove_file(&node->sysdev, &attr_numastat);
> +	sysdev_remove_file(&node->sysdev, &attr_distance);
> +
> +	sysdev_unregister(&node->sysdev);
> +}
> +EXPORT_SYMBOL_GPL(register_node);
> +EXPORT_SYMBOL_GPL(unregister_node);
> +#else /* !CONFIG_HOTPLUG */
> +void unregister_node(struct node *node)
> +{
> +}
> +#endif /* !CONFIG_HOTPLUG */

Oops, you only exported the symbols if CONFIG_HOTPLUG is enabled, not a
good idea.  Either make the null functions in a .h file if the option is
not enabled, or always export them.

And the comment for the #endif isn't really needed, as the whole #ifdef
fits on a single screen :)

And hey, what's the real big deal here, why not always have this
function no matter if CONFIG_HOTPLUG is enabled or not?  I really want
to just make that an option that is always enabled anyway, but changable
if you are using CONFIG_TINY or something...

thanks,

greg k-h

Re: [RFC/PATCH] unregister_node() for hotplug use

[Keiichiro Tokunaga]

On Thu, 21 Apr 2005 17:39:20 -0700 Greg KH wrote:
> On Fri, Apr 22, 2005 at 12:30:09AM +0900, Keiichiro Tokunaga wrote:
> > +#ifdef CONFIG_HOTPLUG
> > +void unregister_node(struct node *node)
> > +{
> > +	sysdev_remove_file(&node->sysdev, &attr_cpumap);
> > +	sysdev_remove_file(&node->sysdev, &attr_meminfo);
> > +	sysdev_remove_file(&node->sysdev, &attr_numastat);
> > +	sysdev_remove_file(&node->sysdev, &attr_distance);
> > +
> > +	sysdev_unregister(&node->sysdev);
> > +}
> > +EXPORT_SYMBOL_GPL(register_node);
> > +EXPORT_SYMBOL_GPL(unregister_node);
> > +#else /* !CONFIG_HOTPLUG */
> > +void unregister_node(struct node *node)
> > +{
> > +}
> > +#endif /* !CONFIG_HOTPLUG */
> 
> Oops, you only exported the symbols if CONFIG_HOTPLUG is enabled, not a
> good idea.  Either make the null functions in a .h file if the option is
> not enabled, or always export them.

  My bad...

> And the comment for the #endif isn't really needed, as the whole #ifdef
> fits on a single screen :)

  I see:)  That makes sense.

> And hey, what's the real big deal here, why not always have this
> function no matter if CONFIG_HOTPLUG is enabled or not?  I really want
> to just make that an option that is always enabled anyway, but changable
> if you are using CONFIG_TINY or something...

  I put the #ifdef there for users who don't need hotplug
stuffs, but I want to make the option always enabled, too.
Also a good side effect, the code would be cleaner:)  I
will be updating my patch without the #ifdef and sending
it here.

Thanks!
Keiichiro Tokunaga

Re: [RFC/PATCH] unregister_node() for hotplug use

[Keiichiro Tokunaga]

On Fri, 22 Apr 2005 11:32:11 +0900 Keiichiro Tokunaga wrote:
> On Thu, 21 Apr 2005 17:39:20 -0700 Greg KH wrote:
> > On Fri, Apr 22, 2005 at 12:30:09AM +0900, Keiichiro Tokunaga wrote:
> > > +#ifdef CONFIG_HOTPLUG
> > > +void unregister_node(struct node *node)
> > > +{
> > > +	sysdev_remove_file(&node->sysdev, &attr_cpumap);
> > > +	sysdev_remove_file(&node->sysdev, &attr_meminfo);
> > > +	sysdev_remove_file(&node->sysdev, &attr_numastat);
> > > +	sysdev_remove_file(&node->sysdev, &attr_distance);
> > > +
> > > +	sysdev_unregister(&node->sysdev);
> > > +}
> > > +EXPORT_SYMBOL_GPL(register_node);
> > > +EXPORT_SYMBOL_GPL(unregister_node);
> > > +#else /* !CONFIG_HOTPLUG */
> > > +void unregister_node(struct node *node)
> > > +{
> > > +}
> > > +#endif /* !CONFIG_HOTPLUG */
<snip>
> > And hey, what's the real big deal here, why not always have this
> > function no matter if CONFIG_HOTPLUG is enabled or not?  I really want
> > to just make that an option that is always enabled anyway, but changable
> > if you are using CONFIG_TINY or something...
> 
>   I put the #ifdef there for users who don't need hotplug
> stuffs, but I want to make the option always enabled, too.
> Also a good side effect, the code would be cleaner:)  I
> will be updating my patch without the #ifdef and sending
> it here.

  Here is the patch.  Please apply.

Thanks,
Keiichiro Tokunaga


Signed-off-by: Keiichiro Tokunaga <tokunaga.keiich@jp.fujitsu.com>
---

 linux-2.6.12-rc2-mm3-kei/drivers/base/node.c  |   15 +++++++++++++--
 linux-2.6.12-rc2-mm3-kei/include/linux/node.h |    1 +
 2 files changed, 14 insertions(+), 2 deletions(-)

diff -puN drivers/base/node.c~numa_hp_base drivers/base/node.c
--- linux-2.6.12-rc2-mm3/drivers/base/node.c~numa_hp_base	2005-04-25 22:37:53.064182320 +0900
+++ linux-2.6.12-rc2-mm3-kei/drivers/base/node.c	2005-04-25 22:41:02.744844059 +0900
@@ -136,7 +136,7 @@ static SYSDEV_ATTR(distance, S_IRUGO, no
  *
  * Initialize and register the node device.
  */
-int __init register_node(struct node *node, int num, struct node *parent)
+int __devinit register_node(struct node *node, int num, struct node *parent)
 {
 	int error;
 
@@ -153,8 +153,19 @@ int __init register_node(struct node *no
 	return error;
 }
 
+void unregister_node(struct node *node)
+{
+	sysdev_remove_file(&node->sysdev, &attr_cpumap);
+	sysdev_remove_file(&node->sysdev, &attr_meminfo);
+	sysdev_remove_file(&node->sysdev, &attr_numastat);
+	sysdev_remove_file(&node->sysdev, &attr_distance);
+
+	sysdev_unregister(&node->sysdev);
+}
+EXPORT_SYMBOL_GPL(register_node);
+EXPORT_SYMBOL_GPL(unregister_node);
 
-int __init register_node_type(void)
+static int __init register_node_type(void)
 {
 	return sysdev_class_register(&node_class);
 }
diff -puN include/linux/node.h~numa_hp_base include/linux/node.h
--- linux-2.6.12-rc2-mm3/include/linux/node.h~numa_hp_base	2005-04-25 22:37:53.067112007 +0900
+++ linux-2.6.12-rc2-mm3-kei/include/linux/node.h	2005-04-25 22:37:53.069065132 +0900
@@ -27,6 +27,7 @@ struct node {
 };
 
 extern int register_node(struct node *, int, struct node *);
+extern void unregister_node(struct node *node);
 
 #define to_node(sys_device) container_of(sys_device, struct node, sysdev)
 

_

Re: [RFC/PATCH] unregister_node() for hotplug use

[Greg KH]

On Mon, Apr 25, 2005 at 11:03:33PM +0900, Keiichiro Tokunaga wrote:
> On Fri, 22 Apr 2005 11:32:11 +0900 Keiichiro Tokunaga wrote:
> > On Thu, 21 Apr 2005 17:39:20 -0700 Greg KH wrote:
> > > On Fri, Apr 22, 2005 at 12:30:09AM +0900, Keiichiro Tokunaga wrote:
> > > > +#ifdef CONFIG_HOTPLUG
> > > > +void unregister_node(struct node *node)
> > > > +{
> > > > +	sysdev_remove_file(&node->sysdev, &attr_cpumap);
> > > > +	sysdev_remove_file(&node->sysdev, &attr_meminfo);
> > > > +	sysdev_remove_file(&node->sysdev, &attr_numastat);
> > > > +	sysdev_remove_file(&node->sysdev, &attr_distance);
> > > > +
> > > > +	sysdev_unregister(&node->sysdev);
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(register_node);
> > > > +EXPORT_SYMBOL_GPL(unregister_node);
> > > > +#else /* !CONFIG_HOTPLUG */
> > > > +void unregister_node(struct node *node)
> > > > +{
> > > > +}
> > > > +#endif /* !CONFIG_HOTPLUG */
> <snip>
> > > And hey, what's the real big deal here, why not always have this
> > > function no matter if CONFIG_HOTPLUG is enabled or not?  I really want
> > > to just make that an option that is always enabled anyway, but changable
> > > if you are using CONFIG_TINY or something...
> > 
> >   I put the #ifdef there for users who don't need hotplug
> > stuffs, but I want to make the option always enabled, too.
> > Also a good side effect, the code would be cleaner:)  I
> > will be updating my patch without the #ifdef and sending
> > it here.
> 
>   Here is the patch.  Please apply.

Care to resend it with a proper change log description that I can use?

thanks,

greg k-h

Re: [RFC/PATCH] unregister_node() for hotplug use

[Keiichiro Tokunaga]

On Mon, 25 Apr 2005 23:54:32 -0700 Greg KH wrote:
> On Mon, Apr 25, 2005 at 11:03:33PM +0900, Keiichiro Tokunaga wrote:
> > On Fri, 22 Apr 2005 11:32:11 +0900 Keiichiro Tokunaga wrote:
> > > On Thu, 21 Apr 2005 17:39:20 -0700 Greg KH wrote:
> > > > On Fri, Apr 22, 2005 at 12:30:09AM +0900, Keiichiro Tokunaga wrote:
> > > > > +#ifdef CONFIG_HOTPLUG
> > > > > +void unregister_node(struct node *node)
> > > > > +{
> > > > > +	sysdev_remove_file(&node->sysdev, &attr_cpumap);
> > > > > +	sysdev_remove_file(&node->sysdev, &attr_meminfo);
> > > > > +	sysdev_remove_file(&node->sysdev, &attr_numastat);
> > > > > +	sysdev_remove_file(&node->sysdev, &attr_distance);
> > > > > +
> > > > > +	sysdev_unregister(&node->sysdev);
> > > > > +}
> > > > > +EXPORT_SYMBOL_GPL(register_node);
> > > > > +EXPORT_SYMBOL_GPL(unregister_node);
> > > > > +#else /* !CONFIG_HOTPLUG */
> > > > > +void unregister_node(struct node *node)
> > > > > +{
> > > > > +}
> > > > > +#endif /* !CONFIG_HOTPLUG */
> > <snip>
> > > > And hey, what's the real big deal here, why not always have this
> > > > function no matter if CONFIG_HOTPLUG is enabled or not?  I really want
> > > > to just make that an option that is always enabled anyway, but changable
> > > > if you are using CONFIG_TINY or something...
> > > 
> > >   I put the #ifdef there for users who don't need hotplug
> > > stuffs, but I want to make the option always enabled, too.
> > > Also a good side effect, the code would be cleaner:)  I
> > > will be updating my patch without the #ifdef and sending
> > > it here.
> > 
> >   Here is the patch.  Please apply.
> 
> Care to resend it with a proper change log description that I can use?

  Sure.  But, please let me ask you something before I
post the update patch.  I think register_node() also
should be always there if unregister_node() is always
there.  So, the '__devinit' attribute for register_node()
does not seem to be necessary.  (Actually, the attribute
'__init' of register_node() was replaced with '__devinit'
in my previous patch.)  What do you think of this?

Thanks,
Keiichiro Tokunaga

Re: [RFC/PATCH] unregister_node() for hotplug use

[Keiichiro Tokunaga]

On Mon, 25 Apr 2005 23:54:32 -0700 Greg KH wrote:
> On Mon, Apr 25, 2005 at 11:03:33PM +0900, Keiichiro Tokunaga wrote:
> > On Fri, 22 Apr 2005 11:32:11 +0900 Keiichiro Tokunaga wrote:
> > > On Thu, 21 Apr 2005 17:39:20 -0700 Greg KH wrote:
> > > > On Fri, Apr 22, 2005 at 12:30:09AM +0900, Keiichiro Tokunaga wrote:
> > > > > +#ifdef CONFIG_HOTPLUG
> > > > > +void unregister_node(struct node *node)
> > > > > +{
> > > > > +	sysdev_remove_file(&node->sysdev, &attr_cpumap);
> > > > > +	sysdev_remove_file(&node->sysdev, &attr_meminfo);
> > > > > +	sysdev_remove_file(&node->sysdev, &attr_numastat);
> > > > > +	sysdev_remove_file(&node->sysdev, &attr_distance);
> > > > > +
> > > > > +	sysdev_unregister(&node->sysdev);
> > > > > +}
> > > > > +EXPORT_SYMBOL_GPL(register_node);
> > > > > +EXPORT_SYMBOL_GPL(unregister_node);
> > > > > +#else /* !CONFIG_HOTPLUG */
> > > > > +void unregister_node(struct node *node)
> > > > > +{
> > > > > +}
> > > > > +#endif /* !CONFIG_HOTPLUG */
> > <snip>
> > > > And hey, what's the real big deal here, why not always have this
> > > > function no matter if CONFIG_HOTPLUG is enabled or not?  I really want
> > > > to just make that an option that is always enabled anyway, but changable
> > > > if you are using CONFIG_TINY or something...
> > > 
> > >   I put the #ifdef there for users who don't need hotplug
> > > stuffs, but I want to make the option always enabled, too.
> > > Also a good side effect, the code would be cleaner:)  I
> > > will be updating my patch without the #ifdef and sending
> > > it here.
> > 
> >   Here is the patch.  Please apply.
> 
> Care to resend it with a proper change log description that I can use?

  I updated the patch for 2.6.12-rc3-mm3 based on my
previous comments.  Please apply.

Thanks,
Keiichiro Tokunaga



This adds a generic function 'unregister_node()'.
It is used to remove objects of a node going away
for hotplug.

Signed-off-by: Keiichiro Tokunaga <tokunaga.keiich@jp.fujitsu.com>
---

 linux-2.6.12-rc3-mm3-kei/drivers/base/node.c  |   15 +++++++++++++--
 linux-2.6.12-rc3-mm3-kei/include/linux/node.h |    1 +
 2 files changed, 14 insertions(+), 2 deletions(-)

diff -puN drivers/base/node.c~numa_hp_base drivers/base/node.c
--- linux-2.6.12-rc3-mm3/drivers/base/node.c~numa_hp_base	2005-05-07 19:58:15.000000000 +0900
+++ linux-2.6.12-rc3-mm3-kei/drivers/base/node.c	2005-05-07 19:58:15.000000000 +0900
@@ -136,7 +136,7 @@ static SYSDEV_ATTR(distance, S_IRUGO, no
  *
  * Initialize and register the node device.
  */
-int __init register_node(struct node *node, int num, struct node *parent)
+int register_node(struct node *node, int num, struct node *parent)
 {
 	int error;
 
@@ -153,8 +153,19 @@ int __init register_node(struct node *no
 	return error;
 }
 
+void unregister_node(struct node *node)
+{
+	sysdev_remove_file(&node->sysdev, &attr_cpumap);
+	sysdev_remove_file(&node->sysdev, &attr_meminfo);
+	sysdev_remove_file(&node->sysdev, &attr_numastat);
+	sysdev_remove_file(&node->sysdev, &attr_distance);
+
+	sysdev_unregister(&node->sysdev);
+}
+EXPORT_SYMBOL_GPL(register_node);
+EXPORT_SYMBOL_GPL(unregister_node);
 
-int __init register_node_type(void)
+static int __init register_node_type(void)
 {
 	return sysdev_class_register(&node_class);
 }
diff -puN include/linux/node.h~numa_hp_base include/linux/node.h
--- linux-2.6.12-rc3-mm3/include/linux/node.h~numa_hp_base	2005-05-07 19:58:15.000000000 +0900
+++ linux-2.6.12-rc3-mm3-kei/include/linux/node.h	2005-05-07 19:58:15.000000000 +0900
@@ -27,6 +27,7 @@ struct node {
 };
 
 extern int register_node(struct node *, int, struct node *);
+extern void unregister_node(struct node *node);
 
 #define to_node(sys_device) container_of(sys_device, struct node, sysdev)
 

_

Re: [RFC/PATCH] unregister_node() for hotplug use

[Nathan Lynch]

> This adds a generic function 'unregister_node()'.
> It is used to remove objects of a node going away
> for hotplug.
> 
> Signed-off-by: Keiichiro Tokunaga <tokunaga.keiich@jp.fujitsu.com>
> ---
> 
>  linux-2.6.12-rc3-mm3-kei/drivers/base/node.c  |   15 +++++++++++++--
>  linux-2.6.12-rc3-mm3-kei/include/linux/node.h |    1 +
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff -puN drivers/base/node.c~numa_hp_base drivers/base/node.c
> --- linux-2.6.12-rc3-mm3/drivers/base/node.c~numa_hp_base	2005-05-07 19:58:15.000000000 +0900
> +++ linux-2.6.12-rc3-mm3-kei/drivers/base/node.c	2005-05-07 19:58:15.000000000 +0900
> @@ -136,7 +136,7 @@ static SYSDEV_ATTR(distance, S_IRUGO, no
>   *
>   * Initialize and register the node device.
>   */
> -int __init register_node(struct node *node, int num, struct node *parent)
> +int register_node(struct node *node, int num, struct node *parent)
>  {
>  	int error;
>  
> @@ -153,8 +153,19 @@ int __init register_node(struct node *no
>  	return error;
>  }
>  
> +void unregister_node(struct node *node)
> +{
> +	sysdev_remove_file(&node->sysdev, &attr_cpumap);
> +	sysdev_remove_file(&node->sysdev, &attr_meminfo);
> +	sysdev_remove_file(&node->sysdev, &attr_numastat);
> +	sysdev_remove_file(&node->sysdev, &attr_distance);
> +
> +	sysdev_unregister(&node->sysdev);
> +}

Is it a bug to call unregister_node() if there are still cpus or
memory present in the node?  Note that register_cpu() creates a
symlink under the node directory to the cpu -- are you assuming that
all the node's cpu sysdevs will have been unregistered by the time
unregister_node is called?  If so, is it possible to enforce that, or
at least document it?

> +EXPORT_SYMBOL_GPL(register_node);
> +EXPORT_SYMBOL_GPL(unregister_node);

What module code needs to call these?  ACPI?


Nathan

Re: [RFC/PATCH] unregister_node() for hotplug use

[Keiichiro Tokunaga]

On Sat, 07 May 2005 19:26:48 -0500 Nathan Lynch wrote:
> > This adds a generic function 'unregister_node()'.
> > It is used to remove objects of a node going away
> > for hotplug.
> > 
> > Signed-off-by: Keiichiro Tokunaga <tokunaga.keiich@jp.fujitsu.com>
> > ---
> > 
> >  linux-2.6.12-rc3-mm3-kei/drivers/base/node.c  |   15 +++++++++++++--
> >  linux-2.6.12-rc3-mm3-kei/include/linux/node.h |    1 +
> >  2 files changed, 14 insertions(+), 2 deletions(-)
> > 
> > diff -puN drivers/base/node.c~numa_hp_base drivers/base/node.c
> > --- linux-2.6.12-rc3-mm3/drivers/base/node.c~numa_hp_base	2005-05-07 19:58:15.000000000 +0900
> > +++ linux-2.6.12-rc3-mm3-kei/drivers/base/node.c	2005-05-07 19:58:15.000000000 +0900
> > @@ -136,7 +136,7 @@ static SYSDEV_ATTR(distance, S_IRUGO, no
> >   *
> >   * Initialize and register the node device.
> >   */
> > -int __init register_node(struct node *node, int num, struct node *parent)
> > +int register_node(struct node *node, int num, struct node *parent)
> >  {
> >  	int error;
> >  
> > @@ -153,8 +153,19 @@ int __init register_node(struct node *no
> >  	return error;
> >  }
> >  
> > +void unregister_node(struct node *node)
> > +{
> > +	sysdev_remove_file(&node->sysdev, &attr_cpumap);
> > +	sysdev_remove_file(&node->sysdev, &attr_meminfo);
> > +	sysdev_remove_file(&node->sysdev, &attr_numastat);
> > +	sysdev_remove_file(&node->sysdev, &attr_distance);
> > +
> > +	sysdev_unregister(&node->sysdev);
> > +}
> 
> Is it a bug to call unregister_node() if there are still cpus or
> memory present in the node?  Note that register_cpu() creates a
> symlink under the node directory to the cpu -- are you assuming that
> all the node's cpu sysdevs will have been unregistered by the time
> unregister_node is called?  If so, is it possible to enforce that, or
> at least document it?

  Yes, this code is assuming that.  I will document it in
a function description of unregister_node().

> > +EXPORT_SYMBOL_GPL(register_node);
> > +EXPORT_SYMBOL_GPL(unregister_node);
> 
> What module code needs to call these?  ACPI?

  Nobody today.  I thought it was necessary for my another
patch to support node hotplug, but I found out it's not.
My mistake...  I will remove them.

  I'm attaching an updated patch.

Thanks,
Keiichiro Tokunaga



This adds a generic function 'unregister_node()'.
It is used to remove objects of a node going away
for hotplug.  All the devices on the node must be
unregistered before calling this function.

Signed-off-by: Keiichiro Tokunaga <tokunaga.keiich@jp.fujitsu.com>
---

 linux-2.6.12-rc3-mm3-kei/drivers/base/node.c  |   20 ++++++++++++++++++--
 linux-2.6.12-rc3-mm3-kei/include/linux/node.h |    1 +
 2 files changed, 19 insertions(+), 2 deletions(-)

diff -puN drivers/base/node.c~numa_hp_base drivers/base/node.c
--- linux-2.6.12-rc3-mm3/drivers/base/node.c~numa_hp_base	2005-05-07 19:58:15.000000000 +0900
+++ linux-2.6.12-rc3-mm3-kei/drivers/base/node.c	2005-05-08 20:27:32.000000000 +0900
@@ -136,7 +136,7 @@ static SYSDEV_ATTR(distance, S_IRUGO, no
  *
  * Initialize and register the node device.
  */
-int __init register_node(struct node *node, int num, struct node *parent)
+int register_node(struct node *node, int num, struct node *parent)
 {
 	int error;
 
@@ -153,8 +153,24 @@ int __init register_node(struct node *no
 	return error;
 }
 
+/**
+ * unregister_node - unregister a node device
+ * @node: node going away
+ *
+ * Unregisters a node device @node.  All the devices on the node must be
+ * unregistered before calling this function.
+ */
+void unregister_node(struct node *node)
+{
+	sysdev_remove_file(&node->sysdev, &attr_cpumap);
+	sysdev_remove_file(&node->sysdev, &attr_meminfo);
+	sysdev_remove_file(&node->sysdev, &attr_numastat);
+	sysdev_remove_file(&node->sysdev, &attr_distance);
+
+	sysdev_unregister(&node->sysdev);
+}
 
-int __init register_node_type(void)
+static int __init register_node_type(void)
 {
 	return sysdev_class_register(&node_class);
 }
diff -puN include/linux/node.h~numa_hp_base include/linux/node.h
--- linux-2.6.12-rc3-mm3/include/linux/node.h~numa_hp_base	2005-05-07 19:58:15.000000000 +0900
+++ linux-2.6.12-rc3-mm3-kei/include/linux/node.h	2005-05-07 19:58:15.000000000 +0900
@@ -27,6 +27,7 @@ struct node {
 };
 
 extern int register_node(struct node *, int, struct node *);
+extern void unregister_node(struct node *node);
 
 #define to_node(sys_device) container_of(sys_device, struct node, sysdev)
 

_

Re: [RFC/PATCH] unregister_node() for hotplug use

[Matthew Dobson]

Keiichiro Tokunaga wrote:
>   I updated the patch for 2.6.12-rc3-mm3 based on my
> previous comments.  Please apply.
> 
> Thanks,
> Keiichiro Tokunaga
> 
> 
> 
> This adds a generic function 'unregister_node()'.
> It is used to remove objects of a node going away
> for hotplug.
> 
> Signed-off-by: Keiichiro Tokunaga <tokunaga.keiich@jp.fujitsu.com>
> ---
> 
>  linux-2.6.12-rc3-mm3-kei/drivers/base/node.c  |   15 +++++++++++++--
>  linux-2.6.12-rc3-mm3-kei/include/linux/node.h |    1 +
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff -puN drivers/base/node.c~numa_hp_base drivers/base/node.c
> --- linux-2.6.12-rc3-mm3/drivers/base/node.c~numa_hp_base	2005-05-07 19:58:15.000000000 +0900
> +++ linux-2.6.12-rc3-mm3-kei/drivers/base/node.c	2005-05-07 19:58:15.000000000 +0900
> @@ -136,7 +136,7 @@ static SYSDEV_ATTR(distance, S_IRUGO, no
>   *
>   * Initialize and register the node device.
>   */
> -int __init register_node(struct node *node, int num, struct node *parent)
> +int register_node(struct node *node, int num, struct node *parent)
>  {
>  	int error;
>  
> @@ -153,8 +153,19 @@ int __init register_node(struct node *no
>  	return error;
>  }
>  
> +void unregister_node(struct node *node)
> +{
> +	sysdev_remove_file(&node->sysdev, &attr_cpumap);
> +	sysdev_remove_file(&node->sysdev, &attr_meminfo);
> +	sysdev_remove_file(&node->sysdev, &attr_numastat);
> +	sysdev_remove_file(&node->sysdev, &attr_distance);
> +
> +	sysdev_unregister(&node->sysdev);
> +}

Is there a reason to not make both register_node() and unregister_node()
__devinit?  If a user has CONFIG_HOTPLUG=y then they want these functions,
otherwise there is no point, as they promised they won't be hotplugging
anything, right?


-Matt

Re: [RFC/PATCH] unregister_node() for hotplug use

[Keiichiro Tokunaga]

On Mon, 09 May 2005 15:44:03 -0700 Matthew Dobson wrote:
> Keiichiro Tokunaga wrote:
> >   I updated the patch for 2.6.12-rc3-mm3 based on my
> > previous comments.  Please apply.
> > 
> > Thanks,
> > Keiichiro Tokunaga
> > 
> > 
> > 
> > This adds a generic function 'unregister_node()'.
> > It is used to remove objects of a node going away
> > for hotplug.
> > 
> > Signed-off-by: Keiichiro Tokunaga <tokunaga.keiich@jp.fujitsu.com>
> > ---
> > 
> >  linux-2.6.12-rc3-mm3-kei/drivers/base/node.c  |   15 +++++++++++++--
> >  linux-2.6.12-rc3-mm3-kei/include/linux/node.h |    1 +
> >  2 files changed, 14 insertions(+), 2 deletions(-)
> > 
> > diff -puN drivers/base/node.c~numa_hp_base drivers/base/node.c
> > --- linux-2.6.12-rc3-mm3/drivers/base/node.c~numa_hp_base	2005-05-07 19:58:15.000000000 +0900
> > +++ linux-2.6.12-rc3-mm3-kei/drivers/base/node.c	2005-05-07 19:58:15.000000000 +0900
> > @@ -136,7 +136,7 @@ static SYSDEV_ATTR(distance, S_IRUGO, no
> >   *
> >   * Initialize and register the node device.
> >   */
> > -int __init register_node(struct node *node, int num, struct node *parent)
> > +int register_node(struct node *node, int num, struct node *parent)
> >  {
> >  	int error;
> >  
> > @@ -153,8 +153,19 @@ int __init register_node(struct node *no
> >  	return error;
> >  }
> >  
> > +void unregister_node(struct node *node)
> > +{
> > +	sysdev_remove_file(&node->sysdev, &attr_cpumap);
> > +	sysdev_remove_file(&node->sysdev, &attr_meminfo);
> > +	sysdev_remove_file(&node->sysdev, &attr_numastat);
> > +	sysdev_remove_file(&node->sysdev, &attr_distance);
> > +
> > +	sysdev_unregister(&node->sysdev);
> > +}
> 
> Is there a reason to not make both register_node() and unregister_node()
> __devinit?  If a user has CONFIG_HOTPLUG=y then they want these functions,
> otherwise there is no point, as they promised they won't be hotplugging
> anything, right?

  The main reason is Greg advised me that we had the function
no matter if CONFIG_HOTPLUG is true or not.  An addtional
advantage of this is that the source becomes cleaner because
I included unregister_node() with '#ifdef CONFIG_HOTPLUG' and
'#endif' in my previous version of patch.

Thanks,
Keiichiro Tokunaga

Re: [RFC/PATCH] unregister_node() for hotplug use

[Matthew Dobson]

Keiichiro Tokunaga wrote:
> On Mon, 09 May 2005 15:44:03 -0700 Matthew Dobson wrote:
>>Is there a reason to not make both register_node() and unregister_node()
>>__devinit?  If a user has CONFIG_HOTPLUG=y then they want these functions,
>>otherwise there is no point, as they promised they won't be hotplugging
>>anything, right?
> 
>   The main reason is Greg advised me that we had the function
> no matter if CONFIG_HOTPLUG is true or not.  An addtional
> advantage of this is that the source becomes cleaner because
> I included unregister_node() with '#ifdef CONFIG_HOTPLUG' and
> '#endif' in my previous version of patch.
> 
> Thanks,
> Keiichiro Tokunaga
> 

You're referring to this comment:

On Thu, 21 Apr 2005 17:39:20 -0700 Greg KH wrote:
>> And hey, what's the real big deal here, why not always have this
>> function no matter if CONFIG_HOTPLUG is enabled or not?  I really want
>> to just make that an option that is always enabled anyway, but changable
>> if you are using CONFIG_TINY or something...

correct?  I think GregKH was referring to you #ifdef'ing the function away
so that it isn't even there if CONFIG_HOTPLUG=n, which is completely
different from marking the function as __devinit.

If you mark the function as __devinit, it will still be there for the
kernel initialization phase whether CONFIG_HOTPLUG is on or off.  What it
does mean, however, is that the function will get freed, along with the
rest of the functions/data marked __init or __initdata, if CONFIG_HOTPLUG
is off.  If CONFIG_HOTPLUG is on, __devinit is defined to be nothing and
the function will remain because there is a possibility that someone will
call the function after the initialization phase.  If CONFIG_HOTPLUG is
off, the user is assuring us that no devices will be hot-added or
hot-removed, so there is no point in bloating the kernel text (albeit very
slightly) with functions that we *know* won't be called.

So I think it's probably a good idea to stick the __devinit on
register_node() and unregister_node(), otherwise we have no marker to know
which functions to remove for CONFIG_TINY.  Greg?

-Matt

Re: [RFC/PATCH] unregister_node() for hotplug use

[Greg KH]

On Tue, May 10, 2005 at 11:15:29AM -0700, Matthew Dobson wrote:
> 
> So I think it's probably a good idea to stick the __devinit on
> register_node() and unregister_node(), otherwise we have no marker to know
> which functions to remove for CONFIG_TINY.  Greg?

Like _anyone_ would have CONFIG_NUMA and CONFIG_TINY enabled at the same
time?  I don't think so...

I'll leave it as is for now.

thanks,

greg k-h

Re: [RFC/PATCH] unregister_node() for hotplug use

[Matthew Dobson]

Greg KH wrote:
> On Tue, May 10, 2005 at 11:15:29AM -0700, Matthew Dobson wrote:
> 
>>So I think it's probably a good idea to stick the __devinit on
>>register_node() and unregister_node(), otherwise we have no marker to know
>>which functions to remove for CONFIG_TINY.  Greg?
> 
> 
> Like _anyone_ would have CONFIG_NUMA and CONFIG_TINY enabled at the same
> time?  I don't think so...
> 
> I'll leave it as is for now.
> 
> thanks,
> 
> greg k-h

No, it seems unlikely that anyone would build with CONFIG_NUMA and
CONFIG_TINY both enabled.  But it is possible and reasonable to build with
CONFIG_NUMA=y and CONFIG_HOTPLUG=n, which is the case I was trying to speak
to.  If NUMA is on and HOTPLUG is off, then we're wasting kernel text
(granted, it's a very small amount of space) for the register_node() &
unregister_node() functions that we *know* will never be called after
initial bootup.  That's why I suggested marking both of those functions as
__devinit.  But it doesn't make a huge difference either way.

-Matt

Re: [RFC/PATCH] unregister_node() for hotplug use

[Greg KH]

On Tue, May 10, 2005 at 11:58:36AM -0700, Matthew Dobson wrote:
> Greg KH wrote:
> > On Tue, May 10, 2005 at 11:15:29AM -0700, Matthew Dobson wrote:
> > 
> >>So I think it's probably a good idea to stick the __devinit on
> >>register_node() and unregister_node(), otherwise we have no marker to know
> >>which functions to remove for CONFIG_TINY.  Greg?
> > 
> > 
> > Like _anyone_ would have CONFIG_NUMA and CONFIG_TINY enabled at the same
> > time?  I don't think so...
> > 
> > I'll leave it as is for now.
> 
> No, it seems unlikely that anyone would build with CONFIG_NUMA and
> CONFIG_TINY both enabled.  But it is possible and reasonable to build with
> CONFIG_NUMA=y and CONFIG_HOTPLUG=n, which is the case I was trying to speak
> to.  If NUMA is on and HOTPLUG is off, then we're wasting kernel text
> (granted, it's a very small amount of space) for the register_node() &
> unregister_node() functions that we *know* will never be called after
> initial bootup.  That's why I suggested marking both of those functions as
> __devinit.  But it doesn't make a huge difference either way.

I do not think this is an issue, and I want to move CONFIG_HOTPLUG to be
under CONFIG_TINY anyway, so you could only disable it if TINY is
enabled.  But that's a different email thread...

thanks,

greg k-h

Re: [RFC/PATCH] unregister_node() for hotplug use

[Matthew Dobson]

Greg KH wrote:
> On Tue, May 10, 2005 at 11:58:36AM -0700, Matthew Dobson wrote:
> 
>>Greg KH wrote:
>>
>>>On Tue, May 10, 2005 at 11:15:29AM -0700, Matthew Dobson wrote:
>>>
>>>
>>>>So I think it's probably a good idea to stick the __devinit on
>>>>register_node() and unregister_node(), otherwise we have no marker to know
>>>>which functions to remove for CONFIG_TINY.  Greg?
>>>
>>>
>>>Like _anyone_ would have CONFIG_NUMA and CONFIG_TINY enabled at the same
>>>time?  I don't think so...
>>>
>>>I'll leave it as is for now.
>>
>>No, it seems unlikely that anyone would build with CONFIG_NUMA and
>>CONFIG_TINY both enabled.  But it is possible and reasonable to build with
>>CONFIG_NUMA=y and CONFIG_HOTPLUG=n, which is the case I was trying to speak
>>to.  If NUMA is on and HOTPLUG is off, then we're wasting kernel text
>>(granted, it's a very small amount of space) for the register_node() &
>>unregister_node() functions that we *know* will never be called after
>>initial bootup.  That's why I suggested marking both of those functions as
>>__devinit.  But it doesn't make a huge difference either way.
> 
> 
> I do not think this is an issue, and I want to move CONFIG_HOTPLUG to be
> under CONFIG_TINY anyway, so you could only disable it if TINY is
> enabled.  But that's a different email thread...
> 
> thanks,
> 
> greg k-h

Fair enough.  We'll leave those functions alone...

-Matt