linux-next.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC -next] linux/linkage.h: fix symbol prefix handling
@ 2013-03-07 11:44 James Hogan
  2013-03-08  0:03 ` Rusty Russell
  0 siblings, 1 reply; 15+ messages in thread
From: James Hogan @ 2013-03-07 11:44 UTC (permalink / raw)
  To: Al Viro
  Cc: Michal Marek, Andrew Morton, Guenter Roeck, Jean Delvare,
	Rusty Russell, linux-kbuild, linux-kernel, Mike Frysinger,
	uclinux-dist-devel, linux-next, James Hogan

The commit "consolidate cond_syscall and SYSCALL_ALIAS declarations"
broke the build on blackfin and metag due to the following code:

  #ifndef SYMBOL_NAME
  #ifdef CONFIG_SYMBOL_PREFIX
  #define SYMBOL_NAME(x) CONFIG_SYMBOL_PREFIX ## x
  #else
  #define SYMBOL_NAME(x) x
  #endif
  #endif
  #define __SYMBOL_NAME(x) __stringify(SYMBOL_NAME(x))

__stringify literally stringifies CONFIG_SYMBOL_PREFIX ##x, so you get
lines like this in the assembly output:

  .weak CONFIG_SYMBOL_PREFIXsys_quotactl
  .set
  CONFIG_SYMBOL_PREFIXsys_quotactl,CONFIG_SYMBOL_PREFIXsys_ni_syscall

This is fixed by defining SYMBOL_PREFIX from the command line for c
files in addition to assembly for architectures that set
CONFIG_SYMBOL_PREFIX (scripts/Makefile.lib), and defining __SYMBOL_NAME
as:

  #define __SYMBOL_NAME(x) __stringify(SYMBOL_PREFIX) #x

We first have to ensure SYMBOL_PREFIX is defined (which avoids polluting
the command lines for architectures that don't use symbol prefixes).
Also the definition of SYMBOL_PREFIX in <linux/kernel.h> is removed as
it conflicts, isn't used anywhere, and is defined as a string so differs
from the assembly definition.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
---
This is just one possible way of fixing it. It felt the cleanest out of
the ways I tried, but feel free to suggest something better!

 include/linux/kernel.h  |  7 -------
 include/linux/linkage.h | 12 +++++-------
 scripts/Makefile.lib    |  1 +
 3 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 80d3687..e13e992 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -723,13 +723,6 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
 /* Trap pasters of __FUNCTION__ at compile-time */
 #define __FUNCTION__ (__func__)
 
-/* This helps us to avoid #ifdef CONFIG_SYMBOL_PREFIX */
-#ifdef CONFIG_SYMBOL_PREFIX
-#define SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX
-#else
-#define SYMBOL_PREFIX ""
-#endif
-
 /* Rebuild everything on CONFIG_FTRACE_MCOUNT_RECORD */
 #ifdef CONFIG_FTRACE_MCOUNT_RECORD
 # define REBUILD_DUE_TO_FTRACE_MCOUNT_RECORD
diff --git a/include/linux/linkage.h b/include/linux/linkage.h
index 829d66c..acb869b 100644
--- a/include/linux/linkage.h
+++ b/include/linux/linkage.h
@@ -15,14 +15,12 @@
 #define asmlinkage CPP_ASMLINKAGE
 #endif
 
-#ifndef SYMBOL_NAME
-#ifdef CONFIG_SYMBOL_PREFIX
-#define SYMBOL_NAME(x) CONFIG_SYMBOL_PREFIX ## x
-#else
-#define SYMBOL_NAME(x) x
-#endif
+/* This helps us to avoid #ifdef SYMBOL_PREFIX */
+#ifndef SYMBOL_PREFIX
+#define SYMBOL_PREFIX
 #endif
-#define __SYMBOL_NAME(x) __stringify(SYMBOL_NAME(x))
+
+#define __SYMBOL_NAME(x) __stringify(SYMBOL_PREFIX) #x
 
 #ifndef cond_syscall
 #define cond_syscall(x) asm(".weak\t" __SYMBOL_NAME(x) \
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 07125e6..f1cce6a 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -123,6 +123,7 @@ ifdef CONFIG_SYMBOL_PREFIX
 _sym_flags = -DSYMBOL_PREFIX=$(patsubst "%",%,$(CONFIG_SYMBOL_PREFIX))
 _cpp_flags += $(_sym_flags)
 _a_flags += $(_sym_flags)
+_c_flags += $(_sym_flags)
 endif
 
 
-- 
1.8.1.2



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [RFC -next] linux/linkage.h: fix symbol prefix handling
  2013-03-07 11:44 [RFC -next] linux/linkage.h: fix symbol prefix handling James Hogan
@ 2013-03-08  0:03 ` Rusty Russell
  2013-03-08  9:15   ` James Hogan
  0 siblings, 1 reply; 15+ messages in thread
From: Rusty Russell @ 2013-03-08  0:03 UTC (permalink / raw)
  To: Al Viro
  Cc: Michal Marek, Andrew Morton, Guenter Roeck, Jean Delvare,
	linux-kbuild, linux-kernel, Mike Frysinger, uclinux-dist-devel,
	linux-next, James Hogan

James Hogan <james.hogan@imgtec.com> writes:
> The commit "consolidate cond_syscall and SYSCALL_ALIAS declarations"
> broke the build on blackfin and metag due to the following code:
>
>   #ifndef SYMBOL_NAME
>   #ifdef CONFIG_SYMBOL_PREFIX
>   #define SYMBOL_NAME(x) CONFIG_SYMBOL_PREFIX ## x
>   #else
>   #define SYMBOL_NAME(x) x
>   #endif
>   #endif
>   #define __SYMBOL_NAME(x) __stringify(SYMBOL_NAME(x))
>
> __stringify literally stringifies CONFIG_SYMBOL_PREFIX ##x, so you get
> lines like this in the assembly output:
>
>   .weak CONFIG_SYMBOL_PREFIXsys_quotactl
>   .set
>   CONFIG_SYMBOL_PREFIXsys_quotactl,CONFIG_SYMBOL_PREFIXsys_ni_syscall
>
> This is fixed by defining SYMBOL_PREFIX from the command line for c
> files in addition to assembly for architectures that set
> CONFIG_SYMBOL_PREFIX (scripts/Makefile.lib), and defining __SYMBOL_NAME
> as:
>
>   #define __SYMBOL_NAME(x) __stringify(SYMBOL_PREFIX) #x
>
> We first have to ensure SYMBOL_PREFIX is defined (which avoids polluting
> the command lines for architectures that don't use symbol prefixes).
> Also the definition of SYMBOL_PREFIX in <linux/kernel.h> is removed as
> it conflicts, isn't used anywhere, and is defined as a string so differs
> from the assembly definition.

So now, if CONFIG_SYMBOL_PREFIX, SYMBOL_PREFIX is defined on the cmdline
as a string.  Otherwise it's empty (not the empty string?):

> +/* This helps us to avoid #ifdef SYMBOL_PREFIX */
> +#ifndef SYMBOL_PREFIX
> +#define SYMBOL_PREFIX
>  #endif
> -#define __SYMBOL_NAME(x) __stringify(SYMBOL_NAME(x))
> +
> +#define __SYMBOL_NAME(x) __stringify(SYMBOL_PREFIX) #x

And why are you __stringify()ing a string?

>  #ifndef cond_syscall
>  #define cond_syscall(x) asm(".weak\t" __SYMBOL_NAME(x) \
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 07125e6..f1cce6a 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -123,6 +123,7 @@ ifdef CONFIG_SYMBOL_PREFIX
>  _sym_flags = -DSYMBOL_PREFIX=$(patsubst "%",%,$(CONFIG_SYMBOL_PREFIX))
>  _cpp_flags += $(_sym_flags)
>  _a_flags += $(_sym_flags)
> +_c_flags += $(_sym_flags)
>  endif

Confused,
Rusty.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC -next] linux/linkage.h: fix symbol prefix handling
  2013-03-08  0:03 ` Rusty Russell
@ 2013-03-08  9:15   ` James Hogan
  2013-03-11  6:35     ` Rusty Russell
  0 siblings, 1 reply; 15+ messages in thread
From: James Hogan @ 2013-03-08  9:15 UTC (permalink / raw)
  To: Rusty Russell
  Cc: Al Viro, Michal Marek, Andrew Morton, Guenter Roeck,
	Jean Delvare, linux-kbuild, linux-kernel, Mike Frysinger,
	uclinux-dist-devel, linux-next

Hi Rusty,

On 08/03/13 00:03, Rusty Russell wrote:
> James Hogan <james.hogan@imgtec.com> writes:
>> Also the definition of SYMBOL_PREFIX in <linux/kernel.h> is removed as
>> it conflicts, isn't used anywhere, and is defined as a string so differs
>> from the assembly definition.
> 
> So now, if CONFIG_SYMBOL_PREFIX, SYMBOL_PREFIX is defined on the cmdline
> as a string.  Otherwise it's empty (not the empty string?):

No, SYMBOL_PREFIX is now defined as a non-string, same as asm files, but
the now unused definition in linux/kernel.h did define it as a string as
it used CONFIG_SYMBOL_PREFIX which is a string. When I said "and is
defined as a string" I was referring to the one in linux/kernel.h that
this removes.

Does that make sense? It's all a bit messy unfortunately (hence RFC).

Cheers
James

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC -next] linux/linkage.h: fix symbol prefix handling
  2013-03-08  9:15   ` James Hogan
@ 2013-03-11  6:35     ` Rusty Russell
  2013-03-11 12:07       ` Stephen Rothwell
  0 siblings, 1 reply; 15+ messages in thread
From: Rusty Russell @ 2013-03-11  6:35 UTC (permalink / raw)
  To: James Hogan
  Cc: Al Viro, Michal Marek, Andrew Morton, Guenter Roeck,
	Jean Delvare, linux-kbuild, linux-kernel, Mike Frysinger,
	uclinux-dist-devel, linux-next

James Hogan <james.hogan@imgtec.com> writes:

> Hi Rusty,
>
> On 08/03/13 00:03, Rusty Russell wrote:
>> James Hogan <james.hogan@imgtec.com> writes:
>>> Also the definition of SYMBOL_PREFIX in <linux/kernel.h> is removed as
>>> it conflicts, isn't used anywhere, and is defined as a string so differs
>>> from the assembly definition.
>> 
>> So now, if CONFIG_SYMBOL_PREFIX, SYMBOL_PREFIX is defined on the cmdline
>> as a string.  Otherwise it's empty (not the empty string?):
>
> No, SYMBOL_PREFIX is now defined as a non-string, same as asm files

Ah, I misread this.  It's stripping the quotes, not adding them:

_sym_flags = -DSYMBOL_PREFIX=$(patsubst "%",%,$(CONFIG_SYMBOL_PREFIX))

> Does that make sense? It's all a bit messy unfortunately (hence RFC).

Yes, let's clear it up once and for all.  Does this work?

CONFIG_SYMBOL_PREFIX: cleanup.

We have CONFIG_SYMBOL_PREFIX, which three archs define to the string
"_".  But Al Viro broke this in "consolidate cond_syscall and
SYSCALL_ALIAS declarations", and he's not the first to do so.

Using CONFIG_SYMBOL_PREFIX is awkward, since we usually just want to
prefix it so something.  So various places define helpers which are
defined to nothing if CONFIG_SYMBOL_PREFIX isn't set:

1) include/asm-generic/unistd.h defines __SYMBOL_PREFIX.
2) include/asm-generic/vmlinux.lds.h defines VMLINUX_SYMBOL(sym)
3) include/linux/export.h defines MODULE_SYMBOL_PREFIX.
4) include/linux/kernel.h defines SYMBOL_PREFIX (which differs from #7)
5) kernel/modsign_certificate.S defines ASM_SYMBOL(sym)
6) scripts/modpost.c defines MODULE_SYMBOL_PREFIX
7) scripts/Makefile.lib defines SYMBOL_PREFIX on the commandline if
   CONFIG_SYMBOL_PREFIX is set, so that we have a non-string version
   for pasting.

(arch/h8300/include/asm/linkage.h defines SYMBOL_NAME(), too).

Let's solve this properly:
1) No more generic prefix, just CONFIG_SYMBOL_PREFIX_UNDERSCORE.
2) Make linux/export.h usable from asm.
3) Define VMLINUX_SYMBOL, VMLINUX_SYMBOL_NAME and VMLINUX_SYMBOL_PREFIX_STR.
4) Make everyone use them.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

diff --git a/Makefile b/Makefile
index 5bd9f77..b2e3e1b 100644
--- a/Makefile
+++ b/Makefile
@@ -1398,7 +1398,7 @@ quiet_cmd_rmfiles = $(if $(wildcard $(rm-files)),CLEAN   $(wildcard $(rm-files))
 # Run depmod only if we have System.map and depmod is executable
 quiet_cmd_depmod = DEPMOD  $(KERNELRELEASE)
       cmd_depmod = $(CONFIG_SHELL) $(srctree)/scripts/depmod.sh $(DEPMOD) \
-                   $(KERNELRELEASE) "$(patsubst "%",%,$(CONFIG_SYMBOL_PREFIX))"
+                   $(KERNELRELEASE) "$(patsubst y,_,$(CONFIG_SYMBOL_PREFIX_UNDERSCORE))"
 
 # Create temporary dir for module support files
 # clean it up only when building all modules
diff --git a/arch/blackfin/Kconfig b/arch/blackfin/Kconfig
index 600494c..cd0f7c7 100644
--- a/arch/blackfin/Kconfig
+++ b/arch/blackfin/Kconfig
@@ -1,6 +1,5 @@
-config SYMBOL_PREFIX
-	string
-	default "_"
+config SYMBOL_PREFIX_UNDERSCORE
+	def_bool y
 
 config MMU
 	def_bool n
diff --git a/arch/h8300/Kconfig b/arch/h8300/Kconfig
index ae8551e..ec9924d 100644
--- a/arch/h8300/Kconfig
+++ b/arch/h8300/Kconfig
@@ -13,9 +13,8 @@ config H8300
 	select OLD_SIGSUSPEND3
 	select OLD_SIGACTION
 
-config SYMBOL_PREFIX
-	string
-	default "_"
+config SYMBOL_PREFIX_UNDERSCORE
+	def_bool y
 
 config MMU
 	bool
diff --git a/arch/metag/Kconfig b/arch/metag/Kconfig
index afc8973..e8719b4 100644
--- a/arch/metag/Kconfig
+++ b/arch/metag/Kconfig
@@ -1,6 +1,5 @@
-config SYMBOL_PREFIX
-	string
-	default "_"
+config SYMBOL_PREFIX_UNDERSCORE
+	def_bool y
 
 config METAG
 	def_bool y
diff --git a/drivers/mtd/chips/gen_probe.c b/drivers/mtd/chips/gen_probe.c
index 3b9a284..00d20ad 100644
--- a/drivers/mtd/chips/gen_probe.c
+++ b/drivers/mtd/chips/gen_probe.c
@@ -204,14 +204,14 @@ static inline struct mtd_info *cfi_cmdset_unknown(struct map_info *map,
 	struct cfi_private *cfi = map->fldrv_priv;
 	__u16 type = primary?cfi->cfiq->P_ID:cfi->cfiq->A_ID;
 #ifdef CONFIG_MODULES
-	char probename[16+sizeof(MODULE_SYMBOL_PREFIX)];
+	char probename[16+sizeof(VMLINUX_SYMBOL_PREFIX_STR)];
 	cfi_cmdset_fn_t *probe_function;
 
-	sprintf(probename, MODULE_SYMBOL_PREFIX "cfi_cmdset_%4.4X", type);
+	sprintf(probename, VMLINUX_SYMBOL_PREFIX_STR "cfi_cmdset_%4.4X", type);
 
 	probe_function = __symbol_get(probename);
 	if (!probe_function) {
-		request_module(probename + sizeof(MODULE_SYMBOL_PREFIX) - 1);
+		request_module(probename + sizeof(VMLINUX_SYMBOL_PREFIX_STR)-1);
 		probe_function = __symbol_get(probename);
 	}
 
diff --git a/include/asm-generic/unistd.h b/include/asm-generic/unistd.h
index 4077b5d..80122d4 100644
--- a/include/asm-generic/unistd.h
+++ b/include/asm-generic/unistd.h
@@ -1,4 +1,5 @@
 #include <uapi/asm-generic/unistd.h>
+#include <linux/export.h>
 
 /*
  * These are required system calls, we should
@@ -17,12 +18,7 @@
  * but it doesn't work on all toolchains, so we just do it by hand
  */
 #ifndef cond_syscall
-#ifdef CONFIG_SYMBOL_PREFIX
-#define __SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX
-#else
-#define __SYMBOL_PREFIX
-#endif
-#define cond_syscall(x) asm(".weak\t" __SYMBOL_PREFIX #x "\n\t" \
-			    ".set\t" __SYMBOL_PREFIX #x "," \
-			    __SYMBOL_PREFIX "sys_ni_syscall")
+#define cond_syscall(x) asm(".weak\t" SYMBOL_NAME_STR(x) "\n\t"	\
+			    ".set\t" SYMBOL_NAME_STR(x) ","	\
+			    SYMBOL_NAME_STR(sys_ni_syscall))
 #endif
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index afa12c7..eb58d2d 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -52,13 +52,7 @@
 #define LOAD_OFFSET 0
 #endif
 
-#ifndef SYMBOL_PREFIX
-#define VMLINUX_SYMBOL(sym) sym
-#else
-#define PASTE2(x,y) x##y
-#define PASTE(x,y) PASTE2(x,y)
-#define VMLINUX_SYMBOL(sym) PASTE(SYMBOL_PREFIX, sym)
-#endif
+#include <linux/export.h>
 
 /* Align . to a 8 byte boundary equals to maximum function alignment. */
 #define ALIGN_FUNCTION()  . = ALIGN(8)
diff --git a/include/linux/export.h b/include/linux/export.h
index 696c0f4..c3c1cfc 100644
--- a/include/linux/export.h
+++ b/include/linux/export.h
@@ -7,15 +7,27 @@
  *
  * If you feel the need to add #include <linux/foo.h> to this file
  * then you are doing something wrong and should go away silently.
+ *
+ * If you think the above arrogance just encourages more people to add
+ * random crap to this file, you're not alone.
  */
 
 /* Some toolchains use a `_' prefix for all user symbols. */
