Re: [PATCH] Add non-zero module sections to sysfs

2013-04-11 Thread Philip Kranz
On Mon, Apr 08, 2013 at 01:44:45PM +0930, Rusty Russell wrote:
> Philip Kranz  writes:
> > Hello.
> >
> > On Fri, Apr 05, 2013 at 12:07:15PM +0200, James Bottomley wrote:
> >> Just so you know: this isn't a parisc specific problem.  Gcc produces
> >> duplicate section names under various circumstances, but the one that
> >> bites us is -ffunction-sections.  Note that there are proposals to use
> >> -ffunction-sections on all architectures (so we can garbage collect
> >> unused functions) in which case you'll induce the bug identified in
> >> 35dead4235e2b67da7275b4122fed37099c2f462 on every architecture
> >
> > I am not able to produce an object file with duplicate section names
> > using gcc on x86. Even with -ffunction-sections, every section gets a
> > unique name. Is this architecture-specific behaviour of gcc?
> 
> Good point.  ld -r will collapse them into the same section (since gcc
> produces them they have to have the same section attributes).
> 
> You can do it with --unique, but no arch uses that.  PARISC has a
> platform-specific toolchain hack which does that for .text sections.
> (Thanks to Alan Modra for that clue...)

Just for clarification, as we are currently preparing a patch set that
depends on this: Would the patch below be an acceptable solution for
this?

Thanks,
Philip



diff --git a/arch/parisc/kernel/module.c b/arch/parisc/kernel/module.c
index 2a625fb..1fd4411 100644
--- a/arch/parisc/kernel/module.c
+++ b/arch/parisc/kernel/module.c
@@ -341,6 +341,11 @@ int module_frob_arch_sections(CONST Elf_Ehdr *hdr,
".PARISC.unwind", 14) == 0)
me->arch.unwind_section = i;
 
+   /* we produce multiple, empty .text sections, and kallsyms
+   * gets upset.  make non-alloc so it doesn't see them. */
+   if (sechdrs[i].sh_size == 0)
+   sechdrs[i].sh_flags &= ~SHF_ALLOC;
+
if (sechdrs[i].sh_type != SHT_RELA)
continue;
 
diff --git a/kernel/module.c b/kernel/module.c
index 3c2c72d..42e0d5a 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1316,7 +1316,7 @@ resolve_symbol_wait(struct module *mod,
 #ifdef CONFIG_KALLSYMS
 static inline bool sect_empty(const Elf_Shdr *sect)
 {
-   return !(sect->sh_flags & SHF_ALLOC) || sect->sh_size == 0;
+   return !(sect->sh_flags & SHF_ALLOC);
 }
 
 struct module_sect_attr

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Add non-zero module sections to sysfs

2013-04-08 Thread Philip Kranz
On Mon, Apr 08, 2013 at 01:44:45PM +0930, Rusty Russell wrote:
> Philip Kranz  writes:
> > I am not able to produce an object file with duplicate section names
> > using gcc on x86. Even with -ffunction-sections, every section gets a
> > unique name. Is this architecture-specific behaviour of gcc?
> 
> Good point.  ld -r will collapse them into the same section (since gcc
> produces them they have to have the same section attributes).
> 
> You can do it with --unique, but no arch uses that.  PARISC has a
> platform-specific toolchain hack which does that for .text sections.
> (Thanks to Alan Modra for that clue...)

So that problem is indeed platform-specific. If it is safe to assume
that kernel modules don't have duplicate section names (except on
PARISC), it would make sense to simply move the check for empty sections
to arch/parisc as you suggested.

James, what do you think about that?

Greetings,
Philip
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Add non-zero module sections to sysfs

2013-04-07 Thread Rusty Russell
Philip Kranz  writes:
> Hello.
>
> On Fri, Apr 05, 2013 at 12:07:15PM +0200, James Bottomley wrote:
>> Just so you know: this isn't a parisc specific problem.  Gcc produces
>> duplicate section names under various circumstances, but the one that
>> bites us is -ffunction-sections.  Note that there are proposals to use
>> -ffunction-sections on all architectures (so we can garbage collect
>> unused functions) in which case you'll induce the bug identified in
>> 35dead4235e2b67da7275b4122fed37099c2f462 on every architecture
>
> I am not able to produce an object file with duplicate section names
> using gcc on x86. Even with -ffunction-sections, every section gets a
> unique name. Is this architecture-specific behaviour of gcc?

