[PATCH -next] drivers/base/inode.c: let vmstat_text be optional

[PATCH -next] drivers/base/inode.c: let vmstat_text be optional

[Randy Dunlap]

From: Randy Dunlap <rdunlap@xenotime.net>

vmstat_text is only available when PROC_FS or SYSFS is enabled.
This causes build errors in drivers/base/node.c when they are
both disabled:

drivers/built-in.o: In function `node_read_vmstat':
node.c:(.text+0x10e28f): undefined reference to `vmstat_text'

Rather than litter drivers/base/node.c with #ifdef/#endif around
the affected lines of code, add macros for optional sysdev
attributes so that those lines of code will be ignored, without
using #ifdef/#endif in the .c file(s).  I.e., the ifdeffery
is done only in a header file with sysdev_create_file_optional()
and sysdev_remove_file_optional().

Signed-off-by: Randy Dunlap <rdunlap@xenotime.net>
---
 drivers/base/node.c    |    6 ++++--
 include/linux/sysdev.h |   14 ++++++++++++++
 include/linux/vmstat.h |    2 ++
 3 files changed, 20 insertions(+), 2 deletions(-)

--- linux-next-20110804.orig/include/linux/vmstat.h
+++ linux-next-20110804/include/linux/vmstat.h
@@ -258,6 +258,8 @@ static inline void refresh_zone_stat_thr
 
 #endif		/* CONFIG_SMP */
 
+#if defined(CONFIG_PROC_FS) || defined(CONFIG_SYSFS)
 extern const char * const vmstat_text[];
+#endif
 
 #endif /* _LINUX_VMSTAT_H */
--- linux-next-20110804.orig/drivers/base/node.c
+++ linux-next-20110804/drivers/base/node.c
@@ -176,6 +176,7 @@ static ssize_t node_read_numastat(struct
 }
 static SYSDEV_ATTR(numastat, S_IRUGO, node_read_numastat, NULL);
 