-#ifdef CONFIG_SYMBOL_PREFIX
-#define MODULE_SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX
+#ifdef CONFIG_SYMBOL_PREFIX_UNDERSCORE
+#define __VMLINUX_SYMBOL(x) _##x
+#define __VMLINUX_SYMBOL_STR(x) "_" #x
+#define VMLINUX_SYMBOL_PREFIX_STR "_"
 #else
-#define MODULE_SYMBOL_PREFIX ""
+#define __VMLINUX_SYMBOL(x) x
+#define __VMLINUX_SYMBOL_STR(x) #x
+#define VMLINUX_SYMBOL_PREFIX_STR ""
 #endif
 
+/* Indirect, so macros are expanded before pasting. */
+#define VMLINUX_SYMBOL(x) __VMLINUX_SYMBOL(x)
+#define VMLINUX_SYMBOL_STR(x) __VMLINUX_SYMBOL_STR(x)
+
+#ifndef __ASSEMBLY__
 struct kernel_symbol
 {
 	unsigned long value;
@@ -51,7 +63,7 @@ extern struct module __this_module;
 	__CRC_SYMBOL(sym, sec)					\
 	static const char __kstrtab_##sym[]			\
 	__attribute__((section("__ksymtab_strings"), aligned(1))) \
-	= MODULE_SYMBOL_PREFIX #sym;				\
+	= VMLINUX_SYMBOL_STR(sym);				\
 	static const struct kernel_symbol __ksymtab_##sym	\
 	__used							\
 	__attribute__((section("___ksymtab" sec "+" #sym), unused))	\
@@ -85,5 +97,6 @@ extern struct module __this_module;
 #define EXPORT_UNUSED_SYMBOL_GPL(sym)
 
 #endif /* CONFIG_MODULES */
+#endif /* !__ASSEMBLY__ */
 
 #endif /* _LINUX_EXPORT_H */
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 80d3687..e13e992 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -723,13 +723,6 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
 /* Trap pasters of __FUNCTION__ at compile-time */
 #define __FUNCTION__ (__func__)
 
-/* This helps us to avoid #ifdef CONFIG_SYMBOL_PREFIX */
-#ifdef CONFIG_SYMBOL_PREFIX
-#define SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX
-#else
-#define SYMBOL_PREFIX ""
-#endif
-
 /* Rebuild everything on CONFIG_FTRACE_MCOUNT_RECORD */
 #ifdef CONFIG_FTRACE_MCOUNT_RECORD
 # define REBUILD_DUE_TO_FTRACE_MCOUNT_RECORD
diff --git a/include/linux/module.h b/include/linux/module.h
index ead1b57..46f1ea0 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -190,7 +190,7 @@ extern int modules_disabled; /* for sysctl */
 /* Get/put a kernel symbol (calls must be symmetric) */
 void *__symbol_get(const char *symbol);
 void *__symbol_get_gpl(const char *symbol);
-#define symbol_get(x) ((typeof(&x))(__symbol_get(MODULE_SYMBOL_PREFIX #x)))
+#define symbol_get(x) ((typeof(&x))(__symbol_get(VMLINUX_SYMBOL_STR(x))))
 
 /* modules using other modules: kdb wants to see this. */
 struct module_use {
@@ -453,7 +453,7 @@ extern void __module_put_and_exit(struct module *mod, long code)
 #ifdef CONFIG_MODULE_UNLOAD
 unsigned long module_refcount(struct module *mod);
 void __symbol_put(const char *symbol);
-#define symbol_put(x) __symbol_put(MODULE_SYMBOL_PREFIX #x)
+#define symbol_put(x) __symbol_put(VMLINUX_SYMBOL_STR(x))
 void symbol_put_addr(void *addr);
 
 /* Sometimes we know we already have a refcount, and it's easier not
diff --git a/kernel/modsign_certificate.S b/kernel/modsign_certificate.S
index 246b4c6..4a9a86d 100644
--- a/kernel/modsign_certificate.S
+++ b/kernel/modsign_certificate.S
@@ -1,15 +1,8 @@
-/* SYMBOL_PREFIX defined on commandline from CONFIG_SYMBOL_PREFIX */
-#ifndef SYMBOL_PREFIX
-#define ASM_SYMBOL(sym) sym
-#else
-#define PASTE2(x,y) x##y
-#define PASTE(x,y) PASTE2(x,y)
-#define ASM_SYMBOL(sym) PASTE(SYMBOL_PREFIX, sym)
-#endif
+#include <linux/export.h>
 
 #define GLOBAL(name)	\
-	.globl ASM_SYMBOL(name);	\
-	ASM_SYMBOL(name):
+	.globl VMLINUX_SYMBOL(name);	\
+	VMLINUX_SYMBOL(name):
 
 	.section ".init.data","aw"
 
diff --git a/kernel/module.c b/kernel/module.c
index 0925c9a..cfd4a3f 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1209,7 +1209,7 @@ static inline int check_modstruct_version(Elf_Shdr *sechdrs,
 
 	/* Since this should be found in kernel (which can't be removed),
 	 * no locking is necessary. */
-	if (!find_symbol(MODULE_SYMBOL_PREFIX "module_layout", NULL,
+	if (!find_symbol(VMLINUX_SYMBOL_STR(module_layout), NULL,
 			 &crc, true, false))
 		BUG();
 	return check_version(sechdrs, versindex, "module_layout", mod, crc,
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 07125e6..a373a1f 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -119,13 +119,6 @@ _c_flags += $(if $(patsubst n%,, \
 		$(CFLAGS_GCOV))
 endif
 
-ifdef CONFIG_SYMBOL_PREFIX
-_sym_flags = -DSYMBOL_PREFIX=$(patsubst "%",%,$(CONFIG_SYMBOL_PREFIX))
-_cpp_flags += $(_sym_flags)
-_a_flags += $(_sym_flags)
-endif
-
-
 # If building the kernel in a separate objtree expand all occurrences
 # of -Idir to -I$(srctree)/dir except for absolute paths (starting with '/').
 
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 3d569d6..d767681 100644
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -74,9 +74,8 @@ kallsyms()
 	info KSYM ${2}
 	local kallsymopt;
 
-	if [ -n "${CONFIG_SYMBOL_PREFIX}" ]; then
-		kallsymopt="${kallsymopt} \
-			    --symbol-prefix=${CONFIG_SYMBOL_PREFIX}"
+	if [ -n "${CONFIG_SYMBOL_PREFIX_UNDERSCORE}" ]; then
+		kallsymopt="${kallsymopt} --symbol-prefix=_"
 	fi
 
 	if [ -n "${CONFIG_KALLSYMS_ALL}" ]; then
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 78b30c1..6985021 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -18,14 +18,7 @@
 #include "modpost.h"
 #include "../../include/generated/autoconf.h"
 #include "../../include/linux/license.h"
-
-/* Some toolchains use a `_' prefix for all user symbols. */
-#ifdef CONFIG_SYMBOL_PREFIX
-#define MODULE_SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX
-#else
-#define MODULE_SYMBOL_PREFIX ""
-#endif
-
+#include "../../include/linux/export.h"
 
 /* Are we using CONFIG_MODVERSIONS? */
 int modversions = 0;
@@ -562,7 +555,7 @@ static void parse_elf_finish(struct elf_info *info)
 static int ignore_undef_symbol(struct elf_info *info, const char *symname)
 {
 	/* ignore __this_module, it will be resolved shortly */
-	if (strcmp(symname, MODULE_SYMBOL_PREFIX "__this_module") == 0)
+	if (strcmp(symname, VMLINUX_SYMBOL_STR(__this_module)) == 0)
 		return 1;
 	/* ignore global offset table */
 	if (strcmp(symname, "_GLOBAL_OFFSET_TABLE_") == 0)
@@ -583,8 +576,8 @@ static int ignore_undef_symbol(struct elf_info *info, const char *symname)
 	return 0;
 }
 
-#define CRC_PFX     MODULE_SYMBOL_PREFIX "__crc_"
-#define KSYMTAB_PFX MODULE_SYMBOL_PREFIX "__ksymtab_"
+#define CRC_PFX     VMLINUX_SYMBOL_STR(__crc_)
+#define KSYMTAB_PFX VMLINUX_SYMBOL_STR(__ksymtab_)
 
 static void handle_modversions(struct module *mod, struct elf_info *info,
 			       Elf_Sym *sym, const char *symname)
@@ -637,11 +630,11 @@ static void handle_modversions(struct module *mod, struct elf_info *info,
 		}
 #endif
 
-		if (memcmp(symname, MODULE_SYMBOL_PREFIX,
-			   strlen(MODULE_SYMBOL_PREFIX)) == 0) {
+		if (memcmp(symname, VMLINUX_SYMBOL_PREFIX_STR,
+			   strlen(VMLINUX_SYMBOL_PREFIX_STR)) == 0) {
 			mod->unres =
 			  alloc_symbol(symname +
-			               strlen(MODULE_SYMBOL_PREFIX),
+			               strlen(VMLINUX_SYMBOL_PREFIX_STR),
 			               ELF_ST_BIND(sym->st_info) == STB_WEAK,
 			               mod->unres);
 		}
@@ -652,9 +645,9 @@ static void handle_modversions(struct module *mod, struct elf_info *info,
 			sym_add_exported(symname + strlen(KSYMTAB_PFX), mod,
 					export);
 		}
-		if (strcmp(symname, MODULE_SYMBOL_PREFIX "init_module") == 0)
+		if (strcmp(symname, VMLINUX_SYMBOL_STR(init_module)) == 0)
 			mod->has_init = 1;
-		if (strcmp(symname, MODULE_SYMBOL_PREFIX "cleanup_module") == 0)
+		if (strcmp(symname, VMLINUX_SYMBOL_STR(cleanup_module)) == 0)
 			mod->has_cleanup = 1;
 		break;
 	}

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [RFC -next] linux/linkage.h: fix symbol prefix handling
  2013-03-11  6:35     ` Rusty Russell
@ 2013-03-11 12:07       ` Stephen Rothwell
  2013-03-12  4:48         ` Rusty Russell
  0 siblings, 1 reply; 15+ messages in thread
From: Stephen Rothwell @ 2013-03-11 12:07 UTC (permalink / raw)
  To: Rusty Russell
  Cc: James Hogan, Al Viro, Michal Marek, Andrew Morton, Guenter Roeck,
	Jean Delvare, linux-kbuild, linux-kernel, Mike Frysinger,
	uclinux-dist-devel, linux-next

[-- Attachment #1: Type: text/plain, Size: 757 bytes --]

Hi Rusty,

On Mon, 11 Mar 2013 17:05:17 +1030 Rusty Russell <rusty@rustcorp.com.au> wrote:
>
> diff --git a/arch/blackfin/Kconfig b/arch/blackfin/Kconfig
> index 600494c..cd0f7c7 100644
> --- a/arch/blackfin/Kconfig
> +++ b/arch/blackfin/Kconfig
> @@ -1,6 +1,5 @@
> -config SYMBOL_PREFIX
> -	string
> -	default "_"
> +config SYMBOL_PREFIX_UNDERSCORE
> +	def_bool y

Please, if you are going to do this, just have

config SYMBOL_PREFIX_UNDERSCORE
	bool

in one Kconfig file (probably arch/Kconfig) and then select it from the
appropriate architecture Kconfig files.  You could even put a help text
on it to explain why a new architecture may need to select it.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC -next] linux/linkage.h: fix symbol prefix handling
  2013-03-11 12:07       ` Stephen Rothwell
@ 2013-03-12  4:48         ` Rusty Russell
  2013-03-12 12:33           ` Stephen Rothwell
  2013-03-12 12:36           ` James Hogan
  0 siblings, 2 replies; 15+ messages in thread
From: Rusty Russell @ 2013-03-12  4:48 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: James Hogan, Al Viro, Michal Marek, Andrew Morton, Guenter Roeck,
	Jean Delvare, linux-kbuild, linux-kernel, Mike Frysinger,
	uclinux-dist-devel, linux-next

Stephen Rothwell <sfr@canb.auug.org.au> writes:

> Hi Rusty,
>
> On Mon, 11 Mar 2013 17:05:17 +1030 Rusty Russell <rusty@rustcorp.com.au> wrote:
>>
>> diff --git a/arch/blackfin/Kconfig b/arch/blackfin/Kconfig
>> index 600494c..cd0f7c7 100644
>> --- a/arch/blackfin/Kconfig
>> +++ b/arch/blackfin/Kconfig
>> @@ -1,6 +1,5 @@
>> -config SYMBOL_PREFIX
>> -	string
>> -	default "_"
>> +config SYMBOL_PREFIX_UNDERSCORE
>> +	def_bool y
>
> Please, if you are going to do this, just have
>
> config SYMBOL_PREFIX_UNDERSCORE
> 	bool
>
> in one Kconfig file (probably arch/Kconfig) and then select it from the
> appropriate architecture Kconfig files.  You could even put a help text
> on it to explain why a new architecture may need to select it.
>
> -- 
> Cheers,
> Stephen Rothwell                    sfr@canb.auug.org.au

OK.

v2: Rename CONFIG_SYMBOL_PREFIX_UNDERSCORE to CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX,
    which is defined in arch/Kconfig and selected by the 3 archs which need it.


Subject: CONFIG_SYMBOL_PREFIX: cleanup.

We have CONFIG_SYMBOL_PREFIX, which three archs define to the string
"_".  But Al Viro broke this in "consolidate cond_syscall and
SYSCALL_ALIAS declarations", and he's not the first to do so.

Using CONFIG_SYMBOL_PREFIX is awkward, since we usually just want to
prefix it so something.  So various places define helpers which are
defined to nothing if CONFIG_SYMBOL_PREFIX isn't set:

1) include/asm-generic/unistd.h defines __SYMBOL_PREFIX.
2) include/asm-generic/vmlinux.lds.h defines VMLINUX_SYMBOL(sym)
3) include/linux/export.h defines MODULE_SYMBOL_PREFIX.
4) include/linux/kernel.h defines SYMBOL_PREFIX (which differs from #7)
5) kernel/modsign_certificate.S defines ASM_SYMBOL(sym)
6) scripts/modpost.c defines MODULE_SYMBOL_PREFIX
7) scripts/Makefile.lib defines SYMBOL_PREFIX on the commandline if
   CONFIG_SYMBOL_PREFIX is set, so that we have a non-string version
   for pasting.

(arch/h8300/include/asm/linkage.h defines SYMBOL_NAME(), too).

Let's solve this properly:
1) No more generic prefix, just CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX.
2) Make linux/export.h usable from asm.
3) Define VMLINUX_SYMBOL, VMLINUX_SYMBOL_NAME and VMLINUX_SYMBOL_PREFIX_STR.
4) Make everyone use them.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

diff --git a/Makefile b/Makefile
index a05ea42..9dc948d 100644
--- a/Makefile
+++ b/Makefile
@@ -1398,7 +1398,7 @@ quiet_cmd_rmfiles = $(if $(wildcard $(rm-files)),CLEAN   $(wildcard $(rm-files))
 # Run depmod only if we have System.map and depmod is executable
 quiet_cmd_depmod = DEPMOD  $(KERNELRELEASE)
       cmd_depmod = $(CONFIG_SHELL) $(srctree)/scripts/depmod.sh $(DEPMOD) \
-                   $(KERNELRELEASE) "$(patsubst "%",%,$(CONFIG_SYMBOL_PREFIX))"
+                   $(KERNELRELEASE) "$(patsubst y,_,$(CONFIG_SYMBOL_PREFIX_UNDERSCORE))"
 
 # Create temporary dir for module support files
 # clean it up only when building all modules
diff --git a/arch/Kconfig b/arch/Kconfig
index 5a1779c..12ba8f9 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -391,6 +391,12 @@ config MODULES_USE_ELF_REL
 	  Modules only use ELF REL relocations.  Modules with ELF RELA
 	  relocations will give an error.
 
+config HAVE_UNDERSCORE_SYMBOL_PREFIX
+	bool
+	help
+	  Some architectures generate an _ in front of C symbols; things like
+	  module loading and assembly files need to know about this.
+
 #
 # ABI hall of shame
 #
diff --git a/arch/blackfin/Kconfig b/arch/blackfin/Kconfig
index 600494c..314ee6a 100644
--- a/arch/blackfin/Kconfig
+++ b/arch/blackfin/Kconfig
@@ -1,7 +1,3 @@
-config SYMBOL_PREFIX
-	string
-	default "_"
-
 config MMU
 	def_bool n
 
@@ -33,6 +29,7 @@ config BLACKFIN
 	select ARCH_HAVE_CUSTOM_GPIO_H
 	select ARCH_WANT_OPTIONAL_GPIOLIB
 	select HAVE_UID16
+	select HAVE_UNDERSCORE_SYMBOL_PREFIX
 	select HAVE_VIRT_TO_BUS
 	select ARCH_WANT_IPC_PARSE_VERSION
 	select HAVE_GENERIC_HARDIRQS
@@ -45,6 +42,7 @@ config BLACKFIN
 	select HAVE_MOD_ARCH_SPECIFIC
 	select MODULES_USE_ELF_RELA
 
+
 config GENERIC_CSUM
 	def_bool y
 
diff --git a/arch/h8300/Kconfig b/arch/h8300/Kconfig
index ae8551e..ba043e3 100644
--- a/arch/h8300/Kconfig
+++ b/arch/h8300/Kconfig
@@ -12,10 +12,10 @@ config H8300
 	select MODULES_USE_ELF_RELA
 	select OLD_SIGSUSPEND3
 	select OLD_SIGACTION
+	select HAVE_UNDERSCORE_SYMBOL_PREFIX
 
-config SYMBOL_PREFIX
-	string
-	default "_"
+config SYMBOL_PREFIX_UNDERSCORE
+	def_bool y
 
 config MMU
 	bool
diff --git a/arch/metag/Kconfig b/arch/metag/Kconfig
index afc8973..88272ce 100644
--- a/arch/metag/Kconfig
+++ b/arch/metag/Kconfig
@@ -1,6 +1,5 @@
-config SYMBOL_PREFIX
-	string
-	default "_"
+config SYMBOL_PREFIX_UNDERSCORE
+	def_bool y
 
 config METAG
 	def_bool y
@@ -27,6 +26,7 @@ config METAG
 	select HAVE_MOD_ARCH_SPECIFIC
 	select HAVE_PERF_EVENTS
 	select HAVE_SYSCALL_TRACEPOINTS
+	select HAVE_UNDERSCORE_SYMBOL_PREFIX
 	select IRQ_DOMAIN
 	select MODULES_USE_ELF_RELA
 	select OF
diff --git a/drivers/mtd/chips/gen_probe.c b/drivers/mtd/chips/gen_probe.c
index 3b9a284..00d20ad 100644
--- a/drivers/mtd/chips/gen_probe.c
+++ b/drivers/mtd/chips/gen_probe.c
@@ -204,14 +204,14 @@ static inline struct mtd_info *cfi_cmdset_unknown(struct map_info *map,
 	struct cfi_private *cfi = map->fldrv_priv;
 	__u16 type = primary?cfi->cfiq->P_ID:cfi->cfiq->A_ID;
 #ifdef CONFIG_MODULES
-	char probename[16+sizeof(MODULE_SYMBOL_PREFIX)];
+	char probename[16+sizeof(VMLINUX_SYMBOL_PREFIX_STR)];
 	cfi_cmdset_fn_t *probe_function;
 
-	sprintf(probename, MODULE_SYMBOL_PREFIX "cfi_cmdset_%4.4X", type);
+	sprintf(probename, VMLINUX_SYMBOL_PREFIX_STR "cfi_cmdset_%4.4X", type);
 
 	probe_function = __symbol_get(probename);
 	if (!probe_function) {
-		request_module(probename + sizeof(MODULE_SYMBOL_PREFIX) - 1);
+		request_module(probename + sizeof(VMLINUX_SYMBOL_PREFIX_STR)-1);
 		probe_function = __symbol_get(probename);
 	}
 
diff --git a/include/asm-generic/unistd.h b/include/asm-generic/unistd.h
index 4077b5d..80122d4 100644
--- a/include/asm-generic/unistd.h
+++ b/include/asm-generic/unistd.h
@@ -1,4 +1,5 @@
 #include <uapi/asm-generic/unistd.h>
+#include <linux/export.h>
 
 /*
  * These are required system calls, we should
@@ -17,12 +18,7 @@
  * but it doesn't work on all toolchains, so we just do it by hand
  */
 #ifndef cond_syscall
-#ifdef CONFIG_SYMBOL_PREFIX
-#define __SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX
-#else
-#define __SYMBOL_PREFIX
-#endif
-#define cond_syscall(x) asm(".weak\t" __SYMBOL_PREFIX #x "\n\t" \
-			    ".set\t" __SYMBOL_PREFIX #x "," \
-			    __SYMBOL_PREFIX "sys_ni_syscall")
+#define cond_syscall(x) asm(".weak\t" SYMBOL_NAME_STR(x) "\n\t"	\
+			    ".set\t" SYMBOL_NAME_STR(x) ","	\
+			    SYMBOL_NAME_STR(sys_ni_syscall))
 #endif
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index afa12c7..eb58d2d 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -52,13 +52,7 @@
 #define LOAD_OFFSET 0
 #endif
 
-#ifndef SYMBOL_PREFIX
-#define VMLINUX_SYMBOL(sym) sym
-#else
-#define PASTE2(x,y) x##y
-#define PASTE(x,y) PASTE2(x,y)
-#define VMLINUX_SYMBOL(sym) PASTE(SYMBOL_PREFIX, sym)
-#endif
+#include <linux/export.h>
 
 /* Align . to a 8 byte boundary equals to maximum function alignment. */
 #define ALIGN_FUNCTION()  . = ALIGN(8)
diff --git a/include/linux/export.h b/include/linux/export.h
index 696c0f4..0080e93 100644
--- a/include/linux/export.h
+++ b/include/linux/export.h
@@ -7,15 +7,27 @@
  *
  * If you feel the need to add #include <linux/foo.h> to this file
  * then you are doing something wrong and should go away silently.
+ *
+ * If you think the above arrogance just encourages more people to add
+ * random crap to this file, you're not alone.
  */
 
 /* Some toolchains use a `_' prefix for all user symbols. */
-#ifdef CONFIG_SYMBOL_PREFIX
-#define MODULE_SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX
+#ifdef CONFIG_SYMBOL_PREFIX_UNDERSCORE
+#define __VMLINUX_SYMBOL(x) _##x
+#define __VMLINUX_SYMBOL_STR(x) "_" #x
+#define VMLINUX_SYMBOL_PREFIX_STR "_"
 #else
-#define MODULE_SYMBOL_PREFIX ""
+#define __VMLINUX_SYMBOL(x) x
+#define __VMLINUX_SYMBOL_STR(x) #x
+#define VMLINUX_SYMBOL_PREFIX_STR ""
 #endif
 
+/* Indirect, so macros are expanded before pasting. */
+#define VMLINUX_SYMBOL(x) __VMLINUX_SYMBOL(x)
+#define VMLINUX_SYMBOL_STR(x) __VMLINUX_SYMBOL_STR(x)
+
+#ifndef __ASSEMBLY__
 struct kernel_symbol
 {
 	unsigned long value;
@@ -51,7 +63,7 @@ extern struct module __this_module;
 	__CRC_SYMBOL(sym, sec)					\
 	static const char __kstrtab_##sym[]			\
 	__attribute__((section("__ksymtab_strings"), aligned(1))) \
-	= MODULE_SYMBOL_PREFIX #sym;				\
+	= VMLINUX_SYMBOL_STR(sym);				\
 	static const struct kernel_symbol __ksymtab_##sym	\
 	__used							\
 	__attribute__((section("___ksymtab" sec "+" #sym), unused))	\
@@ -85,5 +97,6 @@ extern struct module __this_module;
 #define EXPORT_UNUSED_SYMBOL_GPL(sym)
 
 #endif /* CONFIG_MODULES */
+#endif /* !__ASSEMBLY__ */
 
 #endif /* _LINUX_EXPORT_H */
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 80d3687..e13e992 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -723,13 +723,6 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
 /* Trap pasters of __FUNCTION__ at compile-time */
 #define __FUNCTION__ (__func__)
 
-/* This helps us to avoid #ifdef CONFIG_SYMBOL_PREFIX */
-#ifdef CONFIG_SYMBOL_PREFIX
-#define SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX
-#else
-#define SYMBOL_PREFIX ""
-#endif
-
 /* Rebuild everything on CONFIG_FTRACE_MCOUNT_RECORD */
 #ifdef CONFIG_FTRACE_MCOUNT_RECORD
 # define REBUILD_DUE_TO_FTRACE_MCOUNT_RECORD
diff --git a/include/linux/module.h b/include/linux/module.h
index ead1b57..46f1ea0 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -190,7 +190,7 @@ extern int modules_disabled; /* for sysctl */
 /* Get/put a kernel symbol (calls must be symmetric) */
 void *__symbol_get(const char *symbol);
 void *__symbol_get_gpl(const char *symbol);
-#define symbol_get(x) ((typeof(&x))(__symbol_get(MODULE_SYMBOL_PREFIX #x)))
+#define symbol_get(x) ((typeof(&x))(__symbol_get(VMLINUX_SYMBOL_STR(x))))
 
 /* modules using other modules: kdb wants to see this. */
 struct module_use {
@@ -453,7 +453,7 @@ extern void __module_put_and_exit(struct module *mod, long code)
 #ifdef CONFIG_MODULE_UNLOAD
 unsigned long module_refcount(struct module *mod);
 void __symbol_put(const char *symbol);
-#define symbol_put(x) __symbol_put(MODULE_SYMBOL_PREFIX #x)
+#define symbol_put(x) __symbol_put(VMLINUX_SYMBOL_STR(x))
 void symbol_put_addr(void *addr);
 
 /* Sometimes we know we already have a refcount, and it's easier not
diff --git a/kernel/modsign_certificate.S b/kernel/modsign_certificate.S
index 246b4c6..4a9a86d 100644
--- a/kernel/modsign_certificate.S
+++ b/kernel/modsign_certificate.S
@@ -1,15 +1,8 @@
-/* SYMBOL_PREFIX defined on commandline from CONFIG_SYMBOL_PREFIX */
-#ifndef SYMBOL_PREFIX
-#define ASM_SYMBOL(sym) sym
-#else
-#define PASTE2(x,y) x##y
-#define PASTE(x,y) PASTE2(x,y)
-#define ASM_SYMBOL(sym) PASTE(SYMBOL_PREFIX, sym)
-#endif
+#include <linux/export.h>
 
 #define GLOBAL(name)	\
-	.globl ASM_SYMBOL(name);	\
-	ASM_SYMBOL(name):
+	.globl VMLINUX_SYMBOL(name);	\
+	VMLINUX_SYMBOL(name):
 
 	.section ".init.data","aw"
 
diff --git a/kernel/module.c b/kernel/module.c
index 0925c9a..cfd4a3f 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1209,7 +1209,7 @@ static inline int check_modstruct_version(Elf_Shdr *sechdrs,
 
 	/* Since this should be found in kernel (which can't be removed),
 	 * no locking is necessary. */
-	if (!find_symbol(MODULE_SYMBOL_PREFIX "module_layout", NULL,
+	if (!find_symbol(VMLINUX_SYMBOL_STR(module_layout), NULL,
 			 &crc, true, false))
 		BUG();
 	return check_version(sechdrs, versindex, "module_layout", mod, crc,
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 07125e6..a373a1f 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -119,13 +119,6 @@ _c_flags += $(if $(patsubst n%,, \
 		$(CFLAGS_GCOV))
 endif
 
-ifdef CONFIG_SYMBOL_PREFIX
-_sym_flags = -DSYMBOL_PREFIX=$(patsubst "%",%,$(CONFIG_SYMBOL_PREFIX))
-_cpp_flags += $(_sym_flags)
-_a_flags += $(_sym_flags)
-endif
-
-
 # If building the kernel in a separate objtree expand all occurrences
 # of -Idir to -I$(srctree)/dir except for absolute paths (starting with '/').
 
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 3d569d6..d767681 100644
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -74,9 +74,8 @@ kallsyms()
 	info KSYM ${2}
 	local kallsymopt;
 
-	if [ -n "${CONFIG_SYMBOL_PREFIX}" ]; then
-		kallsymopt="${kallsymopt} \
-			    --symbol-prefix=${CONFIG_SYMBOL_PREFIX}"
+	if [ -n "${CONFIG_SYMBOL_PREFIX_UNDERSCORE}" ]; then
+		kallsymopt="${kallsymopt} --symbol-prefix=_"
 	fi
 
 	if [ -n "${CONFIG_KALLSYMS_ALL}" ]; then
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 78b30c1..6985021 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -18,14 +18,7 @@
 #include "modpost.h"
 #include "../../include/generated/autoconf.h"
 #include "../../include/linux/license.h"
-
-/* Some toolchains use a `_' prefix for all user symbols. */
-#ifdef CONFIG_SYMBOL_PREFIX
-#define MODULE_SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX
-#else
-#define MODULE_SYMBOL_PREFIX ""
-#endif
-
+#include "../../include/linux/export.h"
 
 /* Are we using CONFIG_MODVERSIONS? */
 int modversions = 0;
@@ -562,7 +555,7 @@ static void parse_elf_finish(struct elf_info *info)
 static int ignore_undef_symbol(struct elf_info *info, const char *symname)
 {
 	/* ignore __this_module, it will be resolved shortly */
-	if (strcmp(symname, MODULE_SYMBOL_PREFIX "__this_module") == 0)
+	if (strcmp(symname, VMLINUX_SYMBOL_STR(__this_module)) == 0)
 		return 1;
 	/* ignore global offset table */
 	if (strcmp(symname, "_GLOBAL_OFFSET_TABLE_") == 0)
@@ -583,8 +576,8 @@ static int ignore_undef_symbol(struct elf_info *info, const char *symname)
 	return 0;
 }
 
-#define CRC_PFX     MODULE_SYMBOL_PREFIX "__crc_"
-#define KSYMTAB_PFX MODULE_SYMBOL_PREFIX "__ksymtab_"
+#define CRC_PFX     VMLINUX_SYMBOL_STR(__crc_)
+#define KSYMTAB_PFX VMLINUX_SYMBOL_STR(__ksymtab_)
 
 static void handle_modversions(struct module *mod, struct elf_info *info,
 			       Elf_Sym *sym, const char *symname)
@@ -637,11 +630,11 @@ static void handle_modversions(struct module *mod, struct elf_info *info,
 		}
 #endif
 
-		if (memcmp(symname, MODULE_SYMBOL_PREFIX,
-			   strlen(MODULE_SYMBOL_PREFIX)) == 0) {
+		if (memcmp(symname, VMLINUX_SYMBOL_PREFIX_STR,
+			   strlen(VMLINUX_SYMBOL_PREFIX_STR)) == 0) {
 			mod->unres =
 			  alloc_symbol(symname +
-			               strlen(MODULE_SYMBOL_PREFIX),
+			               strlen(VMLINUX_SYMBOL_PREFIX_STR),
 			               ELF_ST_BIND(sym->st_info) == STB_WEAK,
 			               mod->unres);
 		}
@@ -652,9 +645,9 @@ static void handle_modversions(struct module *mod, struct elf_info *info,
 			sym_add_exported(symname + strlen(KSYMTAB_PFX), mod,
 					export);
 		}
-		if (strcmp(symname, MODULE_SYMBOL_PREFIX "init_module") == 0)
+		if (strcmp(symname, VMLINUX_SYMBOL_STR(init_module)) == 0)
 			mod->has_init = 1;
-		if (strcmp(symname, MODULE_SYMBOL_PREFIX "cleanup_module") == 0)
+		if (strcmp(symname, VMLINUX_SYMBOL_STR(cleanup_module)) == 0)
 			mod->has_cleanup = 1;
 		break;
 	}

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [RFC -next] linux/linkage.h: fix symbol prefix handling
  2013-03-12  4:48         ` Rusty Russell
@ 2013-03-12 12:33           ` Stephen Rothwell
  2013-03-13  0:00             ` Rusty Russell
  2013-03-12 12:36           ` James Hogan
  1 sibling, 1 reply; 15+ messages in thread
From: Stephen Rothwell @ 2013-03-12 12:33 UTC (permalink / raw)
  To: Rusty Russell
  Cc: James Hogan, Al Viro, Michal Marek, Andrew Morton, Guenter Roeck,
	Jean Delvare, linux-kbuild, linux-kernel, Mike Frysinger,
	uclinux-dist-devel, linux-next

[-- Attachment #1: Type: text/plain, Size: 725 bytes --]

Hi Rusty,

Looks partly better.   You seem to be using
CONFIG_SYMBOL_PREFIX_UNDERSCORE but selecting
CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX.  One trivial comment below.

Maybe this was an unfinished version of the patch?

On Tue, 12 Mar 2013 15:18:15 +1030 Rusty Russell <rusty@rustcorp.com.au> wrote:
>
> diff --git a/arch/blackfin/Kconfig b/arch/blackfin/Kconfig
> index 600494c..314ee6a 100644
> --- a/arch/blackfin/Kconfig
> +++ b/arch/blackfin/Kconfig
> @@ -45,6 +42,7 @@ config BLACKFIN
>  	select HAVE_MOD_ARCH_SPECIFIC
>  	select MODULES_USE_ELF_RELA
>  
> +
>  config GENERIC_CSUM
>  	def_bool y

Extra blank line added.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC -next] linux/linkage.h: fix symbol prefix handling
  2013-03-12  4:48         ` Rusty Russell
  2013-03-12 12:33           ` Stephen Rothwell
@ 2013-03-12 12:36           ` James Hogan
  1 sibling, 0 replies; 15+ messages in thread
From: James Hogan @ 2013-03-12 12:36 UTC (permalink / raw)
  To: Rusty Russell
  Cc: Stephen Rothwell, Al Viro, Michal Marek, Andrew Morton,
	Guenter Roeck, Jean Delvare, linux-kbuild, linux-kernel,
	Mike Frysinger, uclinux-dist-devel, linux-next

Hi Rusty,

On 12/03/13 04:48, Rusty Russell wrote:
> v2: Rename CONFIG_SYMBOL_PREFIX_UNDERSCORE to CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX,
>     which is defined in arch/Kconfig and selected by the 3 archs which need it.

Sorry I didn't get a chance to try your patch yesterday.

> Subject: CONFIG_SYMBOL_PREFIX: cleanup.
> 
> We have CONFIG_SYMBOL_PREFIX, which three archs define to the string
> "_".  But Al Viro broke this in "consolidate cond_syscall and
> SYSCALL_ALIAS declarations", and he's not the first to do so.
> 
> Using CONFIG_SYMBOL_PREFIX is awkward, since we usually just want to
> prefix it so something.  So various places define helpers which are
> defined to nothing if CONFIG_SYMBOL_PREFIX isn't set:
> 
> 1) include/asm-generic/unistd.h defines __SYMBOL_PREFIX.
> 2) include/asm-generic/vmlinux.lds.h defines VMLINUX_SYMBOL(sym)
> 3) include/linux/export.h defines MODULE_SYMBOL_PREFIX.
> 4) include/linux/kernel.h defines SYMBOL_PREFIX (which differs from #7)
> 5) kernel/modsign_certificate.S defines ASM_SYMBOL(sym)
> 6) scripts/modpost.c defines MODULE_SYMBOL_PREFIX
> 7) scripts/Makefile.lib defines SYMBOL_PREFIX on the commandline if
>    CONFIG_SYMBOL_PREFIX is set, so that we have a non-string version
>    for pasting.
> 
> (arch/h8300/include/asm/linkage.h defines SYMBOL_NAME(), too).
> 
> Let's solve this properly:
> 1) No more generic prefix, just CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX.
> 2) Make linux/export.h usable from asm.
> 3) Define VMLINUX_SYMBOL, VMLINUX_SYMBOL_NAME and VMLINUX_SYMBOL_PREFIX_STR.

You don't seem to define VMLINUX_SYMBOL_NAME

> 4) Make everyone use them.
> 
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

After a few modifications detailed below (with a fixup patch at the
end for convenience) it seems to work no worse than before for metag
(module symbol versioning was apparently already broken, I need to
look into that).

Reviewed-by: James Hogan <james.hogan@imgtec.com>
Tested-by: James Hogan <james.hogan@imgtec.com> (on metag)

Cheers
James

> 
> diff --git a/Makefile b/Makefile
> index a05ea42..9dc948d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1398,7 +1398,7 @@ quiet_cmd_rmfiles = $(if $(wildcard $(rm-files)),CLEAN   $(wildcard $(rm-files))
>  # Run depmod only if we have System.map and depmod is executable
>  quiet_cmd_depmod = DEPMOD  $(KERNELRELEASE)
>        cmd_depmod = $(CONFIG_SHELL) $(srctree)/scripts/depmod.sh $(DEPMOD) \
> -                   $(KERNELRELEASE) "$(patsubst "%",%,$(CONFIG_SYMBOL_PREFIX))"
> +                   $(KERNELRELEASE) "$(patsubst y,_,$(CONFIG_SYMBOL_PREFIX_UNDERSCORE))"

should this be CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX now?


> diff --git a/arch/h8300/Kconfig b/arch/h8300/Kconfig
> index ae8551e..ba043e3 100644
> --- a/arch/h8300/Kconfig
> +++ b/arch/h8300/Kconfig
> @@ -12,10 +12,10 @@ config H8300
>  	select MODULES_USE_ELF_RELA
>  	select OLD_SIGSUSPEND3
>  	select OLD_SIGACTION
> +	select HAVE_UNDERSCORE_SYMBOL_PREFIX
>  
> -config SYMBOL_PREFIX
> -	string
> -	default "_"
> +config SYMBOL_PREFIX_UNDERSCORE
> +	def_bool y

Not needed anymore I think


> diff --git a/arch/metag/Kconfig b/arch/metag/Kconfig
> index afc8973..88272ce 100644
> --- a/arch/metag/Kconfig
> +++ b/arch/metag/Kconfig
> @@ -1,6 +1,5 @@
> -config SYMBOL_PREFIX
> -	string
> -	default "_"
> +config SYMBOL_PREFIX_UNDERSCORE
> +	def_bool y

Same again.


> diff --git a/include/asm-generic/unistd.h b/include/asm-generic/unistd.h
> index 4077b5d..80122d4 100644
> --- a/include/asm-generic/unistd.h
> +++ b/include/asm-generic/unistd.h
> @@ -1,4 +1,5 @@
>  #include <uapi/asm-generic/unistd.h>
> +#include <linux/export.h>
>  
>  /*
>   * These are required system calls, we should
> @@ -17,12 +18,7 @@
>   * but it doesn't work on all toolchains, so we just do it by hand
>   */
>  #ifndef cond_syscall
> -#ifdef CONFIG_SYMBOL_PREFIX
> -#define __SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX
> -#else
> -#define __SYMBOL_PREFIX
> -#endif
> -#define cond_syscall(x) asm(".weak\t" __SYMBOL_PREFIX #x "\n\t" \
> -			    ".set\t" __SYMBOL_PREFIX #x "," \
> -			    __SYMBOL_PREFIX "sys_ni_syscall")
> +#define cond_syscall(x) asm(".weak\t" SYMBOL_NAME_STR(x) "\n\t"	\
> +			    ".set\t" SYMBOL_NAME_STR(x) ","	\
> +			    SYMBOL_NAME_STR(sys_ni_syscall))

s/SYMBOL_NAME_STR/VMLINUX_SYMBOL_STR/



> diff --git a/include/linux/export.h b/include/linux/export.h
> index 696c0f4..0080e93 100644
> --- a/include/linux/export.h
> +++ b/include/linux/export.h
> @@ -7,15 +7,27 @@
>   *
>   * If you feel the need to add #include <linux/foo.h> to this file
>   * then you are doing something wrong and should go away silently.
> + *
> + * If you think the above arrogance just encourages more people to add
> + * random crap to this file, you're not alone.

:-)

>   */
>  
>  /* Some toolchains use a `_' prefix for all user symbols. */
> -#ifdef CONFIG_SYMBOL_PREFIX
> -#define MODULE_SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX
> +#ifdef CONFIG_SYMBOL_PREFIX_UNDERSCORE

CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX


> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> index 3d569d6..d767681 100644
> --- a/scripts/link-vmlinux.sh
> +++ b/scripts/link-vmlinux.sh
> @@ -74,9 +74,8 @@ kallsyms()
>  	info KSYM ${2}
>  	local kallsymopt;
>  
> -	if [ -n "${CONFIG_SYMBOL_PREFIX}" ]; then
> -		kallsymopt="${kallsymopt} \
> -			    --symbol-prefix=${CONFIG_SYMBOL_PREFIX}"
> +	if [ -n "${CONFIG_SYMBOL_PREFIX_UNDERSCORE}" ]; then

CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX




diff --git a/Makefile b/Makefile
index 9dc948d..0b09ba5 100644
--- a/Makefile
+++ b/Makefile
@@ -1398,7 +1398,7 @@ quiet_cmd_rmfiles = $(if $(wildcard $(rm-files)),CLEAN   $(wildcard $(rm-files))
 # Run depmod only if we have System.map and depmod is executable
 quiet_cmd_depmod = DEPMOD  $(KERNELRELEASE)
       cmd_depmod = $(CONFIG_SHELL) $(srctree)/scripts/depmod.sh $(DEPMOD) \
-                   $(KERNELRELEASE) "$(patsubst y,_,$(CONFIG_SYMBOL_PREFIX_UNDERSCORE))"
+                   $(KERNELRELEASE) "$(patsubst y,_,$(CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX))"
 
 # Create temporary dir for module support files
 # clean it up only when building all modules
diff --git a/arch/h8300/Kconfig b/arch/h8300/Kconfig
index ba043e3..2333cf6 100644
--- a/arch/h8300/Kconfig
+++ b/arch/h8300/Kconfig
@@ -14,9 +14,6 @@ config H8300
 	select OLD_SIGACTION
 	select HAVE_UNDERSCORE_SYMBOL_PREFIX
 
-config SYMBOL_PREFIX_UNDERSCORE
-	def_bool y
-
 config MMU
 	bool
 	default n
diff --git a/arch/metag/Kconfig b/arch/metag/Kconfig
index 88272ce..2099617 100644
--- a/arch/metag/Kconfig
+++ b/arch/metag/Kconfig
@@ -1,6 +1,3 @@
-config SYMBOL_PREFIX_UNDERSCORE
-	def_bool y
-
 config METAG
 	def_bool y
 	select EMBEDDED
diff --git a/include/asm-generic/unistd.h b/include/asm-generic/unistd.h
index 80122d4..15c0598 100644
--- a/include/asm-generic/unistd.h
+++ b/include/asm-generic/unistd.h
@@ -18,7 +18,7 @@
  * but it doesn't work on all toolchains, so we just do it by hand
  */
 #ifndef cond_syscall
-#define cond_syscall(x) asm(".weak\t" SYMBOL_NAME_STR(x) "\n\t"	\
-			    ".set\t" SYMBOL_NAME_STR(x) ","	\
-			    SYMBOL_NAME_STR(sys_ni_syscall))
+#define cond_syscall(x) asm(".weak\t" VMLINUX_SYMBOL_STR(x) "\n\t"	\
+			    ".set\t" VMLINUX_SYMBOL_STR(x) ","	\
+			    VMLINUX_SYMBOL_STR(sys_ni_syscall))
 #endif
diff --git a/include/linux/export.h b/include/linux/export.h
index 0080e93..fc83b2a 100644
--- a/include/linux/export.h
+++ b/include/linux/export.h
@@ -13,7 +13,7 @@
  */
 
 /* Some toolchains use a `_' prefix for all user symbols. */
-#ifdef CONFIG_SYMBOL_PREFIX_UNDERSCORE
+#ifdef CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX
 #define __VMLINUX_SYMBOL(x) _##x
 #define __VMLINUX_SYMBOL_STR(x) "_" #x
 #define VMLINUX_SYMBOL_PREFIX_STR "_"
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index d767681..0149949 100644
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -74,7 +74,7 @@ kallsyms()
 	info KSYM ${2}
 	local kallsymopt;
 
-	if [ -n "${CONFIG_SYMBOL_PREFIX_UNDERSCORE}" ]; then
+	if [ -n "${CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX}" ]; then
 		kallsymopt="${kallsymopt} --symbol-prefix=_"
 	fi
 

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [RFC -next] linux/linkage.h: fix symbol prefix handling
  2013-03-12 12:33           ` Stephen Rothwell
@ 2013-03-13  0:00             ` Rusty Russell
  2013-03-13  6:31               ` Sam Ravnborg
  0 siblings, 1 reply; 15+ messages in thread
From: Rusty Russell @ 2013-03-13  0:00 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: James Hogan, Al Viro, Michal Marek, Andrew Morton, Guenter Roeck,
	Jean Delvare, linux-kbuild, linux-kernel, Mike Frysinger,
	uclinux-dist-devel, linux-next

Stephen Rothwell <sfr@canb.auug.org.au> writes:

> Hi Rusty,
>
> Looks partly better.   You seem to be using
> CONFIG_SYMBOL_PREFIX_UNDERSCORE but selecting
> CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX.  One trivial comment below.
>
> Maybe this was an unfinished version of the patch?

Indeed.  It was crap.... now I've applied James' copious fixes, and
actually, y'know, proof-read it.

This is what I'll push into my fixes/ branch now, and if it survives a
day in linux-next, will push to Linus.

Thanks,
Rusty.

v2: Rename CONFIG_SYMBOL_PREFIX_UNDERSCORE to
    CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX, which is defined in
    arch/Kconfig and selected by the 3 archs which need it.

v3: Clean up remnants of v1, s/VMLINUX_SYMBOL_NAME/VMLINUX_SYMBOL_STR/
    in description, note that Al's commit is in linux-next only,
    remove whitespace-only hunk.

CONFIG_SYMBOL_PREFIX: cleanup.

We have CONFIG_SYMBOL_PREFIX, which three archs define to the string
"_".  But Al Viro broke this in "consolidate cond_syscall and
SYSCALL_ALIAS declarations" (in linux-next), and he's not the first to
do so.

Using CONFIG_SYMBOL_PREFIX is awkward, since we usually just want to
prefix it so something.  So various places define helpers which are
defined to nothing if CONFIG_SYMBOL_PREFIX isn't set:

1) include/asm-generic/unistd.h defines __SYMBOL_PREFIX.
2) include/asm-generic/vmlinux.lds.h defines VMLINUX_SYMBOL(sym)
3) include/linux/export.h defines MODULE_SYMBOL_PREFIX.
4) include/linux/kernel.h defines SYMBOL_PREFIX (which differs from #7)
5) kernel/modsign_certificate.S defines ASM_SYMBOL(sym)
6) scripts/modpost.c defines MODULE_SYMBOL_PREFIX
7) scripts/Makefile.lib defines SYMBOL_PREFIX on the commandline if
   CONFIG_SYMBOL_PREFIX is set, so that we have a non-string version
   for pasting.

(arch/h8300/include/asm/linkage.h defines SYMBOL_NAME(), too).

Let's solve this properly:
1) No more generic prefix, just CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX.
2) Make linux/export.h usable from asm.
3) Define VMLINUX_SYMBOL, VMLINUX_SYMBOL_STR and VMLINUX_SYMBOL_PREFIX_STR.
4) Make everyone use them.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Reviewed-by: James Hogan <james.hogan@imgtec.com>
Tested-by: James Hogan <james.hogan@imgtec.com> (on metag)

diff --git a/Makefile b/Makefile
index a05ea42..10fb6c7 100644
--- a/Makefile
+++ b/Makefile
@@ -1398,7 +1398,7 @@ quiet_cmd_rmfiles = $(if $(wildcard $(rm-files)),CLEAN   $(wildcard $(rm-files))
 # Run depmod only if we have System.map and depmod is executable
 quiet_cmd_depmod = DEPMOD  $(KERNELRELEASE)
       cmd_depmod = $(CONFIG_SHELL) $(srctree)/scripts/depmod.sh $(DEPMOD) \
-                   $(KERNELRELEASE) "$(patsubst "%",%,$(CONFIG_SYMBOL_PREFIX))"
+                   $(KERNELRELEASE) "$(patsubst y,_,$(CONFIG_HAVE_SYMBOL_PREFIX_UNDERSCORE))"
 
 # Create temporary dir for module support files
 # clean it up only when building all modules
diff --git a/arch/Kconfig b/arch/Kconfig
index 5a1779c..12ba8f9 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -391,6 +391,12 @@ config MODULES_USE_ELF_REL
 	  Modules only use ELF REL relocations.  Modules with ELF RELA
 	  relocations will give an error.
 
+config HAVE_UNDERSCORE_SYMBOL_PREFIX
+	bool
+	help
+	  Some architectures generate an _ in front of C symbols; things like
+	  module loading and assembly files need to know about this.
+
 #
 # ABI hall of shame
 #
diff --git a/arch/blackfin/Kconfig b/arch/blackfin/Kconfig
index 600494c..314ee6a 100644
--- a/arch/blackfin/Kconfig
+++ b/arch/blackfin/Kconfig
@@ -1,7 +1,3 @@
-config SYMBOL_PREFIX
-	string
-	default "_"
-
 config MMU
 	def_bool n
 
@@ -33,6 +29,7 @@ config BLACKFIN
 	select ARCH_HAVE_CUSTOM_GPIO_H
 	select ARCH_WANT_OPTIONAL_GPIOLIB
 	select HAVE_UID16
+	select HAVE_UNDERSCORE_SYMBOL_PREFIX
 	select HAVE_VIRT_TO_BUS
 	select ARCH_WANT_IPC_PARSE_VERSION
 	select HAVE_GENERIC_HARDIRQS
diff --git a/arch/h8300/Kconfig b/arch/h8300/Kconfig
index ae8551e..2333cf6 100644
--- a/arch/h8300/Kconfig
+++ b/arch/h8300/Kconfig
@@ -12,10 +12,7 @@ config H8300
 	select MODULES_USE_ELF_RELA
 	select OLD_SIGSUSPEND3
 	select OLD_SIGACTION
-
-config SYMBOL_PREFIX
-	string
-	default "_"
+	select HAVE_UNDERSCORE_SYMBOL_PREFIX
 
 config MMU
 	bool
diff --git a/arch/metag/Kconfig b/arch/metag/Kconfig
index afc8973..2099617 100644
--- a/arch/metag/Kconfig
+++ b/arch/metag/Kconfig
@@ -1,7 +1,3 @@
-config SYMBOL_PREFIX
-	string
-	default "_"
-
 config METAG
 	def_bool y
 	select EMBEDDED
@@ -27,6 +23,7 @@ config METAG
 	select HAVE_MOD_ARCH_SPECIFIC
 	select HAVE_PERF_EVENTS
 	select HAVE_SYSCALL_TRACEPOINTS
+	select HAVE_UNDERSCORE_SYMBOL_PREFIX
 	select IRQ_DOMAIN
 	select MODULES_USE_ELF_RELA
 	select OF
diff --git a/drivers/mtd/chips/gen_probe.c b/drivers/mtd/chips/gen_probe.c
index 3b9a284..00d20ad 100644
--- a/drivers/mtd/chips/gen_probe.c
+++ b/drivers/mtd/chips/gen_probe.c
@@ -204,14 +204,14 @@ static inline struct mtd_info *cfi_cmdset_unknown(struct map_info *map,
 	struct cfi_private *cfi = map->fldrv_priv;
 	__u16 type = primary?cfi->cfiq->P_ID:cfi->cfiq->A_ID;
 #ifdef CONFIG_MODULES
-	char probename[16+sizeof(MODULE_SYMBOL_PREFIX)];
+	char probename[16+sizeof(VMLINUX_SYMBOL_PREFIX_STR)];
 	cfi_cmdset_fn_t *probe_function;
 
-	sprintf(probename, MODULE_SYMBOL_PREFIX "cfi_cmdset_%4.4X", type);
+	sprintf(probename, VMLINUX_SYMBOL_PREFIX_STR "cfi_cmdset_%4.4X", type);
 
 	probe_function = __symbol_get(probename);
 	if (!probe_function) {
-		request_module(probename + sizeof(MODULE_SYMBOL_PREFIX) - 1);
+		request_module(probename + sizeof(VMLINUX_SYMBOL_PREFIX_STR)-1);
 		probe_function = __symbol_get(probename);
 	}
 
diff --git a/include/asm-generic/unistd.h b/include/asm-generic/unistd.h
index 4077b5d..15c0598 100644
--- a/include/asm-generic/unistd.h
+++ b/include/asm-generic/unistd.h
@@ -1,4 +1,5 @@
 #include <uapi/asm-generic/unistd.h>
+#include <linux/export.h>
 
 /*
  * These are required system calls, we should
@@ -17,12 +18,7 @@
  * but it doesn't work on all toolchains, so we just do it by hand
  */
 #ifndef cond_syscall
-#ifdef CONFIG_SYMBOL_PREFIX
-#define __SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX
-#else
-#define __SYMBOL_PREFIX
-#endif
-#define cond_syscall(x) asm(".weak\t" __SYMBOL_PREFIX #x "\n\t" \
-			    ".set\t" __SYMBOL_PREFIX #x "," \
-			    __SYMBOL_PREFIX "sys_ni_syscall")
+#define cond_syscall(x) asm(".weak\t" VMLINUX_SYMBOL_STR(x) "\n\t"	\
+			    ".set\t" VMLINUX_SYMBOL_STR(x) ","	\
+			    VMLINUX_SYMBOL_STR(sys_ni_syscall))
 #endif
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index afa12c7..eb58d2d 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -52,13 +52,7 @@
 #define LOAD_OFFSET 0
 #endif
 
-#ifndef SYMBOL_PREFIX
-#define VMLINUX_SYMBOL(sym) sym
-#else
-#define PASTE2(x,y) x##y
-#define PASTE(x,y) PASTE2(x,y)
-#define VMLINUX_SYMBOL(sym) PASTE(SYMBOL_PREFIX, sym)
-#endif
+#include <linux/export.h>
 
 /* Align . to a 8 byte boundary equals to maximum function alignment. */
 #define ALIGN_FUNCTION()  . = ALIGN(8)
diff --git a/include/linux/export.h b/include/linux/export.h
index 696c0f4..fc83b2a 100644
--- a/include/linux/export.h
+++ b/include/linux/export.h
@@ -7,15 +7,27 @@
  *
  * If you feel the need to add #include <linux/foo.h> to this file
  * then you are doing something wrong and should go away silently.
+ *
+ * If you think the above arrogance just encourages more people to add
+ * random crap to this file, you're not alone.
  */
 
 /* Some toolchains use a `_' prefix for all user symbols. */
-#ifdef CONFIG_SYMBOL_PREFIX
-#define MODULE_SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX
+#ifdef CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX
+#define __VMLINUX_SYMBOL(x) _##x
+#define __VMLINUX_SYMBOL_STR(x) "_" #x
+#define VMLINUX_SYMBOL_PREFIX_STR "_"
 #else
-#define MODULE_SYMBOL_PREFIX ""
+#define __VMLINUX_SYMBOL(x) x
+#define __VMLINUX_SYMBOL_STR(x) #x
+#define VMLINUX_SYMBOL_PREFIX_STR ""
 #endif
 
+/* Indirect, so macros are expanded before pasting. */
+#define VMLINUX_SYMBOL(x) __VMLINUX_SYMBOL(x)
+#define VMLINUX_SYMBOL_STR(x) __VMLINUX_SYMBOL_STR(x)
+
+#ifndef __ASSEMBLY__
 struct kernel_symbol
 {
 	unsigned long value;
@@ -51,7 +63,7 @@ extern struct module __this_module;
 	__CRC_SYMBOL(sym, sec)					\
 	static const char __kstrtab_##sym[]			\
 	__attribute__((section("__ksymtab_strings"), aligned(1))) \
-	= MODULE_SYMBOL_PREFIX #sym;				\
+	= VMLINUX_SYMBOL_STR(sym);				\
 	static const struct kernel_symbol __ksymtab_##sym	\
 	__used							\
 	__attribute__((section("___ksymtab" sec "+" #sym), unused))	\
@@ -85,5 +97,6 @@ extern struct module __this_module;
 #define EXPORT_UNUSED_SYMBOL_GPL(sym)
 
 #endif /* CONFIG_MODULES */
+#endif /* !__ASSEMBLY__ */
 
 #endif /* _LINUX_EXPORT_H */
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 80d3687..e13e992 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -723,13 +723,6 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
 /* Trap pasters of __FUNCTION__ at compile-time */
 #define __FUNCTION__ (__func__)
 
-/* This helps us to avoid #ifdef CONFIG_SYMBOL_PREFIX */
-#ifdef CONFIG_SYMBOL_PREFIX
-#define SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX
-#else
-#define SYMBOL_PREFIX ""
-#endif
-
 /* Rebuild everything on CONFIG_FTRACE_MCOUNT_RECORD */
 #ifdef CONFIG_FTRACE_MCOUNT_RECORD
 # define REBUILD_DUE_TO_FTRACE_MCOUNT_RECORD
diff --git a/include/linux/module.h b/include/linux/module.h
index ead1b57..46f1ea0 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -190,7 +190,7 @@ extern int modules_disabled; /* for sysctl */
 /* Get/put a kernel symbol (calls must be symmetric) */
 void *__symbol_get(const char *symbol);
 void *__symbol_get_gpl(const char *symbol);
-#define symbol_get(x) ((typeof(&x))(__symbol_get(MODULE_SYMBOL_PREFIX #x)))
+#define symbol_get(x) ((typeof(&x))(__symbol_get(VMLINUX_SYMBOL_STR(x))))
 
 /* modules using other modules: kdb wants to see this. */
 struct module_use {
@@ -453,7 +453,7 @@ extern void __module_put_and_exit(struct module *mod, long code)
 #ifdef CONFIG_MODULE_UNLOAD
 unsigned long module_refcount(struct module *mod);
 void __symbol_put(const char *symbol);
-#define symbol_put(x) __symbol_put(MODULE_SYMBOL_PREFIX #x)
+#define symbol_put(x) __symbol_put(VMLINUX_SYMBOL_STR(x))
 void symbol_put_addr(void *addr);
 
 /* Sometimes we know we already have a refcount, and it's easier not
diff --git a/kernel/modsign_certificate.S b/kernel/modsign_certificate.S
index 246b4c6..4a9a86d 100644
--- a/kernel/modsign_certificate.S
+++ b/kernel/modsign_certificate.S
@@ -1,15 +1,8 @@
-/* SYMBOL_PREFIX defined on commandline from CONFIG_SYMBOL_PREFIX */
-#ifndef SYMBOL_PREFIX
-#define ASM_SYMBOL(sym) sym
-#else
-#define PASTE2(x,y) x##y
-#define PASTE(x,y) PASTE2(x,y)
-#define ASM_SYMBOL(sym) PASTE(SYMBOL_PREFIX, sym)
-#endif
+#include <linux/export.h>
 
 #define GLOBAL(name)	\
-	.globl ASM_SYMBOL(name);	\
-	ASM_SYMBOL(name):
+	.globl VMLINUX_SYMBOL(name);	\
+	VMLINUX_SYMBOL(name):
 
 	.section ".init.data","aw"
 
diff --git a/kernel/module.c b/kernel/module.c
index 0925c9a..cfd4a3f 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1209,7 +1209,7 @@ static inline int check_modstruct_version(Elf_Shdr *sechdrs,
 
 	/* Since this should be found in kernel (which can't be removed),
 	 * no locking is necessary. */
-	if (!find_symbol(MODULE_SYMBOL_PREFIX "module_layout", NULL,
+	if (!find_symbol(VMLINUX_SYMBOL_STR(module_layout), NULL,
 			 &crc, true, false))
 		BUG();
 	return check_version(sechdrs, versindex, "module_layout", mod, crc,
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 07125e6..a373a1f 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -119,13 +119,6 @@ _c_flags += $(if $(patsubst n%,, \
 		$(CFLAGS_GCOV))
 endif
 
-ifdef CONFIG_SYMBOL_PREFIX
-_sym_flags = -DSYMBOL_PREFIX=$(patsubst "%",%,$(CONFIG_SYMBOL_PREFIX))
-_cpp_flags += $(_sym_flags)
-_a_flags += $(_sym_flags)
-endif
-
-
 # If building the kernel in a separate objtree expand all occurrences
 # of -Idir to -I$(srctree)/dir except for absolute paths (starting with '/').
 
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 3d569d6..0149949 100644
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -74,9 +74,8 @@ kallsyms()
 	info KSYM ${2}
 	local kallsymopt;
 
-	if [ -n "${CONFIG_SYMBOL_PREFIX}" ]; then
-		kallsymopt="${kallsymopt} \
-			    --symbol-prefix=${CONFIG_SYMBOL_PREFIX}"
+	if [ -n "${CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX}" ]; then
+		kallsymopt="${kallsymopt} --symbol-prefix=_"
 	fi
 
 	if [ -n "${CONFIG_KALLSYMS_ALL}" ]; then
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 78b30c1..6985021 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -18,14 +18,7 @@
 #include "modpost.h"
 #include "../../include/generated/autoconf.h"
 #include "../../include/linux/license.h"
-
-/* Some toolchains use a `_' prefix for all user symbols. */
-#ifdef CONFIG_SYMBOL_PREFIX
-#define MODULE_SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX
-#else
-#define MODULE_SYMBOL_PREFIX ""
-#endif
-
+#include "../../include/linux/export.h"
 
 /* Are we using CONFIG_MODVERSIONS? */
 int modversions = 0;
@@ -562,7 +555,7 @@ static void parse_elf_finish(struct elf_info *info)
 static int ignore_undef_symbol(struct elf_info *info, const char *symname)
 {
 	/* ignore __this_module, it will be resolved shortly */
-	if (strcmp(symname, MODULE_SYMBOL_PREFIX "__this_module") == 0)
+	if (strcmp(symname, VMLINUX_SYMBOL_STR(__this_module)) == 0)
 		return 1;
 	/* ignore global offset table */
 	if (strcmp(symname, "_GLOBAL_OFFSET_TABLE_") == 0)
@@ -583,8 +576,8 @@ static int ignore_undef_symbol(struct elf_info *info, const char *symname)
 	return 0;
 }
 
-#define CRC_PFX     MODULE_SYMBOL_PREFIX "__crc_"
-#define KSYMTAB_PFX MODULE_SYMBOL_PREFIX "__ksymtab_"
+#define CRC_PFX     VMLINUX_SYMBOL_STR(__crc_)
+#define KSYMTAB_PFX VMLINUX_SYMBOL_STR(__ksymtab_)
 
 static void handle_modversions(struct module *mod, struct elf_info *info,
 			       Elf_Sym *sym, const char *symname)
@@ -637,11 +630,11 @@ static void handle_modversions(struct module *mod, struct elf_info *info,
 		}
 #endif
 
-		if (memcmp(symname, MODULE_SYMBOL_PREFIX,
-			   strlen(MODULE_SYMBOL_PREFIX)) == 0) {
+		if (memcmp(symname, VMLINUX_SYMBOL_PREFIX_STR,
+			   strlen(VMLINUX_SYMBOL_PREFIX_STR)) == 0) {
 			mod->unres =
 			  alloc_symbol(symname +
-			               strlen(MODULE_SYMBOL_PREFIX),
+			               strlen(VMLINUX_SYMBOL_PREFIX_STR),
 			               ELF_ST_BIND(sym->st_info) == STB_WEAK,
 			               mod->unres);
 		}
@@ -652,9 +645,9 @@ static void handle_modversions(struct module *mod, struct elf_info *info,
 			sym_add_exported(symname + strlen(KSYMTAB_PFX), mod,
 					export);
 		}
-		if (strcmp(symname, MODULE_SYMBOL_PREFIX "init_module") == 0)
+		if (strcmp(symname, VMLINUX_SYMBOL_STR(init_module)) == 0)
 			mod->has_init = 1;
-		if (strcmp(symname, MODULE_SYMBOL_PREFIX "cleanup_module") == 0)
+		if (strcmp(symname, VMLINUX_SYMBOL_STR(cleanup_module)) == 0)
 			mod->has_cleanup = 1;
 		break;
 	}

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [RFC -next] linux/linkage.h: fix symbol prefix handling
  2013-03-13  0:00             ` Rusty Russell
@ 2013-03-13  6:31               ` Sam Ravnborg
  2013-03-13  9:21                 ` James Hogan
  2013-03-14  4:00                 ` Rusty Russell
  0 siblings, 2 replies; 15+ messages in thread
From: Sam Ravnborg @ 2013-03-13  6:31 UTC (permalink / raw)
  To: Rusty Russell
  Cc: Stephen Rothwell, James Hogan, Al Viro, Michal Marek,
	Andrew Morton, Guenter Roeck, Jean Delvare, linux-kbuild,
	linux-kernel, Mike Frysinger, uclinux-dist-devel, linux-next

> actually, y'know, proof-read it.
Hmm..


> +                   $(KERNELRELEASE) "$(patsubst y,_,$(CONFIG_HAVE_SYMBOL_PREFIX_UNDERSCORE))"
>  
> +config HAVE_UNDERSCORE_SYMBOL_PREFIX

HAVE_UNDERSCORE_... or HAVE_SYMBOL_... confusion.
I prefer the HAVE_SYMBOL_... variant but no strong feelings..

> + *
> + * If you think the above arrogance just encourages more people to add
> + * random crap to this file, you're not alone.
Kill this.

>  /* Some toolchains use a `_' prefix for all user symbols. */
> -#ifdef CONFIG_SYMBOL_PREFIX
> -#define MODULE_SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX
> +#ifdef CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX
> +#define __VMLINUX_SYMBOL(x) _##x
> +#define __VMLINUX_SYMBOL_STR(x) "_" #x
> +#define VMLINUX_SYMBOL_PREFIX_STR "_"
>  #else
> -#define MODULE_SYMBOL_PREFIX ""
> +#define __VMLINUX_SYMBOL(x) x
> +#define __VMLINUX_SYMBOL_STR(x) #x
> +#define VMLINUX_SYMBOL_PREFIX_STR ""
>  #endif

We know the prefix is an underscore. No benefits from defining
VMLINUX_SYMBOL_PREFIX_STR. The config name even syas so.
Skipping the above give us only one way to check
for the prefix - today we mix the two.

	Sam


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC -next] linux/linkage.h: fix symbol prefix handling
  2013-03-13  6:31               ` Sam Ravnborg
@ 2013-03-13  9:21                 ` James Hogan
  2013-03-13 18:15                   ` Sam Ravnborg
  2013-03-14  4:00                 ` Rusty Russell
  1 sibling, 1 reply; 15+ messages in thread
From: James Hogan @ 2013-03-13  9:21 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: Rusty Russell, Stephen Rothwell, Al Viro, Michal Marek,
	Andrew Morton, Guenter Roeck, Jean Delvare, linux-kbuild,
	linux-kernel, Mike Frysinger, uclinux-dist-devel, linux-next

Hi Sam,

On 13/03/13 06:31, Sam Ravnborg wrote:
>>  /* Some toolchains use a `_' prefix for all user symbols. */
>> -#ifdef CONFIG_SYMBOL_PREFIX
>> -#define MODULE_SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX
>> +#ifdef CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX
>> +#define __VMLINUX_SYMBOL(x) _##x
>> +#define __VMLINUX_SYMBOL_STR(x) "_" #x
>> +#define VMLINUX_SYMBOL_PREFIX_STR "_"
>>  #else
>> -#define MODULE_SYMBOL_PREFIX ""
>> +#define __VMLINUX_SYMBOL(x) x
>> +#define __VMLINUX_SYMBOL_STR(x) #x
>> +#define VMLINUX_SYMBOL_PREFIX_STR ""
>>  #endif
> 
> We know the prefix is an underscore. No benefits from defining
> VMLINUX_SYMBOL_PREFIX_STR.

scripts/modpost.c uses the actual prefix string though:

>	if (memcmp(symname, MODULE_SYMBOL_PREFIX,
>		   strlen(MODULE_SYMBOL_PREFIX)) == 0) {
>		mod->unres =
>		  alloc_symbol(symname +
>		               strlen(MODULE_SYMBOL_PREFIX),
> 		               ELF_ST_BIND(sym->st_info) == STB_WEAK,
> 		               mod->unres);

See also my "module: fix symbol versioning with symbol prefixes" patch
for an additional use in modpost.

Having a MODULE_SYMBOL_PREFIX/VMLINUX_SYMBOL_PREFIX_STR seems like the
simplest way to keep this code clean and readable without special cases.

Cheers
James

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC -next] linux/linkage.h: fix symbol prefix handling
  2013-03-13  9:21                 ` James Hogan
@ 2013-03-13 18:15                   ` Sam Ravnborg
  0 siblings, 0 replies; 15+ messages in thread
From: Sam Ravnborg @ 2013-03-13 18:15 UTC (permalink / raw)
  To: James Hogan
  Cc: Rusty Russell, Stephen Rothwell, Al Viro, Michal Marek,
	Andrew Morton, Guenter Roeck, Jean Delvare, linux-kbuild,
	linux-kernel, Mike Frysinger, uclinux-dist-devel, linux-next

On Wed, Mar 13, 2013 at 09:21:01AM +0000, James Hogan wrote:
> Hi Sam,
> 
> On 13/03/13 06:31, Sam Ravnborg wrote:
> >>  /* Some toolchains use a `_' prefix for all user symbols. */
> >> -#ifdef CONFIG_SYMBOL_PREFIX
> >> -#define MODULE_SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX
> >> +#ifdef CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX
> >> +#define __VMLINUX_SYMBOL(x) _##x
> >> +#define __VMLINUX_SYMBOL_STR(x) "_" #x
> >> +#define VMLINUX_SYMBOL_PREFIX_STR "_"
> >>  #else
> >> -#define MODULE_SYMBOL_PREFIX ""
> >> +#define __VMLINUX_SYMBOL(x) x
> >> +#define __VMLINUX_SYMBOL_STR(x) #x
> >> +#define VMLINUX_SYMBOL_PREFIX_STR ""
> >>  #endif
> > 
> > We know the prefix is an underscore. No benefits from defining
> > VMLINUX_SYMBOL_PREFIX_STR.
> 
> scripts/modpost.c uses the actual prefix string though:
> 
> >	if (memcmp(symname, MODULE_SYMBOL_PREFIX,
> >		   strlen(MODULE_SYMBOL_PREFIX)) == 0) {
> >		mod->unres =
> >		  alloc_symbol(symname +
> >		               strlen(MODULE_SYMBOL_PREFIX),
> > 		               ELF_ST_BIND(sym->st_info) == STB_WEAK,
> > 		               mod->unres);
> 

Allowing ourself to know this is "_" then we could make the above code look
something like:

	if (have_symbol_prefix_underscore) {
		if (symname[0] == '_') {
			mod->unres =
			...

But I do not feel strong about this part.

It is much more important to clean up so we use the
same definition all over the kernel.

	Sam

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC -next] linux/linkage.h: fix symbol prefix handling
  2013-03-13  6:31               ` Sam Ravnborg
  2013-03-13  9:21                 ` James Hogan
@ 2013-03-14  4:00                 ` Rusty Russell
  2013-03-14 10:49                   ` James Hogan
  1 sibling, 1 reply; 15+ messages in thread
From: Rusty Russell @ 2013-03-14  4:00 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: Stephen Rothwell, James Hogan, Al Viro, Michal Marek,
	Andrew Morton, Guenter Roeck, Jean Delvare, linux-kbuild,
	linux-kernel, Mike Frysinger, uclinux-dist-devel, linux-next,
	Paul Gortmaker, linux-mtd

Sam Ravnborg <sam@ravnborg.org> writes:
>> actually, y'know, proof-read it.
> Hmm..

>> +                   $(KERNELRELEASE) "$(patsubst y,_,$(CONFIG_HAVE_SYMBOL_PREFIX_UNDERSCORE))"
>>  
>> +config HAVE_UNDERSCORE_SYMBOL_PREFIX
>
> HAVE_UNDERSCORE_... or HAVE_SYMBOL_... confusion.
> I prefer the HAVE_SYMBOL_... variant but no strong feelings..

Thanks, fixed.  (HAVE_UNDERSCORE_ won by grep majority vote, but I don't
really care either).

>> + *
>> + * If you think the above arrogance just encourages more people to add
>> + * random crap to this file, you're not alone.
> Kill this.

??  It went in without my Ack, and it's a particularly offensive turd.

I can replace it with a civilized comment, instead of being snarky
though...

>>  /* Some toolchains use a `_' prefix for all user symbols. */
>> -#ifdef CONFIG_SYMBOL_PREFIX
>> -#define MODULE_SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX
>> +#ifdef CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX
>> +#define __VMLINUX_SYMBOL(x) _##x
>> +#define __VMLINUX_SYMBOL_STR(x) "_" #x
>> +#define VMLINUX_SYMBOL_PREFIX_STR "_"
>>  #else
>> -#define MODULE_SYMBOL_PREFIX ""
>> +#define __VMLINUX_SYMBOL(x) x
>> +#define __VMLINUX_SYMBOL_STR(x) #x
>> +#define VMLINUX_SYMBOL_PREFIX_STR ""
>>  #endif
>
> We know the prefix is an underscore. No benefits from defining
> VMLINUX_SYMBOL_PREFIX_STR. The config name even syas so.
> Skipping the above give us only one way to check
> for the prefix - today we mix the two.

After testing, I think you're right.  The two users are both slightly
clearer after this (though mtd is a tiny bit less optimal).

Note: James, your patch hunk now looks like this:

  -		buf_printf(b, "\t{ %#8x, \"%s\" },\n", s->crc, s->name);
  +		buf_printf(b, "\t{ %#8x, VMLINUX_SYMBOL_STR(%s) },\n",
  +			   s->crc, s->name);

Here's version 3:

From: Rusty Russell <rusty@rustcorp.com.au>
Subject: CONFIG_SYMBOL_PREFIX: cleanup.

We have CONFIG_SYMBOL_PREFIX, which three archs define to the string
"_".  But Al Viro broke this in "consolidate cond_syscall and
SYSCALL_ALIAS declarations" (in linux-next), and he's not the first to
do so.

Using CONFIG_SYMBOL_PREFIX is awkward, since we usually just want to
prefix it so something.  So various places define helpers which are
defined to nothing if CONFIG_SYMBOL_PREFIX isn't set:

1) include/asm-generic/unistd.h defines __SYMBOL_PREFIX.
2) include/asm-generic/vmlinux.lds.h defines VMLINUX_SYMBOL(sym)
3) include/linux/export.h defines MODULE_SYMBOL_PREFIX.
4) include/linux/kernel.h defines SYMBOL_PREFIX (which differs from #7)
5) kernel/modsign_certificate.S defines ASM_SYMBOL(sym)
6) scripts/modpost.c defines MODULE_SYMBOL_PREFIX
7) scripts/Makefile.lib defines SYMBOL_PREFIX on the commandline if
   CONFIG_SYMBOL_PREFIX is set, so that we have a non-string version
   for pasting.

(arch/h8300/include/asm/linkage.h defines SYMBOL_NAME(), too).

Let's solve this properly:
1) No more generic prefix, just CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX.
2) Make linux/export.h usable from asm.
3) Define VMLINUX_SYMBOL() and VMLINUX_SYMBOL_STR().
4) Make everyone use them.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

diff --git a/Makefile b/Makefile
index a05ea42..0b09ba5 100644
--- a/Makefile
+++ b/Makefile
@@ -1398,7 +1398,7 @@ quiet_cmd_rmfiles = $(if $(wildcard $(rm-files)),CLEAN   $(wildcard $(rm-files))
 # Run depmod only if we have System.map and depmod is executable
 quiet_cmd_depmod = DEPMOD  $(KERNELRELEASE)
       cmd_depmod = $(CONFIG_SHELL) $(srctree)/scripts/depmod.sh $(DEPMOD) \
-                   $(KERNELRELEASE) "$(patsubst "%",%,$(CONFIG_SYMBOL_PREFIX))"
+                   $(KERNELRELEASE) "$(patsubst y,_,$(CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX))"
 
 # Create temporary dir for module support files
 # clean it up only when building all modules
diff --git a/arch/Kconfig b/arch/Kconfig
index 1455579..7b433a4 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -384,6 +384,12 @@ config MODULES_USE_ELF_REL
 	  Modules only use ELF REL relocations.  Modules with ELF RELA
 	  relocations will give an error.
 
+config HAVE_UNDERSCORE_SYMBOL_PREFIX
+	bool
+	help
+	  Some architectures generate an _ in front of C symbols; things like
+	  module loading and assembly files need to know about this.
+
 #
 # ABI hall of shame
 #
diff --git a/arch/blackfin/Kconfig b/arch/blackfin/Kconfig
index c3f2e0b..453ebe4 100644
--- a/arch/blackfin/Kconfig
+++ b/arch/blackfin/Kconfig
@@ -1,7 +1,3 @@
-config SYMBOL_PREFIX
-	string
-	default "_"
-
 config MMU
 	def_bool n
 
@@ -33,6 +29,7 @@ config BLACKFIN
 	select ARCH_HAVE_CUSTOM_GPIO_H
 	select ARCH_WANT_OPTIONAL_GPIOLIB
 	select HAVE_UID16
+	select HAVE_UNDERSCORE_SYMBOL_PREFIX
 	select VIRT_TO_BUS
 	select ARCH_WANT_IPC_PARSE_VERSION
 	select HAVE_GENERIC_HARDIRQS
diff --git a/arch/h8300/Kconfig b/arch/h8300/Kconfig
index 79250de..303e4f9 100644
--- a/arch/h8300/Kconfig
+++ b/arch/h8300/Kconfig
@@ -12,10 +12,7 @@ config H8300
 	select MODULES_USE_ELF_RELA
 	select OLD_SIGSUSPEND3
 	select OLD_SIGACTION
-
-config SYMBOL_PREFIX
-	string
-	default "_"
+	select HAVE_UNDERSCORE_SYMBOL_PREFIX
 
 config MMU
 	bool
diff --git a/arch/metag/Kconfig b/arch/metag/Kconfig
index afc8973..2099617 100644
--- a/arch/metag/Kconfig
+++ b/arch/metag/Kconfig
@@ -1,7 +1,3 @@
-config SYMBOL_PREFIX
-	string
-	default "_"
-
 config METAG
 	def_bool y
 	select EMBEDDED
@@ -27,6 +23,7 @@ config METAG
 	select HAVE_MOD_ARCH_SPECIFIC
 	select HAVE_PERF_EVENTS
 	select HAVE_SYSCALL_TRACEPOINTS
+	select HAVE_UNDERSCORE_SYMBOL_PREFIX
 	select IRQ_DOMAIN
 	select MODULES_USE_ELF_RELA
 	select OF
diff --git a/drivers/mtd/chips/gen_probe.c b/drivers/mtd/chips/gen_probe.c
index 3b9a284..74dbb6b 100644
--- a/drivers/mtd/chips/gen_probe.c
+++ b/drivers/mtd/chips/gen_probe.c
@@ -204,14 +204,16 @@ static inline struct mtd_info *cfi_cmdset_unknown(struct map_info *map,
 	struct cfi_private *cfi = map->fldrv_priv;
 	__u16 type = primary?cfi->cfiq->P_ID:cfi->cfiq->A_ID;
 #ifdef CONFIG_MODULES
-	char probename[16+sizeof(MODULE_SYMBOL_PREFIX)];
+	char probename[sizeof(VMLINUX_SYMBOL_STR(cfi_cmdset_%4.4X))];
 	cfi_cmdset_fn_t *probe_function;
 
-	sprintf(probename, MODULE_SYMBOL_PREFIX "cfi_cmdset_%4.4X", type);
+	sprintf(probename, VMLINUX_SYMBOL_STR(cfi_cmdset_%4.4X), type);
 
 	probe_function = __symbol_get(probename);
 	if (!probe_function) {
-		request_module(probename + sizeof(MODULE_SYMBOL_PREFIX) - 1);
+		char modname[sizeof("cfi_cmdset_%4.4X")];
+		sprintf(modname, "cfi_cmdset_%4.4X", type);
+		request_module(modname);
 		probe_function = __symbol_get(probename);
 	}
 
diff --git a/include/asm-generic/unistd.h b/include/asm-generic/unistd.h
index 4077b5d..15c0598 100644
--- a/include/asm-generic/unistd.h
+++ b/include/asm-generic/unistd.h
@@ -1,4 +1,5 @@
 #include <uapi/asm-generic/unistd.h>
+#include <linux/export.h>
 
 /*
  * These are required system calls, we should
@@ -17,12 +18,7 @@
  * but it doesn't work on all toolchains, so we just do it by hand
  */
 #ifndef cond_syscall
-#ifdef CONFIG_SYMBOL_PREFIX
-#define __SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX
-#else
-#define __SYMBOL_PREFIX
-#endif
-#define cond_syscall(x) asm(".weak\t" __SYMBOL_PREFIX #x "\n\t" \
-			    ".set\t" __SYMBOL_PREFIX #x "," \
-			    __SYMBOL_PREFIX "sys_ni_syscall")
+#define cond_syscall(x) asm(".weak\t" VMLINUX_SYMBOL_STR(x) "\n\t"	\
+			    ".set\t" VMLINUX_SYMBOL_STR(x) ","	\
+			    VMLINUX_SYMBOL_STR(sys_ni_syscall))
 #endif
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index afa12c7..eb58d2d 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -52,13 +52,7 @@
 #define LOAD_OFFSET 0
 #endif
 
-#ifndef SYMBOL_PREFIX
-#define VMLINUX_SYMBOL(sym) sym
-#else
-#define PASTE2(x,y) x##y
-#define PASTE(x,y) PASTE2(x,y)
-#define VMLINUX_SYMBOL(sym) PASTE(SYMBOL_PREFIX, sym)
-#endif
+#include <linux/export.h>
 
 /* Align . to a 8 byte boundary equals to maximum function alignment. */
 #define ALIGN_FUNCTION()  . = ALIGN(8)
diff --git a/include/linux/export.h b/include/linux/export.h
index 696c0f4..412cd50 100644
--- a/include/linux/export.h
+++ b/include/linux/export.h
@@ -5,17 +5,24 @@
  * to reduce the amount of pointless cruft we feed to gcc when only
  * exporting a simple symbol or two.
  *
- * If you feel the need to add #include <linux/foo.h> to this file
- * then you are doing something wrong and should go away silently.
+ * Try not to add #includes here.  It slows compilation and makes kernel
+ * hackers place grumpy comments in header files.
  */
 
 /* Some toolchains use a `_' prefix for all user symbols. */
-#ifdef CONFIG_SYMBOL_PREFIX
-#define MODULE_SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX
+#ifdef CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX
+#define __VMLINUX_SYMBOL(x) _##x
+#define __VMLINUX_SYMBOL_STR(x) "_" #x
 #else
-#define MODULE_SYMBOL_PREFIX ""
+#define __VMLINUX_SYMBOL(x) x
+#define __VMLINUX_SYMBOL_STR(x) #x
 #endif
 
+/* Indirect, so macros are expanded before pasting. */
+#define VMLINUX_SYMBOL(x) __VMLINUX_SYMBOL(x)
+#define VMLINUX_SYMBOL_STR(x) __VMLINUX_SYMBOL_STR(x)
+
+#ifndef __ASSEMBLY__
 struct kernel_symbol
 {
 	unsigned long value;
@@ -51,7 +58,7 @@ extern struct module __this_module;
 	__CRC_SYMBOL(sym, sec)					\
 	static const char __kstrtab_##sym[]			\
 	__attribute__((section("__ksymtab_strings"), aligned(1))) \
-	= MODULE_SYMBOL_PREFIX #sym;				\
+	= VMLINUX_SYMBOL_STR(sym);				\
 	static const struct kernel_symbol __ksymtab_##sym	\
 	__used							\
 	__attribute__((section("___ksymtab" sec "+" #sym), unused))	\
@@ -85,5 +92,6 @@ extern struct module __this_module;
 #define EXPORT_UNUSED_SYMBOL_GPL(sym)
 
 #endif /* CONFIG_MODULES */
+#endif /* !__ASSEMBLY__ */
 
 #endif /* _LINUX_EXPORT_H */
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 80d3687..e13e992 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -723,13 +723,6 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
 /* Trap pasters of __FUNCTION__ at compile-time */
 #define __FUNCTION__ (__func__)
 
-/* This helps us to avoid #ifdef CONFIG_SYMBOL_PREFIX */
-#ifdef CONFIG_SYMBOL_PREFIX
-#define SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX
-#else
-#define SYMBOL_PREFIX ""
-#endif
-
 /* Rebuild everything on CONFIG_FTRACE_MCOUNT_RECORD */
 #ifdef CONFIG_FTRACE_MCOUNT_RECORD
 # define REBUILD_DUE_TO_FTRACE_MCOUNT_RECORD
diff --git a/include/linux/module.h b/include/linux/module.h
index ead1b57..46f1ea0 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -190,7 +190,7 @@ extern int modules_disabled; /* for sysctl */
 /* Get/put a kernel symbol (calls must be symmetric) */
 void *__symbol_get(const char *symbol);
 void *__symbol_get_gpl(const char *symbol);
-#define symbol_get(x) ((typeof(&x))(__symbol_get(MODULE_SYMBOL_PREFIX #x)))
+#define symbol_get(x) ((typeof(&x))(__symbol_get(VMLINUX_SYMBOL_STR(x))))
 
 /* modules using other modules: kdb wants to see this. */
 struct module_use {
@@ -453,7 +453,7 @@ extern void __module_put_and_exit(struct module *mod, long code)
 #ifdef CONFIG_MODULE_UNLOAD
 unsigned long module_refcount(struct module *mod);
 void __symbol_put(const char *symbol);
-#define symbol_put(x) __symbol_put(MODULE_SYMBOL_PREFIX #x)
+#define symbol_put(x) __symbol_put(VMLINUX_SYMBOL_STR(x))
 void symbol_put_addr(void *addr);
 
 /* Sometimes we know we already have a refcount, and it's easier not
diff --git a/kernel/modsign_certificate.S b/kernel/modsign_certificate.S
index 246b4c6..4a9a86d 100644
--- a/kernel/modsign_certificate.S
+++ b/kernel/modsign_certificate.S
@@ -1,15 +1,8 @@
-/* SYMBOL_PREFIX defined on commandline from CONFIG_SYMBOL_PREFIX */
-#ifndef SYMBOL_PREFIX
-#define ASM_SYMBOL(sym) sym
-#else
-#define PASTE2(x,y) x##y
-#define PASTE(x,y) PASTE2(x,y)
-#define ASM_SYMBOL(sym) PASTE(SYMBOL_PREFIX, sym)
-#endif
+#include <linux/export.h>
 
 #define GLOBAL(name)	\
-	.globl ASM_SYMBOL(name);	\
-	ASM_SYMBOL(name):
+	.globl VMLINUX_SYMBOL(name);	\
+	VMLINUX_SYMBOL(name):
 
 	.section ".init.data","aw"
 
diff --git a/kernel/module.c b/kernel/module.c
index 0925c9a..cfd4a3f 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1209,7 +1209,7 @@ static inline int check_modstruct_version(Elf_Shdr *sechdrs,
 
 	/* Since this should be found in kernel (which can't be removed),
 	 * no locking is necessary. */
-	if (!find_symbol(MODULE_SYMBOL_PREFIX "module_layout", NULL,
+	if (!find_symbol(VMLINUX_SYMBOL_STR(module_layout), NULL,
 			 &crc, true, false))
 		BUG();
 	return check_version(sechdrs, versindex, "module_layout", mod, crc,
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 07125e6..a373a1f 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -119,13 +119,6 @@ _c_flags += $(if $(patsubst n%,, \
 		$(CFLAGS_GCOV))
 endif
 
-ifdef CONFIG_SYMBOL_PREFIX
-_sym_flags = -DSYMBOL_PREFIX=$(patsubst "%",%,$(CONFIG_SYMBOL_PREFIX))
-_cpp_flags += $(_sym_flags)
-_a_flags += $(_sym_flags)
-endif
-
-
 # If building the kernel in a separate objtree expand all occurrences
 # of -Idir to -I$(srctree)/dir except for absolute paths (starting with '/').
 
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 3d569d6..0149949 100644
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -74,9 +74,8 @@ kallsyms()
 	info KSYM ${2}
 	local kallsymopt;
 
-	if [ -n "${CONFIG_SYMBOL_PREFIX}" ]; then
-		kallsymopt="${kallsymopt} \
-			    --symbol-prefix=${CONFIG_SYMBOL_PREFIX}"
+	if [ -n "${CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX}" ]; then
+		kallsymopt="${kallsymopt} --symbol-prefix=_"
 	fi
 
 	if [ -n "${CONFIG_KALLSYMS_ALL}" ]; then
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 78b30c1..282decf 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -18,14 +18,7 @@
 #include "modpost.h"
 #include "../../include/generated/autoconf.h"
 #include "../../include/linux/license.h"
-
-/* Some toolchains use a `_' prefix for all user symbols. */
-#ifdef CONFIG_SYMBOL_PREFIX
-#define MODULE_SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX
-#else
-#define MODULE_SYMBOL_PREFIX ""
-#endif
-
+#include "../../include/linux/export.h"
 
 /* Are we using CONFIG_MODVERSIONS? */
 int modversions = 0;
@@ -562,7 +555,7 @@ static void parse_elf_finish(struct elf_info *info)
 static int ignore_undef_symbol(struct elf_info *info, const char *symname)
 {
 	/* ignore __this_module, it will be resolved shortly */
-	if (strcmp(symname, MODULE_SYMBOL_PREFIX "__this_module") == 0)
+	if (strcmp(symname, VMLINUX_SYMBOL_STR(__this_module)) == 0)
 		return 1;
 	/* ignore global offset table */
 	if (strcmp(symname, "_GLOBAL_OFFSET_TABLE_") == 0)
@@ -583,8 +576,8 @@ static int ignore_undef_symbol(struct elf_info *info, const char *symname)
 	return 0;
 }
 
-#define CRC_PFX     MODULE_SYMBOL_PREFIX "__crc_"
-#define KSYMTAB_PFX MODULE_SYMBOL_PREFIX "__ksymtab_"
+#define CRC_PFX     VMLINUX_SYMBOL_STR(__crc_)
+#define KSYMTAB_PFX VMLINUX_SYMBOL_STR(__ksymtab_)
 
 static void handle_modversions(struct module *mod, struct elf_info *info,
 			       Elf_Sym *sym, const char *symname)
@@ -637,14 +630,15 @@ static void handle_modversions(struct module *mod, struct elf_info *info,
 		}
 #endif
 
-		if (memcmp(symname, MODULE_SYMBOL_PREFIX,
-			   strlen(MODULE_SYMBOL_PREFIX)) == 0) {
-			mod->unres =
-			  alloc_symbol(symname +
-			               strlen(MODULE_SYMBOL_PREFIX),
-			               ELF_ST_BIND(sym->st_info) == STB_WEAK,
-			               mod->unres);
-		}
+#ifdef CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX
+		if (symname[0] != '_')
+			break;
+		else
+			symname++;
+#endif
+		mod->unres = alloc_symbol(symname,
+					  ELF_ST_BIND(sym->st_info) == STB_WEAK,
+					  mod->unres);
 		break;
 	default:
 		/* All exported symbols */
@@ -652,9 +646,9 @@ static void handle_modversions(struct module *mod, struct elf_info *info,
 			sym_add_exported(symname + strlen(KSYMTAB_PFX), mod,
 					export);
 		}
-		if (strcmp(symname, MODULE_SYMBOL_PREFIX "init_module") == 0)
+		if (strcmp(symname, VMLINUX_SYMBOL_STR(init_module)) == 0)
 			mod->has_init = 1;
-		if (strcmp(symname, MODULE_SYMBOL_PREFIX "cleanup_module") == 0)
+		if (strcmp(symname, VMLINUX_SYMBOL_STR(cleanup_module)) == 0)
 			mod->has_cleanup = 1;
 		break;
 	}

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [RFC -next] linux/linkage.h: fix symbol prefix handling
  2013-03-14  4:00                 ` Rusty Russell
@ 2013-03-14 10:49                   ` James Hogan
  2013-03-15  4:37                     ` Rusty Russell
  0 siblings, 1 reply; 15+ messages in thread
From: James Hogan @ 2013-03-14 10:49 UTC (permalink / raw)
  To: Rusty Russell
  Cc: Sam Ravnborg, Stephen Rothwell, Al Viro, Michal Marek,
	Andrew Morton, Guenter Roeck, Jean Delvare, linux-kbuild,
	linux-kernel, Mike Frysinger, uclinux-dist-devel, linux-next,
	Paul Gortmaker, linux-mtd

On 14/03/13 04:00, Rusty Russell wrote:
> From: Rusty Russell <rusty@rustcorp.com.au>
> Subject: CONFIG_SYMBOL_PREFIX: cleanup.
> 
> We have CONFIG_SYMBOL_PREFIX, which three archs define to the string
> "_".  But Al Viro broke this in "consolidate cond_syscall and
> SYSCALL_ALIAS declarations" (in linux-next), and he's not the first to
> do so.
> 
> Using CONFIG_SYMBOL_PREFIX is awkward, since we usually just want to
> prefix it so something.  So various places define helpers which are
> defined to nothing if CONFIG_SYMBOL_PREFIX isn't set:
> 
> 1) include/asm-generic/unistd.h defines __SYMBOL_PREFIX.
> 2) include/asm-generic/vmlinux.lds.h defines VMLINUX_SYMBOL(sym)
> 3) include/linux/export.h defines MODULE_SYMBOL_PREFIX.
> 4) include/linux/kernel.h defines SYMBOL_PREFIX (which differs from #7)
> 5) kernel/modsign_certificate.S defines ASM_SYMBOL(sym)
> 6) scripts/modpost.c defines MODULE_SYMBOL_PREFIX
> 7) scripts/Makefile.lib defines SYMBOL_PREFIX on the commandline if
>    CONFIG_SYMBOL_PREFIX is set, so that we have a non-string version
>    for pasting.
> 
> (arch/h8300/include/asm/linkage.h defines SYMBOL_NAME(), too).
> 
> Let's solve this properly:
> 1) No more generic prefix, just CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX.
> 2) Make linux/export.h usable from asm.
> 3) Define VMLINUX_SYMBOL() and VMLINUX_SYMBOL_STR().
> 4) Make everyone use them.
> 
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

Reviewed-by: James Hogan <james.hogan@imgtec.com>
Tested-by: James Hogan <james.hogan@imgtec.com> (metag)

The only other special case of symbol prefixing I'm aware of is in
scripts/genksyms/genksyms.c. It makes the decision at runtime based
on the --arch=$ARCH argument, and is the only use of the arch argument:

> 	if ((strcmp(arch, "h8300") == 0) || (strcmp(arch, "blackfin") == 0) ||
> 	    (strcmp(arch, "metag") == 0))
> 		mod_prefix = "_";

Does the patch below look reasonable in addition to your patch?
(Note: I'm not sure if genksyms is only used internally or whether
it's API should be kept stable?).

Thanks
James

Subject: [PATCH] genksyms: pass symbol-prefix instead of arch

Pass symbol-prefix to genksyms instead of arch, so that the decision
what symbol prefix to use is kept in one place.

Basically genksyms used to take a -a $ARCH argument and it used that to
determine whether to add an underscore symbol prefix. It's now changed
to take a -s $SYMBOL_PREFIX argument so that the caller decides whether
a symbol prefix is required. The build system then uses
CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX to determine whether to pass the
argument.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
---
 scripts/Makefile.build      |  3 ++-
 scripts/genksyms/genksyms.c | 18 +++++++-----------
 2 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 0e801c3..d5d859c 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -211,7 +211,8 @@ $(obj)/%.i: $(src)/%.c FORCE
 
 cmd_gensymtypes =                                                           \
     $(CPP) -D__GENKSYMS__ $(c_flags) $< |                                   \
-    $(GENKSYMS) $(if $(1), -T $(2)) -a $(ARCH)                              \
+    $(GENKSYMS) $(if $(1), -T $(2))                                         \
+     $(patsubst y,-s _,$(CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX))             \
      $(if $(KBUILD_PRESERVE),-p)                                            \
      -r $(firstword $(wildcard $(2:.symtypes=.symref) /dev/null))
 
diff --git a/scripts/genksyms/genksyms.c b/scripts/genksyms/genksyms.c
index d25e4a1..88632df 100644
--- a/scripts/genksyms/genksyms.c
+++ b/scripts/genksyms/genksyms.c
@@ -45,7 +45,6 @@ int in_source_file;
 
 static int flag_debug, flag_dump_defs, flag_reference, flag_dump_types,
 	   flag_preserve, flag_warnings;
-static const char *arch = "";
 static const char *mod_prefix = "";
 
 static int errors;
@@ -731,7 +730,7 @@ static void genksyms_usage(void)
 {
 	fputs("Usage:\n" "genksyms [-adDTwqhV] > /path/to/.tmp_obj.ver\n" "\n"
 #ifdef __GNU_LIBRARY__
-	      "  -a, --arch            Select architecture\n"
+	      "  -s, --symbol-prefix   Select symbol prefix\n"
 	      "  -d, --debug           Increment the debug level (repeatable)\n"
 	      "  -D, --dump            Dump expanded symbol defs (for debugging only)\n"
 	      "  -r, --reference file  Read reference symbols from a file\n"
@@ -742,7 +741,7 @@ static void genksyms_usage(void)
 	      "  -h, --help            Print this message\n"
 	      "  -V, --version         Print the release version\n"
 #else				/* __GNU_LIBRARY__ */
-	      "  -a                    Select architecture\n"
+	      "  -s                    Select symbol prefix\n"
 	      "  -d                    Increment the debug level (repeatable)\n"
 	      "  -D                    Dump expanded symbol defs (for debugging only)\n"
 	      "  -r file               Read reference symbols from a file\n"
@@ -763,7 +762,7 @@ int main(int argc, char **argv)
 
 #ifdef __GNU_LIBRARY__
 	struct option long_opts[] = {
-		{"arch", 1, 0, 'a'},
+		{"symbol-prefix", 1, 0, 's'},
 		{"debug", 0, 0, 'd'},
 		{"warnings", 0, 0, 'w'},
 		{"quiet", 0, 0, 'q'},
@@ -776,14 +775,14 @@ int main(int argc, char **argv)
 		{0, 0, 0, 0}
 	};
 
-	while ((o = getopt_long(argc, argv, "a:dwqVDr:T:ph",
+	while ((o = getopt_long(argc, argv, "s:dwqVDr:T:ph",
 				&long_opts[0], NULL)) != EOF)
 #else				/* __GNU_LIBRARY__ */
-	while ((o = getopt(argc, argv, "a:dwqVDr:T:ph")) != EOF)
+	while ((o = getopt(argc, argv, "s:dwqVDr:T:ph")) != EOF)
 #endif				/* __GNU_LIBRARY__ */
 		switch (o) {
-		case 'a':
-			arch = optarg;
+		case 's':
+			mod_prefix = optarg;
 			break;
 		case 'd':
 			flag_debug++;
@@ -826,9 +825,6 @@ int main(int argc, char **argv)
 			genksyms_usage();
 			return 1;
 		}
-	if ((strcmp(arch, "h8300") == 0) || (strcmp(arch, "blackfin") == 0) ||
-	    (strcmp(arch, "metag") == 0))
-		mod_prefix = "_";
 	{
 		extern int yydebug;
 		extern int yy_flex_debug;
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [RFC -next] linux/linkage.h: fix symbol prefix handling
  2013-03-14 10:49                   ` James Hogan
@ 2013-03-15  4:37                     ` Rusty Russell
  0 siblings, 0 replies; 15+ messages in thread
From: Rusty Russell @ 2013-03-15  4:37 UTC (permalink / raw)
  To: James Hogan
  Cc: Sam Ravnborg, Stephen Rothwell, Al Viro, Michal Marek,
	Andrew Morton, Guenter Roeck, Jean Delvare, linux-kbuild,
	linux-kernel, Mike Frysinger, uclinux-dist-devel, linux-next,
	Paul Gortmaker, linux-mtd

James Hogan <james.hogan@imgtec.com> writes:
> On 14/03/13 04:00, Rusty Russell wrote:
>> From: Rusty Russell <rusty@rustcorp.com.au>
>> Subject: CONFIG_SYMBOL_PREFIX: cleanup.
...
> Reviewed-by: James Hogan <james.hogan@imgtec.com>
> Tested-by: James Hogan <james.hogan@imgtec.com> (metag)

Thanks.

> The only other special case of symbol prefixing I'm aware of is in
> scripts/genksyms/genksyms.c. It makes the decision at runtime based
> on the --arch=$ARCH argument, and is the only use of the arch argument:
>
>> 	if ((strcmp(arch, "h8300") == 0) || (strcmp(arch, "blackfin") == 0) ||
>> 	    (strcmp(arch, "metag") == 0))
>> 		mod_prefix = "_";
>
> Does the patch below look reasonable in addition to your patch?
> (Note: I'm not sure if genksyms is only used internally or whether
> it's API should be kept stable?).
>
> Thanks
> James
>
> Subject: [PATCH] genksyms: pass symbol-prefix instead of arch

Agreed, I've applied this.

Thanks,
Rusty.

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2013-03-15  5:04 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-03-07 11:44 [RFC -next] linux/linkage.h: fix symbol prefix handling James Hogan
2013-03-08  0:03 ` Rusty Russell
2013-03-08  9:15   ` James Hogan
2013-03-11  6:35     ` Rusty Russell
2013-03-11 12:07       ` Stephen Rothwell
2013-03-12  4:48         ` Rusty Russell
2013-03-12 12:33           ` Stephen Rothwell
2013-03-13  0:00             ` Rusty Russell
2013-03-13  6:31               ` Sam Ravnborg
2013-03-13  9:21                 ` James Hogan
2013-03-13 18:15                   ` Sam Ravnborg
2013-03-14  4:00                 ` Rusty Russell
2013-03-14 10:49                   ` James Hogan
2013-03-15  4:37                     ` Rusty Russell
2013-03-12 12:36           ` James Hogan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).