section mismatches from sun4v*_patch in trampoline.S

section mismatches from sun4v*_patch in trampoline.S

[Sam Ravnborg]

Hi David.

As per discussion in other mail I have looked into the section
mismatch warnings coming from trampoline.S


If the section is changed to .init.text it gives the following warnings:
WARNING: arch/sparc64/kernel/built-in.o(.sun4v_1insn_patch+0x8): Section mismatch: reference to .init.text:
WARNING: arch/sparc64/kernel/built-in.o(.sun4v_1insn_patch+0x10): Section mismatch: reference to .init.text:
WARNING: arch/sparc64/kernel/built-in.o(.sun4v_1insn_patch+0x18): Section mismatch: reference to .init.text:
WARNING: arch/sparc64/kernel/built-in.o(.cpuid_patch+0x0): Section mismatch: reference to .init.text:

The good news is that there is a nice pattern here. All sections that
have the references end in _patch.
So a simple solution is to teach modpost to not warn about references
from a section named *_patch to .init.text

I have such a change queued to modpost but would like to know if
you have any better ideas before pushing the change.

	Sam

Relevant patches (for info only).
I do not know if it is OK to use .init.text??
I would assume that CONFIG_HOTPLUG_CPU had impact here.

diff --git a/arch/sparc64/kernel/trampoline.S b/arch/sparc64/kernel/trampoline.S
index a4dc01a..2588cf0 100644
--- a/arch/sparc64/kernel/trampoline.S
+++ b/arch/sparc64/kernel/trampoline.S
@@ -37,6 +37,7 @@ tramp_stack:
 	.skip	TRAMP_STACK_SIZE
 
 	.text
+	.section	.init.text, "ax"
 	.align		8
 	.globl		sparc64_cpu_startup, sparc64_cpu_startup_end
 sparc64_cpu_startup:

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 0f9130f..8464938 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1173,6 +1173,11 @@ static int init_section_ref_ok(const char *name)
 		".rodata",
 		NULL
 	};
+	/* End of section names */
+	const char *end_of_sec[] = {
+		"_patch",
+		NULL
+	};
 
 	if (initexit_section_ref_ok(name))
 		return 1;
@@ -1183,6 +1188,11 @@ static int init_section_ref_ok(const char *name)
 	for (s = namelist2; *s; s++)
 		if (strncmp(*s, name, strlen(*s)) = 0)
 			return 1;
+
+	for (s = end_of_sec; *s; s++)
+		if (strrcmp(name, *s) = 0)
+			return 1;
+
 	if (strrcmp(name, ".init") = 0)
 		return 1;
 	return 0;

Re: section mismatches from sun4v*_patch in trampoline.S

[David Miller]

From: Sam Ravnborg <sam@ravnborg.org>
Date: Sat, 2 Jun 2007 22:32:06 +0200

> The good news is that there is a nice pattern here. All sections that
> have the references end in _patch.
> So a simple solution is to teach modpost to not warn about references
> from a section named *_patch to .init.text
> 
> I have such a change queued to modpost but would like to know if
> you have any better ideas before pushing the change.

I think it would be great if we adopted a consistent naming
convention for these things across as many platforms as possible.

So if it makes things easier, I can change the naming pattern
I use on sparc64 for the patch sections to match what other
platforms do.

IA64 uses .data.patch.* for example.

Re: section mismatches from sun4v*_patch in trampoline.S

[Sam Ravnborg]

On Sat, Jun 02, 2007 at 02:02:20PM -0700, David Miller wrote:
> From: Sam Ravnborg <sam@ravnborg.org>
> Date: Sat, 2 Jun 2007 22:32:06 +0200
> 
> > The good news is that there is a nice pattern here. All sections that
> > have the references end in _patch.
> > So a simple solution is to teach modpost to not warn about references
> > from a section named *_patch to .init.text
> > 
> > I have such a change queued to modpost but would like to know if
> > you have any better ideas before pushing the change.
> 
> I think it would be great if we adopted a consistent naming
> convention for these things across as many platforms as possible.
> 
> So if it makes things easier, I can change the naming pattern
> I use on sparc64 for the patch sections to match what other
> platforms do.
> 
> IA64 uses .data.patch.* for example.

