Re: [Xenomai-core] [PATCH] optimise syscall mux-code calculation

[Jan Kiszka <jan.kiszka@domain.hid>]

[-- Attachment #1.1: Type: text/plain, Size: 1674 bytes --]

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>  > Jan Kiszka wrote:
>  > > ...
>  > > Ok, I will rework this patch.
>  > > 
>  > 
>  > And here is -v2 (for post 2.2 trunk). It keeps the ABI but changes the
>  > API wrt __xeno_mux_code. As this change gets widely caught by
>  > xeno_user_skin_init, the modifications are limited.
>  > 
>  > At this chance I also converted the rtdm library to the generic
>  > initialisation code. I tweaked nucleus/bind.h a bit so that one can
>  > control via XENO_DISABLE_MLOCKALL_DETECTION if the related code gets
>  > built into the lib. This appeared to be reasonable to me because librtdm
>  > doesn't create its own RT threads, thus the check should be better left
>  > to those skins who do so. Anyone any better idea, or is this acceptable?
>  > If a separate patch is preferred, I will break out this change.
> 
> Another reason why rtdm did not use the generic initialization code is
> that RTDM did not call exit upon failure to bind, and let a later call
> to open fail. I do not know if it was desirable, but it was a feature of
> the previous implementation.
> 

Damn right. So let's try this more systematically:

The first patch prepares the generic skin-init code for the usage by
rtdm/init.c. I'm not totally happy with the #ifndef tweak. I would
prefer to control the signal setup also via a flag, but I have no idea
how to avoid linking the unused xeno_handle_mlock_alert into librtdm.
Any ideas?

Based on this conversion is the optimisation patch -v4. There was
another quirk hidden in libpthread_rt which binds to RTDM "manually".
That's now also covered by the patch.

Jan

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: generic-skin-init.patch --]
[-- Type: text/x-patch; name="generic-skin-init.patch", Size: 7650 bytes --]

---
 include/nucleus/bind.h   |   43 +++++++++++++++++++++++-------------
 src/skins/native/init.c  |    2 -
 src/skins/posix/init.c   |    2 -
 src/skins/rtai/init.c    |    2 -
 src/skins/rtdm/init.c    |   55 ++++++++---------------------------------------
 src/skins/uvm/init.c     |    3 +-
 src/skins/vrtx/init.c    |    2 -
 src/skins/vxworks/init.c |    2 -
 8 files changed, 45 insertions(+), 66 deletions(-)

Index: xenomai/include/nucleus/bind.h
===================================================================
--- xenomai.orig/include/nucleus/bind.h
+++ xenomai/include/nucleus/bind.h
@@ -9,6 +9,9 @@
 #include <pthread.h>
 #include <asm/xenomai/syscall.h>
 
+#define XNSKIN_LAZY_BIND_FAIL	0x01 /* do not fail on missing skin */
+
+#ifndef XNSKIN_NO_MLOCK_ALERT
 __attribute__((weak)) int xeno_sigxcpu_no_mlock = 1;
 
 static void  xeno_handle_mlock_alert (int sig)
@@ -34,18 +37,19 @@ static void  xeno_handle_mlock_alert (in
 	pthread_kill(pthread_self(),SIGXCPU);
 	}
 }
+#endif /* !XNSKIN_NO_MLOCK_ALERT */
 
 static inline int