+#if defined(CONFIG_PROC_FS) || defined(CONFIG_SYSFS)
 static ssize_t node_read_vmstat(struct sys_device *dev,
 				struct sysdev_attribute *attr, char *buf)
 {
@@ -190,6 +191,7 @@ static ssize_t node_read_vmstat(struct s
 	return n;
 }
 static SYSDEV_ATTR(vmstat, S_IRUGO, node_read_vmstat, NULL);
+#endif
 
 static ssize_t node_read_distance(struct sys_device * dev,
 			struct sysdev_attribute *attr, char * buf)
@@ -274,7 +276,7 @@ int register_node(struct node *node, int
 		sysdev_create_file(&node->sysdev, &attr_meminfo);
 		sysdev_create_file(&node->sysdev, &attr_numastat);
 		sysdev_create_file(&node->sysdev, &attr_distance);
-		sysdev_create_file(&node->sysdev, &attr_vmstat);
+		sysdev_create_file_optional(&node->sysdev, &attr_vmstat);
 
 		scan_unevictable_register_node(node);
 
@@ -299,7 +301,7 @@ void unregister_node(struct node *node)
 	sysdev_remove_file(&node->sysdev, &attr_meminfo);
 	sysdev_remove_file(&node->sysdev, &attr_numastat);
 	sysdev_remove_file(&node->sysdev, &attr_distance);
-	sysdev_remove_file(&node->sysdev, &attr_vmstat);
+	sysdev_remove_file_optional(&node->sysdev, &attr_vmstat);
 
 	scan_unevictable_unregister_node(node);
 	hugetlb_unregister_node(node);		/* no-op, if memoryless node */
--- linux-next-20110804.orig/include/linux/sysdev.h
+++ linux-next-20110804/include/linux/sysdev.h
@@ -114,6 +114,20 @@ struct sysdev_attribute { 
 extern int sysdev_create_file(struct sys_device *, struct sysdev_attribute *);
 extern void sysdev_remove_file(struct sys_device *, struct sysdev_attribute *);
 
+#if defined(CONFIG_PROC_FS) || defined(CONFIG_SYSFS)
+#define sysdev_create_file_optional(sysdev, sysdevattr)	\
+	return sysdev_create_file(sysdev, sysdevattr);
+
+#define sysdev_remove_file_optional(sysdev, sysdevattr)	\
+		sysdev_remove_file(sysdev, sysdevattr);
+#else
+#define sysdev_create_file_optional(sysdev, sysdevattr)	\
+		(0)
+
+#define sysdev_remove_file_optional(sysdev, sysdevattr)	\
+		do {} while (0)
+#endif
+
 /* Create/remove NULL terminated attribute list */
 static inline int
 sysdev_create_files(struct sys_device *d, struct sysdev_attribute **a)

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH -next] drivers/base/inode.c: let vmstat_text be optional

[Cong Wang]

ao? 2011a1'08ae??05ae?JPY 06:22, Randy Dunlap a??e??:
> From: Randy Dunlap<rdunlap@xenotime.net>
>
> vmstat_text is only available when PROC_FS or SYSFS is enabled.
> This causes build errors in drivers/base/node.c when they are
> both disabled:
>
> drivers/built-in.o: In function `node_read_vmstat':
> node.c:(.text+0x10e28f): undefined reference to `vmstat_text'
>
> Rather than litter drivers/base/node.c with #ifdef/#endif around
> the affected lines of code, add macros for optional sysdev
> attributes so that those lines of code will be ignored, without
> using #ifdef/#endif in the .c file(s).  I.e., the ifdeffery
> is done only in a header file with sysdev_create_file_optional()
> and sysdev_remove_file_optional().
>

This looks ugly for me, because other sysfs files here are not optional
only due to that they don't rely on vmstat_text.

I still think my approach to fix this is better, that is, introducing
a new Kconfig for drivers/base/node.c which depends on CONFIG_SYSFS.

Thanks.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH -next] drivers/base/inode.c: let vmstat_text be optional

[Randy Dunlap]

On 08/04/11 19:38, Cong Wang wrote:
> ao? 2011a1'08ae??05ae?JPY 06:22, Randy Dunlap a??e??:
>> From: Randy Dunlap<rdunlap@xenotime.net>
>>
>> vmstat_text is only available when PROC_FS or SYSFS is enabled.
>> This causes build errors in drivers/base/node.c when they are
>> both disabled:
>>
>> drivers/built-in.o: In function `node_read_vmstat':
>> node.c:(.text+0x10e28f): undefined reference to `vmstat_text'
>>
>> Rather than litter drivers/base/node.c with #ifdef/#endif around
>> the affected lines of code, add macros for optional sysdev
>> attributes so that those lines of code will be ignored, without
>> using #ifdef/#endif in the .c file(s).  I.e., the ifdeffery
>> is done only in a header file with sysdev_create_file_optional()
>> and sysdev_remove_file_optional().
>>
> 
> This looks ugly for me, because other sysfs files here are not optional
> only due to that they don't rely on vmstat_text.
> 
> I still think my approach to fix this is better, that is, introducing
> a new Kconfig for drivers/base/node.c which depends on CONFIG_SYSFS.

Did you post a patch for that?  I must have missed it.

thanks,
-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH -next] drivers/base/inode.c: let vmstat_text be optional

[Cong Wang]

ao? 2011a1'08ae??05ae?JPY 13:43, Randy Dunlap a??e??:
> On 08/04/11 19:38, Cong Wang wrote:
>> ao? 2011a1'08ae??05ae?JPY 06:22, Randy Dunlap a??e??:
>>> From: Randy Dunlap<rdunlap@xenotime.net>
>>>
>>> vmstat_text is only available when PROC_FS or SYSFS is enabled.
>>> This causes build errors in drivers/base/node.c when they are
>>> both disabled:
>>>
>>> drivers/built-in.o: In function `node_read_vmstat':
>>> node.c:(.text+0x10e28f): undefined reference to `vmstat_text'
>>>
>>> Rather than litter drivers/base/node.c with #ifdef/#endif around
>>> the affected lines of code, add macros for optional sysdev
>>> attributes so that those lines of code will be ignored, without
>>> using #ifdef/#endif in the .c file(s).  I.e., the ifdeffery
>>> is done only in a header file with sysdev_create_file_optional()
>>> and sysdev_remove_file_optional().
>>>
>>
>> This looks ugly for me, because other sysfs files here are not optional
>> only due to that they don't rely on vmstat_text.
>>
>> I still think my approach to fix this is better, that is, introducing
>> a new Kconfig for drivers/base/node.c which depends on CONFIG_SYSFS.
>
> Did you post a patch for that?  I must have missed it.

