[PATCH] mm: Fix section mismatch in memcontrol.c

[PATCH] mm: Fix section mismatch in memcontrol.c

[Rakib Mullick]

Impact: Fix section mismatch warning.

The annotation for __init in enable_swap_cgroup() and __initdata for
really_do_swap_account produces the following warning. Which is not
right, since this function is called from non-init section. This patch
fixes it. If anything else please notice.

  LD      mm/built-in.o
WARNING: mm/built-in.o(.text+0x277d9): Section mismatch in reference
from the function mem_cgroup_create() to the function
.init.text:enable_swap_cgroup()
The function mem_cgroup_create() references
the function __init enable_swap_cgroup().
This is often because mem_cgroup_create lacks a __init
annotation or the annotation of enable_swap_cgroup is wrong.

Signed-off-by: Rakib Mullick <rakib.mullick@gmail.com>

--- linux-2.6-orig/mm/memcontrol.c	2009-01-12 09:53:10.000000000 +0600
+++ linux-2.6/mm/memcontrol.c	2009-01-12 19:40:06.134648480 +0600
@@ -46,7 +46,7 @@ struct cgroup_subsys mem_cgroup_subsys _
 #ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP
 /* Turned on only when memory cgroup is enabled &&
really_do_swap_account = 0 */
 int do_swap_account __read_mostly;
-static int really_do_swap_account __initdata = 1; /* for remember boot option*/
+static int really_do_swap_account = 1; /* for remember boot option*/
 #else
 #define do_swap_account		(0)
 #endif