-xeno_user_skin_init(unsigned skin_magic, const char *skin, const char *module)
+xeno_user_skin_init(unsigned skin_magic, const char *skin, const char *module,
+		    int flags)
 {
-    struct sigaction sa;
     xnfeatinfo_t finfo;
-    int muxid;
+    int muxid = -1;
 
 #ifdef xeno_arch_features_check
     xeno_arch_features_check();
 #endif /* xeno_arch_features_check */
-    
+
     muxid = XENOMAI_SYSBIND(skin_magic,
 			    XENOMAI_FEAT_DEP,
 			    XENOMAI_ABI_REV,
@@ -69,24 +73,33 @@ xeno_user_skin_init(unsigned skin_magic,
 	case -ENOSYS:
 	case -ESRCH:
 
+	    if (flags & XNSKIN_LAZY_BIND_FAIL)
+		break; /* We are ordered to ignore the missing skin. */
+
 	    fprintf(stderr,"Xenomai: %s skin or CONFIG_XENO_OPT_PERVASIVE disabled.\n"
-                    "(modprobe %s?)\n", skin, module);
+		    "(modprobe %s?)\n", skin, module);
 	    exit(1);
-        }
-    
-    if (muxid < 0)
-        {
-        fprintf(stderr,"Xenomai: binding failed: %s.\n",strerror(-muxid));
-        exit(1);
+
+	default:
+	    if (muxid < 0)
+		{
+		fprintf(stderr,"Xenomai: binding failed: %s.\n",strerror(-muxid));
+		exit(1);
+		}
         }
 
+#ifndef XNSKIN_NO_MLOCK_ALERT
     /* Install a SIGXCPU handler to intercept alerts about unlocked
        process memory. */
+    {
+	struct sigaction sa;
 
-    sa.sa_handler = &xeno_handle_mlock_alert;
-    sigemptyset(&sa.sa_mask);
-    sa.sa_flags = 0;
-    sigaction(SIGXCPU,&sa,NULL);
+	sa.sa_handler = &xeno_handle_mlock_alert;
+	sigemptyset(&sa.sa_mask);
+	sa.sa_flags = 0;
+	sigaction(SIGXCPU,&sa,NULL);
+    }
+#endif /* !XNSKIN_NO_MLOCK_ALERT */
 
     return muxid;
 }