Good point.  ld -r will collapse them into the same section (since gcc
produces them they have to have the same section attributes).

You can do it with --unique, but no arch uses that.  PARISC has a
platform-specific toolchain hack which does that for .text sections.
(Thanks to Alan Modra for that clue...)

Thanks,
Rusty.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Add non-zero module sections to sysfs

2013-04-06 Thread John David Anglin

On 6-Apr-13, at 9:22 PM, James Bottomley wrote:




John David Anglin  wrote:


On 6-Apr-13, at 6:52 AM, James Bottomley wrote:


On Sat, 2013-04-06 at 15:22 +1030, Rusty Russell wrote:

The problem is our assumption that section names be unique.  This
assumption is wrong.  The ELF spec says (version 1.1 page 1-15):

"An

object file may have more than one section with the same name."
We need
to fix the kernel not to rely on a bogus assumption ... but we had



no
idea how to do that in a way that preserved the backwards
compatibility
of sections subdirectory.

I admit that 35dead4235e2b67da7275b4122fed37099c2f462 is a hack,
but now
the problem has got attention, can we fix it properly?


Yep.  The original patch didn't go through me, or we would have had



this
discussion back then...

The use of section names in sysfs goes back to one Mr. Corbet.  Why



did
he do it that way?  Because gdb's add-symbol-file makes the same
assumption.  So if we fixed the sysfs somehow, it still wouldn't be
useful, since there's no way to tell gdb :(

The real answer don't use -ffunction-sections on modules: probably
not
as important as the rest of the kernel.  And the new shiny is
-flto anyway.

And that leaves us with a PA-RISC specific issue, for which we

should

move the fix to PA-RISC.

Thoughts?


Well, we don't have much of a choice.  Our ELF stub jump on 32 bits
is a
PCREL17.  That means once a module size is over 128k there's a
chance we
might not be able to link it because the jump is too big for the
instruction.  IPV6 is one such big module today, but I'm sure there
are
others.  The only way I know to fix this is to allow the linker to
insert stubs between functions, so we only fail at linking if a

single

function is >128k big.  The way to do this is -ffunction-sections,
unless there's something else we could do (all we really need is a

way

to ensure we can insert ELF stubs every 128k).


There is now a config work around for this.  See:
http://www.spinics.net/lists/linux-parisc/msg04521.html


The longcalls config option only works on pa2 doesn't it? Although  
we could just deprecate pa1.



No, it works on pa1.1 but the calls are more efficient on pa2.  On  
linux with a flat space, they should be
about the same in terms of instruction count.  On HP-UX, an additional  
space register load is needed.
For calls within the same space, the be instruction is pretty  
efficient but we never implemented linker
support for it in binutils.  If I recall correctly, it works if the  
call is local to a module but not for global calls.

It does work in HP-UX.

Dave
--
John David Anglin   dave.ang...@bell.net



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Add non-zero module sections to sysfs

2013-04-06 Thread James Bottomley


John David Anglin  wrote:

>On 6-Apr-13, at 6:52 AM, James Bottomley wrote:
>
>> On Sat, 2013-04-06 at 15:22 +1030, Rusty Russell wrote:
 The problem is our assumption that section names be unique.  This
 assumption is wrong.  The ELF spec says (version 1.1 page 1-15):
>"An
 object file may have more than one section with the same name."   
 We need
 to fix the kernel not to rely on a bogus assumption ... but we had 
>
 no
 idea how to do that in a way that preserved the backwards  
 compatibility
 of sections subdirectory.

 I admit that 35dead4235e2b67da7275b4122fed37099c2f462 is a hack,  
 but now
 the problem has got attention, can we fix it properly?
