LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* sparc compile error
@ 2008-02-07 23:12 Adrian Bunk
  2008-02-07 23:38 ` David Rientjes
  0 siblings, 1 reply; 10+ messages in thread
From: Adrian Bunk @ 2008-02-07 23:12 UTC (permalink / raw)
  To: David Rientjes, wli; +Cc: linux-kernel, sparclinux

Commit 3062fc67dad01b1d2a15d58c709eff946389eca4 broke sparc:

<--  snip  -->

...
  CC      init/do_mounts.o
In file included from include/linux/mm.h:39,
                 from include/linux/memcontrol.h:24,
                 from include/linux/swap.h:8,
                 from include/linux/suspend.h:7,
                 from init/do_mounts.c:6:
include/asm/pgtable.h:344: warning: parameter names (without types) in function declaration
include/asm/pgtable.h:345: warning: parameter names (without types) in function declaration
include/asm/pgtable.h:346: error: expected '=', ',', ';', 'asm' or '__attribute__' before '___f___swp_entry'
make[1]: *** [init/do_mounts.o] Error 1

<--  snip  -->

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: sparc compile error
  2008-02-07 23:12 sparc compile error Adrian Bunk
@ 2008-02-07 23:38 ` David Rientjes
  2008-02-10 15:33   ` Martin Habets
  0 siblings, 1 reply; 10+ messages in thread
From: David Rientjes @ 2008-02-07 23:38 UTC (permalink / raw)
  To: Adrian Bunk; +Cc: wli, linux-kernel, sparclinux

3062fc67 introduced far too many build errors for the convenience it
allows.  It's not even always necessary to rcu deference mm->mem_cgroup in
the first place, so we'll use it on a case by case basis.

Signed-off-by: David Rientjes <rientjes@google.com>
---
 include/linux/memcontrol.h |   13 -------------
 mm/memcontrol.c            |    2 +-
 mm/rmap.c                  |    6 ++++--
 3 files changed, 5 insertions(+), 16 deletions(-)

diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -20,9 +20,6 @@
 #ifndef _LINUX_MEMCONTROL_H
 #define _LINUX_MEMCONTROL_H
 
-#include <linux/rcupdate.h>
-#include <linux/mm.h>
-
 struct mem_cgroup;
 struct page_cgroup;
 struct page;
@@ -51,11 +48,6 @@ extern int mem_cgroup_cache_charge(struct page *page, struct mm_struct *mm,
 					gfp_t gfp_mask);
 int task_in_mem_cgroup(struct task_struct *task, const struct mem_cgroup *mem);
 
-static inline struct mem_cgroup *mm_cgroup(const struct mm_struct *mm)
-{
-	return rcu_dereference(mm->mem_cgroup);
-}
-
 extern int mem_cgroup_prepare_migration(struct page *page);
 extern void mem_cgroup_end_migration(struct page *page);
 extern void mem_cgroup_page_migration(struct page *page, struct page *newpage);
@@ -123,11 +115,6 @@ static inline int mem_cgroup_cache_charge(struct page *page,
 	return 0;
 }
 
-static inline struct mem_cgroup *mm_cgroup(const struct mm_struct *mm)
-{
-	return NULL;
-}
-
 static inline int task_in_mem_cgroup(struct task_struct *task,
 				     const struct mem_cgroup *mem)
 {
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -399,7 +399,7 @@ int task_in_mem_cgroup(struct task_struct *task, const struct mem_cgroup *mem)
 	int ret;
 
 	task_lock(task);
-	ret = task->mm && mm_cgroup(task->mm) == mem;
+	ret = task->mm && rcu_dereference(task->mm->mem_cgroup) == mem;
 	task_unlock(task);
 	return ret;
 }
diff --git a/mm/rmap.c b/mm/rmap.c
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -321,7 +321,8 @@ static int page_referenced_anon(struct page *page,
 		 * counting on behalf of references from different
 		 * cgroups
 		 */
-		if (mem_cont && (mm_cgroup(vma->vm_mm) != mem_cont))
+		if (mem_cont && mem_cont !=
+		    rcu_dereference(vma->vm_mm->mem_cgroup))
 			continue;
 		referenced += page_referenced_one(page, vma, &mapcount);
 		if (!mapcount)
@@ -382,7 +383,8 @@ static int page_referenced_file(struct page *page,
 		 * counting on behalf of references from different
 		 * cgroups
 		 */
-		if (mem_cont && (mm_cgroup(vma->vm_mm) != mem_cont))
+		if (mem_cont && mem_cont !=
+		    rcu_dereference(vma->vm_mm->mem_cgroup))
 			continue;
 		if ((vma->vm_flags & (VM_LOCKED|VM_MAYSHARE))
 				  == (VM_LOCKED|VM_MAYSHARE)) {

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: sparc compile error
  2008-02-07 23:38 ` David Rientjes
