diff mbox

[1/2] Allow constructor name selection by architecture.

Message ID CAE1uF8dCy3m3TO4xa6Cc3i-CM3QYUFADZH_6o+hgy6MSPEWnyQ@mail.gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

George G. Davis April 1, 2013, 10:21 p.m. UTC
On Mon, Apr 1, 2013 at 5:58 PM, Sam Ravnborg <sam@ravnborg.org> wrote:
> On Mon, Apr 01, 2013 at 05:47:38PM -0400, George G. Davis wrote:
>> On Jun 6, 2012, at 6:12 AM, Russell King - ARM Linux wrote:
>> > On Tue, May 29, 2012 at 10:06:14AM +0100, Russell King - ARM Linux wrote:
>> >> On Mon, May 28, 2012 at 10:30:05PM +0200, Sam Ravnborg wrote:
>> >>> On Mon, May 28, 2012 at 07:33:37PM +0100, Vincent Sanders wrote:
>> >>>> From: Vincent Sanders <vince@collabora.co.uk>
>> >>>>
>> >>>> The constructor symbol name is different between platforms. Allow this
>> >>>> to be selected by configuration and set suitable default values.
>> >>>>
>> >>>> Signed-off-by: Vincent Sanders <vincent.sanders@collabora.co.uk>
>> >>>> ---
>> >>>> include/asm-generic/vmlinux.lds.h |    6 +++---
>> >>>> init/Kconfig                      |    6 ++++++
>> >>>> kernel/module.c                   |    2 +-
>> >>>> 3 files changed, 10 insertions(+), 4 deletions(-)
>> >>>>
>> >>>> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
>> >>>> index 8aeadf6..fd34808 100644
>> >>>> --- a/include/asm-generic/vmlinux.lds.h
>> >>>> +++ b/include/asm-generic/vmlinux.lds.h
>> >>>> @@ -471,9 +471,9 @@
>> >>>>  }
>> >>>>
>> >>>> #ifdef CONFIG_CONSTRUCTORS
>> >>>> -#define KERNEL_CTORS()  . = ALIGN(8);                      \
>> >>>> -                        VMLINUX_SYMBOL(__ctors_start) = .; \
>> >>>> -                        *(.ctors)                          \
>> >>>> +#define KERNEL_CTORS()  . = ALIGN(8);                                   \
>> >>>> +                        VMLINUX_SYMBOL(__ctors_start) = .;              \
>> >>>> +                        *(CONFIG_CONSTRUCTORS_NAME)                     \
>> >>>>                  VMLINUX_SYMBOL(__ctors_end) = .;
>> >>>
>> >>> What is wrong with adding both "standard" names for ctors uncnditionally?
>> >>> Like this:
>> >>>>                  *(.ctors)                          \
>> >>>> +                        *(.init_array)                     \
>> >>
>> >> That doesn't get rid of CONFIG_CONSTRUCTORS_NAME, because it's needed
>> >> in the module code.  Do you have a suggestion to solve that as well?
>> >
>> > Ping.
>>
>> Pinging this back to life.  I'd like to see GCOV for ARM eABI finally make it upstream.
>> So, any objections to this?  Should it be resubmitted?
>
> Why is CONFIG_CONSTRUCTORS_NAME needed in module code?

Because ARM eABI uses the .init_array section for C++ constructors,
rather than .ctors.
So the patch defined CONFIG_CONSTRUCTORS_NAME to set the correct section name
for ARM eABI while leaving non-ARM-eABI as before, .ctors.  Here are the changes
from the patch for reference:


W/o the above, GCOV does not work on ARM eABI for kernel modules.
Meanwhile, it still works as before for non-ARM-eABI kernel modules.

Thanks!

--
Regards,
George

Comments

Russell King - ARM Linux April 18, 2013, 12:39 p.m. UTC | #1
Looks to me like this has died again.  Unless Sam responds shortly,
I'm going to ask for this to be put into the patch system and I'll
just apply it for 3.10 and be done with it.

