* Re: [PATCH][RFC] 2.6 && module + -g && kernel w/o -g
[not found] <20040108003040.GA18481@stop.crashing.org>
@ 2004-01-14 21:09 ` Tom Rini
2004-01-14 22:23 ` David Mosberger
2004-01-14 23:00 ` Rusty Russell
0 siblings, 2 replies; 5+ messages in thread
From: Tom Rini @ 2004-01-14 21:09 UTC (permalink / raw)
To: Paul Mackerras, Benjamin Herrenschmidt, Christoph Hellwig,
Andrew Morton, Rusty Russell
Cc: Kernel Mailing List
On Wed, Jan 07, 2004 at 05:30:40PM -0700, Tom Rini wrote:
> Okay, this is what seems to fix the bug that hch found for me. Does
> this seem right to everyone else? I'm going to poke at it a bit more
> tomorrow, pending time.
>
> ===== arch/ppc/kernel/module.c 1.10 vs edited =====
> --- 1.10/arch/ppc/kernel/module.c Fri Sep 12 09:26:52 2003
> +++ edited//home/trini/work/kernel/testing/linux-2.6/arch/ppc/kernel/module.c Wed Jan 7 17:07:30 2004
> @@ -87,6 +87,9 @@
> if ((strstr(secstrings + sechdrs[i].sh_name, ".init") != 0)
> != is_init)
> continue;
> + /* Skip over debug bits. */
> + if (strstr(secstrings + sechdrs[i].sh_name, ".debug") != 0)
> + continue;
>
> if (sechdrs[i].sh_type == SHT_RELA) {
> DEBUGP("Found relocations in section %u\n", i);
Okay. I've been looking at stock 2.6.1 noticed that the fix for this
issue that Rusty proposed, and that ultimately made it into 2.6.1-rc3
(or so) is not correct. The problem is that we do:
err = module_frob_arch_sections(hdr, sechdrs, secstrings, mod);
/* Which goes over every .debug section and can take _ages_ on something
* like ipv6 */
... skip 100 lines ...
.... Checks Rusty / Linus added ...
The following patch fixes the problem for me on PPC32:
--- 1.96/kernel/module.c Wed Jan 7 22:46:59 2004
+++ edited/kernel/module.c Wed Jan 14 14:05:12 2004
@@ -1439,6 +1439,13 @@
strindex = sechdrs[i].sh_link;
strtab = (char *)hdr + sechdrs[strindex].sh_offset;
}
+
+ /* If we find any debug RELAs, frob these away now. */
+ if (sechdrs[i].sh_type == SHT_RELA &&
+ (strstr(secstrings+sechdrs[i].sh_name, ".debug")
+ != 0))
+ sechdrs[i].sh_type = SHT_NULL;
+
#ifndef CONFIG_MODULE_UNLOAD
/* Don't load .exit sections */
if (strncmp(secstrings+sechdrs[i].sh_name, ".exit", 5) == 0)
IMHO, this shouldn't be covered under a PPC32 test since at least PPC32,
PPC64 and Alpha have this issue, and I suspect that ia64, parisc, s390
and v850 have the problem as well (based on what their module_arch_frob
bits look to be doing).
--
Tom Rini
http://gate.crashing.org/~trini/
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH][RFC] 2.6 && module + -g && kernel w/o -g
2004-01-14 21:09 ` [PATCH][RFC] 2.6 && module + -g && kernel w/o -g Tom Rini
@ 2004-01-14 22:23 ` David Mosberger
2004-01-14 22:33 ` Tom Rini
2004-01-14 23:00 ` Rusty Russell
1 sibling, 1 reply; 5+ messages in thread
From: David Mosberger @ 2004-01-14 22:23 UTC (permalink / raw)
To: Tom Rini
Cc: Paul Mackerras, Benjamin Herrenschmidt, Christoph Hellwig,
Andrew Morton, Rusty Russell, Kernel Mailing List
>>>>> On Wed, 14 Jan 2004 14:09:37 -0700, Tom Rini <trini@kernel.crashing.org> said:
Tom> The following patch fixes the problem for me on PPC32:
Tom> --- 1.96/kernel/module.c Wed Jan 7 22:46:59 2004
Tom> +++ edited/kernel/module.c Wed Jan 14 14:05:12 2004
Tom> @@ -1439,6 +1439,13 @@
Tom> strindex = sechdrs[i].sh_link;
Tom> strtab = (char *)hdr + sechdrs[strindex].sh_offset;
Tom> }
Tom> +
Tom> + /* If we find any debug RELAs, frob these away now. */
Tom> + if (sechdrs[i].sh_type == SHT_RELA &&
Tom> + (strstr(secstrings+sechdrs[i].sh_name, ".debug")
Tom> + != 0))
Tom> + sechdrs[i].sh_type = SHT_NULL;
Tom> +
Tom> #ifndef CONFIG_MODULE_UNLOAD
Tom> /* Don't load .exit sections */
Tom> if (strncmp(secstrings+sechdrs[i].sh_name, ".exit", 5) == 0)
Tom> IMHO, this shouldn't be covered under a PPC32 test since at
Tom> least PPC32, PPC64 and Alpha have this issue, and I suspect
Tom> that ia64, parisc, s390 and v850 have the problem as well
Tom> (based on what their module_arch_frob bits look to be doing).
As far as ia64 is concerned, adding a check for .debug should be OK,
but since the debug sections do not have any relocs anyhow, it
shouldn't make much of a difference one way or another (addresses in
the debug section a segment-relative).
--david
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH][RFC] 2.6 && module + -g && kernel w/o -g
2004-01-14 22:23 ` David Mosberger
@ 2004-01-14 22:33 ` Tom Rini
0 siblings, 0 replies; 5+ messages in thread
From: Tom Rini @ 2004-01-14 22:33 UTC (permalink / raw)
To: davidm
Cc: Paul Mackerras, Benjamin Herrenschmidt, Christoph Hellwig,
Andrew Morton, Rusty Russell, Kernel Mailing List
On Wed, Jan 14, 2004 at 02:23:29PM -0800, David Mosberger wrote:
> >>>>> On Wed, 14 Jan 2004 14:09:37 -0700, Tom Rini <trini@kernel.crashing.org> said:
>
> Tom> The following patch fixes the problem for me on PPC32:
>
> Tom> --- 1.96/kernel/module.c Wed Jan 7 22:46:59 2004
> Tom> +++ edited/kernel/module.c Wed Jan 14 14:05:12 2004
> Tom> @@ -1439,6 +1439,13 @@
> Tom> strindex = sechdrs[i].sh_link;
> Tom> strtab = (char *)hdr + sechdrs[strindex].sh_offset;
> Tom> }
> Tom> +
> Tom> + /* If we find any debug RELAs, frob these away now. */
> Tom> + if (sechdrs[i].sh_type == SHT_RELA &&
> Tom> + (strstr(secstrings+sechdrs[i].sh_name, ".debug")
> Tom> + != 0))
> Tom> + sechdrs[i].sh_type = SHT_NULL;
> Tom> +
> Tom> #ifndef CONFIG_MODULE_UNLOAD
> Tom> /* Don't load .exit sections */
> Tom> if (strncmp(secstrings+sechdrs[i].sh_name, ".exit", 5) == 0)
>
> Tom> IMHO, this shouldn't be covered under a PPC32 test since at
> Tom> least PPC32, PPC64 and Alpha have this issue, and I suspect
> Tom> that ia64, parisc, s390 and v850 have the problem as well
> Tom> (based on what their module_arch_frob bits look to be doing).
>
> As far as ia64 is concerned, adding a check for .debug should be OK,
> but since the debug sections do not have any relocs anyhow, it
> shouldn't make much of a difference one way or another (addresses in
> the debug section a segment-relative).
OK, I wasn't sure. I just did a real quick skim of everyones module.c
to see if they did any for loops and checking of SHT_RELA.
--
Tom Rini
http://gate.crashing.org/~trini/
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH][RFC] 2.6 && module + -g && kernel w/o -g
2004-01-14 21:09 ` [PATCH][RFC] 2.6 && module + -g && kernel w/o -g Tom Rini
2004-01-14 22:23 ` David Mosberger
@ 2004-01-14 23:00 ` Rusty Russell
2004-01-15 15:38 ` Tom Rini
1 sibling, 1 reply; 5+ messages in thread
From: Rusty Russell @ 2004-01-14 23:00 UTC (permalink / raw)
To: Tom Rini, Paul Mackerras, Benjamin Herrenschmidt,
Christoph Hellwig, Andrew Morton, Rusty Russell
Cc: Kernel Mailing List
In message <20040114210937.GA983@stop.crashing.org> you write:
> Okay. I've been looking at stock 2.6.1 noticed that the fix for this
> issue that Rusty proposed, and that ultimately made it into 2.6.1-rc3
> (or so) is not correct. The problem is that we do:
>
> err = module_frob_arch_sections(hdr, sechdrs, secstrings, mod);
> /* Which goes over every .debug section and can take _ages_ on something
> * like ipv6 */
Right. So the arch-specific module_frob_arch_sections() can be slow.
Logically, the fix should be in those module_frob_arch_sections(), not
in the generic code.
> + /* If we find any debug RELAs, frob these away now. */
> + if (sechdrs[i].sh_type == SHT_RELA &&
> + (strstr(secstrings+sechdrs[i].sh_name, ".debug")
> + != 0))
> + sechdrs[i].sh_type = SHT_NULL;
> +
Doesn't cover SHT_REL, and I really dislike name matches: they've bitten
us before.
Really, I prefer the arch-specific optimization.
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH][RFC] 2.6 && module + -g && kernel w/o -g
2004-01-14 23:00 ` Rusty Russell
@ 2004-01-15 15:38 ` Tom Rini
0 siblings, 0 replies; 5+ messages in thread
From: Tom Rini @ 2004-01-15 15:38 UTC (permalink / raw)
To: Rusty Russell
Cc: Paul Mackerras, Benjamin Herrenschmidt, Christoph Hellwig,
Andrew Morton, Kernel Mailing List
On Thu, Jan 15, 2004 at 10:00:11AM +1100, Rusty Russell wrote:
> In message <20040114210937.GA983@stop.crashing.org> you write:
> > Okay. I've been looking at stock 2.6.1 noticed that the fix for this
> > issue that Rusty proposed, and that ultimately made it into 2.6.1-rc3
> > (or so) is not correct. The problem is that we do:
> >
> > err = module_frob_arch_sections(hdr, sechdrs, secstrings, mod);
> > /* Which goes over every .debug section and can take _ages_ on something
> > * like ipv6 */
>
> Right. So the arch-specific module_frob_arch_sections() can be slow.
> Logically, the fix should be in those module_frob_arch_sections(), not
> in the generic code.
So it was right the first time, OK. :)
> > + /* If we find any debug RELAs, frob these away now. */
> > + if (sechdrs[i].sh_type == SHT_RELA &&
> > + (strstr(secstrings+sechdrs[i].sh_name, ".debug")
> > + != 0))
> > + sechdrs[i].sh_type = SHT_NULL;
> > +
>
> Doesn't cover SHT_REL, and I really dislike name matches: they've bitten
> us before.
>
> Really, I prefer the arch-specific optimization.
FWIW, this isn't an optimization, taking 12 minutes to load the ipv6
module is a bug. :)
Andrew, can you please apply the following patch? Thanks.
--- 1.10/arch/ppc/kernel/module.c Fri Sep 12 09:26:52 2003
+++ edited/arch/ppc/kernel/module.c Thu Jan 15 08:35:40 2004
@@ -88,6 +88,10 @@
!= is_init)
continue;
+ /* We don't want to look at debug sections. */
+ if (strstr(secstrings + sechdrs[i].sh_name, ".debug") != 0)
+ continue;
+
if (sechdrs[i].sh_type == SHT_RELA) {
DEBUGP("Found relocations in section %u\n", i);
DEBUGP("Ptr: %p. Number: %u\n",
--
Tom Rini
http://gate.crashing.org/~trini/
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2004-01-15 15:38 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20040108003040.GA18481@stop.crashing.org>
2004-01-14 21:09 ` [PATCH][RFC] 2.6 && module + -g && kernel w/o -g Tom Rini
2004-01-14 22:23 ` David Mosberger
2004-01-14 22:33 ` Tom Rini
2004-01-14 23:00 ` Rusty Russell
2004-01-15 15:38 ` Tom Rini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox