[RFC] making HAVE_SYSCALL_WRAPPERS universal (Re: Issues with "x86, um: switch to generic fork/vfork/clone" commit)

[RFC] making HAVE_SYSCALL_WRAPPERS universal (Re: Issues with "x86, um: switch to generic fork/vfork/clone" commit)

[Al Viro]

[linux-arch Cc'd]
On Mon, Jan 21, 2013 at 02:30:11AM +0000, Al Viro wrote:

> Neither do I, to be honest.  It might be saving us a few cycles on
> some architectures, but I'd like to see examples of that.  amd64
> doesn't seem to be one, at least...

*grumble*

OK, there is one problem - SYSCALL_ALIAS() is as arch-dependent as
cond_syscall().  As far as I can tell, with cond_syscall() we have
the following variants:
	1) alpha, mips
.weak foo
foo = sys_ni_syscall
	2) itanic, ppc
asmlinkage long x (void) __attribute__((weak,alias("sys_ni_syscall")))
	3) very common - most of the architectures
.weak foo
.set foo sys_ni_syscall
	3a) blackfin, h8300 - same as (3), except that asm symbols get
underscore prepended.
.weak _foo
.set _foo _sys_ni_syscall

For SYSCALL_ALIAS(foo,bar):
	1) alpha, mips
foo = bar
.globl foo
	2) ppc64
.globl foo
.set foo bar
.globl .foo
.set .foo .bar
[note that ppc64 cond_syscall ends up generating similar pair, only with
.globl replaced with .weak]
	3) s390, sparc, tile
.globl foo
.set foo bar

I suspect that (3) here should cover the same things as (3,3a) for
cond_syscalls(), with the same "prepend _" variation on blackfin/h8300.
I really wonder what's going on with ppc64, though - as far as I can
tell, .foo is the real function there, while foo is a OPD entry refering
to it.  Do we have anything similar on other architectures?  And what
about itanic?  After looking at the assembler generated by cond_syscall
there, I suspect that
foo# = bar#
.globl foo#
would be the right answer there, but I'd really like somebody familiar with
ia64 to confirm that...

Another question: what's the following comment from spu_callbacks.c about?
 * 4. They are optional and we can't rely on them being
 *    linked into the kernel. Unfortunately, the cond_syscall
 *    helper does not work here as it does not add the necessary
 *    opd symbols:
 *      mbind, mq_open, ipc, ...
What isn't added?  sys_ni_syscall is an OPD entry and weak aliases for it
*are* created.  That comment predates the conversion of cond_syscall()
definition to the current one; it used to be
.weak .foo
.set .foo .sys_ni_syscall
and that wouldn't have created the second alias; is it simply obsolete?
Note that mbind and friends still are not available to SPU; should that
be changed now that we can do that?

I've tried to sanitize cond_syscall/SYSCALL_ALIAS situation; the tree is in
git.kernel.org/pub/scm/linux/kernel/git/viro/signal experimental-syscalls
NOTE: this is absolutely untested and might very well blow up
on any number of architectures.

Review and comments would be very welcome.

Re: [RFC] making HAVE_SYSCALL_WRAPPERS universal (Re: Issues with "x86, um: switch to generic fork/vfork/clone" commit)

[James Hogan]

On 21/01/13 22:55, Al Viro wrote:
> I've tried to sanitize cond_syscall/SYSCALL_ALIAS situation; the tree is in
> git.kernel.org/pub/scm/linux/kernel/git/viro/signal experimental-syscalls
> NOTE: this is absolutely untested and might very well blow up
> on any number of architectures.
> 
> Review and comments would be very welcome.

Looking at "consolidate cond_syscall and SYSCALL_ALIAS declarations",
is it worth having the default __SYMBOL_NAME declaration in
include/linux/linkage.h make use of CONFIG_SYMBOL_PREFIX?

E.g. in the metag patchset we currently have the patch below.

Admittedly CONFIG_SYMBOL_PREFIX is a string which would make it's use in
SYMBOL_NAME a bit awkward.

Cheers
James


Subject: [PATCH v3 02/44] asm-generic/unistd.h: handle symbol prefixes in
 cond_syscall

Some architectures have symbol prefixes and set CONFIG_SYMBOL_PREFIX,
but this wasn't taken into account by the generic cond_syscall. It's
easy enough to fix in a generic fashion, so add the symbol prefix to
symbol names in cond_syscall when CONFIG_SYMBOL_PREFIX is set.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Mike Frysinger <vapier@gentoo.org>
---
 include/asm-generic/unistd.h |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/include/asm-generic/unistd.h b/include/asm-generic/unistd.h
index 257c55e..4077b5d 100644
--- a/include/asm-generic/unistd.h
+++ b/include/asm-generic/unistd.h
@@ -17,5 +17,12 @@
  * 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" #x "\n\t.set\t" #x ",sys_ni_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")
 #endif
