[RFC,-next] linux/linkage.h: fix symbol prefix handling

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC,-next] linux/linkage.h: fix symbol prefix handling

Commit Message

Patch