(hacky) [PATCH] silence MODPOST section mismatch warnings

(hacky) [PATCH] silence MODPOST section mismatch warnings

[Chris Wedgwood]

MODPOST seems to be spewing bogus warnings.  It's not clear how best
to fix it so perhaps we should silence it for now?

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 113dc77..bd6fe7b 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -872,6 +872,10 @@ static void warn_sec_mismatch(const char *modname, const char *fromsec,
 				 sechdrs[hdr->e_shstrndx].sh_offset;
 	const char *secname = secstrings + sechdrs[sym->st_shndx].sh_name;
 
+	/* FIXME: this function doesn't work correctly anymore, it's
+	 * not clear if it should be fixed or removed. */
+	return;
+
 	find_symbols_between(elf, r.r_offset, fromsec, &before, &after);
 
 	refsym = find_elf_symbol(elf, r.r_addend, sym);

Re: (hacky) [PATCH] silence MODPOST section mismatch warnings

[Russell King]

On Thu, May 10, 2007 at 01:34:18PM -0700, Chris Wedgwood wrote:
> MODPOST seems to be spewing bogus warnings.  It's not clear how best
> to fix it so perhaps we should silence it for now?

I agree.  Example bogus warning:

WARNING: arch/arm/mach-iop32x/built-in.o - Section mismatch:
 reference to .init.text: from .data between 'iq80321_timer'
 (at offset 0x428) and 'iq80321_serial_device'

c04088fc d iq80321_timer
c0408950 d iq80321_serial_device

It's completely unclear what is referencing what, what the two named
symbols mean, and even what "at offset" relates to.

What I can say is that iq80321_timer doesn't reference iq80321_serial_device
nor vice versa, and iq80321_timer is far smaller than 0x428 bytes.

> 
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 113dc77..bd6fe7b 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -872,6 +872,10 @@ static void warn_sec_mismatch(const char *modname, const char *fromsec,
>  				 sechdrs[hdr->e_shstrndx].sh_offset;
>  	const char *secname = secstrings + sechdrs[sym->st_shndx].sh_name;
>  
> +	/* FIXME: this function doesn't work correctly anymore, it's
> +	 * not clear if it should be fixed or removed. */
> +	return;
> +
>  	find_symbols_between(elf, r.r_offset, fromsec, &before, &after);
>  
>  	refsym = find_elf_symbol(elf, r.r_addend, sym);

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: (hacky) [PATCH] silence MODPOST section mismatch warnings

[David Miller]

From: Russell King <rmk+lkml@arm.linux.org.uk>
Date: Thu, 10 May 2007 21:40:38 +0100

> On Thu, May 10, 2007 at 01:34:18PM -0700, Chris Wedgwood wrote:
> > MODPOST seems to be spewing bogus warnings.  It's not clear how best
> > to fix it so perhaps we should silence it for now?
> 
> I agree.  Example bogus warning:
> 
> WARNING: arch/arm/mach-iop32x/built-in.o - Section mismatch:
>  reference to .init.text: from .data between 'iq80321_timer'
>  (at offset 0x428) and 'iq80321_serial_device'
> 
> c04088fc d iq80321_timer
> c0408950 d iq80321_serial_device
> 
> It's completely unclear what is referencing what, what the two named
> symbols mean, and even what "at offset" relates to.
> 
> What I can say is that iq80321_timer doesn't reference iq80321_serial_device
> nor vice versa, and iq80321_timer is far smaller than 0x428 bytes.

The range of symbols is just provided to handle inaccuracies
in mid-function and mid-object references, it actually helps.

In this particular case iq80321_timer, which is not init-data,
is referencing iq80321_timer_init() which is init.text

If this is legitimate (which I believe this case is, the init function
method can be init but the offset one is not), add an exception case
like we already have for console drivers et al. to modpost.c and this
warning will go away.

Re: (hacky) [PATCH] silence MODPOST section mismatch warnings

[Sam Ravnborg]

On Thu, May 10, 2007 at 09:40:38PM +0100, Russell King wrote:
> On Thu, May 10, 2007 at 01:34:18PM -0700, Chris Wedgwood wrote:
> > MODPOST seems to be spewing bogus warnings.  It's not clear how best
> > to fix it so perhaps we should silence it for now?
> 
> I agree.  Example bogus warning:
> 
> WARNING: arch/arm/mach-iop32x/built-in.o - Section mismatch:
>  reference to .init.text: from .data between 'iq80321_timer'
>  (at offset 0x428) and 'iq80321_serial_device'
> 
> c04088fc d iq80321_timer
> c0408950 d iq80321_serial_device
> 
> It's completely unclear what is referencing what, what the two named
> symbols mean, and even what "at offset" relates to.

The error message tells you that somewhere _between_ the symbol
iq80312_timer and iq80321_serial_device there is a recerence to an address
in the .init.text section.
The referenced address could not be resolved to an actual symbol.

Lets take a look:
git grep iq80321_timer reveals:
arch/arm/mach-iop32x/iq80321.c:static void __init iq80321_timer_init(void)
arch/arm/mach-iop32x/iq80321.c:static struct sys_timer iq80321_timer = {
arch/arm/mach-iop32x/iq80321.c: .init           = iq80321_timer_init,
arch/arm/mach-iop32x/iq80321.c: .timer          = &iq80321_timer,

So the one to look at is this structure:

static struct sys_timer iq80321_timer = {
	.init		= iq80321_timer_init,
	.offset		= iop_gettimeoffset,
};


iq80321_timer_init is a function marked __init
So we have a reference from .data to .init.text

If you remove the static definition of iq80321_timer then you will see
that modpost can resolve the symbol for you.

The usage of iq80321_timer is obscufated inside a MACHINCE_START/MACHINE_END
macor so I did not look further.
But based on the naming of the member in the struct ".init" I will assume
this is legitime reference to .init.text from .data.

Today modpost whitelist (avoid warnings) if a variable are named on of the 
following:

*driver, *_template, *_sht, *_ops, *_probe, *probe_one

We could add *timer to silence this particular warning indeed.
Or we could rename the varibale.
Or we could find a way to annotate the data to say "references to .init.*
is OK and should not be warned.


The latter seems to be most appropriate so this is the direction I'm heading.

	Sam

Re: (hacky) [PATCH] silence MODPOST section mismatch warnings

[David Miller]

From: Chris Wedgwood <cw@f00f.org>
Date: Thu, 10 May 2007 13:34:18 -0700

> MODPOST seems to be spewing bogus warnings.  It's not clear how best
> to fix it so perhaps we should silence it for now?

Most of them are legitimate, the only one that needs sorting
is the mm/slab.c case and people are working on that.

The rest are useful and I've been working to fix things up
on sparc64 and the networking, and in fact I'm very happy
about these notifications.

Please don't apply a sledgehammer to this issue, thanks.

Re: (hacky) [PATCH] silence MODPOST section mismatch warnings

[Russell King]

On Thu, May 10, 2007 at 01:51:47PM -0700, David Miller wrote:
> From: Chris Wedgwood <cw@f00f.org>
> Date: Thu, 10 May 2007 13:34:18 -0700
> 
> > MODPOST seems to be spewing bogus warnings.  It's not clear how best
> > to fix it so perhaps we should silence it for now?
> 
> Most of them are legitimate, the only one that needs sorting
> is the mm/slab.c case and people are working on that.
> 
> The rest are useful and I've been working to fix things up
> on sparc64 and the networking, and in fact I'm very happy
> about these notifications.
> 
> Please don't apply a sledgehammer to this issue, thanks.

I've not had one accurate one on ARM yet.

Here's another example:

WARNING: init/built-in.o - Section mismatch: reference to .init.text:
 from .text between 'rest_init' (at offset 0x4c) and 'run_init_process'

from init/main.c:

static void noinline rest_init(void)
        __releases(kernel_lock)

static void run_init_process(char *init_filename)

Clearly, it just does _not_ work.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: (hacky) [PATCH] silence MODPOST section mismatch warnings

[Sam Ravnborg]

On Thu, May 10, 2007 at 09:54:27PM +0100, Russell King wrote:
> On Thu, May 10, 2007 at 01:51:47PM -0700, David Miller wrote:
> > From: Chris Wedgwood <cw@f00f.org>
> > Date: Thu, 10 May 2007 13:34:18 -0700
> > 
> > > MODPOST seems to be spewing bogus warnings.  It's not clear how best
> > > to fix it so perhaps we should silence it for now?
> > 
> > Most of them are legitimate, the only one that needs sorting
> > is the mm/slab.c case and people are working on that.
> > 
> > The rest are useful and I've been working to fix things up
> > on sparc64 and the networking, and in fact I'm very happy
> > about these notifications.
> > 
> > Please don't apply a sledgehammer to this issue, thanks.
> 
> I've not had one accurate one on ARM yet.
You had one patch from me in latest submission to linus that
was a clear bug.

> 
> Here's another example:
> 
> WARNING: init/built-in.o - Section mismatch: reference to .init.text:
>  from .text between 'rest_init' (at offset 0x4c) and 'run_init_process'
> 
> from init/main.c:
> 
> static void noinline rest_init(void)
>         __releases(kernel_lock)
> 
> static void run_init_process(char *init_filename)
> 
> Clearly, it just does _not_ work.
As I have already explained to you this is a binutils issue
that causes this false positive.

The plan is to annotate functions that are not __init that
they intentional reference a function or data in a init section.
I just not there yet.

	Sam

Re: (hacky) [PATCH] silence MODPOST section mismatch warnings

[Satyam Sharma]

Hi Sam,

On 5/11/07, Sam Ravnborg <sam@ravnborg.org> wrote:
> On Thu, May 10, 2007 at 09:54:27PM +0100, Russell King wrote:
> > On Thu, May 10, 2007 at 01:51:47PM -0700, David Miller wrote:
> > > From: Chris Wedgwood <cw@f00f.org>
> > > Date: Thu, 10 May 2007 13:34:18 -0700
> > >
> > > > MODPOST seems to be spewing bogus warnings.  It's not clear how best
> > > > to fix it so perhaps we should silence it for now?
> > >
> > > Most of them are legitimate, the only one that needs sorting
> > > is the mm/slab.c case and people are working on that.
> > >
> > > The rest are useful and I've been working to fix things up
> > > on sparc64 and the networking, and in fact I'm very happy
> > > about these notifications.
> > >
> > > Please don't apply a sledgehammer to this issue, thanks.
> >
> > I've not had one accurate one on ARM yet.
> You had one patch from me in latest submission to linus that
> was a clear bug.
>
> >
> > Here's another example:
> >
> > WARNING: init/built-in.o - Section mismatch: reference to .init.text:
> >  from .text between 'rest_init' (at offset 0x4c) and 'run_init_process'
> >
> > from init/main.c:
> >
> > static void noinline rest_init(void)
> >         __releases(kernel_lock)
> >
> > static void run_init_process(char *init_filename)
> >
> > Clearly, it just does _not_ work.
> As I have already explained to you this is a binutils issue
> that causes this false positive.
>
> The plan is to annotate functions that are not __init that
> they intentional reference a function or data in a init section.
> I just not there yet.

The thought makes me *extremely* uncomfortable. Better to _heed_
warnings, than _hide_ them, isn't it -- they only tell us what the
code says, after all. Personally, I'd much rather see a few warnings
on my screen (which I _know_ are false positives) than hide a genuine
potential issue only to crash later.

For bogus warnings caused by binutils issues, the solution clearly
lies in fixing _binutils_.

For the "genuine false positives" (where we *do* call .init.text from
.text, but somehow ensure that it doesn't happen after boot) -- I
don't really know, but a solution that solves the (kmem_cache_create
-> setup_cpu_cache + g_cpucache_up -> set_up_list3s) problem in
_logic_ sounds more elegant to me, than inventing something like
__nowarn_calls_init (?). The problem with stuff of the latter sort is
that they often tend to be abused later by everyone even where they
are avoidable / inapplicable, which opens up a can of worms.

Just my 2 paise,
Satyam

Re: (hacky) [PATCH] silence MODPOST section mismatch warnings

[David Miller]

From: Russell King <rmk+lkml@arm.linux.org.uk>
Date: Thu, 10 May 2007 21:54:27 +0100

> I've not had one accurate one on ARM yet.
> 
> Here's another example:
> 
> WARNING: init/built-in.o - Section mismatch: reference to .init.text:
>  from .text between 'rest_init' (at offset 0x4c) and 'run_init_process'
> 
> from init/main.c:
> 
> static void noinline rest_init(void)
>         __releases(kernel_lock)
> 
> static void run_init_process(char *init_filename)
> 
> Clearly, it just does _not_ work.

Russell, the symbols are where the reference to an .init.text
section are coming from, they are not the .init.text function
being referenced itself.

It is saying that something between rest_init and run_init_processes,
which are not .init.text, are referencing an .init.text object.

It's unfortunate that the .init.text object being referenced for some
reason can't get the symbol printed out properly on ARM, it does get
printed out on other platforms, but once you fix that it will be very
easy for you to decode these messages.

But they are still useful.

The other one you posted was legitimate and as I explained just
needs an exception entry added to modpost.c

Re: (hacky) [PATCH] silence MODPOST section mismatch warnings

[Russell King]

On Thu, May 10, 2007 at 02:07:25PM -0700, David Miller wrote:
> From: Russell King <rmk+lkml@arm.linux.org.uk>
> Date: Thu, 10 May 2007 21:54:27 +0100
> 
> > I've not had one accurate one on ARM yet.
> > 
> > Here's another example:
> > 
> > WARNING: init/built-in.o - Section mismatch: reference to .init.text:
> >  from .text between 'rest_init' (at offset 0x4c) and 'run_init_process'
> > 
> > from init/main.c:
> > 
> > static void noinline rest_init(void)
> >         __releases(kernel_lock)
> > 
> > static void run_init_process(char *init_filename)
> > 
> > Clearly, it just does _not_ work.
> 
> Russell, the symbols are where the reference to an .init.text
> section are coming from, they are not the .init.text function
> being referenced itself.
> 
> It is saying that something between rest_init and run_init_processes,
> which are not .init.text, are referencing an .init.text object.

Ah, so they aren't supposed to be the two objects themselves.  Now
that I understand what the message means, I can make some headway
into investigating what's up.

> It's unfortunate that the .init.text object being referenced for some
> reason can't get the symbol printed out properly on ARM, it does get
> printed out on other platforms, but once you fix that it will be very
> easy for you to decode these messages.

Well, I guess it's because of the way the ARM ABI works.  We have
literal pools mixed in with the text:

Disassembly of section .text:

00000000 <rest_init>:
...
  14:   e59f0030        ldr     r0, [pc, #48]   ; 4c <.text+0x4c>
  18:   ebfffffe        bl      0 <kernel_thread>
                        18: R_ARM_PC24  kernel_thread
...
  4c:   000007a0        andeq   r0, r0, r0, lsr #15
                        4c: R_ARM_ABS32 .init.text
        ...
                        50: R_ARM_ABS32 kthreadd
                        54: R_ARM_ABS32 kthreadd_task

00000058 <run_init_process>:

Disassembly of section .init.text:

000007a0 <kernel_init>:

It looks like we're going to have to jump through hoops to get the
right symbols on ARM - to get at the right symbol being referenced
you need to take account of the relocation type and therefore the
properties of the relocation, and add any offset found at the
reference address.

IOW, you need to resolve the relocation (if possible) and see which
symbol you end up pointing at.

I'm sorely lacking in detailed understanding of the ARM binutils
(it's been 12 years since I touched it), especially after the
advent of ARM EABI, so I guess I need to find someone willing in
the ARM community prepared to resolve this.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: (hacky) [PATCH] silence MODPOST section mismatch warnings

[Russell King]

On Thu, May 10, 2007 at 02:07:25PM -0700, David Miller wrote:
> From: Russell King <rmk+lkml@arm.linux.org.uk>
> Date: Thu, 10 May 2007 21:54:27 +0100
> 
> > I've not had one accurate one on ARM yet.
> > 
> > Here's another example:
> > 
> > WARNING: init/built-in.o - Section mismatch: reference to .init.text:
> >  from .text between 'rest_init' (at offset 0x4c) and 'run_init_process'
> > 
> > from init/main.c:
> > 
> > static void noinline rest_init(void)
> >         __releases(kernel_lock)
> > 
> > static void run_init_process(char *init_filename)
> > 
> > Clearly, it just does _not_ work.
> 
> Russell, the symbols are where the reference to an .init.text
> section are coming from, they are not the .init.text function
> being referenced itself.
> 
> It is saying that something between rest_init and run_init_processes,
> which are not .init.text, are referencing an .init.text object.

I'd like to make a suggestion to make the wording of the warning
clearer:

WARNING: init/built-in.o(.text+0x4c): section mismatch: reference to
 .init.text:blah (between 'rest_init' and 'run_init_process')

I think this would remove the confusion - the primary information
relating to where the reference is located is contained together
("init/built-in.o(.text+0x4c)") and the confusing "between" clause
which seems to only be a hint becomes entirely secondary.

Moreover, it's similar to binutils warnings - which are of the form:

file:(section+offset): message

so is of a form that we're used to reading.

Patch below.  Also fixes the obviously wrong ordering of arguments
in the final case in the string of ifs.





Change modpost section mismatch warnings to be less confusing;
model them on the binutils linker warnings which we all know how
to interpret.

Also, fix the wrong ordering of arguments for the final case -
fromsec and refsymname were reversed.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 scripts/mod/modpost.c |   32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 113dc77..6d7864d 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -885,29 +885,29 @@ static void warn_sec_mismatch(const char *modname, const char *fromsec,
 		return;
 
 	if (before && after) {
-		warn("%s - Section mismatch: reference to %s:%s from %s "
-		     "between '%s' (at offset 0x%llx) and '%s'\n",
-		     modname, secname, refsymname, fromsec,
+		warn("%s(%s+0x%llx): Section mismatch: reference to %s:%s "
+		     "(between '%s' and '%s')\n",
+		     modname, fromsec, (long long)r.r_offset,
+		     secname, refsymname,
 		     elf->strtab + before->st_name,
-		     (long long)r.r_offset,
 		     elf->strtab + after->st_name);
 	} else if (before) {
-		warn("%s - Section mismatch: reference to %s:%s from %s "
-		     "after '%s' (at offset 0x%llx)\n",
-		     modname, secname, refsymname, fromsec,
-		     elf->strtab + before->st_name,
-		     (long long)r.r_offset);
+		warn("%s(%s+0x%llx): Section mismatch: reference to %s:%s "
+		     "(after '%s')\n",
+		     modname, fromsec, (long long)r.r_offset,
+		     secname, refsymname,
+		     elf->strtab + before->st_name);
 	} else if (after) {
-		warn("%s - Section mismatch: reference to %s:%s from %s "
+		warn("%s(%s+0x%llx): Section mismatch: reference to %s:%s "
 		     "before '%s' (at offset -0x%llx)\n",
-		     modname, secname, refsymname, fromsec,
-		     elf->strtab + after->st_name,
-		     (long long)r.r_offset);
+		     modname, fromsec, (long long)r.r_offset,
+		     secname, refsymname,
+		     elf->strtab + after->st_name);
 	} else {
-		warn("%s - Section mismatch: reference to %s:%s from %s "
+		warn("%s(%s+0x%llx): Section mismatch: reference to %s:%s "
 		     "(offset 0x%llx)\n",
-		     modname, secname, fromsec, refsymname,
-		     (long long)r.r_offset);
+		     modname, fromsec, (long long)r.r_offset,
+		     secname, refsymname);
 	}
 }
 


-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: [PATCH] Make modpost section warnings clearer

[Russell King]

Change modpost section mismatch warnings to be less confusing;
model them on the binutils linker warnings which we all know how
to interpret.

Also, fix the wrong ordering of arguments for the final case -
fromsec and refsymname were reversed.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
Yes, I build-tested the last patch but it didn't throw any warnings.
Here's a corrected patch.

 scripts/mod/modpost.c |   33 ++++++++++++++++-----------------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 113dc77..acd28ab 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -885,29 +885,28 @@ static void warn_sec_mismatch(const char *modname, const char *fromsec,
 		return;
 
 	if (before && after) {
-		warn("%s - Section mismatch: reference to %s:%s from %s "
-		     "between '%s' (at offset 0x%llx) and '%s'\n",
-		     modname, secname, refsymname, fromsec,
+		warn("%s(%s+0x%llx): Section mismatch: reference to %s:%s "
+		     "(between '%s' and '%s')\n",
+		     modname, fromsec, (long long)r.r_offset,
+		     secname, refsymname,
 		     elf->strtab + before->st_name,
-		     (long long)r.r_offset,
 		     elf->strtab + after->st_name);
 	} else if (before) {
-		warn("%s - Section mismatch: reference to %s:%s from %s "
-		     "after '%s' (at offset 0x%llx)\n",
-		     modname, secname, refsymname, fromsec,
-		     elf->strtab + before->st_name,
-		     (long long)r.r_offset);
+		warn("%s(%s+0x%llx): Section mismatch: reference to %s:%s "
+		     "(after '%s')\n",
+		     modname, fromsec, (long long)r.r_offset,
+		     secname, refsymname,
+		     elf->strtab + before->st_name);
 	} else if (after) {
-		warn("%s - Section mismatch: reference to %s:%s from %s "
+		warn("%s(%s+0x%llx): Section mismatch: reference to %s:%s "
 		     "before '%s' (at offset -0x%llx)\n",
-		     modname, secname, refsymname, fromsec,
-		     elf->strtab + after->st_name,
-		     (long long)r.r_offset);
+		     modname, fromsec, (long long)r.r_offset,
+		     secname, refsymname,
+		     elf->strtab + after->st_name);
 	} else {
-		warn("%s - Section mismatch: reference to %s:%s from %s "
-		     "(offset 0x%llx)\n",
-		     modname, secname, fromsec, refsymname,
-		     (long long)r.r_offset);
+		warn("%s(%s+0x%llx): Section mismatch: reference to %s:%s\n",
+		     modname, fromsec, (long long)r.r_offset,
+		     secname, refsymname);
 	}
 }
 
-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: [PATCH] Make modpost section warnings clearer

[David Miller]

From: Russell King <rmk+lkml@arm.linux.org.uk>
Date: Thu, 10 May 2007 23:03:25 +0100

> Change modpost section mismatch warnings to be less confusing;
> model them on the binutils linker warnings which we all know how
> to interpret.
> 
> Also, fix the wrong ordering of arguments for the final case -
> fromsec and refsymname were reversed.
> 
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>

FWIW I'm fine with the new wording.

Re: (hacky) [PATCH] silence MODPOST section mismatch warnings

[Sam Ravnborg]

On Thu, May 10, 2007 at 10:59:20PM +0100, Russell King wrote:
> On Thu, May 10, 2007 at 02:07:25PM -0700, David Miller wrote:
> > From: Russell King <rmk+lkml@arm.linux.org.uk>
> > Date: Thu, 10 May 2007 21:54:27 +0100
> > 
> > > I've not had one accurate one on ARM yet.
> > > 
> > > Here's another example:
> > > 
> > > WARNING: init/built-in.o - Section mismatch: reference to .init.text:
> > >  from .text between 'rest_init' (at offset 0x4c) and 'run_init_process'
> > > 
> > > from init/main.c:
> > > 
> > > static void noinline rest_init(void)
> > >         __releases(kernel_lock)
> > > 
> > > static void run_init_process(char *init_filename)
> > > 
> > > Clearly, it just does _not_ work.
> > 
> > Russell, the symbols are where the reference to an .init.text
> > section are coming from, they are not the .init.text function
> > being referenced itself.
> > 
> > It is saying that something between rest_init and run_init_processes,
> > which are not .init.text, are referencing an .init.text object.
> 
> I'd like to make a suggestion to make the wording of the warning
> clearer:
> 
> WARNING: init/built-in.o(.text+0x4c): section mismatch: reference to
>  .init.text:blah (between 'rest_init' and 'run_init_process')
> 
> I think this would remove the confusion - the primary information
> relating to where the reference is located is contained together
> ("init/built-in.o(.text+0x4c)") and the confusing "between" clause
> which seems to only be a hint becomes entirely secondary.
> 
> Moreover, it's similar to binutils warnings - which are of the form:
> 
> file:(section+offset): message

I like the new format - thanks!
Did you drop the ':' after the file on purpose?

[I am not familiar with this particular binutils warning so I did not
know an easy way to provoke it]

PS. Will apply the path you submitted in next mail.

	Sam

Re: (hacky) [PATCH] silence MODPOST section mismatch warnings

[Russell King]

On Fri, May 11, 2007 at 12:16:59AM +0200, Sam Ravnborg wrote:
> On Thu, May 10, 2007 at 10:59:20PM +0100, Russell King wrote:
> > file:(section+offset): message
> 
> I like the new format - thanks!
> Did you drop the ':' after the file on purpose?

Oops, yes.

> PS. Will apply the path you submitted in next mail.

Do you want a patch with added colons?

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: (hacky) [PATCH] silence MODPOST section mismatch warnings

[Sam Ravnborg]

On Thu, May 10, 2007 at 11:18:50PM +0100, Russell King wrote:
> On Fri, May 11, 2007 at 12:16:59AM +0200, Sam Ravnborg wrote:
> > On Thu, May 10, 2007 at 10:59:20PM +0100, Russell King wrote:
> > > file:(section+offset): message
> > 
> > I like the new format - thanks!
> > Did you drop the ':' after the file on purpose?
> 
> Oops, yes.
> 
> > PS. Will apply the path you submitted in next mail.
> 
> Do you want a patch with added colons?
I will add them locally - no problem.

	Sam

Re: (hacky) [PATCH] silence MODPOST section mismatch warnings

[Sam Ravnborg]

On Thu, May 10, 2007 at 09:54:27PM +0100, Russell King wrote:
> On Thu, May 10, 2007 at 01:51:47PM -0700, David Miller wrote:
> > From: Chris Wedgwood <cw@f00f.org>
> > Date: Thu, 10 May 2007 13:34:18 -0700
> > 
> > > MODPOST seems to be spewing bogus warnings.  It's not clear how best
> > > to fix it so perhaps we should silence it for now?
> > 
> > Most of them are legitimate, the only one that needs sorting
> > is the mm/slab.c case and people are working on that.
> > 
> > The rest are useful and I've been working to fix things up
> > on sparc64 and the networking, and in fact I'm very happy
> > about these notifications.
> > 
> > Please don't apply a sledgehammer to this issue, thanks.
> 
> I've not had one accurate one on ARM yet.

To be able to test my modpost changes I am building arm for all configs.
I can see a number of legitime warnings for the ARM code.
Those I have investigated has all been of the case where functions
are used only during init but not marked init.
So no real bugs just potential savings and consistency.
Shall I try to fix them up and drop you a path?

Samples (with old warning format):

WARNING: drivers/built-in.o - Section mismatch: reference to .init.text:pcmcia_assabet_init from .text between 'sa11x0_drv_pcmcia_probe' (at offset 0x49ed8) an
d 'assabet_pcmcia_socket_state'
WARNING: drivers/built-in.o - Section mismatch: reference to .init.text:pcmcia_collie_init from .text between 'sa11x0_drv_pcmcia_probe' (at offset 0x5f250) and
 'sharpsl_pcmcia_init_reset'
WARNING: drivers/pcmcia/sa1100_cs.o - Section mismatch: reference to .init.text:pcmcia_cerf_init from .text between 'sa11x0_drv_pcmcia_probe' (at offset 0xc) a
nd 'cerf_pcmcia_socket_state'

WARNING: arch/arm/mach-at91/built-in.o - Section mismatch: reference to .init.data: from .text after 'nand_partitions' (at offset 0xbdc)
WARNING: arch/arm/mach-at91/built-in.o - Section mismatch: reference to .init.data: from .text between 'nand_partitions' (at offset 0xbdc) and 'ads7843_pendown
_state'
WARNING: arch/arm/mach-at91/built-in.o - Section mismatch: reference to .init.data: from .text between 'nand_partitions' (at offset 0xdf4) and 'at91_leds_event
'


	Sam

Re: (hacky) [PATCH] silence MODPOST section mismatch warnings

[Russell King]

On Fri, May 11, 2007 at 08:54:31PM +0200, Sam Ravnborg wrote:
> On Thu, May 10, 2007 at 09:54:27PM +0100, Russell King wrote:
> > On Thu, May 10, 2007 at 01:51:47PM -0700, David Miller wrote:
> > > From: Chris Wedgwood <cw@f00f.org>
> > > Date: Thu, 10 May 2007 13:34:18 -0700
> > > 
> > > > MODPOST seems to be spewing bogus warnings.  It's not clear how best
> > > > to fix it so perhaps we should silence it for now?
> > > 
> > > Most of them are legitimate, the only one that needs sorting
> > > is the mm/slab.c case and people are working on that.
> > > 
> > > The rest are useful and I've been working to fix things up
> > > on sparc64 and the networking, and in fact I'm very happy
> > > about these notifications.
> > > 
> > > Please don't apply a sledgehammer to this issue, thanks.
> > 
> > I've not had one accurate one on ARM yet.
> 
> To be able to test my modpost changes I am building arm for all configs.
> I can see a number of legitime warnings for the ARM code.
> Those I have investigated has all been of the case where functions
> are used only during init but not marked init.
> So no real bugs just potential savings and consistency.
> Shall I try to fix them up and drop you a path?

It'll probably be worth doing this after -rc1 - there's still some
stuff in flux at the moment.

However, note that my comments above were based on a mis-understanding
of the warnings.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: (hacky) [PATCH] silence MODPOST section mismatch warnings

[Sam Ravnborg]

On Thu, May 10, 2007 at 01:34:18PM -0700, Chris Wedgwood wrote:
> MODPOST seems to be spewing bogus warnings.  It's not clear how best
> to fix it so perhaps we should silence it for now?
> 

There is a number of know bogus warnings being looked at.
Could you please post the ones you have in mind so we can see if this is
know ones or new (bogus)? warnings.

PS. I count at least 30 section mismatch warnings being fixed in the kernel lately.
So the check has an effect.

	Sam