On Mon, Apr 01, 2013 at 06:21:51PM -0400, George G. Davis wrote:
> On Mon, Apr 1, 2013 at 5:58 PM, Sam Ravnborg <sam@ravnborg.org> wrote:
> > On Mon, Apr 01, 2013 at 05:47:38PM -0400, George G. Davis wrote:
> >> On Jun 6, 2012, at 6:12 AM, Russell King - ARM Linux wrote:
> >> > On Tue, May 29, 2012 at 10:06:14AM +0100, Russell King - ARM Linux wrote:
> >> >> On Mon, May 28, 2012 at 10:30:05PM +0200, Sam Ravnborg wrote:
> >> >>> On Mon, May 28, 2012 at 07:33:37PM +0100, Vincent Sanders wrote:
> >> >>>> From: Vincent Sanders <vince@collabora.co.uk>
> >> >>>>
> >> >>>> The constructor symbol name is different between platforms. Allow this
> >> >>>> to be selected by configuration and set suitable default values.
> >> >>>>
> >> >>>> Signed-off-by: Vincent Sanders <vincent.sanders@collabora.co.uk>
> >> >>>> ---
> >> >>>> include/asm-generic/vmlinux.lds.h |    6 +++---
> >> >>>> init/Kconfig                      |    6 ++++++
> >> >>>> kernel/module.c                   |    2 +-
> >> >>>> 3 files changed, 10 insertions(+), 4 deletions(-)
> >> >>>>
> >> >>>> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> >> >>>> index 8aeadf6..fd34808 100644
> >> >>>> --- a/include/asm-generic/vmlinux.lds.h
> >> >>>> +++ b/include/asm-generic/vmlinux.lds.h
> >> >>>> @@ -471,9 +471,9 @@
> >> >>>>  }
> >> >>>>
> >> >>>> #ifdef CONFIG_CONSTRUCTORS
> >> >>>> -#define KERNEL_CTORS()  . = ALIGN(8);                      \
> >> >>>> -                        VMLINUX_SYMBOL(__ctors_start) = .; \
> >> >>>> -                        *(.ctors)                          \
> >> >>>> +#define KERNEL_CTORS()  . = ALIGN(8);                                   \
> >> >>>> +                        VMLINUX_SYMBOL(__ctors_start) = .;              \
> >> >>>> +                        *(CONFIG_CONSTRUCTORS_NAME)                     \
> >> >>>>                  VMLINUX_SYMBOL(__ctors_end) = .;
> >> >>>
> >> >>> What is wrong with adding both "standard" names for ctors uncnditionally?
> >> >>> Like this:
> >> >>>>                  *(.ctors)                          \
> >> >>>> +                        *(.init_array)                     \
> >> >>
> >> >> That doesn't get rid of CONFIG_CONSTRUCTORS_NAME, because it's needed
> >> >> in the module code.  Do you have a suggestion to solve that as well?
> >> >
> >> > Ping.
> >>
> >> Pinging this back to life.  I'd like to see GCOV for ARM eABI finally make it upstream.
> >> So, any objections to this?  Should it be resubmitted?
> >
> > Why is CONFIG_CONSTRUCTORS_NAME needed in module code?
> 
> Because ARM eABI uses the .init_array section for C++ constructors,
> rather than .ctors.
> So the patch defined CONFIG_CONSTRUCTORS_NAME to set the correct section name
> for ARM eABI while leaving non-ARM-eABI as before, .ctors.  Here are the changes
> from the patch for reference:
> 
> diff --git a/init/Kconfig b/init/Kconfig
> index 6cfd71d..52181a1 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -20,6 +20,12 @@ config CONSTRUCTORS
>         bool
>         depends on !UML
> 
> +config CONSTRUCTORS_NAME
> +       string
> +       depends on CONSTRUCTORS
> +       default ".init_array" if ARM && AEABI
> +       default ".ctors"
> +
>  config HAVE_IRQ_WORK
>         bool
> 
> diff --git a/kernel/module.c b/kernel/module.c
> index 78ac6ec..e5fad5e 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2600,7 +2600,7 @@ static void find_module_sections(struct module
> *mod, struct load_info *info)
>         mod->unused_gpl_crcs = section_addr(info, "__kcrctab_unused_gpl");
>  #endif
>  #ifdef CONFIG_CONSTRUCTORS
> -       mod->ctors = section_objs(info, ".ctors",
> +       mod->ctors = section_objs(info, CONFIG_CONSTRUCTORS_NAME,
>                                   sizeof(*mod->ctors), &mod->num_ctors);
>  #endif
> 
> W/o the above, GCOV does not work on ARM eABI for kernel modules.
> Meanwhile, it still works as before for non-ARM-eABI kernel modules.
> 
> Thanks!
> 
> --
> Regards,
> George
Sam Ravnborg April 22, 2013, 8:56 a.m. UTC | #2
On Thu, Apr 18, 2013 at 01:39:31PM +0100, Russell King - ARM Linux wrote:
> Looks to me like this has died again.  Unless Sam responds shortly,
> I'm going to ask for this to be put into the patch system and I'll
> just apply it for 3.10 and be done with it.

I had preferred it was a solution where we supported both
variants in the code.
But I do not feel strongly about it.

So you can add my ack and apply it.
Acked-by: Sam Ravnborg <sam@ravnborg.org>

	Sam
diff mbox

Patch

diff --git a/init/Kconfig b/init/Kconfig
index 6cfd71d..52181a1 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -20,6 +20,12 @@  config CONSTRUCTORS
        bool
        depends on !UML

+config CONSTRUCTORS_NAME
+       string
+       depends on CONSTRUCTORS
+       default ".init_array" if ARM && AEABI
+       default ".ctors"
+
 config HAVE_IRQ_WORK
        bool

diff --git a/kernel/module.c b/kernel/module.c
index 78ac6ec..e5fad5e 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2600,7 +2600,7 @@  static void find_module_sections(struct module
*mod, struct load_info *info)
        mod->unused_gpl_crcs = section_addr(info, "__kcrctab_unused_gpl");
 #endif
 #ifdef CONFIG_CONSTRUCTORS
-       mod->ctors = section_objs(info, ".ctors",
+       mod->ctors = section_objs(info, CONFIG_CONSTRUCTORS_NAME,
                                  sizeof(*mod->ctors), &mod->num_ctors);
 #endif