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 <[email protected]>
--- 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
Cc: KAMEZAWA Hiroyuki <[email protected]>
Cc: Balbir Singh <[email protected]>
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 <[email protected]>
>
> --- 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
> --
On Tue, 13 Jan 2009 13:49:03 +0800
Li Zefan <[email protected]> wrote:
> Cc: KAMEZAWA Hiroyuki <[email protected]>
> Cc: Balbir Singh <[email protected]>
>
> 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
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 <[email protected]> wrote:
> On Tue, 13 Jan 2009 13:49:03 +0800
> Li Zefan <[email protected]> wrote:
>
>> Cc: KAMEZAWA Hiroyuki <[email protected]>
>> Cc: Balbir Singh <[email protected]>
>>
>> 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
>
>
>
On Wed, 14 Jan 2009 10:43:24 +0600
"Rakib Mullick" <[email protected]> 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 <[email protected]>
> --- 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 <[email protected]> wrote:
> > On Tue, 13 Jan 2009 13:49:03 +0800
> > Li Zefan <[email protected]> wrote:
> >
> >> Cc: KAMEZAWA Hiroyuki <[email protected]>
> >> Cc: Balbir Singh <[email protected]>
> >>
> >> 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
> >
> >
> >
>
Oh, yes. I forgot to signed off the patch. Here it is.
Signed-off-by: Rakib Mullick <[email protected]>
--- 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 <[email protected]> wrote:
> On Wed, 14 Jan 2009 10:43:24 +0600
> "Rakib Mullick" <[email protected]> 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 <[email protected]>
>
>
>
>> --- 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 <[email protected]> wrote:
>> > On Tue, 13 Jan 2009 13:49:03 +0800
>> > Li Zefan <[email protected]> wrote:
>> >
>> >> Cc: KAMEZAWA Hiroyuki <[email protected]>
>> >> Cc: Balbir Singh <[email protected]>
>> >>
>> >> 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
>> >
>> >
>> >
>>
>
>
KAMEZAWA Hiroyuki wrote:
> On Wed, 14 Jan 2009 10:43:24 +0600
> "Rakib Mullick" <[email protected]> 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 <[email protected]>
>
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 <[email protected]>
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 <[email protected]>
Signed-off-by: Li Zefan <[email protected]>
---
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
On Wed, 14 Jan 2009 13:14:54 +0800
Li Zefan <[email protected]> wrote:
> KAMEZAWA Hiroyuki wrote:
> > On Wed, 14 Jan 2009 10:43:24 +0600
> > "Rakib Mullick" <[email protected]> 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 <[email protected]>
> >
>
> 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 <[email protected]>
> 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 <[email protected]>
> Signed-off-by: Li Zefan <[email protected]>
> ---
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>