Add gcov support to ARM

Add gcov support to ARM

[Vincent Sanders]

These two patches add the necessary support for gcov on ARM. The idea
is based on an original patch in 2010 by George G. Davis
<gdavis@mvista.com> but re-implemented against 3.4.

The approach requires the constructor symbol name be adjusted as it is
different on ARM to other platforms (I have directly tested on x86 and
arm). Currently I cannot find another platform where the symbol name
is not .ctors but it can be readily added.

Once the constructors are called correctly the actual changes to the
ARM architecture is minimal and principally involves avoiding adding
gcov compilation options to the early boot code.

Add gcov support to ARM

[Vincent Sanders]

These two patches add the necessary support for gcov on ARM. The idea
is based on an original patch in 2010 by George G. Davis
<gdavis@mvista.com> but re-implemented against 3.4.

The approach requires the constructor symbol name be adjusted as it is
different on ARM to other platforms (I have directly tested on x86 and
arm). Currently I cannot find another platform where the symbol name
is not .ctors but it can be readily added.

Once the constructors are called correctly the actual changes to the
ARM architecture is minimal and principally involves avoiding adding
gcov compilation options to the early boot code.

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

[Vincent Sanders]

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) = .;
 #else
 #define KERNEL_CTORS()
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
 
-- 
1.7.10

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

[Vincent Sanders]

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) = .;
 #else
 #define KERNEL_CTORS()
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
 
-- 
1.7.10

[PATCH 2/2] Enable gcov support on the ARM architecture

[Vincent Sanders]

From: Vincent Sanders <vince@collabora.co.uk>

Enable gcov support for ARM based on original patches by David
Singleton and George G. Davis

Signed-off-by: Vincent Sanders <vincent.sanders@collabora.co.uk>
---
 arch/arm/boot/bootp/Makefile      |    2 ++
 arch/arm/boot/compressed/Makefile |    2 ++
 arch/arm/include/asm/elf.h        |    1 +
 arch/arm/kernel/module.c          |    1 +
 kernel/gcov/Kconfig               |    2 +-
 5 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/bootp/Makefile b/arch/arm/boot/bootp/Makefile
index c394e30..5761f00 100644
--- a/arch/arm/boot/bootp/Makefile
+++ b/arch/arm/boot/bootp/Makefile
@@ -5,6 +5,8 @@
 # architecture-specific flags and dependencies.
 #
 
+GCOV_PROFILE	:= n
+
 LDFLAGS_bootp	:=-p --no-undefined -X \
 		 --defsym initrd_phys=$(INITRD_PHYS) \
 		 --defsym params_phys=$(PARAMS_PHYS) -T
diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile
index bb26756..7482fcd 100644
--- a/arch/arm/boot/compressed/Makefile
+++ b/arch/arm/boot/compressed/Makefile
@@ -30,6 +30,8 @@ FONTC	= $(srctree)/drivers/video/console/font_acorn_8x8.c
 OBJS		+= string.o
 CFLAGS_string.o	:= -Os
 
+GCOV_PROFILE		:= n
+
 #
 # Architecture dependencies
 #
diff --git a/arch/arm/include/asm/elf.h b/arch/arm/include/asm/elf.h
index 38050b1..64afbd0 100644
--- a/arch/arm/include/asm/elf.h
+++ b/arch/arm/include/asm/elf.h
@@ -52,6 +52,7 @@ typedef struct user_fp elf_fpregset_t;
 #define R_ARM_ABS32		2
 #define R_ARM_CALL		28
 #define R_ARM_JUMP24		29
+#define R_ARM_TARGET1		38
 #define R_ARM_V4BX		40
 #define R_ARM_PREL31		42
 #define R_ARM_MOVW_ABS_NC	43