@@ -2170,13 +2170,13 @@ static void mem_cgroup_put(struct mem_cg


 #ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP
-static void __init enable_swap_cgroup(void)
+static void enable_swap_cgroup(void)
 {
 	if (!mem_cgroup_disabled() && really_do_swap_account)
 		do_swap_account = 1;
 }
 #else
-static void __init enable_swap_cgroup(void)
+static void enable_swap_cgroup(void)
 {
 }
 #endif

Re: [PATCH] mm: Fix section mismatch in memcontrol.c

[Li Zefan]

Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: Balbir Singh <balbir@linux.vnet.ibm.com>

Rakib Mullick wrote:
> Impact: Fix section mismatch warning.
> 
> The annotation for __init in enable_swap_cgroup() and __initdata for
> really_do_swap_account produces the following warning. Which is not
> right, since this function is called from non-init section. This patch
> fixes it. If anything else please notice.
> 

I saw this warning on IA64. But this fix is wrong IMO.

enable_swap_cgroup() will be called at system boot only:

start_kernel()
  cgroup_init()
    mem_cgroup_create()
      enable_swap_cgroup()

I think the proper fix is annotate mem_cgroup_create() with __init_refok.

>   LD      mm/built-in.o
> WARNING: mm/built-in.o(.text+0x277d9): Section mismatch in reference
> from the function mem_cgroup_create() to the function
> .init.text:enable_swap_cgroup()
> The function mem_cgroup_create() references
> the function __init enable_swap_cgroup().
> This is often because mem_cgroup_create lacks a __init
> annotation or the annotation of enable_swap_cgroup is wrong.
> 
> Signed-off-by: Rakib Mullick <rakib.mullick@gmail.com>
> 
> --- linux-2.6-orig/mm/memcontrol.c	2009-01-12 09:53:10.000000000 +0600
> +++ linux-2.6/mm/memcontrol.c	2009-01-12 19:40:06.134648480 +0600
> @@ -46,7 +46,7 @@ struct cgroup_subsys mem_cgroup_subsys _
>  #ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP
>  /* Turned on only when memory cgroup is enabled &&
> really_do_swap_account = 0 */
>  int do_swap_account __read_mostly;
> -static int really_do_swap_account __initdata = 1; /* for remember boot option*/
> +static int really_do_swap_account = 1; /* for remember boot option*/
>  #else
>  #define do_swap_account		(0)
>  #endif
> @@ -2170,13 +2170,13 @@ static void mem_cgroup_put(struct mem_cg
> 
> 
>  #ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP
> -static void __init enable_swap_cgroup(void)
> +static void enable_swap_cgroup(void)
>  {
>  	if (!mem_cgroup_disabled() && really_do_swap_account)
>  		do_swap_account = 1;
>  }
>  #else
> -static void __init enable_swap_cgroup(void)
> +static void enable_swap_cgroup(void)
>  {
>  }
>  #endif
> --

Re: [PATCH] mm: Fix section mismatch in memcontrol.c

[KAMEZAWA Hiroyuki]

On Tue, 13 Jan 2009 13:49:03 +0800
Li Zefan <lizf@cn.fujitsu.com> wrote:

> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Cc: Balbir Singh <balbir@linux.vnet.ibm.com>
> 
> Rakib Mullick wrote:
> > Impact: Fix section mismatch warning.
> > 
> > The annotation for __init in enable_swap_cgroup() and __initdata for
> > really_do_swap_account produces the following warning. Which is not
> > right, since this function is called from non-init section. This patch
> > fixes it. If anything else please notice.
> > 
> 
> I saw this warning on IA64. But this fix is wrong IMO.
> 
> enable_swap_cgroup() will be called at system boot only:
> 
> start_kernel()
>   cgroup_init()
>     mem_cgroup_create()
>       enable_swap_cgroup()
> 
> I think the proper fix is annotate mem_cgroup_create() with __init_refok.
> 
I think Li Zefan's fix is correct. 
Could you make a patch ? or I'll schedule this as my work.

Thanks,
-Kame

Re: [PATCH] mm: Fix section mismatch in memcontrol.c

[Rakib Mullick]

Sure. You can carryout your other jobs. Here is the patch.
Thanks for your help guys.

--- linux-2.6-orig/mm/memcontrol.c	2009-01-12 09:53:10.000000000 +0600
+++ linux-2.6/mm/memcontrol.c	2009-01-13 19:55:02.846459224 +0600
@@ -2170,13 +2170,13 @@ static void mem_cgroup_put(struct mem_cg


 #ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP
-static void __init enable_swap_cgroup(void)
+static void __init_refok enable_swap_cgroup(void)
 {
 	if (!mem_cgroup_disabled() && really_do_swap_account)
 		do_swap_account = 1;
 }
 #else
-static void __init enable_swap_cgroup(void)
+static void __init_refok enable_swap_cgroup(void)
 {
 }
 #endif


On 1/13/09, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> On Tue, 13 Jan 2009 13:49:03 +0800
> Li Zefan <lizf@cn.fujitsu.com> wrote:
>
>> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>> Cc: Balbir Singh <balbir@linux.vnet.ibm.com>
>>
>> Rakib Mullick wrote:
>> > Impact: Fix section mismatch warning.
>> >
>> > The annotation for __init in enable_swap_cgroup() and __initdata for
>> > really_do_swap_account produces the following warning. Which is not
>> > right, since this function is called from non-init section. This patch
>> > fixes it. If anything else please notice.
>> >
>>
>> I saw this warning on IA64. But this fix is wrong IMO.
>>
>> enable_swap_cgroup() will be called at system boot only:
>>
>> start_kernel()
>>   cgroup_init()
>>     mem_cgroup_create()
>>       enable_swap_cgroup()
>>
>> I think the proper fix is annotate mem_cgroup_create() with __init_refok.
>>
> I think Li Zefan's fix is correct.
> Could you make a patch ? or I'll schedule this as my work.
>
> Thanks,
> -Kame
>
>
>

Re: [PATCH] mm: Fix section mismatch in memcontrol.c

[KAMEZAWA Hiroyuki]

On Wed, 14 Jan 2009 10:43:24 +0600
"Rakib Mullick" <rakib.mullick@gmail.com> wrote:

> Sure. You can carryout your other jobs. Here is the patch.
> Thanks for your help guys.
> 
Thank you. I confirmed this fixes mismatch.

Could you send this with your Signed-off-by, again ?

you got my Ack.
Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>



> --- linux-2.6-orig/mm/memcontrol.c	2009-01-12 09:53:10.000000000 +0600
> +++ linux-2.6/mm/memcontrol.c	2009-01-13 19:55:02.846459224 +0600
> @@ -2170,13 +2170,13 @@ static void mem_cgroup_put(struct mem_cg
> 
> 
>  #ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP
> -static void __init enable_swap_cgroup(void)
> +static void __init_refok enable_swap_cgroup(void)
>  {
>  	if (!mem_cgroup_disabled() && really_do_swap_account)
>  		do_swap_account = 1;
>  }
>  #else
> -static void __init enable_swap_cgroup(void)
> +static void __init_refok enable_swap_cgroup(void)
>  {
>  }
>  #endif
> 
> 
> On 1/13/09, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> > On Tue, 13 Jan 2009 13:49:03 +0800
> > Li Zefan <lizf@cn.fujitsu.com> wrote:
> >
> >> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> >> Cc: Balbir Singh <balbir@linux.vnet.ibm.com>
> >>
> >> Rakib Mullick wrote:
> >> > Impact: Fix section mismatch warning.
> >> >
> >> > The annotation for __init in enable_swap_cgroup() and __initdata for
> >> > really_do_swap_account produces the following warning. Which is not
> >> > right, since this function is called from non-init section. This patch
> >> > fixes it. If anything else please notice.
> >> >
> >>
> >> I saw this warning on IA64. But this fix is wrong IMO.
> >>
> >> enable_swap_cgroup() will be called at system boot only:
> >>
> >> start_kernel()
> >>   cgroup_init()
> >>     mem_cgroup_create()
> >>       enable_swap_cgroup()
> >>
> >> I think the proper fix is annotate mem_cgroup_create() with __init_refok.
> >>
> > I think Li Zefan's fix is correct.
> > Could you make a patch ? or I'll schedule this as my work.
> >
> > Thanks,
> > -Kame
> >
> >
> >
> 

Re: [PATCH] mm: Fix section mismatch in memcontrol.c

[Rakib Mullick]

Oh, yes. I forgot to signed off the patch. Here it is.

Signed-off-by: Rakib Mullick <rakib.mullick@gmail.com>

--- linux-2.6-orig/mm/memcontrol.c	2009-01-12 09:53:10.000000000 +0600
+++ linux-2.6/mm/memcontrol.c	2009-01-13 19:55:02.846459224 +0600
@@ -2170,13 +2170,13 @@ static void mem_cgroup_put(struct mem_cg


 #ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP
-static void __init enable_swap_cgroup(void)
+static void __init_refok enable_swap_cgroup(void)
 {
 	if (!mem_cgroup_disabled() && really_do_swap_account)
 		do_swap_account = 1;
 }
 #else
-static void __init enable_swap_cgroup(void)
+static void __init_refok enable_swap_cgroup(void)
 {
 }
 #endif


On 1/14/09, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> On Wed, 14 Jan 2009 10:43:24 +0600
> "Rakib Mullick" <rakib.mullick@gmail.com> wrote:
>
>> Sure. You can carryout your other jobs. Here is the patch.
>> Thanks for your help guys.
>>
> Thank you. I confirmed this fixes mismatch.
>
> Could you send this with your Signed-off-by, again ?
>
> you got my Ack.
> Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>
>
>
>> --- linux-2.6-orig/mm/memcontrol.c	2009-01-12 09:53:10.000000000 +0600
>> +++ linux-2.6/mm/memcontrol.c	2009-01-13 19:55:02.846459224 +0600
>> @@ -2170,13 +2170,13 @@ static void mem_cgroup_put(struct mem_cg
>>
>>
>>  #ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP
>> -static void __init enable_swap_cgroup(void)
>> +static void __init_refok enable_swap_cgroup(void)
>>  {
>>  	if (!mem_cgroup_disabled() && really_do_swap_account)
>>  		do_swap_account = 1;
>>  }
>>  #else
>> -static void __init enable_swap_cgroup(void)
>> +static void __init_refok enable_swap_cgroup(void)
>>  {
>>  }
>>  #endif
>>
>>
>> On 1/13/09, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
>> > On Tue, 13 Jan 2009 13:49:03 +0800
>> > Li Zefan <lizf@cn.fujitsu.com> wrote:
>> >
>> >> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>> >> Cc: Balbir Singh <balbir@linux.vnet.ibm.com>
>> >>
>> >> Rakib Mullick wrote:
>> >> > Impact: Fix section mismatch warning.
>> >> >
>> >> > The annotation for __init in enable_swap_cgroup() and __initdata for
>> >> > really_do_swap_account produces the following warning. Which is not
>> >> > right, since this function is called from non-init section. This
>> >> > patch
>> >> > fixes it. If anything else please notice.
>> >> >
>> >>
>> >> I saw this warning on IA64. But this fix is wrong IMO.
>> >>
>> >> enable_swap_cgroup() will be called at system boot only:
>> >>
>> >> start_kernel()
>> >>   cgroup_init()
>> >>     mem_cgroup_create()
>> >>       enable_swap_cgroup()
>> >>
>> >> I think the proper fix is annotate mem_cgroup_create() with
>> >> __init_refok.
>> >>
>> > I think Li Zefan's fix is correct.
>> > Could you make a patch ? or I'll schedule this as my work.
>> >
>> > Thanks,
>> > -Kame
>> >
>> >
>> >
>>
>
>

Re: [PATCH] mm: Fix section mismatch in memcontrol.c

[Li Zefan]

KAMEZAWA Hiroyuki wrote:
> On Wed, 14 Jan 2009 10:43:24 +0600
> "Rakib Mullick" <rakib.mullick@gmail.com> wrote:
> 
>> Sure. You can carryout your other jobs. Here is the patch.
>> Thanks for your help guys.
>>
> Thank you. I confirmed this fixes mismatch.
> 
> Could you send this with your Signed-off-by, again ?
> 
> you got my Ack.
> Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> 

No... This patch is still not correct..

It's mem_cgroup_create() but not enable_swap_cgroup() that needs
to be marked as __init_refok.

Here is the correct one:

===============

From: Li Zefan <lizf@cn.fujitsu.com>
Date: Wed, 14 Jan 2009 13:07:31 +0800
Subject: [PATCH] memcg: fix section mismatch

At system boot when creating the top cgroup, mem_cgroup_create() calls
enable_swap_cgroup() which is marked as __init, so mark mem_cgroup_create()
as __init_refok to avoid false section mismatch warning.

Reported-by: Rakib Mullick <rakib.mullick@gmail.com>
Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
 mm/memcontrol.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index e2996b8..a2c929f 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2181,7 +2181,7 @@ static void __init enable_swap_cgroup(void)
 }
 #endif
 
-static struct cgroup_subsys_state *
+static struct cgroup_subsys_state * __init_refok
 mem_cgroup_create(struct cgroup_subsys *ss, struct cgroup *cont)
 {
 	struct mem_cgroup *mem, *parent;
-- 
1.5.4.rc3

Re: [PATCH] mm: Fix section mismatch in memcontrol.c

[KAMEZAWA Hiroyuki]

On Wed, 14 Jan 2009 13:14:54 +0800
Li Zefan <lizf@cn.fujitsu.com> wrote:

> KAMEZAWA Hiroyuki wrote:
> > On Wed, 14 Jan 2009 10:43:24 +0600
> > "Rakib Mullick" <rakib.mullick@gmail.com> wrote:
> > 
> >> Sure. You can carryout your other jobs. Here is the patch.
> >> Thanks for your help guys.
> >>
> > Thank you. I confirmed this fixes mismatch.
> > 
> > Could you send this with your Signed-off-by, again ?
> > 
> > you got my Ack.
> > Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> > 
> 
> No... This patch is still not correct..
> 
> It's mem_cgroup_create() but not enable_swap_cgroup() that needs
> to be marked as __init_refok.
> 
> Here is the correct one:
> 
Ah..maybe you're right.

BTW, reading inclinde/linux/init.h again

==
/* modpost check for section mismatches during the kernel build.
 * A section mismatch happens when there are references from a
 * code or data section to an init section (both code or data).
 * The init sections are (for most archs) discarded by the kernel
 * when early init has completed so all such references are potential bugs.
 * For exit sections the same issue exists.
 * The following markers are used for the cases where the reference to
 * the *init / *exit section (code or data) is valid and will teach
 * modpost not to issue a warning.
 * The markers follow same syntax rules as __init / __initdata. */
#define __ref            __section(.ref.text) noinline
#define __refdata        __section(.ref.data)
#define __refconst       __section(.ref.rodata)

==

It seems init_refok is old style.

Could you convert to __ref ?

-Kame



> ===============
> 
> From: Li Zefan <lizf@cn.fujitsu.com>
> Date: Wed, 14 Jan 2009 13:07:31 +0800
> Subject: [PATCH] memcg: fix section mismatch
> 
> At system boot when creating the top cgroup, mem_cgroup_create() calls
> enable_swap_cgroup() which is marked as __init, so mark mem_cgroup_create()
> as __init_refok to avoid false section mismatch warning.
> 
> Reported-by: Rakib Mullick <rakib.mullick@gmail.com>
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
> ---
>  mm/memcontrol.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index e2996b8..a2c929f 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -2181,7 +2181,7 @@ static void __init enable_swap_cgroup(void)
>  }
>  #endif
>  
> -static struct cgroup_subsys_state *
> +static struct cgroup_subsys_state * __init_refok
>  mem_cgroup_create(struct cgroup_subsys *ss, struct cgroup *cont)
>  {
>  	struct mem_cgroup *mem, *parent;
> -- 
> 1.5.4.rc3
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

[PATCH] mm: Fix section mismatch in memcontrol.c

[Rakib Mullick]

Impact: Fix section mismatch warning.

The annotation for __init in enable_swap_cgroup() and __initdata for
really_do_swap_account produces the following warning. Which is not
right, since this function is called from non-init section. This patch
fixes it.

  LD      mm/built-in.o
WARNING: mm/built-in.o(.text+0x277d9): Section mismatch in reference
from the function mem_cgroup_create() to the function
.init.text:enable_swap_cgroup()
The function mem_cgroup_create() references
the function __init enable_swap_cgroup().
This is often because mem_cgroup_create lacks a __init
annotation or the annotation of enable_swap_cgroup is wrong.

Signed-off-by: Rakib Mullick <rakib.mullick@gmail.com>

--- linux-2.6-orig/mm/memcontrol.c	2009-01-12 09:53:10.000000000 +0600
+++ linux-2.6/mm/memcontrol.c	2009-01-12 19:40:06.134648480 +0600
@@ -46,7 +46,7 @@ struct cgroup_subsys mem_cgroup_subsys _
 #ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP
 /* Turned on only when memory cgroup is enabled &&
really_do_swap_account = 0 */
 int do_swap_account __read_mostly;
-static int really_do_swap_account __initdata = 1; /* for remember boot option*/
+static int really_do_swap_account = 1; /* for remember boot option*/
 #else
 #define do_swap_account		(0)
 #endif
@@ -2170,13 +2170,13 @@ static void mem_cgroup_put(struct mem_cg


 #ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP
-static void __init enable_swap_cgroup(void)
+static void enable_swap_cgroup(void)
 {
 	if (!mem_cgroup_disabled() && really_do_swap_account)
 		do_swap_account = 1;
 }
 #else
-static void __init enable_swap_cgroup(void)
+static void enable_swap_cgroup(void)
 {
 }
 #endif