Index: xenomai/src/skins/native/init.c
===================================================================
--- xenomai.orig/src/skins/native/init.c
+++ xenomai/src/skins/native/init.c
@@ -39,7 +39,7 @@ static __attribute__ ((constructor))
 void __init_xeno_interface(void)
 {
 	__native_muxid =
-	    xeno_user_skin_init(XENO_SKIN_MAGIC, "native", "xeno_native");
+	    xeno_user_skin_init(XENO_SKIN_MAGIC, "native", "xeno_native", 0);
 
 	/* Allocate a TSD key for indexing self task pointers. */
 
Index: xenomai/src/skins/posix/init.c
===================================================================
--- xenomai.orig/src/skins/posix/init.c
+++ xenomai/src/skins/posix/init.c
@@ -40,7 +40,7 @@ void __init_posix_interface(void)
 	int muxid, err;
 
 	__pse51_muxid =
-	    xeno_user_skin_init(PSE51_SKIN_MAGIC, "POSIX", "xeno_posix");
+	    xeno_user_skin_init(PSE51_SKIN_MAGIC, "POSIX", "xeno_posix", 0);
 
 	muxid = XENOMAI_SYSBIND(RTDM_SKIN_MAGIC,
 				XENOMAI_FEAT_DEP, XENOMAI_ABI_REV, NULL);
Index: xenomai/src/skins/rtai/init.c
===================================================================
--- xenomai.orig/src/skins/rtai/init.c
+++ xenomai/src/skins/rtai/init.c
@@ -27,5 +27,5 @@ static __attribute__ ((constructor))
 void __init_rtai_interface(void)
 {
 	__rtai_muxid =
-	    xeno_user_skin_init(RTAI_SKIN_MAGIC, "RTAI", "xeno_rtai");
+	    xeno_user_skin_init(RTAI_SKIN_MAGIC, "RTAI", "xeno_rtai", 0);
 }
Index: xenomai/src/skins/rtdm/init.c
===================================================================
--- xenomai.orig/src/skins/rtdm/init.c
+++ xenomai/src/skins/rtdm/init.c
@@ -20,6 +20,11 @@
 #include <errno.h>
 #include <stdlib.h>
 #include <string.h>
+
+/* Checking for mlockall is only done by skins that create real-time
+   user-space threads, which we do not. */
+#define XNSKIN_NO_MLOCK_ALERT
+#include <nucleus/bind.h>
 #include <rtdm/syscall.h>
 
 int __rtdm_muxid = -1;
@@ -27,49 +32,9 @@ int __rtdm_muxid = -1;
 static __attribute__((constructor)) void __init_rtdm_interface(void)
 
 {
-    xnfeatinfo_t finfo;
-    int muxid;
-
-#ifdef xeno_arch_features_check
-    xeno_arch_features_check();
-#endif /* xeno_arch_features_check */
-
-    muxid = XENOMAI_SYSBIND(RTDM_SKIN_MAGIC,
-			    XENOMAI_FEAT_DEP,
-			    XENOMAI_ABI_REV,
-			    &finfo);
-    switch (muxid)
-	{
-	case -EINVAL:
-
-	    fprintf(stderr,"Xenomai: incompatible feature set\n");
-	    fprintf(stderr,"(required=\"%s\", present=\"%s\", missing=\"%s\").\n",
-		    finfo.feat_man_s,finfo.feat_all_s,finfo.feat_mis_s);
-	    exit(1);
-
-	case -ENOEXEC:
-
-	    fprintf(stderr,"Xenomai: incompatible ABI revision level\n");
-	    fprintf(stderr,"(needed=%lu, current=%lu).\n",
-		    XENOMAI_ABI_REV,finfo.abirev);
-	    exit(1);
-
-	case -ENOSYS:
-	case -ESRCH:
-
-	    /* we ignore this and fail later when the user tries to open
-	       a device or socket. */
-	    break;
-
-	default:
-
-	    if (muxid < 0)
-		{
-		fprintf(stderr,"Xenomai: binding failed: %s.\n",strerror(-muxid));
-		exit(1);
-		}
-
-	    __rtdm_muxid = muxid;
-	    break;
-	}
+	/* We ignore a missing skin and fail later when the user tries to open
+	   a device or socket. */
+	__rtdm_muxid =
+	    xeno_user_skin_init(RTDM_SKIN_MAGIC, "RTDM", "xeno_rtdm",
+				XNSKIN_LAZY_BIND_FAIL);
 }
Index: xenomai/src/skins/uvm/init.c
===================================================================
--- xenomai.orig/src/skins/uvm/init.c
+++ xenomai/src/skins/uvm/init.c
@@ -29,7 +29,8 @@ xnsysinfo_t __uvm_info;
 static __attribute__ ((constructor))
 void __init_uvm_interface(void)
 {
-	__uvm_muxid = xeno_user_skin_init(UVM_SKIN_MAGIC, "UVM", "xeno_uvm");
+	__uvm_muxid =
+	    xeno_user_skin_init(UVM_SKIN_MAGIC, "UVM", "xeno_uvm", 0);
 
 	XENOMAI_SYSCALL2(__xn_sys_info, __uvm_muxid, &__uvm_info);
 }
Index: xenomai/src/skins/vrtx/init.c
===================================================================
--- xenomai.orig/src/skins/vrtx/init.c
+++ xenomai/src/skins/vrtx/init.c
@@ -40,7 +40,7 @@ void __init_xeno_interface(void)
 	TCB *tcb;
 
 	__vrtx_muxid =
-	    xeno_user_skin_init(VRTX_SKIN_MAGIC, "VRTX", "xeno_vrtx");
+	    xeno_user_skin_init(VRTX_SKIN_MAGIC, "VRTX", "xeno_vrtx", 0);
 
 	/* Allocate a TSD key for indexing self task pointers. */
 
Index: xenomai/src/skins/vxworks/init.c
===================================================================
--- xenomai.orig/src/skins/vxworks/init.c
+++ xenomai/src/skins/vxworks/init.c
@@ -38,7 +38,7 @@ static __attribute__ ((constructor))
 void __init_xeno_interface(void)
 {
 	__vxworks_muxid = xeno_user_skin_init(VXWORKS_SKIN_MAGIC,
-					      "VxWorks", "xeno_vxworks");
+					      "VxWorks", "xeno_vxworks", 0);
 
 	/* Allocate a TSD key for indexing self task pointers. */
 

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.3: optimise-muxcode-calculation-v4.patch --]
[-- Type: text/x-patch; name="optimise-muxcode-calculation-v4.patch", Size: 7892 bytes --]

---
 include/asm-arm/syscall.h      |   29 ++++++++++++------------
 include/asm-blackfin/syscall.h |    6 +++--
 include/asm-i386/syscall.h     |    7 +++--
 include/asm-ia64/syscall.h     |   25 ++++++++++----------
 include/asm-powerpc/syscall.h  |    7 +++--
 include/nucleus/bind.h         |   19 ++++++++++-----
 src/skins/rtdm/init.c          |   49 +++--------------------------------------
 7 files changed, 56 insertions(+), 86 deletions(-)

Index: xenomai/include/asm-arm/syscall.h
===================================================================
--- xenomai.orig/include/asm-arm/syscall.h
+++ xenomai/include/asm-arm/syscall.h
@@ -25,7 +25,8 @@
 
 #include <asm-generic/xenomai/syscall.h>
 
-#define __xn_mux_code(id,op)    ((op << 24)|((id << 16) & 0xff0000)|(__xn_sys_mux & 0xffff))
+#define __xn_mux_code(shifted_id,op) ((op << 24)|shifted_id|(__xn_sys_mux & 0xffff))
+#define __xn_mux_shifted_id(id) ((id << 16) & 0xff0000)
 
 #define XENO_ARM_SYSCALL        0x009F0042	/* carefully chosen... */
 