@ 2008-02-10 15:33   ` Martin Habets
  2008-02-10 19:19     ` David Rientjes
  0 siblings, 1 reply; 10+ messages in thread
From: Martin Habets @ 2008-02-10 15:33 UTC (permalink / raw)
  To: David Rientjes; +Cc: Adrian Bunk, wli, linux-kernel, sparclinux

This still gives build errors with CGROUP_MEM_CONT off.
Some ifdef-ing will fix that.

 Martin

	 Signed-off-by: Martin Habets <errandir_news@mph.eclipse.co.uk>

Index: sparc-2.6.git/mm/rmap.c
===================================================================
--- sparc-2.6.git.orig/mm/rmap.c	2008-02-10 13:01:37.000000000 +0000
+++ sparc-2.6.git/mm/rmap.c	2008-02-10 14:16:34.000000000 +0000
@@ -321,8 +321,11 @@
 		 * counting on behalf of references from different
 		 * cgroups
 		 */
-		if (mem_cont && (mm_cgroup(vma->vm_mm) != mem_cont))
+#ifdef CONFIG_CGROUP_MEM_CONT
+		if (mem_cont && mem_cont !=
+		    rcu_dereference(vma->vm_mm->mem_cgroup))
 			continue;
+#endif
 		referenced += page_referenced_one(page, vma, &mapcount);
 		if (!mapcount)
 			break;
@@ -382,8 +385,11 @@
 		 * counting on behalf of references from different
 		 * cgroups
 		 */
-		if (mem_cont && (mm_cgroup(vma->vm_mm) != mem_cont))
+#ifdef CONFIG_CGROUP_MEM_CONT
+		if (mem_cont && mem_cont !=
+		    rcu_dereference(vma->vm_mm->mem_cgroup))
 			continue;