Yes, see https://lkml.org/lkml/2011/7/19/31.

I think I need to resend it. :)

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH -next] drivers/base/inode.c: let vmstat_text be optional

[Andrew Morton]

> Subject: [PATCH -next] drivers/base/inode.c: let vmstat_text be optional

The patch-against-next thing always disturbs me.  It implies that the
patch is only needed in linux-next, but often that's wrong.  So I have
to go off and work out what kernel it really is applicable to.

And that's OK, it keeps me healthy.  But two minds are better than one,
and if the originator has already worked this out, they should state it
explicitly, please.

On Thu, 4 Aug 2011 15:22:11 -0700
Randy Dunlap <rdunlap@xenotime.net> wrote:

> From: Randy Dunlap <rdunlap@xenotime.net>
> 
> vmstat_text is only available when PROC_FS or SYSFS is enabled.
> This causes build errors in drivers/base/node.c when they are
> both disabled:
> 
> drivers/built-in.o: In function `node_read_vmstat':
> node.c:(.text+0x10e28f): undefined reference to `vmstat_text'
> 
> Rather than litter drivers/base/node.c with #ifdef/#endif around
> the affected lines of code, add macros for optional sysdev
> attributes so that those lines of code will be ignored, without
> using #ifdef/#endif in the .c file(s).  I.e., the ifdeffery
> is done only in a header file with sysdev_create_file_optional()
> and sysdev_remove_file_optional().
> 
> --- linux-next-20110804.orig/include/linux/vmstat.h
> +++ linux-next-20110804/include/linux/vmstat.h
> @@ -258,6 +258,8 @@ static inline void refresh_zone_stat_thr
>  
>  #endif		/* CONFIG_SMP */
>  
> +#if defined(CONFIG_PROC_FS) || defined(CONFIG_SYSFS)
>  extern const char * const vmstat_text[];
> +#endif
>

The ifdef around the declaration isn't needed, really.  If we remove
it then a build-time error becomes a link-time (or even moddep-time) error,
which is a bit of a pain.  But it's less painful than having ifdefs
around squillions of declarations.


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[Patch] numa: introduce CONFIG_NUMA_SYSFS for drivers/base/node.c

[Cong Wang]