@@ -130,20 +131,20 @@ static inline int __xn_interrupted_p(str
 #define __sys2(x)	#x
 #define __sys1(x)	__sys2(x)
 
-#define XENOMAI_DO_SYSCALL(nr, id, op, args...)			\
-  ({								                    \
-        unsigned long __res;					        \
+#define XENOMAI_DO_SYSCALL(nr, shifted_id, op, args...)		\
+  ({								\
+        unsigned long __res;					\
 	register unsigned long __res_r0 __asm__ ("r0");		\
-   	ASM_INDECL_##nr;					                \
-								                        \
-	LOADARGS_##nr(__xn_mux_code(id,op), args);		    \
-	__asm__ __volatile__ (					            \
-"       swi " __sys1(XENO_ARM_SYSCALL)			        \
-		: "=r" (__res_r0)				                \
-		: ASM_INPUT_##nr				                \
-		: "memory");					                \
-   	__res = __res_r0;					                \
-   	(int) __res;						                \
+   	ASM_INDECL_##nr;					\
+								\
+	LOADARGS_##nr(__xn_mux_code(shifted_id,op), args);	\
+	__asm__ __volatile__ (					\
+"       swi " __sys1(XENO_ARM_SYSCALL)				\
+		: "=r" (__res_r0)				\
+		: ASM_INPUT_##nr				\
+		: "memory");					\
+   	__res = __res_r0;					\
+   	(int) __res;						\
   })
 
 #define XENOMAI_SYSCALL0(op)                XENOMAI_DO_SYSCALL(0,0,op)
Index: xenomai/include/asm-blackfin/syscall.h
===================================================================
--- xenomai.orig/include/asm-blackfin/syscall.h
+++ xenomai/include/asm-blackfin/syscall.h
@@ -26,7 +26,8 @@
    marker. Note: watch out for the p0 sign convention used by Linux
    (i.e. negative syscall number in orig_p0 meaning "non-syscall
    entry"). */
-#define __xn_mux_code(id,op)   ((id << 24)|((op << 16) & 0xff0000)|(__xn_sys_mux & 0xffff))
+#define __xn_mux_code(shifted_id,op) (shifted_id|((op << 16) & 0xff0000)|(__xn_sys_mux & 0xffff))
+#define __xn_mux_shifted_id(id) (id << 24)
 
 /* Local syscalls -- the braindamage thing about this arch is the
    absence of atomic ops usable from user-space; so we export what
@@ -213,7 +214,8 @@ static inline int __xn_interrupted_p(str
   __res;								\
 })
 
-#define XENOMAI_DO_SYSCALL(nr, id, op, args...)	__emit_syscall##nr(__xn_mux_code(id,op), ##args)
+#define XENOMAI_DO_SYSCALL(nr, shifted_id, op, args...) \
+    __emit_syscall##nr(__xn_mux_code(shifted_id,op), ##args)
 
 #define XENOMAI_SYSCALL0(op)                XENOMAI_DO_SYSCALL(0,0,op)
 #define XENOMAI_SYSCALL1(op,a1)             XENOMAI_DO_SYSCALL(1,0,op,a1)
Index: xenomai/include/asm-i386/syscall.h
===================================================================
--- xenomai.orig/include/asm-i386/syscall.h
+++ xenomai/include/asm-i386/syscall.h
@@ -22,7 +22,8 @@
 
 #include <asm-generic/xenomai/syscall.h>
 
-#define __xn_mux_code(id,op)  ((op << 24)|((id << 16) & 0xff0000)|(__xn_sys_mux & 0x7fff))
+#define __xn_mux_code(shifted_id,op) ((op << 24)|shifted_id|(__xn_sys_mux & 0x7fff))
+#define __xn_mux_shifted_id(id) ((id << 16) & 0xff0000)
 
 #ifdef __KERNEL__
 
@@ -165,9 +166,9 @@ asm (".L__X'%ebx = 1\n\t"
     : "i" (__xn_mux_code(0,op)) ASMFMT_##nr(args) : "memory", "cc");  \
     (int) resultvar; })
 
-#define XENOMAI_SKIN_MUX(nr, id, op, args...) \
+#define XENOMAI_SKIN_MUX(nr, shifted_id, op, args...) \
   ({								      \
-    int muxcode = __xn_mux_code(id,op);                               \
+    int muxcode = __xn_mux_code(shifted_id,op);			      \
     unsigned resultvar;						      \
     asm volatile (						      \
     LOADARGS_##nr						      \
Index: xenomai/include/asm-ia64/syscall.h
===================================================================
--- xenomai.orig/include/asm-ia64/syscall.h
+++ xenomai/include/asm-ia64/syscall.h
@@ -23,7 +23,8 @@
 
 #include <asm-generic/xenomai/syscall.h>
 
-#define __xn_mux_code(id,op)        ((op << 24)|(((id << 16) & 0xff0000UL)|(__xn_sys_mux & 0xffffUL)))
+#define __xn_mux_code(shifted_id,op) ((op << 24)|shifted_id|(__xn_sys_mux & 0xffffUL))
+#define __xn_mux_shifted_id(id) ((id << 16) & 0xff0000UL)
 
 #ifdef __KERNEL__
 
@@ -151,17 +152,17 @@ static inline int __xn_interrupted_p(str
   /* Branch registers.  */						\
   "b6", "b7"
 
-#define XENOMAI_SKIN_MUX(nr, id, op, args...)                   \
-  ({                                                            \
-    register long _r15 asm ("r15") = (__xn_mux_code(id,op));    \
-    register long _retval asm ("r8");                           \
-    register long err asm ("r10");                              \
-    LOAD_ARGS_##nr (args);                                      \
-    __asm __volatile ("break %3;;\n\t"                          \
-                      : "=r" (_retval), "=r" (_r15), "=r" (err) \
-                      : "i" (__BREAK_SYSCALL), "1" (_r15)       \
-			ASM_ARGS_##nr                           \
-                      : "memory" ASM_CLOBBERS_##nr);            \
+#define XENOMAI_SKIN_MUX(nr, shifted_id, op, args...)			\
+  ({									\
+    register long _r15 asm ("r15") = (__xn_mux_code(shifted_id,op));	\
+    register long _retval asm ("r8");					\
+    register long err asm ("r10");					\
+    LOAD_ARGS_##nr (args);						\
+    __asm __volatile ("break %3;;\n\t"					\
+		      : "=r" (_retval), "=r" (_r15), "=r" (err)		\
+		      : "i" (__BREAK_SYSCALL), "1" (_r15)		\
+			ASM_ARGS_##nr					\
+		      : "memory" ASM_CLOBBERS_##nr);			\
     err < 0 ? -_retval : _retval; })
 
 #define XENOMAI_SYS_MUX(nr, op, args...) XENOMAI_SKIN_MUX(nr, 0, op , ##args)
Index: xenomai/include/asm-powerpc/syscall.h
===================================================================
--- xenomai.orig/include/asm-powerpc/syscall.h
+++ xenomai/include/asm-powerpc/syscall.h
@@ -25,7 +25,8 @@
 
 #include <asm-generic/xenomai/syscall.h>
 
-#define __xn_mux_code(id,op)        ((op << 24)|((id << 16) & 0xff0000)|(__xn_sys_mux & 0xffff))
+#define __xn_mux_code(shifted_id,op) ((op << 24)|shifted_id|(__xn_sys_mux & 0xffff))
+#define __xn_mux_shifted_id(id) ((id << 16) & 0xff0000)
 
 #ifdef __KERNEL__
 
@@ -123,7 +124,7 @@ static inline int __xn_interrupted_p(str
 #define ASM_INPUT_4 ASM_INPUT_3, "4" (__sc_6)
 #define ASM_INPUT_5 ASM_INPUT_4, "5" (__sc_7)
 
-#define XENOMAI_DO_SYSCALL(nr, id, op, args...)			\
+#define XENOMAI_DO_SYSCALL(nr, shifted_id, op, args...)		\
   ({								\
 	register unsigned long __sc_0  __asm__ ("r0");		\
 	register unsigned long __sc_3  __asm__ ("r3");		\
@@ -132,7 +133,7 @@ static inline int __xn_interrupted_p(str
 	register unsigned long __sc_6  __asm__ ("r6");		\
 	register unsigned long __sc_7  __asm__ ("r7");		\
 								\
-	LOADARGS_##nr(__xn_mux_code(id,op), args);		\
+	LOADARGS_##nr(__xn_mux_code(shifted_id,op), args);	\
 	__asm__ __volatile__					\
 		("sc           \n\t"				\
 		 "mfcr %0      "				\

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 249 bytes --]