linux-next: build failure after merge of the final tree

linux-next: build failure after merge of the final tree

[Stephen Rothwell]

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

Hi all,

After merging the final tree, today's linux-next build (sparc32 defconfig)
failed like this:

lib/lib.a(show_mem.o): In function `show_mem':
show_mem.c:(.text+0x1f4): multiple definition of `show_mem'
arch/sparc/mm/built-in.o:(.text+0xd70): first defined here

This has been reported elsewhere, caused by commit ddd588b5dd55 ("oom:
suppress nodes that are not allowed from meminfo on oom kill").

I presume that this will be fixed soon, and so have left it as is.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

Re: linux-next: build failure after merge of the final tree

[Andrew Morton]

On Thu, 24 Mar 2011 14:30:19 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:

> Hi all,
> 
> After merging the final tree, today's linux-next build (sparc32 defconfig)
> failed like this:
> 
> lib/lib.a(show_mem.o): In function `show_mem':
> show_mem.c:(.text+0x1f4): multiple definition of `show_mem'
> arch/sparc/mm/built-in.o:(.text+0xd70): first defined here

I discovered this on ia64 today.

> This has been reported elsewhere, caused by commit ddd588b5dd55 ("oom:
> suppress nodes that are not allowed from meminfo on oom kill").
> 

Yeah, this is tricky.  The addition of the symbol __show_mem caused the
linker to newly take lib/show_mem.o out of lib/lib.a, whereas
previously that object file would have been omitted from the build
altogether.  Even though we compile it!

Making show_mem() __weak would "fix" it, but it's still wrong.  Given
that architectures can implement private versions of show_mem(), the
patch was wrong to call the generic version (renamed to __show_mem) at
all.

So...  a suitable fix would be to remove __show_mem and to add the
extra arg to show_mem() itself then fix up all callers and to fix up
all implementations to honour "filter".

Meanwhile, ddd588b5dd55f143203799 reverts cleanly and I'd suggest we do
that.

The dependence on linker .a handling is just gross and we should clean
that up.  CONFIG_ARCH_HAS_SHOW_MEM or something.

Re: linux-next: build failure after merge of the final tree

[Tony Luck]

On Wed, Mar 23, 2011 at 9:13 PM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> Meanwhile, ddd588b5dd55f143203799 reverts cleanly and I'd suggest we do
> that.

I tried that revert (from Linus' tree this morning HEAD
=6c5103890057b1bb781b26b7aae38d33e4c517d8)
and got:


  CC      mm/page_alloc.o
mm/page_alloc.c: In function ‘__alloc_pages_slowpath’:
mm/page_alloc.c:2180: error: ‘SHOW_MEM_FILTER_NODES’ undeclared (first
use in this function)
mm/page_alloc.c:2180: error: (Each undeclared identifier is reported only once
mm/page_alloc.c:2180: error: for each function it appears in.)
mm/page_alloc.c:2198: error: implicit declaration of function ‘__show_mem’
make[1]: *** [mm/page_alloc.o] Error 1
make: *** [mm] Error 2

-Tony

[patch] lib, arch: add filter argument to show_mem and fix private implementations

[David Rientjes]

On Wed, 23 Mar 2011, Andrew Morton wrote:

> So...  a suitable fix would be to remove __show_mem and to add the
> extra arg to show_mem() itself then fix up all callers and to fix up
> all implementations to honour "filter".
> 

I've done that below, I hope it's not too late to prevent the revert.  
Compile tested on arm, ia64, powerpc, and sparc32 defconfig.


lib, arch: add filter argument to show_mem and fix private implementations

ddd588b5dd55 (oom: suppress nodes that are not allowed from meminfo on oom 
kill) moved lib/show_mem.o out of lib/lib.a, which resulted in build 
warnings on all architectures that implement their own versions of 
show_mem():

	lib/lib.a(show_mem.o): In function `show_mem':
	show_mem.c:(.text+0x1f4): multiple definition of `show_mem'
	arch/sparc/mm/built-in.o:(.text+0xd70): first defined here

The fix is to remove __show_mem() and add its argument to show_mem() in 
all implementations to prevent this breakage.

Architectures that implement their own show_mem() actually don't do 
anything with the argument yet, but they could be made to filter nodes 
that aren't allowed in the current context in the future just like the 
generic implementation.

Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Reported-by: James Bottomley <James.Bottomley@hansenpartnership.com>
Suggested-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: David Rientjes <rientjes@google.com>
---
 arch/arm/mm/init.c        |    2 +-
 arch/ia64/mm/contig.c     |    2 +-
 arch/ia64/mm/discontig.c  |    2 +-
 arch/parisc/mm/init.c     |    2 +-
 arch/powerpc/xmon/xmon.c  |    2 +-
 arch/sparc/mm/init_32.c   |    2 +-
 arch/tile/mm/pgtable.c    |    2 +-
 arch/unicore32/mm/init.c  |    2 +-
 drivers/tty/sysrq.c       |    2 +-
 drivers/tty/vt/keyboard.c |    2 +-
 include/linux/mm.h        |    5 ++---
 lib/show_mem.c            |    7 +------
 mm/oom_kill.c             |    2 +-
 mm/page_alloc.c           |    2 +-
 14 files changed, 15 insertions(+), 21 deletions(-)

diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
--- a/arch/arm/mm/init.c
+++ b/arch/arm/mm/init.c
@@ -78,7 +78,7 @@ __tagtable(ATAG_INITRD2, parse_tag_initrd2);
  */
 struct meminfo meminfo;
 
-void show_mem(void)
+void show_mem(unsigned int filter)
 {
 	int free = 0, total = 0, reserved = 0;
 	int shared = 0, cached = 0, slab = 0, i;
diff --git a/arch/ia64/mm/contig.c b/arch/ia64/mm/contig.c
--- a/arch/ia64/mm/contig.c
+++ b/arch/ia64/mm/contig.c
@@ -36,7 +36,7 @@ static unsigned long max_gap;
  * Shows a simple page count of reserved and used pages in the system.
  * For discontig machines, it does this on a per-pgdat basis.
  */
-void show_mem(void)
+void show_mem(unsigned int filter)
 {
 	int i, total_reserved = 0;
 	int total_shared = 0, total_cached = 0;
diff --git a/arch/ia64/mm/discontig.c b/arch/ia64/mm/discontig.c
--- a/arch/ia64/mm/discontig.c
+++ b/arch/ia64/mm/discontig.c
@@ -614,7 +614,7 @@ void __cpuinit *per_cpu_init(void)
  * Shows a simple page count of reserved and used pages in the system.
  * For discontig machines, it does this on a per-pgdat basis.
  */
-void show_mem(void)
+void show_mem(unsigned int filter)
 {
 	int i, total_reserved = 0;
 	int total_shared = 0, total_cached = 0;
diff --git a/arch/parisc/mm/init.c b/arch/parisc/mm/init.c
--- a/arch/parisc/mm/init.c
+++ b/arch/parisc/mm/init.c
@@ -544,7 +544,7 @@ void __init mem_init(void)
 unsigned long *empty_zero_page __read_mostly;
 EXPORT_SYMBOL(empty_zero_page);
 
-void show_mem(void)
+void show_mem(unsigned int filter)
 {
 	int i,free = 0,total = 0,reserved = 0;
 	int shared = 0, cached = 0;
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -821,7 +821,7 @@ cmds(struct pt_regs *excp)
 				memzcan();
 				break;
 			case 'i':
-				show_mem();
+				show_mem(0);
 				break;
 			default:
 				termch = cmd;
diff --git a/arch/sparc/mm/init_32.c b/arch/sparc/mm/init_32.c
--- a/arch/sparc/mm/init_32.c
+++ b/arch/sparc/mm/init_32.c
@@ -75,7 +75,7 @@ void __init kmap_init(void)
 	kmap_prot = __pgprot(SRMMU_ET_PTE | SRMMU_PRIV | SRMMU_CACHE);
 }
 
-void show_mem(void)
+void show_mem(unsigned int filter)
 {
 	printk("Mem-info:\n");
 	show_free_areas();
diff --git a/arch/tile/mm/pgtable.c b/arch/tile/mm/pgtable.c
--- a/arch/tile/mm/pgtable.c
+++ b/arch/tile/mm/pgtable.c
@@ -41,7 +41,7 @@
  * The normal show_free_areas() is too verbose on Tile, with dozens
  * of processors and often four NUMA zones each with high and lowmem.
  */
-void show_mem(void)
+void show_mem(unsigned int filter)
 {
 	struct zone *zone;
 
diff --git a/arch/unicore32/mm/init.c b/arch/unicore32/mm/init.c
--- a/arch/unicore32/mm/init.c
+++ b/arch/unicore32/mm/init.c
@@ -55,7 +55,7 @@ early_param("initrd", early_initrd);
  */
 struct meminfo meminfo;
 
-void show_mem(void)
+void show_mem(unsigned int filter)
 {
 	int free = 0, total = 0, reserved = 0;
 	int shared = 0, cached = 0, slab = 0, i;
diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -306,7 +306,7 @@ static struct sysrq_key_op sysrq_ftrace_dump_op = {
 
 static void sysrq_handle_showmem(int key)
 {
-	show_mem();
+	show_mem(0);
 }
 static struct sysrq_key_op sysrq_showmem_op = {
 	.handler	= sysrq_handle_showmem,
diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c
--- a/drivers/tty/vt/keyboard.c
+++ b/drivers/tty/vt/keyboard.c
@@ -600,7 +600,7 @@ static void fn_scroll_back(struct vc_data *vc)
 
 static void fn_show_mem(struct vc_data *vc)
 {
-	show_mem();
+	show_mem(0);
 }
 
 static void fn_show_state(struct vc_data *vc)
diff --git a/include/linux/mm.h b/include/linux/mm.h
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -861,7 +861,7 @@ extern void pagefault_out_of_memory(void);
 #define offset_in_page(p)	((unsigned long)(p) & ~PAGE_MASK)
 
 /*
- * Flags passed to __show_mem() and __show_free_areas() to suppress output in
+ * Flags passed to show_mem() and __show_free_areas() to suppress output in
  * various contexts.
  */
 #define SHOW_MEM_FILTER_NODES	(0x0001u)	/* filter disallowed nodes */
@@ -1360,8 +1360,7 @@ extern void setup_per_zone_wmarks(void);
 extern void calculate_zone_inactive_ratio(struct zone *zone);
 extern void mem_init(void);
 extern void __init mmap_init(void);
-extern void show_mem(void);
-extern void __show_mem(unsigned int flags);
+extern void show_mem(unsigned int flags);
 extern void si_meminfo(struct sysinfo * val);
 extern void si_meminfo_node(struct sysinfo *val, int nid);
 extern int after_bootmem;
diff --git a/lib/show_mem.c b/lib/show_mem.c
--- a/lib/show_mem.c
+++ b/lib/show_mem.c
@@ -9,7 +9,7 @@
 #include <linux/nmi.h>
 #include <linux/quicklist.h>
 
-void __show_mem(unsigned int filter)
+void show_mem(unsigned int filter)
 {
 	pg_data_t *pgdat;
 	unsigned long total = 0, reserved = 0, shared = 0,
@@ -61,8 +61,3 @@ void __show_mem(unsigned int filter)
 		quicklist_total_size());
 #endif
 }
-
-void show_mem(void)
-{
-	__show_mem(0);
-}
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -406,7 +406,7 @@ static void dump_header(struct task_struct *p, gfp_t gfp_mask, int order,
 	task_unlock(current);
 	dump_stack();
 	mem_cgroup_print_oom_info(mem, p);
-	__show_mem(SHOW_MEM_FILTER_NODES);
+	show_mem(SHOW_MEM_FILTER_NODES);
 	if (sysctl_oom_dump_tasks)
 		dump_tasks(mem, nodemask);
 }
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -2195,7 +2195,7 @@ nopage:
 			current->comm, order, gfp_mask);
 		dump_stack();
 		if (!should_suppress_show_mem())
-			__show_mem(filter);
+			show_mem(filter);
 	}
 	return page;
 got_pg:

Re: [patch] lib, arch: add filter argument to show_mem and fix private implementations

[Tony Luck]

On Thu, Mar 24, 2011 at 3:18 PM, David Rientjes <rientjes@google.com> wrote:
> I've done that below, I hope it's not too late to prevent the revert.
> Compile tested on arm, ia64, powerpc, and sparc32 defconfig.

ia64 builds and boots again - thanks David.

-Tony

But now I see an occasional kernel unaligned access in "m_stop()" ...
cscope says we have four
different functions named "m_stop()" ... process running was mingetty