>>>
>>> Yep.  The original patch didn't go through me, or we would have had 
>
>>> this
>>> discussion back then...
>>>
>>> The use of section names in sysfs goes back to one Mr. Corbet.  Why 
>
>>> did
>>> he do it that way?  Because gdb's add-symbol-file makes the same
>>> assumption.  So if we fixed the sysfs somehow, it still wouldn't be
>>> useful, since there's no way to tell gdb :(
>>>
>>> The real answer don't use -ffunction-sections on modules: probably  
>>> not
>>> as important as the rest of the kernel.  And the new shiny is
>>> -flto anyway.
>>>
>>> And that leaves us with a PA-RISC specific issue, for which we
>should
>>> move the fix to PA-RISC.
>>>
>>> Thoughts?
>>
>> Well, we don't have much of a choice.  Our ELF stub jump on 32 bits  
>> is a
>> PCREL17.  That means once a module size is over 128k there's a  
>> chance we
>> might not be able to link it because the jump is too big for the
>> instruction.  IPV6 is one such big module today, but I'm sure there  
>> are
>> others.  The only way I know to fix this is to allow the linker to
>> insert stubs between functions, so we only fail at linking if a
>single
>> function is >128k big.  The way to do this is -ffunction-sections,
>> unless there's something else we could do (all we really need is a
>way
>> to ensure we can insert ELF stubs every 128k).
>
>There is now a config work around for this.  See:
>http://www.spinics.net/lists/linux-parisc/msg04521.html

The longcalls config option only works on pa2 doesn't it? Although we could 
just deprecate pa1.

James
-- 
Sent from my Android phone with K-9 Mail. Please excuse my brevity and top 
posting.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Add non-zero module sections to sysfs

2013-04-06 Thread John David Anglin

On 6-Apr-13, at 6:52 AM, James Bottomley wrote:


On Sat, 2013-04-06 at 15:22 +1030, Rusty Russell wrote:

The problem is our assumption that section names be unique.  This
assumption is wrong.  The ELF spec says (version 1.1 page 1-15): "An
object file may have more than one section with the same name."   
We need
to fix the kernel not to rely on a bogus assumption ... but we had  
no
idea how to do that in a way that preserved the backwards  
compatibility

of sections subdirectory.

I admit that 35dead4235e2b67da7275b4122fed37099c2f462 is a hack,  
but now

the problem has got attention, can we fix it properly?


Yep.  The original patch didn't go through me, or we would have had  
this

discussion back then...

The use of section names in sysfs goes back to one Mr. Corbet.  Why  
did

he do it that way?  Because gdb's add-symbol-file makes the same
assumption.  So if we fixed the sysfs somehow, it still wouldn't be
useful, since there's no way to tell gdb :(

The real answer don't use -ffunction-sections on modules: probably  
not

as important as the rest of the kernel.  And the new shiny is
-flto anyway.

And that leaves us with a PA-RISC specific issue, for which we should
move the fix to PA-RISC.

Thoughts?


Well, we don't have much of a choice.  Our ELF stub jump on 32 bits  
is a
PCREL17.  That means once a module size is over 128k there's a  
chance we

might not be able to link it because the jump is too big for the
instruction.  IPV6 is one such big module today, but I'm sure there  
are

others.  The only way I know to fix this is to allow the linker to
insert stubs between functions, so we only fail at linking if a single
function is >128k big.  The way to do this is -ffunction-sections,
unless there's something else we could do (all we really need is a way
to ensure we can insert ELF stubs every 128k).


There is now a config work around for this.  See:
http://www.spinics.net/lists/linux-parisc/msg04521.html

Dave
--
John David Anglin   dave.ang...@bell.net



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Add non-zero module sections to sysfs

2013-04-06 Thread James Bottomley
On Sat, 2013-04-06 at 15:22 +1030, Rusty Russell wrote:
> > The problem is our assumption that section names be unique.  This
> > assumption is wrong.  The ELF spec says (version 1.1 page 1-15): "An
> > object file may have more than one section with the same name."  We need
> > to fix the kernel not to rely on a bogus assumption ... but we had no
> > idea how to do that in a way that preserved the backwards compatibility
> > of sections subdirectory.
> >
> > I admit that 35dead4235e2b67da7275b4122fed37099c2f462 is a hack, but now
> > the problem has got attention, can we fix it properly?
> 
> Yep.  The original patch didn't go through me, or we would have had this
> discussion back then...
>
> The use of section names in sysfs goes back to one Mr. Corbet.  Why did
> he do it that way?  Because gdb's add-symbol-file makes the same
> assumption.  So if we fixed the sysfs somehow, it still wouldn't be
> useful, since there's no way to tell gdb :(
> 
> The real answer don't use -ffunction-sections on modules: probably not
> as important as the rest of the kernel.  And the new shiny is
> -flto anyway.
> 
> And that leaves us with a PA-RISC specific issue, for which we should
> move the fix to PA-RISC.
> 
> Thoughts?