I would like a common namespace too.
To me .data.* gives association of storage for variables of
some kind which was why I did not suggest that sparc64 adapted
ia64 namescheme.

I thought of .fixup.patch.* but I did not manage to fully understand
how .fixup sections are used.
So a common naming scheme are welcome under the condition
that we find a good name.
If this requires update of both ia64 and sparc64 does not matter.

Btw I cannot see why ia64 does not give warnings with their use
of .data.patch.*. It could be a modpost bug...

	Sam

Re: section mismatches from sun4v*_patch in trampoline.S

[David Miller]

From: Sam Ravnborg <sam@ravnborg.org>
Date: Sun, 3 Jun 2007 01:00:50 +0200

> Btw I cannot see why ia64 does not give warnings with their use
> of .data.patch.*. It could be a modpost bug...

Keep in mind that if you prefix a section name with ".data.foo"
or ".text.foo" this might have meaning for a given platform
and ELF format.

Re: section mismatches from sun4v*_patch in trampoline.S

[David Miller]

From: Sam Ravnborg <sam@ravnborg.org>
Date: Sat, 2 Jun 2007 22:32:06 +0200

> I do not know if it is OK to use .init.text??
> I would assume that CONFIG_HOTPLUG_CPU had impact here.

The __INIT and __INITDATA assembler macros in linux/init.h for a while
would select the section properly based upon whether CPU hotplug was
enabled or not, but that got reverted by:

838c41184fee5e151c09972f2ba90c16493af614

I guess it's much better to put the ugly ifdefs into the assembler
code by someone's estimation :-( That's how the x86 trampoline.S, for
example, is handling this.

The sparc64 trampoline.S should definitely use .init.foo when
CPU hotplug is enabled.

Re: section mismatches from sun4v*_patch in trampoline.S

[Sam Ravnborg]

On Mon, Jun 04, 2007 at 11:58:28PM -0700, David Miller wrote:
> From: Sam Ravnborg <sam@ravnborg.org>
> Date: Sat, 2 Jun 2007 22:32:06 +0200
> 
> > I do not know if it is OK to use .init.text??
> > I would assume that CONFIG_HOTPLUG_CPU had impact here.
> 
> The __INIT and __INITDATA assembler macros in linux/init.h for a while
> would select the section properly based upon whether CPU hotplug was
> enabled or not, but that got reverted by:
> 
> 838c41184fee5e151c09972f2ba90c16493af614
> 
> I guess it's much better to put the ugly ifdefs into the assembler
> code by someone's estimation :-( That's how the x86 trampoline.S, for
> example, is handling this.
That change was anyway bogus.
There should be a 1:1 between __init and __INIT

So the HOTPLUG aware definitions should be named:
__DEVINIT, __DEVINITDATA

and the HOTPLUG_CPU aware definitions should be named_
__CPUINIT, __CPUINITDATA

Will cook up a patch for this.

> 
> The sparc64 trampoline.S should definitely use .init.foo when
> CPU hotplug is enabled.
Will include that too.

	Sam

Re: section mismatches from sun4v*_patch in trampoline.S

[David Miller]

From: Sam Ravnborg <sam@ravnborg.org>
Date: Tue, 5 Jun 2007 11:13:45 +0200

> On Mon, Jun 04, 2007 at 11:58:28PM -0700, David Miller wrote:
> > The __INIT and __INITDATA assembler macros in linux/init.h for a while
> > would select the section properly based upon whether CPU hotplug was
> > enabled or not, but that got reverted by:
> > 
> > 838c41184fee5e151c09972f2ba90c16493af614
> > 
> > I guess it's much better to put the ugly ifdefs into the assembler
> > code by someone's estimation :-( That's how the x86 trampoline.S, for
> > example, is handling this.
> That change was anyway bogus.
> There should be a 1:1 between __init and __INIT

I see, that's right.

> Will cook up a patch for this.

Thanks a lot Sam.