+#endif
 		if ((vma->vm_flags & (VM_LOCKED|VM_MAYSHARE))
 				  == (VM_LOCKED|VM_MAYSHARE)) {
 			referenced++;

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: sparc compile error
  2008-02-10 15:33   ` Martin Habets
@ 2008-02-10 19:19     ` David Rientjes
  2008-02-10 21:44       ` Robert Reif
  0 siblings, 1 reply; 10+ messages in thread
From: David Rientjes @ 2008-02-10 19:19 UTC (permalink / raw)
  To: Martin Habets; +Cc: Adrian Bunk, wli, linux-kernel, sparclinux

On Sun, 10 Feb 2008, Martin Habets wrote:

> This still gives build errors with CGROUP_MEM_CONT off.
> Some ifdef-ing will fix that.
> 

Please try Linus' latest git that includes 60c12b12 (from yesterday) that 
fixes this issue.

		David

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: sparc compile error
  2008-02-10 19:19     ` David Rientjes
@ 2008-02-10 21:44       ` Robert Reif
  2008-02-10 23:30         ` [patch] sparc: fix build David Rientjes
  0 siblings, 1 reply; 10+ messages in thread
From: Robert Reif @ 2008-02-10 21:44 UTC (permalink / raw)
  To: David Rientjes; +Cc: Martin Habets, Adrian Bunk, wli, linux-kernel, sparclinux

David Rientjes wrote:

>Please try Linus' latest git that includes 60c12b12 (from yesterday) that 
>fixes this issue.
>
>  
>
I get this with latest git:

  CC      init/do_mounts.o
In file included from include/linux/mm.h:39,
                 from include/linux/memcontrol.h:24,
                 from include/linux/swap.h:8,
                 from include/linux/suspend.h:7,
                 from init/do_mounts.c:6:
include/asm/pgtable.h:344: warning: parameter names (without types) in 
function declaration
include/asm/pgtable.h:345: warning: parameter names (without types) in 
function declaration
include/asm/pgtable.h:346: error: expected '=', ',', ';', 'asm' or 
'__attribute__' before '___f___swp_entry'
make[1]: *** [init/do_mounts.o] Error 1



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [patch] sparc: fix build
  2008-02-10 21:44       ` Robert Reif
@ 2008-02-10 23:30         ` David Rientjes
  2008-02-11  9:37           ` Balbir Singh
  2008-02-13  2:46           ` Andrew Morton
  0 siblings, 2 replies; 10+ messages in thread
From: David Rientjes @ 2008-02-10 23:30 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Adrian Bunk, linux-kernel, sparclinux, Robert Reif

Fix build failures on sparc:

	In file included from include/linux/mm.h:39,
	                from include/linux/memcontrol.h:24,
	                from include/linux/swap.h:8,
	                from include/linux/suspend.h:7,
	                from init/do_mounts.c:6:
	include/asm/pgtable.h:344: warning: parameter names (without
		types) in function declaration
	include/asm/pgtable.h:345: warning: parameter names (without
		types) in function declaration
	include/asm/pgtable.h:346: error: expected '=', ',', ';', 'asm' or
		'__attribute__' before '___f___swp_entry'

and

	arch/sparc/kernel/led.c: In function 'led_blink':
	arch/sparc/kernel/led.c:35: error: invalid use of undefined type
		'struct timer_list'
	arch/sparc/kernel/led.c:35: error: 'jiffies' undeclared (first use
		in this function)
	arch/sparc/kernel/led.c:35: error: (Each undeclared identifier is
		reported only once
	arch/sparc/kernel/led.c:35: error: for each function it appears
		in.)
	arch/sparc/kernel/led.c:36: error: 'avenrun' undeclared (first use
		in this function)
	arch/sparc/kernel/led.c:36: error: 'FSHIFT' undeclared (first use
		in this function)
	arch/sparc/kernel/led.c:36: error: 'HZ' undeclared (first use in
		this function)
	arch/sparc/kernel/led.c:37: error: invalid use of undefined type
		'struct timer_list'
	arch/sparc/kernel/led.c:39: error: invalid use of undefined type
		'struct timer_list'
	arch/sparc/kernel/led.c:40: error: invalid use of undefined type
		'struct timer_list'
	arch/sparc/kernel/led.c:42: error: implicit declaration of
		function 'add_timer'
	arch/sparc/kernel/led.c: In function 'led_write_proc':
	arch/sparc/kernel/led.c:70: error: implicit declaration of
		function 'copy_from_user'
	arch/sparc/kernel/led.c:84: error: implicit declaration of
		function 'del_timer_sync'
	arch/sparc/kernel/led.c: In function 'led_init':
	arch/sparc/kernel/led.c:109: error: implicit declaration of
		function 'init_timer'
	arch/sparc/kernel/led.c:110: error: invalid use of undefined type
		'struct timer_list'

Cc: Adrian Bunk <bunk@kernel.org>
Cc: Robert Reif <reif@earthlink.net>
Signed-off-by: David Rientjes <rientjes@google.com>
---
 arch/sparc/kernel/led.c    |    3 ++-
 include/linux/memcontrol.h |    3 ---
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/sparc/kernel/led.c b/arch/sparc/kernel/led.c
--- a/arch/sparc/kernel/led.c
+++ b/arch/sparc/kernel/led.c
@@ -1,8 +1,9 @@
-#include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/proc_fs.h>
 #include <linux/string.h>
+#include <linux/sched.h>
+#include <linux/uaccess.h>
 
 #include <asm/auxio.h>
 
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -20,9 +20,6 @@
 #ifndef _LINUX_MEMCONTROL_H
 #define _LINUX_MEMCONTROL_H
 
-#include <linux/rcupdate.h>
-#include <linux/mm.h>
-
 struct mem_cgroup;
 struct page_cgroup;
 struct page;

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [patch] sparc: fix build
  2008-02-10 23:30         ` [patch] sparc: fix build David Rientjes
@ 2008-02-11  9:37           ` Balbir Singh
  2008-02-13  2:46           ` Andrew Morton
  1 sibling, 0 replies; 10+ messages in thread
From: Balbir Singh @ 2008-02-11  9:37 UTC (permalink / raw)
  To: David Rientjes
  Cc: Andrew Morton, Adrian Bunk, linux-kernel, sparclinux, Robert Reif

David Rientjes wrote:
> Fix build failures on sparc:
> 
> 	In file included from include/linux/mm.h:39,
> 	                from include/linux/memcontrol.h:24,
> 	                from include/linux/swap.h:8,
> 	                from include/linux/suspend.h:7,
> 	                from init/do_mounts.c:6:
> 	include/asm/pgtable.h:344: warning: parameter names (without
> 		types) in function declaration
> 	include/asm/pgtable.h:345: warning: parameter names (without
> 		types) in function declaration
> 	include/asm/pgtable.h:346: error: expected '=', ',', ';', 'asm' or
> 		'__attribute__' before '___f___swp_entry'
> 
> and
> 
> 	arch/sparc/kernel/led.c: In function 'led_blink':
> 	arch/sparc/kernel/led.c:35: error: invalid use of undefined type
> 		'struct timer_list'
> 	arch/sparc/kernel/led.c:35: error: 'jiffies' undeclared (first use
> 		in this function)
> 	arch/sparc/kernel/led.c:35: error: (Each undeclared identifier is
> 		reported only once
> 	arch/sparc/kernel/led.c:35: error: for each function it appears
> 		in.)
> 	arch/sparc/kernel/led.c:36: error: 'avenrun' undeclared (first use
> 		in this function)
> 	arch/sparc/kernel/led.c:36: error: 'FSHIFT' undeclared (first use
> 		in this function)
> 	arch/sparc/kernel/led.c:36: error: 'HZ' undeclared (first use in
> 		this function)
> 	arch/sparc/kernel/led.c:37: error: invalid use of undefined type
> 		'struct timer_list'
> 	arch/sparc/kernel/led.c:39: error: invalid use of undefined type
> 		'struct timer_list'
> 	arch/sparc/kernel/led.c:40: error: invalid use of undefined type
> 		'struct timer_list'
> 	arch/sparc/kernel/led.c:42: error: implicit declaration of
> 		function 'add_timer'
> 	arch/sparc/kernel/led.c: In function 'led_write_proc':
> 	arch/sparc/kernel/led.c:70: error: implicit declaration of
> 		function 'copy_from_user'
> 	arch/sparc/kernel/led.c:84: error: implicit declaration of
> 		function 'del_timer_sync'
> 	arch/sparc/kernel/led.c: In function 'led_init':
> 	arch/sparc/kernel/led.c:109: error: implicit declaration of
> 		function 'init_timer'
> 	arch/sparc/kernel/led.c:110: error: invalid use of undefined type
> 		'struct timer_list'
> 
> Cc: Adrian Bunk <bunk@kernel.org>
> Cc: Robert Reif <reif@earthlink.net>
> Signed-off-by: David Rientjes <rientjes@google.com>
> ---
>  arch/sparc/kernel/led.c    |    3 ++-
>  include/linux/memcontrol.h |    3 ---
>  2 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/sparc/kernel/led.c b/arch/sparc/kernel/led.c
> --- a/arch/sparc/kernel/led.c
> +++ b/arch/sparc/kernel/led.c
> @@ -1,8 +1,9 @@
> -#include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/init.h>
>  #include <linux/proc_fs.h>
>  #include <linux/string.h>
> +#include <linux/sched.h>
> +#include <linux/uaccess.h>
> 
>  #include <asm/auxio.h>
> 
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -20,9 +20,6 @@
>  #ifndef _LINUX_MEMCONTROL_H
>  #define _LINUX_MEMCONTROL_H
> 
> -#include <linux/rcupdate.h>
> -#include <linux/mm.h>
> -
>  struct mem_cgroup;
>  struct page_cgroup;
>  struct page;

This patch allowed my sparc cross compiler to build beyond mm/memcontrol.o, but
I faced a build issue at

drivers/net/e1000e/netdev.c: In function 'e1000e_update_stats':
drivers/net/e1000e/netdev.c:2583: error: unable to find a register to spill in
class 'FP_REGS'
drivers/net/e1000e/netdev.c:2583: error: this is the insn:
(insn 66 2379 2236 2 drivers/net/e1000e/netdev.c:2459 (set (reg:DI 487 [ D.34076 ])
        (mem/s/j:DI (plus:SI (reg/v/f:SI 11 %o3 [orig:492 adapter ] [492])
                (reg:SI 7 %g7 [514])) [0 <variable>.stats.gprc+0 S8 A128])) 45
{*movdi_insn_sp32} (insn_list:REG_DEP_TRUE 65 (insn_list:REG_DEP_ANTI 32 (nil)))
    (expr_list:REG_EQUIV (mem/s/j:DI (plus:SI (reg/v/f:SI 11 %o3 [orig:492
adapter ] [492])
                (reg:SI 7 %g7 [514])) [0 <variable>.stats.gprc+0 S8 A128])
        (nil)))
drivers/net/e1000e/netdev.c:2583: confused by earlier errors, bailing out
make[3]: *** [drivers/net/e1000e/netdev.o] Error 1
make[2]: *** [drivers/net/e1000e] Error 2
make[1]: *** [drivers/net] Error 2

So for the problem reported

Tested-by: Balbir Singh <balbir@linux.vnet.ibm.com>

-- 
	Warm Regards,
	Balbir Singh
	Linux Technology Center
	IBM, ISTL

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [patch] sparc: fix build
  2008-02-10 23:30         ` [patch] sparc: fix build David Rientjes
  2008-02-11  9:37           ` Balbir Singh
@ 2008-02-13  2:46           ` Andrew Morton
  2008-02-13  2:57             ` Al Viro
  1 sibling, 1 reply; 10+ messages in thread
From: Andrew Morton @ 2008-02-13  2:46 UTC (permalink / raw)
  To: David Rientjes; +Cc: Adrian Bunk, linux-kernel, sparclinux, Robert Reif

On Sun, 10 Feb 2008 15:30:51 -0800 (PST) David Rientjes <rientjes@google.com> wrote:

> Fix build failures on sparc:
> 
> 	In file included from include/linux/mm.h:39,
> 	                from include/linux/memcontrol.h:24,
> 	                from include/linux/swap.h:8,
> 	                from include/linux/suspend.h:7,
> 	                from init/do_mounts.c:6:
> 	include/asm/pgtable.h:344: warning: parameter names (without
> 		types) in function declaration
> 	include/asm/pgtable.h:345: warning: parameter names (without
> 		types) in function declaration
> 	include/asm/pgtable.h:346: error: expected '=', ',', ';', 'asm' or
> 		'__attribute__' before '___f___swp_entry'
> 
> and
> 
> 	arch/sparc/kernel/led.c: In function 'led_blink':
> 	arch/sparc/kernel/led.c:35: error: invalid use of undefined type
> 		'struct timer_list'
> 	arch/sparc/kernel/led.c:35: error: 'jiffies' undeclared (first use
> 		in this function)
> 	arch/sparc/kernel/led.c:35: error: (Each undeclared identifier is
> 		reported only once
> 	arch/sparc/kernel/led.c:35: error: for each function it appears
> 		in.)
> 	arch/sparc/kernel/led.c:36: error: 'avenrun' undeclared (first use
> 		in this function)
> 	arch/sparc/kernel/led.c:36: error: 'FSHIFT' undeclared (first use
> 		in this function)
> 	arch/sparc/kernel/led.c:36: error: 'HZ' undeclared (first use in
> 		this function)
> 	arch/sparc/kernel/led.c:37: error: invalid use of undefined type
> 		'struct timer_list'
> 	arch/sparc/kernel/led.c:39: error: invalid use of undefined type
> 		'struct timer_list'
> 	arch/sparc/kernel/led.c:40: error: invalid use of undefined type
> 		'struct timer_list'
> 	arch/sparc/kernel/led.c:42: error: implicit declaration of
> 		function 'add_timer'
> 	arch/sparc/kernel/led.c: In function 'led_write_proc':
> 	arch/sparc/kernel/led.c:70: error: implicit declaration of
> 		function 'copy_from_user'
> 	arch/sparc/kernel/led.c:84: error: implicit declaration of
> 		function 'del_timer_sync'
> 	arch/sparc/kernel/led.c: In function 'led_init':
> 	arch/sparc/kernel/led.c:109: error: implicit declaration of
> 		function 'init_timer'
> 	arch/sparc/kernel/led.c:110: error: invalid use of undefined type
> 		'struct timer_list'
> 
> ...
>
> diff --git a/arch/sparc/kernel/led.c b/arch/sparc/kernel/led.c
> --- a/arch/sparc/kernel/led.c
> +++ b/arch/sparc/kernel/led.c
> @@ -1,8 +1,9 @@
> -#include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/init.h>
>  #include <linux/proc_fs.h>
>  #include <linux/string.h>
> +#include <linux/sched.h>
> +#include <linux/uaccess.h>
>  
>  #include <asm/auxio.h>

OK, that'll fix one error.
  
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -20,9 +20,6 @@
>  #ifndef _LINUX_MEMCONTROL_H
>  #define _LINUX_MEMCONTROL_H
>  
> -#include <linux/rcupdate.h>
> -#include <linux/mm.h>
> -
>  struct mem_cgroup;
>  struct page_cgroup;
>  struct page;

This really should have been in a separate patch and extensively tested.

Have we checked that every file which directly or indirectly includes
memcontrol.h does not have an requirement for rcupdate.h and mm.h, where
that requirement was satisfied only via this nested inclusion?  For all
architectures and for all config selections?  Think not.

Sadly, removal of nested includes is a *big* deal, and it takes quite a lot
of time to get it all shaken down.

If we can confirm that all files (.c and .h) which include memcontrol.h
also directly include rcupdate.h and mm.h then we're _probably_ ok (modulo
ordering issues).

Otherwise we should perhaps be taking a second look at how to fix the sparc
problem.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [patch] sparc: fix build
  2008-02-13  2:46           ` Andrew Morton
@ 2008-02-13  2:57             ` Al Viro
  2008-02-13  3:08               ` Andrew Morton
  0 siblings, 1 reply; 10+ messages in thread
From: Al Viro @ 2008-02-13  2:57 UTC (permalink / raw)
  To: Andrew Morton
  Cc: David Rientjes, Adrian Bunk, linux-kernel, sparclinux, Robert Reif

On Tue, Feb 12, 2008 at 06:46:54PM -0800, Andrew Morton wrote:
> > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> > --- a/include/linux/memcontrol.h
> > +++ b/include/linux/memcontrol.h
> > @@ -20,9 +20,6 @@
> >  #ifndef _LINUX_MEMCONTROL_H
> >  #define _LINUX_MEMCONTROL_H
> >  
> > -#include <linux/rcupdate.h>
> > -#include <linux/mm.h>
> > -
> >  struct mem_cgroup;
> >  struct page_cgroup;
> >  struct page;
> 
> This really should have been in a separate patch and extensively tested.
> 
> Have we checked that every file which directly or indirectly includes
> memcontrol.h does not have an requirement for rcupdate.h and mm.h, where
> that requirement was satisfied only via this nested inclusion?  For all
> architectures and for all config selections?  Think not.
> 
> Sadly, removal of nested includes is a *big* deal, and it takes quite a lot
> of time to get it all shaken down.
> 
> If we can confirm that all files (.c and .h) which include memcontrol.h
> also directly include rcupdate.h and mm.h then we're _probably_ ok (modulo
> ordering issues).
> 
> Otherwise we should perhaps be taking a second look at how to fix the sparc
> problem.