Well, we don't have much of a choice.  Our ELF stub jump on 32 bits is a
PCREL17.  That means once a module size is over 128k there's a chance we
might not be able to link it because the jump is too big for the
instruction.  IPV6 is one such big module today, but I'm sure there are
others.  The only way I know to fix this is to allow the linker to
insert stubs between functions, so we only fail at linking if a single
function is >128k big.  The way to do this is -ffunction-sections,
unless there's something else we could do (all we really need is a way
to ensure we can insert ELF stubs every 128k).

We're not the only architecture that has these problems: frv, metag and
score seem to as well.

James


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Add non-zero module sections to sysfs

2013-04-06 Thread Philip Kranz
Hello.

On Fri, Apr 05, 2013 at 12:07:15PM +0200, James Bottomley wrote:
> Just so you know: this isn't a parisc specific problem.  Gcc produces
> duplicate section names under various circumstances, but the one that
> bites us is -ffunction-sections.  Note that there are proposals to use
> -ffunction-sections on all architectures (so we can garbage collect
> unused functions) in which case you'll induce the bug identified in
> 35dead4235e2b67da7275b4122fed37099c2f462 on every architecture

I am not able to produce an object file with duplicate section names
using gcc on x86. Even with -ffunction-sections, every section gets a
unique name. Is this architecture-specific behaviour of gcc?

Greetings,
Philip

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Add non-zero module sections to sysfs

2013-04-05 Thread Rusty Russell
Sebastian Wankerl  writes:
> On 04/05/13 06:00, Rusty Russell wrote:
>> Sebastian Wankerl  writes:
>>> On 04/04/13 03:00, Rusty Russell wrote:
 Sebastian Wankerl  writes:
> Add non-zero module sections to sysfs on architectures unequal to PARISC.
> KGDB needs all module sections for proper module debugging. Therefore, 
> commit 
> 35dead4235e2b67da7275b4122fed37099c2f462 is revoked except for PARISC
> architecture.
 #ifdef CONFIG_PARISC in the middle of kernel/module.c is super-ugly, and
 wrong.
>>> I don't see why this is wrong. It used to load all sections to sysfs
>>> until the patch mentioned. Actually, it is the PARISC build chain which
>>> is broken.
>
> We worked on that topic further. Now we have another suggestion: would
> it be okay to add a field to struct module for use by kgdb where we save
> the section names for our use. This seems to be the most valuable
> solution as solving the sysfs stuff is rather hard.

It is hard.  But being a kernel hacker isn't just about making newbies
eat flaming death; sometimes we need to solve problems.

We'll see what we can do; but we'll continue this in the branch of the
thread that cc's linux-parisc...

Thanks,
Rusty.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Add non-zero module sections to sysfs

2013-04-05 Thread Rusty Russell
James Bottomley  writes:
> On Fri, 2013-04-05 at 14:30 +1030, Rusty Russell wrote:
>> Sebastian Wankerl  writes:
>> > On 04/04/13 03:00, Rusty Russell wrote:
>> >> Sebastian Wankerl  writes:
>> >>> Add non-zero module sections to sysfs on architectures unequal to PARISC.
>> >>> KGDB needs all module sections for proper module debugging. Therefore, 
>> >>> commit 
>> >>> 35dead4235e2b67da7275b4122fed37099c2f462 is revoked except for PARISC
>> >>> architecture.
>
> Thanks for actually cc'ing us.
>
>> >> #ifdef CONFIG_PARISC in the middle of kernel/module.c is super-ugly, and
>> >> wrong.
>> >
>> > I don't see why this is wrong. It used to load all sections to sysfs
>> > until the patch mentioned. Actually, it is the PARISC build chain which
>> > is broken.
>> 
>> Exactly.  Don't workaround it here, revert it and put the
>> duplicate-section-name fixup in parisc where it belongs.
>> 
>> Assuming parisc still produces these dup sections: that patch is 4 years
>> old now.
>
> Just so you know: this isn't a parisc specific problem.  Gcc produces
> duplicate section names under various circumstances, but the one that
> bites us is -ffunction-sections.