diff --git a/arch/arm/kernel/module.c b/arch/arm/kernel/module.c
index 1e9be5d..b22fc22 100644
--- a/arch/arm/kernel/module.c
+++ b/arch/arm/kernel/module.c
@@ -89,6 +89,7 @@ apply_relocate(Elf32_Shdr *sechdrs, const char *strtab, unsigned int symindex,
 			break;
 
 		case R_ARM_ABS32:
+		case R_ARM_TARGET1:
 			*(u32 *)loc += sym->st_value;
 			break;
 
diff --git a/kernel/gcov/Kconfig b/kernel/gcov/Kconfig
index a920281..2a02dae 100644
--- a/kernel/gcov/Kconfig
+++ b/kernel/gcov/Kconfig
@@ -35,7 +35,7 @@ config GCOV_KERNEL
 config GCOV_PROFILE_ALL
 	bool "Profile entire Kernel"
 	depends on GCOV_KERNEL
-	depends on SUPERH || S390 || X86 || (PPC && EXPERIMENTAL) || MICROBLAZE
+	depends on SUPERH || S390 || X86 || (PPC && EXPERIMENTAL) || MICROBLAZE || ARM
 	default n
 	---help---
 	This options activates profiling for the entire kernel.
-- 
1.7.10

[PATCH 2/2] Enable gcov support on the ARM architecture

[Vincent Sanders]

From: Vincent Sanders <vince@collabora.co.uk>

Enable gcov support for ARM based on original patches by David
Singleton and George G. Davis

Signed-off-by: Vincent Sanders <vincent.sanders@collabora.co.uk>
---
 arch/arm/boot/bootp/Makefile      |    2 ++
 arch/arm/boot/compressed/Makefile |    2 ++
 arch/arm/include/asm/elf.h        |    1 +
 arch/arm/kernel/module.c          |    1 +
 kernel/gcov/Kconfig               |    2 +-
 5 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/bootp/Makefile b/arch/arm/boot/bootp/Makefile
index c394e30..5761f00 100644
--- a/arch/arm/boot/bootp/Makefile
+++ b/arch/arm/boot/bootp/Makefile
@@ -5,6 +5,8 @@
 # architecture-specific flags and dependencies.
 #
 
+GCOV_PROFILE	:= n
+
 LDFLAGS_bootp	:=-p --no-undefined -X \
 		 --defsym initrd_phys=$(INITRD_PHYS) \
 		 --defsym params_phys=$(PARAMS_PHYS) -T
diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile
index bb26756..7482fcd 100644
--- a/arch/arm/boot/compressed/Makefile
+++ b/arch/arm/boot/compressed/Makefile
@@ -30,6 +30,8 @@ FONTC	= $(srctree)/drivers/video/console/font_acorn_8x8.c
 OBJS		+= string.o
 CFLAGS_string.o	:= -Os
 
+GCOV_PROFILE		:= n
+
 #
 # Architecture dependencies
 #
diff --git a/arch/arm/include/asm/elf.h b/arch/arm/include/asm/elf.h
index 38050b1..64afbd0 100644
--- a/arch/arm/include/asm/elf.h
+++ b/arch/arm/include/asm/elf.h
@@ -52,6 +52,7 @@ typedef struct user_fp elf_fpregset_t;
 #define R_ARM_ABS32		2
 #define R_ARM_CALL		28
 #define R_ARM_JUMP24		29
+#define R_ARM_TARGET1		38
 #define R_ARM_V4BX		40
 #define R_ARM_PREL31		42
 #define R_ARM_MOVW_ABS_NC	43
diff --git a/arch/arm/kernel/module.c b/arch/arm/kernel/module.c
index 1e9be5d..b22fc22 100644
--- a/arch/arm/kernel/module.c
+++ b/arch/arm/kernel/module.c
@@ -89,6 +89,7 @@ apply_relocate(Elf32_Shdr *sechdrs, const char *strtab, unsigned int symindex,
 			break;
 
 		case R_ARM_ABS32:
+		case R_ARM_TARGET1:
 			*(u32 *)loc += sym->st_value;
 			break;
 
diff --git a/kernel/gcov/Kconfig b/kernel/gcov/Kconfig
index a920281..2a02dae 100644
--- a/kernel/gcov/Kconfig
+++ b/kernel/gcov/Kconfig
@@ -35,7 +35,7 @@ config GCOV_KERNEL
 config GCOV_PROFILE_ALL
 	bool "Profile entire Kernel"
 	depends on GCOV_KERNEL
-	depends on SUPERH || S390 || X86 || (PPC && EXPERIMENTAL) || MICROBLAZE
+	depends on SUPERH || S390 || X86 || (PPC && EXPERIMENTAL) || MICROBLAZE || ARM
 	default n
 	---help---
 	This options activates profiling for the entire kernel.
-- 
1.7.10

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

[Sam Ravnborg]

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)			   \


	Sam

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

[Sam Ravnborg]

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)			   \


	Sam

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

[Russell King - ARM Linux]

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?

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

[Russell King - ARM Linux]

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?

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

[Vincent Sanders]

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)			   \
> 

Persoinally I have no strong opinion and would alter approach.

As I understand it the previous objections to the "put them all in"
approach (this change has been round the loop before) were:

 - It contaminates the x86 symbol namespace and if init_array was used on
   not ARM it might lead to surprises

 - If another architecture needs something different again then we are
   going to end up with another symbol.

 - The module loading code has to be changed to allow either symbol.

The only technical issue really is the module symbol handling and I am
unsure how to robustly handle that.

-- 
Regards Vincent
http://www.kyllikki.org/

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

[Vincent Sanders]

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)			   \
> 

Persoinally I have no strong opinion and would alter approach.

As I understand it the previous objections to the "put them all in"
approach (this change has been round the loop before) were:

 - It contaminates the x86 symbol namespace and if init_array was used on
   not ARM it might lead to surprises

 - If another architecture needs something different again then we are
   going to end up with another symbol.

 - The module loading code has to be changed to allow either symbol.

The only technical issue really is the module symbol handling and I am
unsure how to robustly handle that.

-- 
Regards Vincent
http://www.kyllikki.org/

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

[Russell King - ARM Linux]

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.

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

[Russell King - ARM Linux]

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.

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

[George G. Davis]

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?

TIA!

--
Regards,
George

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

[George G. Davis]

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?

TIA!

--
Regards,
George

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

[Sam Ravnborg]

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?

	Sam

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

[Sam Ravnborg]

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?

	Sam

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

[George G. Davis]

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

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

[George G. Davis]

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

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

[Russell King - ARM Linux]

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

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

[Russell King - ARM Linux]

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

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

[Sam Ravnborg]

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

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

[Sam Ravnborg]

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