-- 
1.7.7.6

Re: [RFC] making HAVE_SYSCALL_WRAPPERS universal (Re: Issues with "x86, um: switch to generic fork/vfork/clone" commit)

[Al Viro]

On Tue, Jan 22, 2013 at 12:47:17PM +0000, James Hogan wrote:
> On 21/01/13 22:55, Al Viro wrote:
> > I've tried to sanitize cond_syscall/SYSCALL_ALIAS situation; the tree is in
> > git.kernel.org/pub/scm/linux/kernel/git/viro/signal experimental-syscalls
> > NOTE: this is absolutely untested and might very well blow up
> > on any number of architectures.
> > 
> > Review and comments would be very welcome.
> 
> Looking at "consolidate cond_syscall and SYSCALL_ALIAS declarations",
> is it worth having the default __SYMBOL_NAME declaration in
> include/linux/linkage.h make use of CONFIG_SYMBOL_PREFIX?
> 
> E.g. in the metag patchset we currently have the patch below.
> 
> Admittedly CONFIG_SYMBOL_PREFIX is a string which would make it's use in
> SYMBOL_NAME a bit awkward.

Umm...  TBH, I simply followed the existing macro (from h8300), but I really
suspect that it's better than your approach - sure, you leave populating the
syscall table to cc(1) and generally have fewer things in asm glue, but
you still have things like _clear_page, _ip_fast_csum, ___ashldi3, etc.
Having a macro converting C identifier to asm one might be a win for such
situations; if nothing else, it helps with grep - blackfin is a constant
source of annoyance, since e.g. git grep -n -w do_notify_resume will miss
things like
        pseudo_long_call _do_notify_resume, p5;

I'm not saying that SYMBOL_NAME is particulary good as identifiers go,
but the functionality is more useful than CONFIG_SYMBOL_PREFIX, IMO...

Re: [RFC] making HAVE_SYSCALL_WRAPPERS universal (Re: Issues with "x86, um: switch to generic fork/vfork/clone" commit)

[Arnd Bergmann]

On Monday 21 January 2013, Al Viro wrote:
> Another question: what's the following comment from spu_callbacks.c about?
>  * 4. They are optional and we can't rely on them being
>  *    linked into the kernel. Unfortunately, the cond_syscall
>  *    helper does not work here as it does not add the necessary
>  *    opd symbols:
>  *      mbind, mq_open, ipc, ...
> What isn't added?  sys_ni_syscall is an OPD entry and weak aliases for it
> are created.  That comment predates the conversion of cond_syscall()
> definition to the current one; it used to be
> .weak .foo
> .set .foo .sys_ni_syscall
> and that wouldn't have created the second alias; is it simply obsolete?

Yes, I think that is correct, it was fixed years ago when the old
toolchains that couldn's support it got deprecated, in

commit 8dc86ab954d28513f75918d743c40cddbff7388a
Author: Geoff Levand <geoffrey.levand@am.sony.com>
Date:   Mon Nov 20 18:44:56 2006 +0100

    [POWERPC] Change ppc_rtas declaration to weak
    
    Change the definition of powerpc's cond_syscall() to use the standard gcc
    weak attribute specifier which provides proper support for C linkage as
    needed by spu_syscall_table[].
    
    Fixes this powerpc build error with CONFIG_SPU_FS=y, CONFIG_PPC_RTAS=n:
    
     arch/powerpc/platforms/built-in.o: undefined reference to `ppc_rtas'

but we forgot enable the remaining ones.

> Note that mbind and friends still are not available to SPU; should that
> be changed now that we can do that?

We could certainly change it, if someone is willing to go through the
list and mark the ones that should be added. It's probably not necessary,
I have never seen any SPU application that actually did any but the most
basic syscalls itself.

	Arnd

Re: [RFC] making HAVE_SYSCALL_WRAPPERS universal (Re: Issues with "x86, um: switch to generic fork/vfork/clone" commit)

[Arnd Bergmann]

On Monday 21 January 2013, Al Viro wrote:
>         1) alpha, mips
> .weak foo
> foo = sys_ni_syscall
>         2) itanic, ppc
> asmlinkage long x (void) __attribute__((weak,alias("sys_ni_syscall")))
>         3) very common - most of the architectures
> .weak foo
> .set foo sys_ni_syscall
>         3a) blackfin, h8300 - same as (3), except that asm symbols get
> underscore prepended.
> .weak _foo
> .set _foo _sys_ni_syscall

One question here: Is there still a reason why we can't use method 2 on
all architectures now? The comment in a lot of architectures says

 * What we want is __attribute__((weak,alias("sys_ni_syscall")))
 * but it doesn't work on all toolchains, so we just do it by hand

and my guess is that all toolchains we support at this point actually
do allow it, and turn it into the appropriate assembler syntax.

	Arnd