*This* is a PA-RISC specific issue.  -ffunction-sections is a different
problem, which this hack wouldn't help.

> Note that there are proposals to use
> -ffunction-sections on all architectures (so we can garbage collect
> unused functions) in which case you'll induce the bug identified in
> 35dead4235e2b67da7275b4122fed37099c2f462 on every architecture

Good point, though I note that we seem to have stalled on
-ffunction-sections.  (And I vaguely recall an issue with
-ffunction-sections and using ld -o which would fold duplicate named
sections back together reducing elimination opportunities).

> The problem is our assumption that section names be unique.  This
> assumption is wrong.  The ELF spec says (version 1.1 page 1-15): "An
> object file may have more than one section with the same name."  We need
> to fix the kernel not to rely on a bogus assumption ... but we had no
> idea how to do that in a way that preserved the backwards compatibility
> of sections subdirectory.
>
> I admit that 35dead4235e2b67da7275b4122fed37099c2f462 is a hack, but now
> the problem has got attention, can we fix it properly?

Yep.  The original patch didn't go through me, or we would have had this
discussion back then...

The use of section names in sysfs goes back to one Mr. Corbet.  Why did
he do it that way?  Because gdb's add-symbol-file makes the same
assumption.  So if we fixed the sysfs somehow, it still wouldn't be
useful, since there's no way to tell gdb :(

The real answer don't use -ffunction-sections on modules: probably not
as important as the rest of the kernel.  And the new shiny is
-flto anyway.

And that leaves us with a PA-RISC specific issue, for which we should
move the fix to PA-RISC.

Thoughts?
Rusty.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Add non-zero module sections to sysfs

2013-04-05 Thread Sebastian Wankerl
On 04/05/13 06:00, Rusty Russell wrote:
> Sebastian Wankerl  writes:
>> On 04/04/13 03:00, Rusty Russell wrote:
>>> Sebastian Wankerl  writes:
 Add non-zero module sections to sysfs on architectures unequal to PARISC.
 KGDB needs all module sections for proper module debugging. Therefore, 
 commit 
 35dead4235e2b67da7275b4122fed37099c2f462 is revoked except for PARISC
 architecture.
>>> #ifdef CONFIG_PARISC in the middle of kernel/module.c is super-ugly, and
>>> wrong.
>> I don't see why this is wrong. It used to load all sections to sysfs
>> until the patch mentioned. Actually, it is the PARISC build chain which
>> is broken.

We worked on that topic further. Now we have another suggestion: would
it be okay to add a field to struct module for use by kgdb where we save
the section names for our use. This seems to be the most valuable
solution as solving the sysfs stuff is rather hard.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Add non-zero module sections to sysfs

2013-04-05 Thread James Bottomley
On Fri, 2013-04-05 at 14:30 +1030, Rusty Russell wrote:
> Sebastian Wankerl  writes:
> > On 04/04/13 03:00, Rusty Russell wrote:
> >> Sebastian Wankerl  writes:
> >>> Add non-zero module sections to sysfs on architectures unequal to PARISC.
> >>> KGDB needs all module sections for proper module debugging. Therefore, 
> >>> commit 
> >>> 35dead4235e2b67da7275b4122fed37099c2f462 is revoked except for PARISC
> >>> architecture.

Thanks for actually cc'ing us.

> >> #ifdef CONFIG_PARISC in the middle of kernel/module.c is super-ugly, and
> >> wrong.
> >
> > I don't see why this is wrong. It used to load all sections to sysfs
> > until the patch mentioned. Actually, it is the PARISC build chain which
> > is broken.
> 
> Exactly.  Don't workaround it here, revert it and put the
> duplicate-section-name fixup in parisc where it belongs.
> 
> Assuming parisc still produces these dup sections: that patch is 4 years
> old now.