[-- Attachment #1: Type: text/plain, Size: 77 bytes --]

Hi, Andrew,

Do you think my patch below is better?

Thanks!

------------->

[-- Attachment #2: numa-depends-on-sysfs.diff --]
[-- Type: text/plain, Size: 1334 bytes --]

Introduce a new Kconfig CONFIG_NUMA_SYSFS for drivers/base/node.c
which just provides sysfs interface, so that when we select
CONFIG_NUMA, we don't have to enable the sysfs interface too.

This by the way fixes a randconfig build error when NUMA && !SYSFS.

Signed-off-by: WANG Cong <amwang@redhat.com>

---
diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index 99a375a..e382338 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -10,7 +10,7 @@ obj-$(CONFIG_HAS_DMA)	+= dma-mapping.o
 obj-$(CONFIG_HAVE_GENERIC_DMA_COHERENT) += dma-coherent.o
 obj-$(CONFIG_ISA)	+= isa.o
 obj-$(CONFIG_FW_LOADER)	+= firmware_class.o
-obj-$(CONFIG_NUMA)	+= node.o
+obj-$(CONFIG_NUMA_SYSFS)	+= node.o
 obj-$(CONFIG_MEMORY_HOTPLUG_SPARSE) += memory.o
 obj-$(CONFIG_SMP)	+= topology.o
 ifeq ($(CONFIG_SYSFS),y)
diff --git a/mm/Kconfig b/mm/Kconfig
index f2f1ca1..77345e7 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -340,6 +340,16 @@ choice
 	  benefit.
 endchoice
 
+config NUMA_SYSFS
+	bool "Enable NUMA sysfs interface for user-space"
+	depends on NUMA
+	depends on SYSFS
+	default y
+	help
+	  This enables NUMA sysfs interface, /sys/devices/system/node/*
+	  files, for user-space tools, like numactl. If you have enabled
+	  NUMA, probably you also need this one.
+
 #
 # UP and nommu archs use km based percpu allocator
 #

Re: [Patch] numa: introduce CONFIG_NUMA_SYSFS for drivers/base/node.c

[Randy Dunlap]

On Wed, 24 Aug 2011 11:34:45 +0800 Cong Wang wrote:

> Hi, Andrew,
> 
> Do you think my patch below is better?

Hi,

This causes build errors for me because node.o is not being built:

arch/x86/built-in.o: In function `topology_init':
topology.c:(.init.text+0x3668): undefined reference to `register_one_node'
drivers/built-in.o: In function `unregister_cpu':
(.text+0x7aecc): undefined reference to `unregister_cpu_under_node'
drivers/built-in.o: In function `register_cpu':
(.cpuinit.text+0xc1): undefined reference to `register_cpu_under_node'


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [Patch] numa: introduce CONFIG_NUMA_SYSFS for drivers/base/node.c

[Cong Wang]

[-- Attachment #1: Type: text/plain, Size: 744 bytes --]

ao? 2011a1'08ae??25ae?JPY 10:14, Randy Dunlap a??e??:
> On Wed, 24 Aug 2011 11:34:45 +0800 Cong Wang wrote:
>
>> Hi, Andrew,
>>
>> Do you think my patch below is better?
>
> Hi,
>
> This causes build errors for me because node.o is not being built:
>
> arch/x86/built-in.o: In function `topology_init':
> topology.c:(.init.text+0x3668): undefined reference to `register_one_node'
> drivers/built-in.o: In function `unregister_cpu':
> (.text+0x7aecc): undefined reference to `unregister_cpu_under_node'
> drivers/built-in.o: In function `register_cpu':
> (.cpuinit.text+0xc1): undefined reference to `register_cpu_under_node'

Ah, this is because I missed the part in include/linux/node.h. :)

Below is the updated version.

Thanks for testing!

[-- Attachment #2: n.diff --]
[-- Type: text/plain, Size: 1586 bytes --]

diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index 99a375a..e382338 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -10,7 +10,7 @@ obj-$(CONFIG_HAS_DMA)	+= dma-mapping.o
 obj-$(CONFIG_HAVE_GENERIC_DMA_COHERENT) += dma-coherent.o
 obj-$(CONFIG_ISA)	+= isa.o
 obj-$(CONFIG_FW_LOADER)	+= firmware_class.o
-obj-$(CONFIG_NUMA)	+= node.o
+obj-$(CONFIG_NUMA_SYSFS)	+= node.o
 obj-$(CONFIG_MEMORY_HOTPLUG_SPARSE) += memory.o
 obj-$(CONFIG_SMP)	+= topology.o
 ifeq ($(CONFIG_SYSFS),y)
diff --git a/include/linux/node.h b/include/linux/node.h
index 92370e2..a0cc5f9 100644
--- a/include/linux/node.h
+++ b/include/linux/node.h
@@ -32,7 +32,7 @@ typedef  void (*node_registration_func_t)(struct node *);
 
 extern int register_node(struct node *, int, struct node *);
 extern void unregister_node(struct node *node);
-#ifdef CONFIG_NUMA
+#ifdef defined(CONFIG_NUMA) && defined(CONFIG_SYSFS)
 extern int register_one_node(int nid);
 extern void unregister_one_node(int nid);
 extern int register_cpu_under_node(unsigned int cpu, unsigned int nid);
diff --git a/mm/Kconfig b/mm/Kconfig
index f2f1ca1..77345e7 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -340,6 +340,16 @@ choice
 	  benefit.
 endchoice
 
+config NUMA_SYSFS
+	bool "Enable NUMA sysfs interface for user-space"
+	depends on NUMA
+	depends on SYSFS
+	default y
+	help
+	  This enables NUMA sysfs interface, /sys/devices/system/node/*
+	  files, for user-space tools, like numactl. If you have enabled
+	  NUMA, probably you also need this one.
+
 #
 # UP and nommu archs use km based percpu allocator
 #

Re: [Patch] numa: introduce CONFIG_NUMA_SYSFS for drivers/base/node.c

[Randy Dunlap]

On Thu, 25 Aug 2011 11:31:45 +0800 Cong Wang wrote:

> ao? 2011a1'08ae??25ae?JPY 10:14, Randy Dunlap a??e??:
> > On Wed, 24 Aug 2011 11:34:45 +0800 Cong Wang wrote:
> >
> >> Hi, Andrew,
> >>
> >> Do you think my patch below is better?
> >
> > Hi,
> >
> > This causes build errors for me because node.o is not being built:
> >
> > arch/x86/built-in.o: In function `topology_init':
> > topology.c:(.init.text+0x3668): undefined reference to `register_one_node'
> > drivers/built-in.o: In function `unregister_cpu':
> > (.text+0x7aecc): undefined reference to `unregister_cpu_under_node'
> > drivers/built-in.o: In function `register_cpu':
> > (.cpuinit.text+0xc1): undefined reference to `register_cpu_under_node'
> 
> Ah, this is because I missed the part in include/linux/node.h. :)
> 
> Below is the updated version.
> 
> Thanks for testing!

Yes, that works after changing #ifdef defined(...)
to #if defined(...)

Acked-by: Randy Dunlap <rdunlap@xenotime.net>


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [Patch] numa: introduce CONFIG_NUMA_SYSFS for drivers/base/node.c

[David Rientjes]

On Wed, 24 Aug 2011, Randy Dunlap wrote:

> > Below is the updated version.
> > 
> > Thanks for testing!
> 
> Yes, that works after changing #ifdef defined(...)
> to #if defined(...)
> 
> Acked-by: Randy Dunlap <rdunlap@xenotime.net>
> 

No, it doesn't work, CONFIG_HUGETLBFS can be enabled with CONFIG_NUMA=y 
and CONFIG_SYSFS=n and that uses data structures from drivers/base/node.c 
which doesn't get compiled with this patch.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[patch] numa: fix NUMA compile error when sysfs and procfs are disabled

[David Rientjes]

On Thu, 25 Aug 2011, Cong Wang wrote:

> Ah, this is because I missed the part in include/linux/node.h. :)
> 
> Below is the updated version.
> 

I've never had a problem building a kernel with CONFIG_NUMA=y and 
CONFIG_SYSFS=n since most of drivers/base/node.c is just an abstraction 
that calls into sysfs functions that will be no-ops in such a 
configuration.

The error you cite in a different thread 
(http://marc.info/?l=linux-mm&m=131098795024186) about an undefined 
reference to vmstat_text is because you have CONFIG_NUMA enabled and both 
CONFIG_SYSFS and CONFIG_PROC_FS disabled and we only define vmstat_text 
for those fs configurations since that's the only way these strings were 
ever emitted before per-node vmstat.

The correct fix is to define the array for CONFIG_NUMA as well.



numa: fix NUMA compile error when sysfs and procfs are disabled

The vmstat_text array is only defined for CONFIG_SYSFS or CONFIG_PROC_FS, 
yet it is referenced for per-node vmstat with CONFIG_NUMA:

	drivers/built-in.o: In function `node_read_vmstat':
	node.c:(.text+0x1106df): undefined reference to `vmstat_text'

in fa25c503dfa2 (mm: per-node vmstat: show proper vmstats).

Define the array for CONFIG_NUMA as well.

Reported-by: Cong Wang <amwang@redhat.com>
Signed-off-by: David Rientjes <rientjes@google.com>
---
 include/linux/vmstat.h |    2 ++
 mm/vmstat.c            |    4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/linux/vmstat.h b/include/linux/vmstat.h
--- a/include/linux/vmstat.h
+++ b/include/linux/vmstat.h
@@ -258,6 +258,8 @@ static inline void refresh_zone_stat_thresholds(void) { }
 
 #endif		/* CONFIG_SMP */
 
+#if defined(CONFIG_PROC_FS) || defined(CONFIG_SYSFS) || defined(CONFIG_NUMA)
 extern const char * const vmstat_text[];
+#endif
 
 #endif /* _LINUX_VMSTAT_H */
diff --git a/mm/vmstat.c b/mm/vmstat.c
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -659,7 +659,7 @@ static void walk_zones_in_node(struct seq_file *m, pg_data_t *pgdat,
 }
 #endif
 
-#if defined(CONFIG_PROC_FS) || defined(CONFIG_SYSFS)
+#if defined(CONFIG_PROC_FS) || defined(CONFIG_SYSFS) || defined(CONFIG_NUMA)
 #ifdef CONFIG_ZONE_DMA
 #define TEXT_FOR_DMA(xx) xx "_dma",
 #else
@@ -788,7 +788,7 @@ const char * const vmstat_text[] = {
 
 #endif /* CONFIG_VM_EVENTS_COUNTERS */
 };
-#endif /* CONFIG_PROC_FS || CONFIG_SYSFS */
+#endif /* CONFIG_PROC_FS || CONFIG_SYSFS || CONFIG_NUMA */
 
 
 #ifdef CONFIG_PROC_FS

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [Patch] numa: introduce CONFIG_NUMA_SYSFS for drivers/base/node.c

[Cong Wang]

ao? 2011a1'08ae??25ae?JPY 13:04, David Rientjes a??e??:
> On Wed, 24 Aug 2011, Randy Dunlap wrote:
>
>>> Below is the updated version.
>>>
>>> Thanks for testing!
>>
>> Yes, that works after changing #ifdef defined(...)
>> to #if defined(...)
>>
>> Acked-by: Randy Dunlap<rdunlap@xenotime.net>
>>
>
> No, it doesn't work, CONFIG_HUGETLBFS can be enabled with CONFIG_NUMA=y
> and CONFIG_SYSFS=n and that uses data structures from drivers/base/node.c
> which doesn't get compiled with this patch.

So, you mean with CONFIG_NUMA=y && CONFIG_SYSFS=n && CONFIG_HUGETLBFS=y
we still get compile error?

Which data structures are used by hugetlbfs?

Thanks.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch] numa: fix NUMA compile error when sysfs and procfs are disabled

[Randy Dunlap]

On Wed, 24 Aug 2011 22:55:02 -0700 (PDT) David Rientjes wrote:

> On Thu, 25 Aug 2011, Cong Wang wrote:
> 
> > Ah, this is because I missed the part in include/linux/node.h. :)
> > 
> > Below is the updated version.
> > 
> 
> I've never had a problem building a kernel with CONFIG_NUMA=y and 
> CONFIG_SYSFS=n since most of drivers/base/node.c is just an abstraction 
> that calls into sysfs functions that will be no-ops in such a 
> configuration.
> 
> The error you cite in a different thread 
> (http://marc.info/?l=linux-mm&m=131098795024186) about an undefined 
> reference to vmstat_text is because you have CONFIG_NUMA enabled and both 
> CONFIG_SYSFS and CONFIG_PROC_FS disabled and we only define vmstat_text 
> for those fs configurations since that's the only way these strings were 
> ever emitted before per-node vmstat.
> 
> The correct fix is to define the array for CONFIG_NUMA as well.
> 
> 
> 
> numa: fix NUMA compile error when sysfs and procfs are disabled
> 
> The vmstat_text array is only defined for CONFIG_SYSFS or CONFIG_PROC_FS, 
> yet it is referenced for per-node vmstat with CONFIG_NUMA:
> 
> 	drivers/built-in.o: In function `node_read_vmstat':
> 	node.c:(.text+0x1106df): undefined reference to `vmstat_text'
> 
> in fa25c503dfa2 (mm: per-node vmstat: show proper vmstats).
> 
> Define the array for CONFIG_NUMA as well.
> 
> Reported-by: Cong Wang <amwang@redhat.com>
> Signed-off-by: David Rientjes <rientjes@google.com>

Sure, that also works.
Acked-by: Randy Dunlap <rdunlap@xenotime.net>


> ---
>  include/linux/vmstat.h |    2 ++
>  mm/vmstat.c            |    4 ++--
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/vmstat.h b/include/linux/vmstat.h
> --- a/include/linux/vmstat.h
> +++ b/include/linux/vmstat.h
> @@ -258,6 +258,8 @@ static inline void refresh_zone_stat_thresholds(void) { }
>  
>  #endif		/* CONFIG_SMP */
>  
> +#if defined(CONFIG_PROC_FS) || defined(CONFIG_SYSFS) || defined(CONFIG_NUMA)
>  extern const char * const vmstat_text[];
> +#endif
>  
>  #endif /* _LINUX_VMSTAT_H */
> diff --git a/mm/vmstat.c b/mm/vmstat.c
> --- a/mm/vmstat.c
> +++ b/mm/vmstat.c
> @@ -659,7 +659,7 @@ static void walk_zones_in_node(struct seq_file *m, pg_data_t *pgdat,
>  }
>  #endif
>  
> -#if defined(CONFIG_PROC_FS) || defined(CONFIG_SYSFS)
> +#if defined(CONFIG_PROC_FS) || defined(CONFIG_SYSFS) || defined(CONFIG_NUMA)
>  #ifdef CONFIG_ZONE_DMA
>  #define TEXT_FOR_DMA(xx) xx "_dma",
>  #else
> @@ -788,7 +788,7 @@ const char * const vmstat_text[] = {
>  
>  #endif /* CONFIG_VM_EVENTS_COUNTERS */
>  };
> -#endif /* CONFIG_PROC_FS || CONFIG_SYSFS */
> +#endif /* CONFIG_PROC_FS || CONFIG_SYSFS || CONFIG_NUMA */
>  
>  
>  #ifdef CONFIG_PROC_FS


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [Patch] numa: introduce CONFIG_NUMA_SYSFS for drivers/base/node.c

[David Rientjes]

On Thu, 25 Aug 2011, Cong Wang wrote:

> > No, it doesn't work, CONFIG_HUGETLBFS can be enabled with CONFIG_NUMA=y
> > and CONFIG_SYSFS=n and that uses data structures from drivers/base/node.c
> > which doesn't get compiled with this patch.
> 
> So, you mean with CONFIG_NUMA=y && CONFIG_SYSFS=n && CONFIG_HUGETLBFS=y
> we still get compile error?
> 
> Which data structures are used by hugetlbfs?
> 

node_states[], which is revealed at link time if you tried to compile your 
patch.  It's obvious that we don't want to add per-node hugetlbfs 
attributes to sysfs directories if sysfs is disabled, so you need to 
modify the hugetlbfs code as well.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [Patch] numa: introduce CONFIG_NUMA_SYSFS for drivers/base/node.c

[Cong Wang]

ao? 2011a1'08ae??26ae?JPY 04:57, David Rientjes a??e??:
> On Thu, 25 Aug 2011, Cong Wang wrote:
>
>>> No, it doesn't work, CONFIG_HUGETLBFS can be enabled with CONFIG_NUMA=y
>>> and CONFIG_SYSFS=n and that uses data structures from drivers/base/node.c
>>> which doesn't get compiled with this patch.
>>
>> So, you mean with CONFIG_NUMA=y&&  CONFIG_SYSFS=n&&  CONFIG_HUGETLBFS=y
>> we still get compile error?
>>
>> Which data structures are used by hugetlbfs?
>>
>
> node_states[], which is revealed at link time if you tried to compile your
> patch.  It's obvious that we don't want to add per-node hugetlbfs
> attributes to sysfs directories if sysfs is disabled, so you need to
> modify the hugetlbfs code as well.

Makes sense. Thanks!

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>