I've run allmodconfig builds on a bunch of target, FWIW (essentially the
same patch).  Note that these includes are recent addition caused by
added inline function that had since then become a define.  So while I
agree with your comments in general, in _this_ case it's pretty safe.

Commit that had done it is 3062fc67dad01b1d2a15d58c709eff946389eca4
and switch to #define is 60c12b1202a60eabb1c61317e5d2678fcea9893f (BTW,
that warranted mentioning in changelog of the latter).

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [patch] sparc: fix build
  2008-02-13  2:57             ` Al Viro
@ 2008-02-13  3:08               ` Andrew Morton
  0 siblings, 0 replies; 10+ messages in thread
From: Andrew Morton @ 2008-02-13  3:08 UTC (permalink / raw)
  To: Al Viro
  Cc: David Rientjes, Adrian Bunk, linux-kernel, sparclinux, Robert Reif

On Wed, 13 Feb 2008 02:57:39 +0000 Al Viro <viro@ZenIV.linux.org.uk> wrote:

> On Tue, Feb 12, 2008 at 06:46:54PM -0800, Andrew Morton wrote:
> > > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> > > --- a/include/linux/memcontrol.h
> > > +++ b/include/linux/memcontrol.h
> > > @@ -20,9 +20,6 @@
> > >  #ifndef _LINUX_MEMCONTROL_H
> > >  #define _LINUX_MEMCONTROL_H
> > >  
> > > -#include <linux/rcupdate.h>
> > > -#include <linux/mm.h>
> > > -
> > >  struct mem_cgroup;
> > >  struct page_cgroup;
> > >  struct page;
> > 
> > This really should have been in a separate patch and extensively tested.
> > 
> > Have we checked that every file which directly or indirectly includes
> > memcontrol.h does not have an requirement for rcupdate.h and mm.h, where
> > that requirement was satisfied only via this nested inclusion?  For all
> > architectures and for all config selections?  Think not.
> > 
> > Sadly, removal of nested includes is a *big* deal, and it takes quite a lot
> > of time to get it all shaken down.
> > 
> > If we can confirm that all files (.c and .h) which include memcontrol.h
> > also directly include rcupdate.h and mm.h then we're _probably_ ok (modulo
> > ordering issues).
> > 
> > Otherwise we should perhaps be taking a second look at how to fix the sparc
> > problem.
> 
> I've run allmodconfig builds on a bunch of target, FWIW (essentially the
> same patch).  Note that these includes are recent addition caused by
> added inline function that had since then become a define.  So while I
> agree with your comments in general, in _this_ case it's pretty safe.

OK, thanks, that increases the comfort level,

> Commit that had done it is 3062fc67dad01b1d2a15d58c709eff946389eca4
> and switch to #define is 60c12b1202a60eabb1c61317e5d2678fcea9893f (BTW,
> that warranted mentioning in changelog of the latter).

I just copied-and-pasted your email ;)

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2008-02-13  3:10 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-02-07 23:12 sparc compile error Adrian Bunk
2008-02-07 23:38 ` David Rientjes
2008-02-10 15:33   ` Martin Habets
2008-02-10 19:19     ` David Rientjes
2008-02-10 21:44       ` Robert Reif
2008-02-10 23:30         ` [patch] sparc: fix build David Rientjes
2008-02-11  9:37           ` Balbir Singh
2008-02-13  2:46           ` Andrew Morton
2008-02-13  2:57             ` Al Viro
2008-02-13  3:08               ` Andrew Morton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).