Just so you know: this isn't a parisc specific problem.  Gcc produces
duplicate section names under various circumstances, but the one that
bites us is -ffunction-sections.  Note that there are proposals to use
-ffunction-sections on all architectures (so we can garbage collect
unused functions) in which case you'll induce the bug identified in
35dead4235e2b67da7275b4122fed37099c2f462 on every architecture

The problem is our assumption that section names be unique.  This
assumption is wrong.  The ELF spec says (version 1.1 page 1-15): "An
object file may have more than one section with the same name."  We need
to fix the kernel not to rely on a bogus assumption ... but we had no
idea how to do that in a way that preserved the backwards compatibility
of sections subdirectory.

I admit that 35dead4235e2b67da7275b4122fed37099c2f462 is a hack, but now
the problem has got attention, can we fix it properly?

James


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Add non-zero module sections to sysfs

2013-04-05 Thread Sebastian Wankerl
On 04/05/13 06:00, Rusty Russell wrote:
> Exactly.  Don't workaround it here, revert it and put the
> duplicate-section-name fixup in parisc where it belongs.
>
> Assuming parisc still produces these dup sections: that patch is 4 years
> old now.
>
> Untested:
>
> diff --git a/arch/parisc/kernel/module.c b/arch/parisc/kernel/module.c
> index 2a625fb..28d32a2 100644
> --- a/arch/parisc/kernel/module.c
> +++ b/arch/parisc/kernel/module.c
> @@ -341,6 +341,11 @@ int module_frob_arch_sections(CONST Elf_Ehdr *hdr,
>   ".PARISC.unwind", 14) == 0)
>   me->arch.unwind_section = i;
>  
> + /* we produce multiple, empty .text sections, and kallsyms
> +  * gets upset.  make non-alloc so it doesn't see them. */
> + if (sechdrs[i].sh_size == 0)
> + sechdrs[i].sh_flags &= ~SHF_ALLOC;
> +
>   if (sechdrs[i].sh_type != SHT_RELA)
>   continue;
We just worked your suggested patch in.

> Why?  Does something refer to this empty section?  Why has noone noticed
> this since 2009?

GDB wants to know all section with attribute ALLOC, regardless whether
they are empty or not. Thus, it is useful if all of them appear in sysfs.
> A zero-length section doesn't change the binary's structure.  You don't
> see non-SHF_ALLOC sections either.

Yes, but they do occupy an index in the section headers of the binary.
GDB needs to know all of them in the right order.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Add non-zero module sections to sysfs

2013-04-04 Thread Rusty Russell
Sebastian Wankerl  writes:
> On 04/04/13 03:00, Rusty Russell wrote:
>> Sebastian Wankerl  writes:
>>> Add non-zero module sections to sysfs on architectures unequal to PARISC.
>>> KGDB needs all module sections for proper module debugging. Therefore, 
>>> commit 
>>> 35dead4235e2b67da7275b4122fed37099c2f462 is revoked except for PARISC
>>> architecture.
>> #ifdef CONFIG_PARISC in the middle of kernel/module.c is super-ugly, and
>> wrong.
>
> I don't see why this is wrong. It used to load all sections to sysfs
> until the patch mentioned. Actually, it is the PARISC build chain which
> is broken.

Exactly.  Don't workaround it here, revert it and put the
duplicate-section-name fixup in parisc where it belongs.

Assuming parisc still produces these dup sections: that patch is 4 years
old now.

Untested:

diff --git a/arch/parisc/kernel/module.c b/arch/parisc/kernel/module.c
index 2a625fb..28d32a2 100644
--- a/arch/parisc/kernel/module.c
+++ b/arch/parisc/kernel/module.c
@@ -341,6 +341,11 @@ int module_frob_arch_sections(CONST Elf_Ehdr *hdr,
".PARISC.unwind", 14) == 0)
me->arch.unwind_section = i;
 
+   /* we produce multiple, empty .text sections, and kallsyms
+* gets upset.  make non-alloc so it doesn't see them. */
+   if (sechdrs[i].sh_size == 0)
+   sechdrs[i].sh_flags &= ~SHF_ALLOC;
+
if (sechdrs[i].sh_type != SHT_RELA)
continue;
 
>> My preference would be to fix kgdb.  If the section is empty, what need
>> does it have to examine it?
>
> GDB needs to know all sections of the binary and its addresses.

Why?  Does something refer to this empty section?  Why has noone noticed
this since 2009?

> It is generally useful to be able to check up all sections of the binary
> regardless if they are empty or not so one can see the binary's
> structure.

A zero-length section doesn't change the binary's structure.  You don't
see non-SHF_ALLOC sections either.

Cheers,
Rusty.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Add non-zero module sections to sysfs

2013-04-04 Thread Sebastian Wankerl
On 04/04/13 03:00, Rusty Russell wrote:
> Sebastian Wankerl  writes:
>> Add non-zero module sections to sysfs on architectures unequal to PARISC.
>> KGDB needs all module sections for proper module debugging. Therefore, 
>> commit 
>> 35dead4235e2b67da7275b4122fed37099c2f462 is revoked except for PARISC
>> architecture.
> #ifdef CONFIG_PARISC in the middle of kernel/module.c is super-ugly, and
> wrong.

I don't see why this is wrong. It used to load all sections to sysfs
until the patch mentioned. Actually, it is the PARISC build chain which
is broken.

> My preference would be to fix kgdb.  If the section is empty, what need
> does it have to examine it?

GDB needs to know all sections of the binary and its addresses. It is
generally useful to be able to check up all sections of the binary
regardless if they are empty or not so one can see the binary's
structure. The alternative would be to redo the work by saving the ELF
header in an additional internal data structure that is not exported.
However, this approach is more ugly as it results in code which is
actually redundant.

Thanks,
Sebastian

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] Add non-zero module sections to sysfs

2013-04-03 Thread Rusty Russell
Sebastian Wankerl  writes:
> Add non-zero module sections to sysfs on architectures unequal to PARISC.
> KGDB needs all module sections for proper module debugging. Therefore, commit 
> 35dead4235e2b67da7275b4122fed37099c2f462 is revoked except for PARISC
> architecture.

#ifdef CONFIG_PARISC in the middle of kernel/module.c is super-ugly, and
wrong.

My preference would be to fix kgdb.  If the section is empty, what need
does it have to examine it?

Thanks,
Rusty.

> Signed-off-by: Sebastian Wankerl 
> Signed-off-by: Philip Kranz 
>
> ---
>  kernel/module.c |4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/kernel/module.c b/kernel/module.c
> index 3c2c72d..5393a54 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -1316,7 +1316,11 @@ resolve_symbol_wait(struct module *mod,
>  #ifdef CONFIG_KALLSYMS
>  static inline bool sect_empty(const Elf_Shdr *sect)
>  {
> +#if defined(CONFIG_PARISC)
>   return !(sect->sh_flags & SHF_ALLOC) || sect->sh_size == 0;
> +#else
> + return !(sect->sh_flags & SHF_ALLOC);
> +#endif
>  }
>  
>  struct module_sect_attr
> -- 
> 1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@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] Add non-zero module sections to sysfs

2013-04-03 Thread Sebastian Wankerl
Add non-zero module sections to sysfs on architectures unequal to PARISC.
KGDB needs all module sections for proper module debugging. Therefore, commit 
35dead4235e2b67da7275b4122fed37099c2f462 is revoked except for PARISC
architecture.

Signed-off-by: Sebastian Wankerl 
Signed-off-by: Philip Kranz 

---
 kernel/module.c |4 
 1 file changed, 4 insertions(+)

diff --git a/kernel/module.c b/kernel/module.c
index 3c2c72d..5393a54 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1316,7 +1316,11 @@ resolve_symbol_wait(struct module *mod,
 #ifdef CONFIG_KALLSYMS
 static inline bool sect_empty(const Elf_Shdr *sect)
 {
+#if defined(CONFIG_PARISC)
return !(sect->sh_flags & SHF_ALLOC) || sect->sh_size == 0;
+#else
+   return !(sect->sh_flags & SHF_ALLOC);
+#endif
 }
 
 struct module_sect_attr
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/