All of lore.kernel.org
 help / color / mirror / Atom feed
* [Xenomai-core] [PATCH 1/1] Use SIGWINCH to trigger priority change in user-space.
@ 2008-10-18 14:22 Gilles Chanteperdrix
  2008-10-18 14:35 ` Gilles Chanteperdrix
  2008-10-19 11:10 ` Jan Kiszka
  0 siblings, 2 replies; 17+ messages in thread
From: Gilles Chanteperdrix @ 2008-10-18 14:22 UTC (permalink / raw)
  To: Xenomai core


Hi,

here is a patch which implements the idea discussed previously of 
re-using SIGWINCH to trigger priority changes in user-space. There is 
only one patch, but the files should have been put in a logical order
to make review easier.

Index: include/nucleus/thread.h
===================================================================
--- include/nucleus/thread.h	(revision 4218)
+++ include/nucleus/thread.h	(working copy)
@@ -116,6 +116,7 @@
 #define XNROBBED  0x00000020 /**< Robbed from resource ownership */
 #define XNATOMIC  0x00000040 /**< In atomic switch from secondary to primary mode */
 #define XNAFFSET  0x00000080 /**< CPU affinity changed from primary mode */
+#define XNPRIOSET 0x00000100 /**< Priority changed from primary mode */
 
 /* These information flags are available to the real-time interfaces */
 #define XNTHREAD_INFO_SPARE0  0x10000000
Index: include/asm-generic/syscall.h
===================================================================
--- include/asm-generic/syscall.h	(revision 4218)
+++ include/asm-generic/syscall.h	(working copy)
@@ -59,7 +59,12 @@ typedef struct xnsysinfo {
     unsigned long tickval;	/* Tick duration (ns) */
 } xnsysinfo_t;
 
-#define SIGHARDEN  SIGWINCH
+#define SIGSHADOW  SIGWINCH
+#define SIGSHADOW_ACTION_HARDEN 1
+#define SIGSHADOW_ACTION_RENICE 2
+#define sigshadow_action(code) ((code) & 0xff)
+#define sigshadow_prio(code) (((code) >> 8) & 0xff)
+#define sigshadow_int(action, prio) ((action) | ((prio) << 8))
 
 #ifdef __KERNEL__
 
Index: include/nucleus/shadow.h
===================================================================
--- include/nucleus/shadow.h	(revision 4218)
+++ include/nucleus/shadow.h	(working copy)
@@ -98,6 +98,7 @@ void xnshadow_reset_shield(void);
 
 void xnshadow_send_sig(struct xnthread *thread,
 		       int sig,
+		       int arg,
 		       int specific);
 
 void xnshadow_rpi_check(void);
Index: ksrc/nucleus/shadow.c
===================================================================
--- ksrc/nucleus/shadow.c	(revision 4218)
+++ ksrc/nucleus/shadow.c	(working copy)
@@ -115,6 +115,14 @@ do { \
    switch_lock_owner[task_cpu(t)] = t; \
 } while(0)
 
+#define xnshadow_sig_mux(sig, arg) ((sig) | (arg << 8))
+#define xnshadow_sig_demux(muxed, sig, arg) \
+	do {				     \
+		int _muxed = (muxed);	     \
+		(sig) = _muxed & 0xff;	     \
+		(arg) = _muxed << 8;	     \
+	} while (0)
+
 static struct task_struct *switch_lock_owner[XNARCH_NR_CPUS];
 
 static int nucleus_muxid = -1;
@@ -879,7 +887,7 @@ static void xnshadow_dereference_skin(un
 
 static void lostage_handler(void *cookie)
 {
-	int cpu = smp_processor_id(), reqnum, sig;
+	int cpu = smp_processor_id(), reqnum, sig, arg;
 	struct __lostagerq *rq = &lostagerq[cpu];
 
 	while ((reqnum = rq->out) != rq->in) {
@@ -927,8 +935,16 @@ static void lostage_handler(void *cookie
 
 		case LO_SIGTHR_REQ:
 
-			sig = rq->req[reqnum].arg;
-			send_sig(sig, p, 1);
+			xnshadow_sig_demux(rq->req[reqnum].arg, sig, arg);
+			if (sig == SIGSHADOW) {
+				siginfo_t si;
+				memset(&si, '\0', sizeof(si));
+				si.si_signo = sig;
+				si.si_code = SI_QUEUE;
+				si.si_int = arg;
+				send_sig_info(sig, &si, p);
+			} else
+				send_sig(sig, p, 1);
 			break;
 
 		case LO_SIGGRP_REQ:
@@ -1235,6 +1251,13 @@ void xnshadow_relax(int notify)
 		/* Help debugging spurious relaxes. */
 		send_sig(SIGXCPU, current, 1);
 
+	if (xnthread_test_info(thread, XNPRIOSET)) {
+		xnthread_clear_info(thread, XNPRIOSET);
+		xnshadow_send_sig(thread, SIGSHADOW,
+				  sigshadow_int(SIGSHADOW_ACTION_RENICE, prio),
+				  1);
+	}
+
 #ifdef CONFIG_SMP
 	/* If the shadow thread changed its CPU affinity while in
 	   primary mode, reset the CPU affinity of its Linux
@@ -1485,13 +1508,13 @@ void xnshadow_start(xnthread_t *thread)
 void xnshadow_renice(xnthread_t *thread)
 {
 	/* Called with nklock locked, Xenomai interrupts off. */
-	struct task_struct *p = xnthread_archtcb(thread)->user_task;
-
 	/* We need to bound the priority values in the [1..MAX_RT_PRIO-1]
 	   range, since the core pod's priority scale is a superset of
 	   Linux's priority scale. */
 	int prio = normalize_priority(xnthread_current_priority(thread));
-	schedule_linux_call(LO_RENICE_REQ, p, prio);
+
+	xnshadow_send_sig(thread, SIGSHADOW,
+			  sigshadow_int(SIGSHADOW_ACTION_RENICE, prio), 1);
 
 	if (!xnthread_test_state(thread, XNDORMANT) &&
 	    xnthread_sched(thread) == xnpod_current_sched())
@@ -1501,8 +1524,7 @@ void xnshadow_renice(xnthread_t *thread)
 void xnshadow_suspend(xnthread_t *thread)
 {
 	/* Called with nklock locked, Xenomai interrupts off. */
-	struct task_struct *p = xnthread_archtcb(thread)->user_task;
-	schedule_linux_call(LO_SIGTHR_REQ, p, SIGHARDEN);
+	xnshadow_send_sig(thread, SIGSHADOW, SIGSHADOW_ACTION_HARDEN, 1);
 }
 
 static int xnshadow_sys_migrate(struct pt_regs *regs)
@@ -1513,7 +1535,7 @@ static int xnshadow_sys_migrate(struct p
 				return -EPERM;
 
 			/* Paranoid: a corner case where the
-			   user-space side fiddles with SIGHARDEN
+			   user-space side fiddles with SIGSHADOW
 			   while the target thread is still waiting to
 			   be started. */
 			if (xnthread_test_state(xnshadow_thread(current), XNDORMANT))
@@ -1979,10 +2001,11 @@ static inline int substitute_linux_sysca
 	return 0;
 }
 
-void xnshadow_send_sig(xnthread_t *thread, int sig, int specific)
+void xnshadow_send_sig(xnthread_t *thread, int sig, int arg, int specific)
 {
 	schedule_linux_call(specific ? LO_SIGTHR_REQ : LO_SIGGRP_REQ,
-			    xnthread_user_task(thread), sig);
+			    xnthread_user_task(thread),
+			    xnshadow_sig_mux(sig, specific ? arg : 0));
 }
 
 static inline int do_hisyscall_event(unsigned event, unsigned domid, void *data)
Index: ksrc/nucleus/synch.c
===================================================================
--- ksrc/nucleus/synch.c	(revision 4218)
+++ ksrc/nucleus/synch.c	(working copy)
@@ -121,6 +121,8 @@ static void xnsynch_renice_thread(xnthre
 #ifdef CONFIG_XENO_OPT_PERVASIVE
 	if (xnthread_test_state(thread, XNRELAX))
 		xnshadow_renice(thread);
+	else if (xnthread_test_state(thread, XNSHADOW))
+		xnthread_set_info(thread, XNPRIOSET);
 #endif /* CONFIG_XENO_OPT_PERVASIVE */
 }
 
Index: ksrc/nucleus/pod.c
===================================================================
--- ksrc/nucleus/pod.c	(revision 4218)
+++ ksrc/nucleus/pod.c	(working copy)
@@ -1162,7 +1162,7 @@ void xnpod_delete_thread(xnthread_t *thr
 	    !xnthread_test_state(thread, XNDORMANT) &&
 	    !xnpod_current_p(thread)) {
 		if (!xnpod_userspace_p())
-			xnshadow_send_sig(thread, SIGKILL, 1);
+			xnshadow_send_sig(thread, SIGKILL, 0, 1);
 		/*
 		 * Otherwise, assume the interface library has issued
 		 * pthread_cancel on the target thread, which should
@@ -1456,7 +1456,7 @@ void xnpod_suspend_thread(xnthread_t *th
 	   is actually running some code under the control of the
 	   Linux scheduler (i.e. it's relaxed).  To make this
 	   possible, we force the target Linux task to migrate back to
-	   the Xenomai domain by sending it a SIGHARDEN signal the
+	   the Xenomai domain by sending it a SIGSHADOW signal the
 	   skin interface libraries trap for this specific internal
 	   purpose, whose handler is expected to call back the
 	   nucleus's migration service. By forcing this migration, we
@@ -1821,8 +1821,12 @@ void xnpod_renice_thread_inner(xnthread_
 			xnpod_resume_thread(thread, 0);
 	}
 #ifdef CONFIG_XENO_OPT_PERVASIVE
-	if (propagate && xnthread_test_state(thread, XNRELAX))
-		xnshadow_renice(thread);
+	if (propagate) {
+		if (xnthread_test_state(thread, XNRELAX))
+			xnshadow_renice(thread);
+		else if (xnthread_test_state(thread, XNSHADOW))
+			xnthread_set_info(thread, XNPRIOSET);
+	}
 #endif /* CONFIG_XENO_OPT_PERVASIVE */
 
 	xnlock_put_irqrestore(&nklock, s);
Index: include/asm-generic/bits/sigshadow.h
===================================================================
--- include/asm-generic/bits/sigshadow.h	(revision 0)
+++ include/asm-generic/bits/sigshadow.h	(revision 0)
@@ -0,0 +1,72 @@
+#ifndef _XENO_ASM_GENERIC_BITS_SIGSHADOW_H
+#define _XENO_ASM_GENERIC_BITS_SIGSHADOW_H
+
+#include <pthread.h>
+#include <signal.h>
+
+static pthread_once_t sigshadow_installed = PTHREAD_ONCE_INIT;
+static struct sigaction old_sigshadow_action;
+
+static void sigshadow_handler(int sig, siginfo_t *si, void *ctxt)
+{
+	int action;
+
+	/* Not a signal sent by Xenomai nucleus */
+	if (si->si_code != SI_QUEUE) {
+		const struct sigaction *const sa = &old_sigshadow_action;
+		sigset_t old_sigset;
+
+	  not_nucleus:
+		if ((!(sa->sa_flags & SA_SIGINFO) && !sa->sa_handler)
+		    || ((sa->sa_flags & SA_SIGINFO) && !sa->sa_sigaction))
+			return;
+
+		pthread_sigmask(SIG_SETMASK, &sa->sa_mask, &old_sigset);
+		if (!(sa->sa_flags & SA_SIGINFO))
+			sa->sa_handler(sig);
+		else
+			sa->sa_sigaction(sig, si, ctxt);
+		pthread_sigmask(SIG_SETMASK, &old_sigset, NULL);
+		return;
+	}
+
+	action = sigshadow_action(si->si_int);
+
+	switch(action) {
+	case SIGSHADOW_ACTION_HARDEN:
+		XENOMAI_SYSCALL1(__xn_sys_migrate, XENOMAI_XENO_DOMAIN);
+		break;
+
+	case SIGSHADOW_ACTION_RENICE: {
+		struct sched_param param;
+		int policy;
+
+		param.sched_priority = sigshadow_prio(si->si_int);
+		policy = param.sched_priority > 0 ? SCHED_FIFO: SCHED_OTHER;
+		pthread_setschedparam(pthread_self(), policy, &param);
+		break;
+	}
+
+	default:
+		goto not_nucleus;
+	}
+}
+
+static void sigshadow_install_once(void)
+{
+	struct sigaction new_sigshadow_action;
+
+	new_sigshadow_action.sa_flags = SA_SIGINFO | SA_RESTART;
+	new_sigshadow_action.sa_sigaction = sigshadow_handler;
+	sigemptyset(&new_sigshadow_action.sa_mask);
+
+	sigaction(SIGSHADOW, &new_sigshadow_action, &old_sigshadow_action);
+	if (!(old_sigshadow_action.sa_flags & SA_NODEFER))
+		sigaddset(&old_sigshadow_action.sa_mask, SIGSHADOW);
+}
+
+static inline void sigshadow_install(void)
+{
+	pthread_once(&sigshadow_installed, sigshadow_install_once);
+}
+#endif /* _XENO_ASM_GENERIC_BITS_SIGSHADOW_H */
Index: src/skins/posix/thread.c
===================================================================
--- src/skins/posix/thread.c	(revision 4218)
+++ src/skins/posix/thread.c	(working copy)
@@ -26,23 +26,13 @@
 #include <semaphore.h>
 #include <posix/syscall.h>
 #include <asm-generic/bits/current.h>
+#include <asm-generic/bits/sigshadow.h>
 
 extern int __pse51_muxid;
 
 static pthread_attr_t default_attr;
 static int linuxthreads;
 
-static void (*old_sigharden_handler)(int sig);
-
-static void __pthread_sigharden_handler(int sig)
-{
-	if (old_sigharden_handler &&
-	    old_sigharden_handler != &__pthread_sigharden_handler)
-		old_sigharden_handler(sig);
-
-	XENOMAI_SYSCALL1(__xn_sys_migrate, XENOMAI_XENO_DOMAIN);
-}
-
 int __wrap_pthread_setschedparam(pthread_t thread,
 				 int policy, const struct sched_param *param)
 {
@@ -59,7 +49,7 @@ int __wrap_pthread_setschedparam(pthread
 		__real_pthread_setschedparam(thread, policy, param);
 
 	if (!err && promoted) {
-		old_sigharden_handler = signal(SIGHARDEN, &__pthread_sigharden_handler);
+		sigshadow_install();
 		xeno_set_current();
 		if (policy != SCHED_OTHER)
 			XENOMAI_SYSCALL1(__xn_sys_migrate, XENOMAI_XENO_DOMAIN);
@@ -118,7 +108,7 @@ static void *__pthread_trampoline(void *
 	int parent_prio, policy;
 	long err;
 
-	old_sigharden_handler = signal(SIGHARDEN, &__pthread_sigharden_handler);
+	sigshadow_install();
 
 	param.sched_priority = iargs->prio;
 	policy = iargs->policy;
Index: src/skins/native/task.c
===================================================================
--- src/skins/native/task.c	(revision 4218)
+++ src/skins/native/task.c	(working copy)
@@ -26,6 +26,7 @@
 #include <limits.h>
 #include <native/syscall.h>
 #include <native/task.h>
+#include <asm-generic/bits/sigshadow.h>
 #include "wrappers.h"
 
 extern pthread_key_t __native_tskey;
@@ -42,17 +43,6 @@ struct rt_task_iargs {
 	xncompletion_t *completionp;
 };
 
-static void (*old_sigharden_handler)(int sig);
-
-static void rt_task_sigharden(int sig)
-{
-	if (old_sigharden_handler &&
-	    old_sigharden_handler != &rt_task_sigharden)
-		old_sigharden_handler(sig);
-
-	XENOMAI_SYSCALL1(__xn_sys_migrate, XENOMAI_XENO_DOMAIN);
-}
-
 static void *rt_task_trampoline(void *cookie)
 {
 	struct rt_task_iargs *iargs = (struct rt_task_iargs *)cookie;
@@ -74,7 +64,7 @@ static void *rt_task_trampoline(void *co
 	/* rt_task_delete requires asynchronous cancellation */
 	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
 
-	old_sigharden_handler = signal(SIGHARDEN, &rt_task_sigharden);
+	sigshadow_install();
 
 	bulk.a1 = (u_long)iargs->task;
 	bulk.a2 = (u_long)iargs->name;
@@ -175,8 +165,7 @@ int rt_task_shadow(RT_TASK *task, const 
 
 	/* rt_task_delete requires asynchronous cancellation */
 	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
-
-	old_sigharden_handler = signal(SIGHARDEN, &rt_task_sigharden);
+	sigshadow_install();
 
 	if (prio > 0) {
 		/* Make sure the POSIX library caches the right priority. */
Index: src/skins/vxworks/taskLib.c
===================================================================
--- src/skins/vxworks/taskLib.c	(revision 4218)
+++ src/skins/vxworks/taskLib.c	(working copy)
@@ -27,6 +27,7 @@
 #include <errno.h>
 #include <limits.h>
 #include <vxworks/vxworks.h>
+#include <asm-generic/bits/sigshadow.h>
 #include "wrappers.h"
 
 extern pthread_key_t __vxworks_tskey;
@@ -54,17 +55,6 @@ struct wind_task_iargs {
 	xncompletion_t *completionp;
 };
 
-static void (*old_sigharden_handler)(int sig);
-
-static void wind_task_sigharden(int sig)
-{
-	if (old_sigharden_handler &&
-	    old_sigharden_handler != &wind_task_sigharden)
-		old_sigharden_handler(sig);
-
-	XENOMAI_SYSCALL1(__xn_sys_migrate, XENOMAI_XENO_DOMAIN);
-}
-
 static int wind_task_set_posix_priority(int prio, struct sched_param *param)
 {
 	int maxpprio, pprio;
@@ -103,8 +93,7 @@ static void *wind_task_trampoline(void *
 
 	/* wind_task_delete requires asynchronous cancellation */
 	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
-
-	old_sigharden_handler = signal(SIGHARDEN, &wind_task_sigharden);
+	sigshadow_install();
 
 	bulk.a1 = (u_long)iargs->name;
 	bulk.a2 = (u_long)iargs->prio;
Index: src/skins/psos+/task.c
===================================================================
--- src/skins/psos+/task.c	(revision 4218)
+++ src/skins/psos+/task.c	(working copy)
@@ -26,6 +26,7 @@
 #include <memory.h>
 #include <string.h>
 #include <psos+/psos.h>
+#include <asm-generic/bits/sigshadow.h>
 
 extern int __psos_muxid;
 
@@ -38,17 +39,6 @@ struct psos_task_iargs {
 	xncompletion_t *completionp;
 };
 
-static void (*old_sigharden_handler)(int sig);
-
-static void psos_task_sigharden(int sig)
-{
-	if (old_sigharden_handler &&
-	    old_sigharden_handler != &psos_task_sigharden)
-		old_sigharden_handler(sig);
-
-	XENOMAI_SYSCALL1(__xn_sys_migrate, XENOMAI_XENO_DOMAIN);
-}
-
 static int psos_task_set_posix_priority(int prio, struct sched_param *param)
 {
 	int maxpprio, pprio;
@@ -79,8 +69,7 @@ static void *psos_task_trampoline(void *
 	pthread_setschedparam(pthread_self(), policy, &param);
 
 	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
-
-	old_sigharden_handler = signal(SIGHARDEN, &psos_task_sigharden);
+	sigshadow_install();
 
 	err = XENOMAI_SKINCALL5(__psos_muxid,
 				__psos_t_create,
@@ -174,8 +163,7 @@ u_long t_shadow(const char *name, /* Xen
 		u_long *tid_r)
 {
 	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
-
-	old_sigharden_handler = signal(SIGHARDEN, &psos_task_sigharden);
+	sigshadow_install();
 
 	return XENOMAI_SKINCALL5(__psos_muxid,
 				 __psos_t_create,
Index: src/skins/vrtx/task.c
===================================================================
--- src/skins/vrtx/task.c	(revision 4218)
+++ src/skins/vrtx/task.c	(working copy)
@@ -27,6 +27,7 @@
 #include <errno.h>
 #include <limits.h>
 #include <vrtx/vrtx.h>
+#include <asm-generic/bits/sigshadow.h>
 
 extern pthread_key_t __vrtx_tskey;
 
@@ -44,17 +45,6 @@ struct vrtx_task_iargs {
 	xncompletion_t *completionp;
 };
 
-static void (*old_sigharden_handler)(int sig);
-
-static void vrtx_task_sigharden(int sig)
-{
-	if (old_sigharden_handler &&
-	    old_sigharden_handler != &vrtx_task_sigharden)
-		old_sigharden_handler(sig);
-
-	XENOMAI_SYSCALL1(__xn_sys_migrate, XENOMAI_XENO_DOMAIN);
-}
-
 static int vrtx_task_set_posix_priority(int prio, struct sched_param *param)
 {
 	int maxpprio, pprio;
@@ -93,8 +83,7 @@ static void *vrtx_task_trampoline(void *
 
 	/* vrtx_task_delete requires asynchronous cancellation */
 	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
-
-	old_sigharden_handler = signal(SIGHARDEN, &vrtx_task_sigharden);
+	sigshadow_install();
 
 	bulk.a1 = (u_long)iargs->tid;
 	bulk.a2 = (u_long)iargs->prio;
Index: src/skins/uitron/task.c
===================================================================
--- src/skins/uitron/task.c	(revision 4218)
+++ src/skins/uitron/task.c	(working copy)
@@ -24,6 +24,7 @@
 #include <errno.h>
 #include <limits.h>
 #include <asm/xenomai/system.h>
+#include <asm-generic/bits/sigshadow.h>
 #include <uitron/uitron.h>
 
 extern int __uitron_muxid;
@@ -35,17 +36,6 @@ struct uitron_task_iargs {
 	xncompletion_t *completionp;
 };
 
-static void (*old_sigharden_handler)(int sig);
-
-static void uitron_task_sigharden(int sig)
-{
-	if (old_sigharden_handler &&
-	    old_sigharden_handler != &uitron_task_sigharden)
-		old_sigharden_handler(sig);
-
-	XENOMAI_SYSCALL1(__xn_sys_migrate, XENOMAI_XENO_DOMAIN);
-}
-
 static int uitron_task_set_posix_priority(int prio, struct sched_param *param)
 {
 	int maxpprio, pprio;
@@ -80,7 +70,7 @@ static void *uitron_task_trampoline(void
 	pthread_setschedparam(pthread_self(), policy, &param);
 
 	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
-	old_sigharden_handler = signal(SIGHARDEN, &uitron_task_sigharden);
+	sigshadow_install();
 
 	err = XENOMAI_SKINCALL3(__uitron_muxid,
 				__uitron_cre_tsk,
@@ -157,8 +147,7 @@ ER shd_tsk(ID tskid, T_CTSK *pk_ctsk) /*
 	pthread_setschedparam(pthread_self(), policy, &param);
 
 	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
-
-	old_sigharden_handler = signal(SIGHARDEN, &uitron_task_sigharden);
+	sigshadow_install();
 
 	return XENOMAI_SKINCALL3(__uitron_muxid,
 				 __uitron_cre_tsk,

-- 
					    Gilles.


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

* Re: [Xenomai-core] [PATCH 1/1] Use SIGWINCH to trigger priority change in user-space.
  2008-10-18 14:22 [Xenomai-core] [PATCH 1/1] Use SIGWINCH to trigger priority change in user-space Gilles Chanteperdrix
@ 2008-10-18 14:35 ` Gilles Chanteperdrix
  2008-10-18 17:09   ` Philippe Gerum
  2008-10-19 15:21   ` Gilles Chanteperdrix
  2008-10-19 11:10 ` Jan Kiszka
  1 sibling, 2 replies; 17+ messages in thread
From: Gilles Chanteperdrix @ 2008-10-18 14:35 UTC (permalink / raw)
  To: Xenomai core

Gilles Chanteperdrix wrote:
> Hi,
> 
> here is a patch which implements the idea discussed previously of 
> re-using SIGWINCH to trigger priority changes in user-space. There is 
> only one patch, but the files should have been put in a logical order
> to make review easier.

Since there are chances of conflicts with Jan patches series, I will
rebase this patch after Jan patches have been commited.

-- 
					    Gilles.


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

* Re: [Xenomai-core] [PATCH 1/1] Use SIGWINCH to trigger priority change in user-space.
  2008-10-18 14:35 ` Gilles Chanteperdrix
@ 2008-10-18 17:09   ` Philippe Gerum
  2008-10-19 15:21   ` Gilles Chanteperdrix
  1 sibling, 0 replies; 17+ messages in thread
From: Philippe Gerum @ 2008-10-18 17:09 UTC (permalink / raw)
  To: Gilles Chanteperdrix; +Cc: Xenomai core

Gilles Chanteperdrix wrote:
> Gilles Chanteperdrix wrote:
>> Hi,
>>
>> here is a patch which implements the idea discussed previously of 
>> re-using SIGWINCH to trigger priority changes in user-space. There is 
>> only one patch, but the files should have been put in a logical order
>> to make review easier.
> 
> Since there are chances of conflicts with Jan patches series, I will
> rebase this patch after Jan patches have been commited.
> 

I'm reviewing those now, this should not be long.

-- 
Philippe.


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

* Re: [Xenomai-core] [PATCH 1/1] Use SIGWINCH to trigger priority change in user-space.
  2008-10-18 14:22 [Xenomai-core] [PATCH 1/1] Use SIGWINCH to trigger priority change in user-space Gilles Chanteperdrix
  2008-10-18 14:35 ` Gilles Chanteperdrix
@ 2008-10-19 11:10 ` Jan Kiszka
  2008-10-19 11:19   ` Gilles Chanteperdrix
  1 sibling, 1 reply; 17+ messages in thread
From: Jan Kiszka @ 2008-10-19 11:10 UTC (permalink / raw)
  To: Gilles Chanteperdrix; +Cc: Xenomai core

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

Gilles Chanteperdrix wrote:
> Hi,
> 
> here is a patch which implements the idea discussed previously of 
> re-using SIGWINCH to trigger priority changes in user-space. There is 
> only one patch, but the files should have been put in a logical order
> to make review easier.

Thanks for providing this. Find a few comments below, primarily
targeting the (not new) signal hooking strategy.

> 
> Index: include/nucleus/thread.h
> ===================================================================
> --- include/nucleus/thread.h	(revision 4218)
> +++ include/nucleus/thread.h	(working copy)
> @@ -116,6 +116,7 @@
>  #define XNROBBED  0x00000020 /**< Robbed from resource ownership */
>  #define XNATOMIC  0x00000040 /**< In atomic switch from secondary to primary mode */
>  #define XNAFFSET  0x00000080 /**< CPU affinity changed from primary mode */
> +#define XNPRIOSET 0x00000100 /**< Priority changed from primary mode */
>  
>  /* These information flags are available to the real-time interfaces */
>  #define XNTHREAD_INFO_SPARE0  0x10000000
> Index: include/asm-generic/syscall.h
> ===================================================================
> --- include/asm-generic/syscall.h	(revision 4218)
> +++ include/asm-generic/syscall.h	(working copy)
> @@ -59,7 +59,12 @@ typedef struct xnsysinfo {
>      unsigned long tickval;	/* Tick duration (ns) */
>  } xnsysinfo_t;
>  
> -#define SIGHARDEN  SIGWINCH
> +#define SIGSHADOW  SIGWINCH
> +#define SIGSHADOW_ACTION_HARDEN 1
> +#define SIGSHADOW_ACTION_RENICE 2
> +#define sigshadow_action(code) ((code) & 0xff)
> +#define sigshadow_prio(code) (((code) >> 8) & 0xff)
> +#define sigshadow_int(action, prio) ((action) | ((prio) << 8))
>  
>  #ifdef __KERNEL__
>  
> Index: include/nucleus/shadow.h
> ===================================================================
> --- include/nucleus/shadow.h	(revision 4218)
> +++ include/nucleus/shadow.h	(working copy)
> @@ -98,6 +98,7 @@ void xnshadow_reset_shield(void);
>  
>  void xnshadow_send_sig(struct xnthread *thread,
>  		       int sig,
> +		       int arg,
>  		       int specific);
>  
>  void xnshadow_rpi_check(void);
> Index: ksrc/nucleus/shadow.c
> ===================================================================
> --- ksrc/nucleus/shadow.c	(revision 4218)
> +++ ksrc/nucleus/shadow.c	(working copy)
> @@ -115,6 +115,14 @@ do { \
>     switch_lock_owner[task_cpu(t)] = t; \
>  } while(0)
>  
> +#define xnshadow_sig_mux(sig, arg) ((sig) | (arg << 8))
> +#define xnshadow_sig_demux(muxed, sig, arg) \
> +	do {				     \
> +		int _muxed = (muxed);	     \
> +		(sig) = _muxed & 0xff;	     \
> +		(arg) = _muxed << 8;	     \
> +	} while (0)
> +
>  static struct task_struct *switch_lock_owner[XNARCH_NR_CPUS];
>  
>  static int nucleus_muxid = -1;
> @@ -879,7 +887,7 @@ static void xnshadow_dereference_skin(un
>  
>  static void lostage_handler(void *cookie)
>  {
> -	int cpu = smp_processor_id(), reqnum, sig;
> +	int cpu = smp_processor_id(), reqnum, sig, arg;
>  	struct __lostagerq *rq = &lostagerq[cpu];
>  
>  	while ((reqnum = rq->out) != rq->in) {
> @@ -927,8 +935,16 @@ static void lostage_handler(void *cookie
>  
>  		case LO_SIGTHR_REQ:
>  
> -			sig = rq->req[reqnum].arg;
> -			send_sig(sig, p, 1);
> +			xnshadow_sig_demux(rq->req[reqnum].arg, sig, arg);
> +			if (sig == SIGSHADOW) {
> +				siginfo_t si;
> +				memset(&si, '\0', sizeof(si));
> +				si.si_signo = sig;
> +				si.si_code = SI_QUEUE;
> +				si.si_int = arg;
> +				send_sig_info(sig, &si, p);
> +			} else
> +				send_sig(sig, p, 1);
>  			break;
>  
>  		case LO_SIGGRP_REQ:
> @@ -1235,6 +1251,13 @@ void xnshadow_relax(int notify)
>  		/* Help debugging spurious relaxes. */
>  		send_sig(SIGXCPU, current, 1);
>  
> +	if (xnthread_test_info(thread, XNPRIOSET)) {
> +		xnthread_clear_info(thread, XNPRIOSET);
> +		xnshadow_send_sig(thread, SIGSHADOW,
> +				  sigshadow_int(SIGSHADOW_ACTION_RENICE, prio),
> +				  1);
> +	}
> +
>  #ifdef CONFIG_SMP
>  	/* If the shadow thread changed its CPU affinity while in
>  	   primary mode, reset the CPU affinity of its Linux
> @@ -1485,13 +1508,13 @@ void xnshadow_start(xnthread_t *thread)
>  void xnshadow_renice(xnthread_t *thread)
>  {
>  	/* Called with nklock locked, Xenomai interrupts off. */

I think this comment line should be pushed in front of the function at
this chance.

> -	struct task_struct *p = xnthread_archtcb(thread)->user_task;
> -
>  	/* We need to bound the priority values in the [1..MAX_RT_PRIO-1]
>  	   range, since the core pod's priority scale is a superset of
>  	   Linux's priority scale. */
>  	int prio = normalize_priority(xnthread_current_priority(thread));
> -	schedule_linux_call(LO_RENICE_REQ, p, prio);
> +
> +	xnshadow_send_sig(thread, SIGSHADOW,
> +			  sigshadow_int(SIGSHADOW_ACTION_RENICE, prio), 1);
>  
>  	if (!xnthread_test_state(thread, XNDORMANT) &&
>  	    xnthread_sched(thread) == xnpod_current_sched())
> @@ -1501,8 +1524,7 @@ void xnshadow_renice(xnthread_t *thread)
>  void xnshadow_suspend(xnthread_t *thread)
>  {
>  	/* Called with nklock locked, Xenomai interrupts off. */
> -	struct task_struct *p = xnthread_archtcb(thread)->user_task;
> -	schedule_linux_call(LO_SIGTHR_REQ, p, SIGHARDEN);
> +	xnshadow_send_sig(thread, SIGSHADOW, SIGSHADOW_ACTION_HARDEN, 1);
>  }
>  
>  static int xnshadow_sys_migrate(struct pt_regs *regs)
> @@ -1513,7 +1535,7 @@ static int xnshadow_sys_migrate(struct p
>  				return -EPERM;
>  
>  			/* Paranoid: a corner case where the
> -			   user-space side fiddles with SIGHARDEN
> +			   user-space side fiddles with SIGSHADOW
>  			   while the target thread is still waiting to
>  			   be started. */
>  			if (xnthread_test_state(xnshadow_thread(current), XNDORMANT))
> @@ -1979,10 +2001,11 @@ static inline int substitute_linux_sysca
>  	return 0;
>  }
>  
> -void xnshadow_send_sig(xnthread_t *thread, int sig, int specific)
> +void xnshadow_send_sig(xnthread_t *thread, int sig, int arg, int specific)
>  {
>  	schedule_linux_call(specific ? LO_SIGTHR_REQ : LO_SIGGRP_REQ,
> -			    xnthread_user_task(thread), sig);
> +			    xnthread_user_task(thread),
> +			    xnshadow_sig_mux(sig, specific ? arg : 0));
>  }
>  
>  static inline int do_hisyscall_event(unsigned event, unsigned domid, void *data)
> Index: ksrc/nucleus/synch.c
> ===================================================================
> --- ksrc/nucleus/synch.c	(revision 4218)
> +++ ksrc/nucleus/synch.c	(working copy)
> @@ -121,6 +121,8 @@ static void xnsynch_renice_thread(xnthre
>  #ifdef CONFIG_XENO_OPT_PERVASIVE
>  	if (xnthread_test_state(thread, XNRELAX))
>  		xnshadow_renice(thread);
> +	else if (xnthread_test_state(thread, XNSHADOW))
> +		xnthread_set_info(thread, XNPRIOSET);
>  #endif /* CONFIG_XENO_OPT_PERVASIVE */
>  }
>  
> Index: ksrc/nucleus/pod.c
> ===================================================================
> --- ksrc/nucleus/pod.c	(revision 4218)
> +++ ksrc/nucleus/pod.c	(working copy)
> @@ -1162,7 +1162,7 @@ void xnpod_delete_thread(xnthread_t *thr
>  	    !xnthread_test_state(thread, XNDORMANT) &&
>  	    !xnpod_current_p(thread)) {
>  		if (!xnpod_userspace_p())
> -			xnshadow_send_sig(thread, SIGKILL, 1);
> +			xnshadow_send_sig(thread, SIGKILL, 0, 1);
>  		/*
>  		 * Otherwise, assume the interface library has issued
>  		 * pthread_cancel on the target thread, which should
> @@ -1456,7 +1456,7 @@ void xnpod_suspend_thread(xnthread_t *th
>  	   is actually running some code under the control of the
>  	   Linux scheduler (i.e. it's relaxed).  To make this
>  	   possible, we force the target Linux task to migrate back to
> -	   the Xenomai domain by sending it a SIGHARDEN signal the
> +	   the Xenomai domain by sending it a SIGSHADOW signal the
>  	   skin interface libraries trap for this specific internal
>  	   purpose, whose handler is expected to call back the
>  	   nucleus's migration service. By forcing this migration, we
> @@ -1821,8 +1821,12 @@ void xnpod_renice_thread_inner(xnthread_
>  			xnpod_resume_thread(thread, 0);
>  	}
>  #ifdef CONFIG_XENO_OPT_PERVASIVE
> -	if (propagate && xnthread_test_state(thread, XNRELAX))
> -		xnshadow_renice(thread);
> +	if (propagate) {
> +		if (xnthread_test_state(thread, XNRELAX))
> +			xnshadow_renice(thread);
> +		else if (xnthread_test_state(thread, XNSHADOW))
> +			xnthread_set_info(thread, XNPRIOSET);
> +	}
>  #endif /* CONFIG_XENO_OPT_PERVASIVE */
>  
>  	xnlock_put_irqrestore(&nklock, s);
> Index: include/asm-generic/bits/sigshadow.h
> ===================================================================
> --- include/asm-generic/bits/sigshadow.h	(revision 0)
> +++ include/asm-generic/bits/sigshadow.h	(revision 0)
> @@ -0,0 +1,72 @@
> +#ifndef _XENO_ASM_GENERIC_BITS_SIGSHADOW_H
> +#define _XENO_ASM_GENERIC_BITS_SIGSHADOW_H
> +
> +#include <pthread.h>
> +#include <signal.h>
> +
> +static pthread_once_t sigshadow_installed = PTHREAD_ONCE_INIT;
> +static struct sigaction old_sigshadow_action;
> +
> +static void sigshadow_handler(int sig, siginfo_t *si, void *ctxt)
> +{
> +	int action;
> +
> +	/* Not a signal sent by Xenomai nucleus */
> +	if (si->si_code != SI_QUEUE) {
> +		const struct sigaction *const sa = &old_sigshadow_action;
> +		sigset_t old_sigset;
> +
> +	  not_nucleus:
> +		if ((!(sa->sa_flags & SA_SIGINFO) && !sa->sa_handler)
> +		    || ((sa->sa_flags & SA_SIGINFO) && !sa->sa_sigaction))
> +			return;
> +
> +		pthread_sigmask(SIG_SETMASK, &sa->sa_mask, &old_sigset);
> +		if (!(sa->sa_flags & SA_SIGINFO))
> +			sa->sa_handler(sig);
> +		else
> +			sa->sa_sigaction(sig, si, ctxt);
> +		pthread_sigmask(SIG_SETMASK, &old_sigset, NULL);
> +		return;
> +	}
> +
> +	action = sigshadow_action(si->si_int);
> +
> +	switch(action) {
> +	case SIGSHADOW_ACTION_HARDEN:
> +		XENOMAI_SYSCALL1(__xn_sys_migrate, XENOMAI_XENO_DOMAIN);
> +		break;
> +
> +	case SIGSHADOW_ACTION_RENICE: {
> +		struct sched_param param;
> +		int policy;
> +
> +		param.sched_priority = sigshadow_prio(si->si_int);
> +		policy = param.sched_priority > 0 ? SCHED_FIFO: SCHED_OTHER;
> +		pthread_setschedparam(pthread_self(), policy, &param);
> +		break;
> +	}
> +
> +	default:
> +		goto not_nucleus;
> +	}
> +}
> +
> +static void sigshadow_install_once(void)
> +{
> +	struct sigaction new_sigshadow_action;
> +
> +	new_sigshadow_action.sa_flags = SA_SIGINFO | SA_RESTART;
> +	new_sigshadow_action.sa_sigaction = sigshadow_handler;
> +	sigemptyset(&new_sigshadow_action.sa_mask);
> +
> +	sigaction(SIGSHADOW, &new_sigshadow_action, &old_sigshadow_action);
> +	if (!(old_sigshadow_action.sa_flags & SA_NODEFER))
> +		sigaddset(&old_sigshadow_action.sa_mask, SIGSHADOW);
> +}
> +
> +static inline void sigshadow_install(void)
> +{
> +	pthread_once(&sigshadow_installed, sigshadow_install_once);
> +}
> +#endif /* _XENO_ASM_GENERIC_BITS_SIGSHADOW_H */

I wonder if we shouldn't switch the signal hooking strategy: So far we
install the handler at shadow thread creation time, saving a potentially
installed handler of the application for redirection of
Xenomai-unrelated events. But that only works if the application
installed the handler before it creates the first shadow thread, right?
If the app decides to install/change the SIGWINCH handler later on
(without taking care of our handler), we will loose.

Suggestion: As this is fragile and cannot be solved transparently, lets
document this signal requirement of Xenomai, e.g. in all task/thread
creation functions of the skins. Tell the user that Xenomai will install
a custom SIGWINCH handler and that, if the app wants to override it, it
has to make sure to _first_ call into a Xenomai-provided handler and
check if that one wants to handle the event. I'm thinking of something
like int xeno_sigwinch_handler(int sig, siginfo_t *si, void *ctxt),
where the return code is non-zero in case the signal was processed.

With this policy in place, we can switch the signal handler installation
above to __attribute__ ((construtor)) and save us all the changes
regarding sigshadow_install below.

> Index: src/skins/posix/thread.c
> ===================================================================
> --- src/skins/posix/thread.c	(revision 4218)
> +++ src/skins/posix/thread.c	(working copy)
> @@ -26,23 +26,13 @@
>  #include <semaphore.h>
>  #include <posix/syscall.h>
>  #include <asm-generic/bits/current.h>
> +#include <asm-generic/bits/sigshadow.h>
>  
>  extern int __pse51_muxid;
>  
>  static pthread_attr_t default_attr;
>  static int linuxthreads;
>  
> -static void (*old_sigharden_handler)(int sig);
> -
> -static void __pthread_sigharden_handler(int sig)
> -{
> -	if (old_sigharden_handler &&
> -	    old_sigharden_handler != &__pthread_sigharden_handler)
> -		old_sigharden_handler(sig);
> -
> -	XENOMAI_SYSCALL1(__xn_sys_migrate, XENOMAI_XENO_DOMAIN);
> -}
> -
>  int __wrap_pthread_setschedparam(pthread_t thread,
>  				 int policy, const struct sched_param *param)
>  {
> @@ -59,7 +49,7 @@ int __wrap_pthread_setschedparam(pthread
>  		__real_pthread_setschedparam(thread, policy, param);
>  
>  	if (!err && promoted) {
> -		old_sigharden_handler = signal(SIGHARDEN, &__pthread_sigharden_handler);
> +		sigshadow_install();
>  		xeno_set_current();
>  		if (policy != SCHED_OTHER)
>  			XENOMAI_SYSCALL1(__xn_sys_migrate, XENOMAI_XENO_DOMAIN);
> @@ -118,7 +108,7 @@ static void *__pthread_trampoline(void *
>  	int parent_prio, policy;
>  	long err;
>  
> -	old_sigharden_handler = signal(SIGHARDEN, &__pthread_sigharden_handler);
> +	sigshadow_install();
>  
>  	param.sched_priority = iargs->prio;
>  	policy = iargs->policy;
> Index: src/skins/native/task.c
> ===================================================================
> --- src/skins/native/task.c	(revision 4218)
> +++ src/skins/native/task.c	(working copy)
> @@ -26,6 +26,7 @@
>  #include <limits.h>
>  #include <native/syscall.h>
>  #include <native/task.h>
> +#include <asm-generic/bits/sigshadow.h>
>  #include "wrappers.h"
>  
>  extern pthread_key_t __native_tskey;
> @@ -42,17 +43,6 @@ struct rt_task_iargs {
>  	xncompletion_t *completionp;
>  };
>  
> -static void (*old_sigharden_handler)(int sig);
> -
> -static void rt_task_sigharden(int sig)
> -{
> -	if (old_sigharden_handler &&
> -	    old_sigharden_handler != &rt_task_sigharden)
> -		old_sigharden_handler(sig);
> -
> -	XENOMAI_SYSCALL1(__xn_sys_migrate, XENOMAI_XENO_DOMAIN);
> -}
> -
>  static void *rt_task_trampoline(void *cookie)
>  {
>  	struct rt_task_iargs *iargs = (struct rt_task_iargs *)cookie;
> @@ -74,7 +64,7 @@ static void *rt_task_trampoline(void *co
>  	/* rt_task_delete requires asynchronous cancellation */
>  	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
>  
> -	old_sigharden_handler = signal(SIGHARDEN, &rt_task_sigharden);
> +	sigshadow_install();
>  
>  	bulk.a1 = (u_long)iargs->task;
>  	bulk.a2 = (u_long)iargs->name;
> @@ -175,8 +165,7 @@ int rt_task_shadow(RT_TASK *task, const 
>  
>  	/* rt_task_delete requires asynchronous cancellation */
>  	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
> -
> -	old_sigharden_handler = signal(SIGHARDEN, &rt_task_sigharden);
> +	sigshadow_install();
>  
>  	if (prio > 0) {
>  		/* Make sure the POSIX library caches the right priority. */
> Index: src/skins/vxworks/taskLib.c
> ===================================================================
> --- src/skins/vxworks/taskLib.c	(revision 4218)
> +++ src/skins/vxworks/taskLib.c	(working copy)
> @@ -27,6 +27,7 @@
>  #include <errno.h>
>  #include <limits.h>
>  #include <vxworks/vxworks.h>
> +#include <asm-generic/bits/sigshadow.h>
>  #include "wrappers.h"
>  
>  extern pthread_key_t __vxworks_tskey;
> @@ -54,17 +55,6 @@ struct wind_task_iargs {
>  	xncompletion_t *completionp;
>  };
>  
> -static void (*old_sigharden_handler)(int sig);
> -
> -static void wind_task_sigharden(int sig)
> -{
> -	if (old_sigharden_handler &&
> -	    old_sigharden_handler != &wind_task_sigharden)
> -		old_sigharden_handler(sig);
> -
> -	XENOMAI_SYSCALL1(__xn_sys_migrate, XENOMAI_XENO_DOMAIN);
> -}
> -
>  static int wind_task_set_posix_priority(int prio, struct sched_param *param)
>  {
>  	int maxpprio, pprio;
> @@ -103,8 +93,7 @@ static void *wind_task_trampoline(void *
>  
>  	/* wind_task_delete requires asynchronous cancellation */
>  	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
> -
> -	old_sigharden_handler = signal(SIGHARDEN, &wind_task_sigharden);
> +	sigshadow_install();
>  
>  	bulk.a1 = (u_long)iargs->name;
>  	bulk.a2 = (u_long)iargs->prio;
> Index: src/skins/psos+/task.c
> ===================================================================
> --- src/skins/psos+/task.c	(revision 4218)
> +++ src/skins/psos+/task.c	(working copy)
> @@ -26,6 +26,7 @@
>  #include <memory.h>
>  #include <string.h>
>  #include <psos+/psos.h>
> +#include <asm-generic/bits/sigshadow.h>
>  
>  extern int __psos_muxid;
>  
> @@ -38,17 +39,6 @@ struct psos_task_iargs {
>  	xncompletion_t *completionp;
>  };
>  
> -static void (*old_sigharden_handler)(int sig);
> -
> -static void psos_task_sigharden(int sig)
> -{
> -	if (old_sigharden_handler &&
> -	    old_sigharden_handler != &psos_task_sigharden)
> -		old_sigharden_handler(sig);
> -
> -	XENOMAI_SYSCALL1(__xn_sys_migrate, XENOMAI_XENO_DOMAIN);
> -}
> -
>  static int psos_task_set_posix_priority(int prio, struct sched_param *param)
>  {
>  	int maxpprio, pprio;
> @@ -79,8 +69,7 @@ static void *psos_task_trampoline(void *
>  	pthread_setschedparam(pthread_self(), policy, &param);
>  
>  	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
> -
> -	old_sigharden_handler = signal(SIGHARDEN, &psos_task_sigharden);
> +	sigshadow_install();
>  
>  	err = XENOMAI_SKINCALL5(__psos_muxid,
>  				__psos_t_create,
> @@ -174,8 +163,7 @@ u_long t_shadow(const char *name, /* Xen
>  		u_long *tid_r)
>  {
>  	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
> -
> -	old_sigharden_handler = signal(SIGHARDEN, &psos_task_sigharden);
> +	sigshadow_install();
>  
>  	return XENOMAI_SKINCALL5(__psos_muxid,
>  				 __psos_t_create,
> Index: src/skins/vrtx/task.c
> ===================================================================
> --- src/skins/vrtx/task.c	(revision 4218)
> +++ src/skins/vrtx/task.c	(working copy)
> @@ -27,6 +27,7 @@
>  #include <errno.h>
>  #include <limits.h>
>  #include <vrtx/vrtx.h>
> +#include <asm-generic/bits/sigshadow.h>
>  
>  extern pthread_key_t __vrtx_tskey;
>  
> @@ -44,17 +45,6 @@ struct vrtx_task_iargs {
>  	xncompletion_t *completionp;
>  };
>  
> -static void (*old_sigharden_handler)(int sig);
> -
> -static void vrtx_task_sigharden(int sig)
> -{
> -	if (old_sigharden_handler &&
> -	    old_sigharden_handler != &vrtx_task_sigharden)
> -		old_sigharden_handler(sig);
> -
> -	XENOMAI_SYSCALL1(__xn_sys_migrate, XENOMAI_XENO_DOMAIN);
> -}
> -
>  static int vrtx_task_set_posix_priority(int prio, struct sched_param *param)
>  {
>  	int maxpprio, pprio;
> @@ -93,8 +83,7 @@ static void *vrtx_task_trampoline(void *
>  
>  	/* vrtx_task_delete requires asynchronous cancellation */
>  	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
> -
> -	old_sigharden_handler = signal(SIGHARDEN, &vrtx_task_sigharden);
> +	sigshadow_install();
>  
>  	bulk.a1 = (u_long)iargs->tid;
>  	bulk.a2 = (u_long)iargs->prio;
> Index: src/skins/uitron/task.c
> ===================================================================
> --- src/skins/uitron/task.c	(revision 4218)
> +++ src/skins/uitron/task.c	(working copy)
> @@ -24,6 +24,7 @@
>  #include <errno.h>
>  #include <limits.h>
>  #include <asm/xenomai/system.h>
> +#include <asm-generic/bits/sigshadow.h>
>  #include <uitron/uitron.h>
>  
>  extern int __uitron_muxid;
> @@ -35,17 +36,6 @@ struct uitron_task_iargs {
>  	xncompletion_t *completionp;
>  };
>  
> -static void (*old_sigharden_handler)(int sig);
> -
> -static void uitron_task_sigharden(int sig)
> -{
> -	if (old_sigharden_handler &&
> -	    old_sigharden_handler != &uitron_task_sigharden)
> -		old_sigharden_handler(sig);
> -
> -	XENOMAI_SYSCALL1(__xn_sys_migrate, XENOMAI_XENO_DOMAIN);
> -}
> -
>  static int uitron_task_set_posix_priority(int prio, struct sched_param *param)
>  {
>  	int maxpprio, pprio;
> @@ -80,7 +70,7 @@ static void *uitron_task_trampoline(void
>  	pthread_setschedparam(pthread_self(), policy, &param);
>  
>  	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
> -	old_sigharden_handler = signal(SIGHARDEN, &uitron_task_sigharden);
> +	sigshadow_install();
>  
>  	err = XENOMAI_SKINCALL3(__uitron_muxid,
>  				__uitron_cre_tsk,
> @@ -157,8 +147,7 @@ ER shd_tsk(ID tskid, T_CTSK *pk_ctsk) /*
>  	pthread_setschedparam(pthread_self(), policy, &param);
>  
>  	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
> -
> -	old_sigharden_handler = signal(SIGHARDEN, &uitron_task_sigharden);
> +	sigshadow_install();
>  
>  	return XENOMAI_SKINCALL3(__uitron_muxid,
>  				 __uitron_cre_tsk,
> 

One question currently remains open for me, both regarding your approach
as well as my suggestion: What happens if some app links against more
than onw skin library? I think your approach will cause multiple
sigshadow_handler invocations (as the stack up due to library-local
pthread_once), while mine suffers from multiple xeno_sigwinch_handler
symbols. The latter might be solvable with __attribute__ ((weak)), correct?

Jan


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

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

* Re: [Xenomai-core] [PATCH 1/1] Use SIGWINCH to trigger priority change in user-space.
  2008-10-19 11:10 ` Jan Kiszka
@ 2008-10-19 11:19   ` Gilles Chanteperdrix
  2008-10-19 11:22     ` Jan Kiszka
  0 siblings, 1 reply; 17+ messages in thread
From: Gilles Chanteperdrix @ 2008-10-19 11:19 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: Xenomai core

Jan Kiszka wrote:
> I wonder if we shouldn't switch the signal hooking strategy: So far we
> install the handler at shadow thread creation time, saving a potentially
> installed handler of the application for redirection of
> Xenomai-unrelated events. But that only works if the application
> installed the handler before it creates the first shadow thread, right?
> If the app decides to install/change the SIGWINCH handler later on
> (without taking care of our handler), we will loose.
> 
> Suggestion: As this is fragile and cannot be solved transparently, lets
> document this signal requirement of Xenomai, e.g. in all task/thread
> creation functions of the skins. Tell the user that Xenomai will install
> a custom SIGWINCH handler and that, if the app wants to override it, it
> has to make sure to _first_ call into a Xenomai-provided handler and
> check if that one wants to handle the event. I'm thinking of something
> like int xeno_sigwinch_handler(int sig, siginfo_t *si, void *ctxt),
> where the return code is non-zero in case the signal was processed.
> 
> With this policy in place, we can switch the signal handler installation
> above to __attribute__ ((construtor)) and save us all the changes
> regarding sigshadow_install below.

You mean to push the burden of identifying the signal source and calling
the proper handler on the user. With the current approach we take care
of that burden, I think it is better. But I agree that this should be
documented somewhere.

> One question currently remains open for me, both regarding your approach
> as well as my suggestion: What happens if some app links against more
> than onw skin library? I think your approach will cause multiple
> sigshadow_handler invocations (as the stack up due to library-local
> pthread_once), while mine suffers from multiple xeno_sigwinch_handler
> symbols. The latter might be solvable with __attribute__ ((weak)), correct?

Yes, the pthread_once_t should be global with __attribute__((weak)), and
 we would get the proper behaviour for the proposed implementation too.

-- 
					    Gilles.


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

* Re: [Xenomai-core] [PATCH 1/1] Use SIGWINCH to trigger priority change in user-space.
  2008-10-19 11:19   ` Gilles Chanteperdrix
@ 2008-10-19 11:22     ` Jan Kiszka
  2008-10-19 11:28       ` Gilles Chanteperdrix
  0 siblings, 1 reply; 17+ messages in thread
From: Jan Kiszka @ 2008-10-19 11:22 UTC (permalink / raw)
  To: Gilles Chanteperdrix; +Cc: Xenomai core

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

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> I wonder if we shouldn't switch the signal hooking strategy: So far we
>> install the handler at shadow thread creation time, saving a potentially
>> installed handler of the application for redirection of
>> Xenomai-unrelated events. But that only works if the application
>> installed the handler before it creates the first shadow thread, right?
>> If the app decides to install/change the SIGWINCH handler later on
>> (without taking care of our handler), we will loose.
>>
>> Suggestion: As this is fragile and cannot be solved transparently, lets
>> document this signal requirement of Xenomai, e.g. in all task/thread
>> creation functions of the skins. Tell the user that Xenomai will install
>> a custom SIGWINCH handler and that, if the app wants to override it, it
>> has to make sure to _first_ call into a Xenomai-provided handler and
>> check if that one wants to handle the event. I'm thinking of something
>> like int xeno_sigwinch_handler(int sig, siginfo_t *si, void *ctxt),
>> where the return code is non-zero in case the signal was processed.
>>
>> With this policy in place, we can switch the signal handler installation
>> above to __attribute__ ((construtor)) and save us all the changes
>> regarding sigshadow_install below.
> 
> You mean to push the burden of identifying the signal source and calling
> the proper handler on the user. With the current approach we take care
> of that burden, I think it is better. But I agree that this should be
> documented somewhere.

Nope, the burden will still be Xenomai's, encoded in
xeno_sigwinch_handler. The app will only have to check for its return code.

> 
>> One question currently remains open for me, both regarding your approach
>> as well as my suggestion: What happens if some app links against more
>> than onw skin library? I think your approach will cause multiple
>> sigshadow_handler invocations (as the stack up due to library-local
>> pthread_once), while mine suffers from multiple xeno_sigwinch_handler
>> symbols. The latter might be solvable with __attribute__ ((weak)), correct?
> 
> Yes, the pthread_once_t should be global with __attribute__((weak)), and
>  we would get the proper behaviour for the proposed implementation too.
> 

OK.

Nevertheless, the installation on thread creation remains fragile, IMHO.

Jan


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

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

* Re: [Xenomai-core] [PATCH 1/1] Use SIGWINCH to trigger priority change in user-space.
  2008-10-19 11:22     ` Jan Kiszka
@ 2008-10-19 11:28       ` Gilles Chanteperdrix
  2008-10-19 16:09         ` Philippe Gerum
  0 siblings, 1 reply; 17+ messages in thread
From: Gilles Chanteperdrix @ 2008-10-19 11:28 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: Xenomai core

Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> Jan Kiszka wrote:
>>> I wonder if we shouldn't switch the signal hooking strategy: So far we
>>> install the handler at shadow thread creation time, saving a potentially
>>> installed handler of the application for redirection of
>>> Xenomai-unrelated events. But that only works if the application
>>> installed the handler before it creates the first shadow thread, right?
>>> If the app decides to install/change the SIGWINCH handler later on
>>> (without taking care of our handler), we will loose.
>>>
>>> Suggestion: As this is fragile and cannot be solved transparently, lets
>>> document this signal requirement of Xenomai, e.g. in all task/thread
>>> creation functions of the skins. Tell the user that Xenomai will install
>>> a custom SIGWINCH handler and that, if the app wants to override it, it
>>> has to make sure to _first_ call into a Xenomai-provided handler and
>>> check if that one wants to handle the event. I'm thinking of something
>>> like int xeno_sigwinch_handler(int sig, siginfo_t *si, void *ctxt),
>>> where the return code is non-zero in case the signal was processed.
>>>
>>> With this policy in place, we can switch the signal handler installation
>>> above to __attribute__ ((construtor)) and save us all the changes
>>> regarding sigshadow_install below.
>> You mean to push the burden of identifying the signal source and calling
>> the proper handler on the user. With the current approach we take care
>> of that burden, I think it is better. But I agree that this should be
>> documented somewhere.
> 
> Nope, the burden will still be Xenomai's, encoded in
> xeno_sigwinch_handler. The app will only have to check for its return code.

Actually, I have no real preference. Philippe, since you designed the
original code, could you tell us what you had in mind?

-- 
					    Gilles.


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

* Re: [Xenomai-core] [PATCH 1/1] Use SIGWINCH to trigger priority change in user-space.
  2008-10-18 14:35 ` Gilles Chanteperdrix
  2008-10-18 17:09   ` Philippe Gerum
@ 2008-10-19 15:21   ` Gilles Chanteperdrix
  1 sibling, 0 replies; 17+ messages in thread
From: Gilles Chanteperdrix @ 2008-10-19 15:21 UTC (permalink / raw)
  To: Xenomai core

Gilles Chanteperdrix wrote:
> Gilles Chanteperdrix wrote:
>> Hi,
>>
>> here is a patch which implements the idea discussed previously of 
>> re-using SIGWINCH to trigger priority changes in user-space. There is 
>> only one patch, but the files should have been put in a logical order
>> to make review easier.
> 
> Since there are chances of conflicts with Jan patches series, I will
> rebase this patch after Jan patches have been commited.

Here comes a newer version.

Index: include/nucleus/thread.h
===================================================================
--- include/nucleus/thread.h	(revision 4251)
+++ include/nucleus/thread.h	(working copy)
@@ -116,6 +116,7 @@
 #define XNROBBED  0x00000020 /**< Robbed from resource ownership */
 #define XNATOMIC  0x00000040 /**< In atomic switch from secondary to primary mode */
 #define XNAFFSET  0x00000080 /**< CPU affinity changed from primary mode */
+#define XNPRIOSET 0x00000100 /**< Priority changed from primary mode */
 
 /* These information flags are available to the real-time interfaces */
 #define XNTHREAD_INFO_SPARE0  0x10000000
Index: include/asm-generic/syscall.h
===================================================================
--- include/asm-generic/syscall.h	(revision 4251)
+++ include/asm-generic/syscall.h	(working copy)
@@ -59,7 +59,12 @@ typedef struct xnsysinfo {
     unsigned long tickval;	/* Tick duration (ns) */
 } xnsysinfo_t;
 
-#define SIGHARDEN  SIGWINCH
+#define SIGSHADOW  SIGWINCH
+#define SIGSHADOW_ACTION_HARDEN 1
+#define SIGSHADOW_ACTION_RENICE 2
+#define sigshadow_action(code) ((code) & 0xff)
+#define sigshadow_prio(code) (((code) >> 8) & 0xff)
+#define sigshadow_int(action, prio) ((action) | ((prio) << 8))
 
 #ifdef __KERNEL__
 
Index: include/nucleus/shadow.h
===================================================================
--- include/nucleus/shadow.h	(revision 4251)
+++ include/nucleus/shadow.h	(working copy)
@@ -99,6 +99,7 @@ void xnshadow_reset_shield(void);
 
 void xnshadow_send_sig(struct xnthread *thread,
 		       int sig,
+		       int arg,
 		       int specific);
 
 void xnshadow_rpi_check(void);
Index: ksrc/nucleus/shadow.c
===================================================================
--- ksrc/nucleus/shadow.c	(revision 4251)
+++ ksrc/nucleus/shadow.c	(working copy)
@@ -115,6 +115,14 @@ do { \
    switch_lock_owner[task_cpu(t)] = t; \
 } while(0)
 
+#define xnshadow_sig_mux(sig, arg) ((sig) | (arg << 8))
+#define xnshadow_sig_demux(muxed, sig, arg) \
+	do {				     \
+		int _muxed = (muxed);	     \
+		(sig) = _muxed & 0xff;	     \
+		(arg) = _muxed << 8;	     \
+	} while (0)
+
 static struct task_struct *switch_lock_owner[XNARCH_NR_CPUS];
 
 static int nucleus_muxid = -1;
@@ -879,7 +887,7 @@ static void xnshadow_dereference_skin(un
 
 static void lostage_handler(void *cookie)
 {
-	int cpu = smp_processor_id(), reqnum, sig;
+	int cpu = smp_processor_id(), reqnum, sig, arg;
 	struct __lostagerq *rq = &lostagerq[cpu];
 
 	while ((reqnum = rq->out) != rq->in) {
@@ -927,8 +935,16 @@ static void lostage_handler(void *cookie
 
 		case LO_SIGTHR_REQ:
 
-			sig = rq->req[reqnum].arg;
-			send_sig(sig, p, 1);
+			xnshadow_sig_demux(rq->req[reqnum].arg, sig, arg);
+			if (sig == SIGSHADOW) {
+				siginfo_t si;
+				memset(&si, '\0', sizeof(si));
+				si.si_signo = sig;
+				si.si_code = SI_QUEUE;
+				si.si_int = arg;
+				send_sig_info(sig, &si, p);
+			} else
+				send_sig(sig, p, 1);
 			break;
 
 		case LO_SIGGRP_REQ:
@@ -1237,6 +1253,13 @@ void xnshadow_relax(int notify)
 		/* Help debugging spurious relaxes. */
 		send_sig(SIGXCPU, current, 1);
 
+	if (xnthread_test_info(thread, XNPRIOSET)) {
+		xnthread_clear_info(thread, XNPRIOSET);
+		xnshadow_send_sig(thread, SIGSHADOW,
+				  sigshadow_int(SIGSHADOW_ACTION_RENICE, prio),
+				  1);
+	}
+
 #ifdef CONFIG_SMP
 	/* If the shadow thread changed its CPU affinity while in
 	   primary mode, reset the CPU affinity of its Linux
@@ -1500,16 +1523,16 @@ void xnshadow_start(xnthread_t *thread)
 		schedule_linux_call(LO_START_REQ, p, 0);
 }
 
+/* Called with nklock locked, Xenomai interrupts off. */
 void xnshadow_renice(xnthread_t *thread)
 {
-	/* Called with nklock locked, Xenomai interrupts off. */
-	struct task_struct *p = xnthread_archtcb(thread)->user_task;
-
 	/* We need to bound the priority values in the [1..MAX_RT_PRIO-1]
 	   range, since the core pod's priority scale is a superset of
 	   Linux's priority scale. */
 	int prio = normalize_priority(xnthread_current_priority(thread));
-	schedule_linux_call(LO_RENICE_REQ, p, prio);
+
+	xnshadow_send_sig(thread, SIGSHADOW,
+			  sigshadow_int(SIGSHADOW_ACTION_RENICE, prio), 1);
 
 	if (!xnthread_test_state(thread, XNDORMANT) &&
 	    xnthread_sched(thread) == xnpod_current_sched())
@@ -1519,8 +1542,7 @@ void xnshadow_renice(xnthread_t *thread)
 void xnshadow_suspend(xnthread_t *thread)
 {
 	/* Called with nklock locked, Xenomai interrupts off. */
-	struct task_struct *p = xnthread_archtcb(thread)->user_task;
-	schedule_linux_call(LO_SIGTHR_REQ, p, SIGHARDEN);
+	xnshadow_send_sig(thread, SIGSHADOW, SIGSHADOW_ACTION_HARDEN, 1);
 }
 
 static int xnshadow_sys_migrate(struct pt_regs *regs)
@@ -1531,7 +1553,7 @@ static int xnshadow_sys_migrate(struct p
 				return -EPERM;
 
 			/* Paranoid: a corner case where the
-			   user-space side fiddles with SIGHARDEN
+			   user-space side fiddles with SIGSHADOW
 			   while the target thread is still waiting to
 			   be started. */
 			if (xnthread_test_state(xnshadow_thread(current), XNDORMANT))
@@ -2007,10 +2029,11 @@ static inline int substitute_linux_sysca
 	return 0;
 }
 
-void xnshadow_send_sig(xnthread_t *thread, int sig, int specific)
+void xnshadow_send_sig(xnthread_t *thread, int sig, int arg, int specific)
 {
 	schedule_linux_call(specific ? LO_SIGTHR_REQ : LO_SIGGRP_REQ,
-			    xnthread_user_task(thread), sig);
+			    xnthread_user_task(thread),
+			    xnshadow_sig_mux(sig, specific ? arg : 0));
 }
 
 static inline int do_hisyscall_event(unsigned event, unsigned domid, void *data)
Index: ksrc/nucleus/synch.c
===================================================================
--- ksrc/nucleus/synch.c	(revision 4251)
+++ ksrc/nucleus/synch.c	(working copy)
@@ -343,6 +343,8 @@ static void xnsynch_renice_thread(xnthre
 #ifdef CONFIG_XENO_OPT_PERVASIVE
 	if (xnthread_test_state(thread, XNRELAX))
 		xnshadow_renice(thread);
+	else if (xnthread_test_state(thread, XNSHADOW))
+		xnthread_set_info(thread, XNPRIOSET);
 #endif /* CONFIG_XENO_OPT_PERVASIVE */
 }
 
Index: ksrc/nucleus/pod.c
===================================================================
--- ksrc/nucleus/pod.c	(revision 4251)
+++ ksrc/nucleus/pod.c	(working copy)
@@ -1162,7 +1162,7 @@ void xnpod_delete_thread(xnthread_t *thr
 	    !xnthread_test_state(thread, XNDORMANT) &&
 	    !xnpod_current_p(thread)) {
 		if (!xnpod_userspace_p())
-			xnshadow_send_sig(thread, SIGKILL, 1);
+			xnshadow_send_sig(thread, SIGKILL, 0, 1);
 		/*
 		 * Otherwise, assume the interface library has issued
 		 * pthread_cancel on the target thread, which should
@@ -1456,7 +1456,7 @@ void xnpod_suspend_thread(xnthread_t *th
 	   is actually running some code under the control of the
 	   Linux scheduler (i.e. it's relaxed).  To make this
 	   possible, we force the target Linux task to migrate back to
-	   the Xenomai domain by sending it a SIGHARDEN signal the
+	   the Xenomai domain by sending it a SIGSHADOW signal the
 	   skin interface libraries trap for this specific internal
 	   purpose, whose handler is expected to call back the
 	   nucleus's migration service. By forcing this migration, we
@@ -1821,8 +1821,12 @@ void xnpod_renice_thread_inner(xnthread_
 			xnpod_resume_thread(thread, 0);
 	}
 #ifdef CONFIG_XENO_OPT_PERVASIVE
-	if (propagate && xnthread_test_state(thread, XNRELAX))
-		xnshadow_renice(thread);
+	if (propagate) {
+		if (xnthread_test_state(thread, XNRELAX))
+			xnshadow_renice(thread);
+		else if (xnthread_test_state(thread, XNSHADOW))
+			xnthread_set_info(thread, XNPRIOSET);
+	}
 #endif /* CONFIG_XENO_OPT_PERVASIVE */
 
 	xnlock_put_irqrestore(&nklock, s);
Index: include/asm-generic/bits/sigshadow.h
===================================================================
--- include/asm-generic/bits/sigshadow.h	(revision 0)
+++ include/asm-generic/bits/sigshadow.h	(revision 0)
@@ -0,0 +1,73 @@
+#ifndef _XENO_ASM_GENERIC_BITS_SIGSHADOW_H
+#define _XENO_ASM_GENERIC_BITS_SIGSHADOW_H
+
+#include <pthread.h>
+#include <signal.h>
+
+pthread_once_t __attribute__((weak))
+	xeno_sigshadow_installed = PTHREAD_ONCE_INIT;
+static struct sigaction old_sigshadow_action;
+
+static void sigshadow_handler(int sig, siginfo_t *si, void *ctxt)
+{
+	int action;
+
+	/* Not a signal sent by Xenomai nucleus */
+	if (si->si_code != SI_QUEUE) {
+		const struct sigaction *const sa = &old_sigshadow_action;
+		sigset_t old_sigset;
+
+	  not_nucleus:
+		if ((!(sa->sa_flags & SA_SIGINFO) && !sa->sa_handler)
+		    || ((sa->sa_flags & SA_SIGINFO) && !sa->sa_sigaction))
+			return;
+
+		pthread_sigmask(SIG_SETMASK, &sa->sa_mask, &old_sigset);
+		if (!(sa->sa_flags & SA_SIGINFO))
+			sa->sa_handler(sig);
+		else
+			sa->sa_sigaction(sig, si, ctxt);
+		pthread_sigmask(SIG_SETMASK, &old_sigset, NULL);
+		return;
+	}
+
+	action = sigshadow_action(si->si_int);
+
+	switch(action) {
+	case SIGSHADOW_ACTION_HARDEN:
+		XENOMAI_SYSCALL1(__xn_sys_migrate, XENOMAI_XENO_DOMAIN);
+		break;
+
+	case SIGSHADOW_ACTION_RENICE: {
+		struct sched_param param;
+		int policy;
+
+		param.sched_priority = sigshadow_prio(si->si_int);
+		policy = param.sched_priority > 0 ? SCHED_FIFO: SCHED_OTHER;
+		pthread_setschedparam(pthread_self(), policy, &param);
+		break;
+	}
+
+	default:
+		goto not_nucleus;
+	}
+}
+
+static void sigshadow_install_once(void)
+{
+	struct sigaction new_sigshadow_action;
+
+	new_sigshadow_action.sa_flags = SA_SIGINFO | SA_RESTART;
+	new_sigshadow_action.sa_sigaction = sigshadow_handler;
+	sigemptyset(&new_sigshadow_action.sa_mask);
+
+	sigaction(SIGSHADOW, &new_sigshadow_action, &old_sigshadow_action);
+	if (!(old_sigshadow_action.sa_flags & SA_NODEFER))
+		sigaddset(&old_sigshadow_action.sa_mask, SIGSHADOW);
+}
+
+static inline void sigshadow_install(void)
+{
+	pthread_once(&xeno_sigshadow_installed, sigshadow_install_once);
+}
+#endif /* _XENO_ASM_GENERIC_BITS_SIGSHADOW_H */
Index: src/skins/posix/thread.c
===================================================================
--- src/skins/posix/thread.c	(revision 4251)
+++ src/skins/posix/thread.c	(working copy)
@@ -26,23 +26,13 @@
 #include <semaphore.h>
 #include <posix/syscall.h>
 #include <asm-generic/bits/current.h>
+#include <asm-generic/bits/sigshadow.h>
 
 extern int __pse51_muxid;
 
 static pthread_attr_t default_attr;
 static int linuxthreads;
 
-static void (*old_sigharden_handler)(int sig);
-
-static void __pthread_sigharden_handler(int sig)
-{
-	if (old_sigharden_handler &&
-	    old_sigharden_handler != &__pthread_sigharden_handler)
-		old_sigharden_handler(sig);
-
-	XENOMAI_SYSCALL1(__xn_sys_migrate, XENOMAI_XENO_DOMAIN);
-}
-
 int __wrap_pthread_setschedparam(pthread_t thread,
 				 int policy, const struct sched_param *param)
 {
@@ -67,11 +57,9 @@ int __wrap_pthread_setschedparam(pthread
 
 	if (err == EPERM)
 		return __real_pthread_setschedparam(thread, policy, param);
-	else
-		__real_pthread_setschedparam(thread, policy, param);
 
 	if (!err && promoted) {
-		old_sigharden_handler = signal(SIGHARDEN, &__pthread_sigharden_handler);
+		sigshadow_install();
 #ifndef HAVE___THREAD
 		pthread_setspecific(xeno_current_mode_key, mode_buf);
 #endif /* !HAVE___THREAD */
@@ -138,7 +126,7 @@ static void *__pthread_trampoline(void *
 	unsigned long *mode_buf;
 	long err;
 
-	old_sigharden_handler = signal(SIGHARDEN, &__pthread_sigharden_handler);
+	sigshadow_install();
 
 	param.sched_priority = iargs->prio;
 	policy = iargs->policy;
Index: src/skins/native/task.c
===================================================================
--- src/skins/native/task.c	(revision 4251)
+++ src/skins/native/task.c	(working copy)
@@ -26,6 +26,7 @@
 #include <limits.h>
 #include <native/syscall.h>
 #include <native/task.h>
+#include <asm-generic/bits/sigshadow.h>
 #include <asm-generic/bits/current.h>
 #include "wrappers.h"
 
@@ -50,17 +51,6 @@ struct rt_task_iargs {
 	xncompletion_t *completionp;
 };
 
-static void (*old_sigharden_handler)(int sig);
-
-static void rt_task_sigharden(int sig)
-{
-	if (old_sigharden_handler &&
-	    old_sigharden_handler != &rt_task_sigharden)
-		old_sigharden_handler(sig);
-
-	XENOMAI_SYSCALL1(__xn_sys_migrate, XENOMAI_XENO_DOMAIN);
-}
-
 static void *rt_task_trampoline(void *cookie)
 {
 	struct rt_task_iargs *iargs = (struct rt_task_iargs *)cookie;
@@ -82,7 +72,7 @@ static void *rt_task_trampoline(void *co
 	/* rt_task_delete requires asynchronous cancellation */
 	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
 
-	old_sigharden_handler = signal(SIGHARDEN, &rt_task_sigharden);
+	sigshadow_install();
 
 	bulk.a1 = (u_long)iargs->task;
 	bulk.a2 = (u_long)iargs->name;
@@ -196,8 +186,7 @@ int rt_task_shadow(RT_TASK *task, const 
 
 	/* rt_task_delete requires asynchronous cancellation */
 	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
-
-	old_sigharden_handler = signal(SIGHARDEN, &rt_task_sigharden);
+	sigshadow_install();
 
 	if (prio > 0) {
 		/* Make sure the POSIX library caches the right priority. */
Index: src/skins/vxworks/taskLib.c
===================================================================
--- src/skins/vxworks/taskLib.c	(revision 4251)
+++ src/skins/vxworks/taskLib.c	(working copy)
@@ -27,6 +27,7 @@
 #include <errno.h>
 #include <limits.h>
 #include <vxworks/vxworks.h>
+#include <asm-generic/bits/sigshadow.h>
 #include <asm-generic/bits/current.h>
 #include "wrappers.h"
 
@@ -62,17 +63,6 @@ struct wind_task_iargs {
 	xncompletion_t *completionp;
 };
 
-static void (*old_sigharden_handler)(int sig);
-
-static void wind_task_sigharden(int sig)
-{
-	if (old_sigharden_handler &&
-	    old_sigharden_handler != &wind_task_sigharden)
-		old_sigharden_handler(sig);
-
-	XENOMAI_SYSCALL1(__xn_sys_migrate, XENOMAI_XENO_DOMAIN);
-}
-
 static int wind_task_set_posix_priority(int prio, struct sched_param *param)
 {
 	int maxpprio, pprio;
@@ -111,8 +101,7 @@ static void *wind_task_trampoline(void *
 
 	/* wind_task_delete requires asynchronous cancellation */
 	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
-
-	old_sigharden_handler = signal(SIGHARDEN, &wind_task_sigharden);
+	sigshadow_install();
 
 	bulk.a1 = (u_long)iargs->name;
 	bulk.a2 = (u_long)iargs->prio;
Index: src/skins/psos+/task.c
===================================================================
--- src/skins/psos+/task.c	(revision 4251)
+++ src/skins/psos+/task.c	(working copy)
@@ -26,6 +26,7 @@
 #include <memory.h>
 #include <string.h>
 #include <psos+/psos.h>
+#include <asm-generic/bits/sigshadow.h>
 #include <asm-generic/bits/current.h>
 
 extern int __psos_muxid;
@@ -39,17 +40,6 @@ struct psos_task_iargs {
 	xncompletion_t *completionp;
 };
 
-static void (*old_sigharden_handler)(int sig);
-
-static void psos_task_sigharden(int sig)
-{
-	if (old_sigharden_handler &&
-	    old_sigharden_handler != &psos_task_sigharden)
-		old_sigharden_handler(sig);
-
-	XENOMAI_SYSCALL1(__xn_sys_migrate, XENOMAI_XENO_DOMAIN);
-}
-
 static int psos_task_set_posix_priority(int prio, struct sched_param *param)
 {
 	int maxpprio, pprio;
@@ -81,8 +71,7 @@ static void *psos_task_trampoline(void *
 	pthread_setschedparam(pthread_self(), policy, &param);
 
 	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
-
-	old_sigharden_handler = signal(SIGHARDEN, &psos_task_sigharden);
+	sigshadow_install();
 
 	bulk.a1 = (u_long)iargs->name;
 	bulk.a2 = (u_long)iargs->prio;
@@ -189,8 +178,7 @@ u_long t_shadow(const char *name, /* Xen
 	int err;
 
 	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
-
-	old_sigharden_handler = signal(SIGHARDEN, &psos_task_sigharden);
+	sigshadow_install();
 
 	err = XENOMAI_SKINCALL5(__psos_muxid,
 				__psos_t_create,
Index: src/skins/vrtx/task.c
===================================================================
--- src/skins/vrtx/task.c	(revision 4251)
+++ src/skins/vrtx/task.c	(working copy)
@@ -27,6 +27,7 @@
 #include <errno.h>
 #include <limits.h>
 #include <vrtx/vrtx.h>
+#include <asm-generic/bits/sigshadow.h>
 #include <asm-generic/bits/current.h>
 
 #ifdef HAVE___THREAD
@@ -49,17 +50,6 @@ struct vrtx_task_iargs {
 	xncompletion_t *completionp;
 };
 
-static void (*old_sigharden_handler)(int sig);
-
-static void vrtx_task_sigharden(int sig)
-{
-	if (old_sigharden_handler &&
-	    old_sigharden_handler != &vrtx_task_sigharden)
-		old_sigharden_handler(sig);
-
-	XENOMAI_SYSCALL1(__xn_sys_migrate, XENOMAI_XENO_DOMAIN);
-}
-
 static int vrtx_task_set_posix_priority(int prio, struct sched_param *param)
 {
 	int maxpprio, pprio;
@@ -113,7 +103,7 @@ static void *vrtx_task_trampoline(void *
 	pthread_setspecific(__vrtx_tskey, tcb);
 #endif /* !HAVE___THREAD */
 
-	old_sigharden_handler = signal(SIGHARDEN, &vrtx_task_sigharden);
+	sigshadow_install();
 
 	bulk.a1 = (u_long)iargs->tid;
 	bulk.a2 = (u_long)iargs->prio;
Index: src/skins/uitron/task.c
===================================================================
--- src/skins/uitron/task.c	(revision 4251)
+++ src/skins/uitron/task.c	(working copy)
@@ -24,6 +24,7 @@
 #include <errno.h>
 #include <limits.h>
 #include <asm/xenomai/system.h>
+#include <asm-generic/bits/sigshadow.h>
 #include <uitron/uitron.h>
 #include <asm-generic/bits/current.h>
 
@@ -36,17 +37,6 @@ struct uitron_task_iargs {
 	xncompletion_t *completionp;
 };
 
-static void (*old_sigharden_handler)(int sig);
-
-static void uitron_task_sigharden(int sig)
-{
-	if (old_sigharden_handler &&
-	    old_sigharden_handler != &uitron_task_sigharden)
-		old_sigharden_handler(sig);
-
-	XENOMAI_SYSCALL1(__xn_sys_migrate, XENOMAI_XENO_DOMAIN);
-}
-
 static int uitron_task_set_posix_priority(int prio, struct sched_param *param)
 {
 	int maxpprio, pprio;
@@ -82,7 +72,7 @@ static void *uitron_task_trampoline(void
 	pthread_setschedparam(pthread_self(), policy, &param);
 
 	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
-	old_sigharden_handler = signal(SIGHARDEN, &uitron_task_sigharden);
+	sigshadow_install();
 
 	mode = xeno_init_current_mode();
 	if (!mode) {
@@ -167,8 +157,7 @@ ER shd_tsk(ID tskid, T_CTSK *pk_ctsk) /*
 	pthread_setschedparam(pthread_self(), policy, &param);
 
 	pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
-
-	old_sigharden_handler = signal(SIGHARDEN, &uitron_task_sigharden);
+	sigshadow_install();
 
 	err = XENOMAI_SKINCALL3(__uitron_muxid,
 				__uitron_cre_tsk,


-- 
					    Gilles.


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

* Re: [Xenomai-core] [PATCH 1/1] Use SIGWINCH to trigger priority change in user-space.
  2008-10-19 11:28       ` Gilles Chanteperdrix
@ 2008-10-19 16:09         ` Philippe Gerum
  2008-10-19 16:14           ` Philippe Gerum
  2008-10-19 16:23           ` Gilles Chanteperdrix
  0 siblings, 2 replies; 17+ messages in thread
From: Philippe Gerum @ 2008-10-19 16:09 UTC (permalink / raw)
  To: Gilles Chanteperdrix; +Cc: Jan Kiszka, Xenomai core

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> Gilles Chanteperdrix wrote:
>>> Jan Kiszka wrote:
>>>> I wonder if we shouldn't switch the signal hooking strategy: So far we
>>>> install the handler at shadow thread creation time, saving a potentially
>>>> installed handler of the application for redirection of
>>>> Xenomai-unrelated events. But that only works if the application
>>>> installed the handler before it creates the first shadow thread, right?
>>>> If the app decides to install/change the SIGWINCH handler later on
>>>> (without taking care of our handler), we will loose.
>>>>
>>>> Suggestion: As this is fragile and cannot be solved transparently, lets
>>>> document this signal requirement of Xenomai, e.g. in all task/thread
>>>> creation functions of the skins. Tell the user that Xenomai will install
>>>> a custom SIGWINCH handler and that, if the app wants to override it, it
>>>> has to make sure to _first_ call into a Xenomai-provided handler and
>>>> check if that one wants to handle the event. I'm thinking of something
>>>> like int xeno_sigwinch_handler(int sig, siginfo_t *si, void *ctxt),
>>>> where the return code is non-zero in case the signal was processed.
>>>>
>>>> With this policy in place, we can switch the signal handler installation
>>>> above to __attribute__ ((construtor)) and save us all the changes
>>>> regarding sigshadow_install below.
>>> You mean to push the burden of identifying the signal source and calling
>>> the proper handler on the user. With the current approach we take care
>>> of that burden, I think it is better. But I agree that this should be
>>> documented somewhere.
>> Nope, the burden will still be Xenomai's, encoded in
>> xeno_sigwinch_handler. The app will only have to check for its return code.
> 
> Actually, I have no real preference. Philippe, since you designed the
> original code, could you tell us what you had in mind?
> 

My reasoning at that time was that the application folks would not want or even
be able to change the signal handling code to insert anything Xenomai might
need, hence the override-then-route-unhandled approach. A typical configuration
I thought of was GUI-based apps, that would include RT code as well. In that
case, I don't think that most of us would want to be dragged into rebuilding
kde/gtk libs, for the sole purpose of routing unhandled SIGSHADOW signals.

At the same time, I agree with Jan that we should not prevent people who prefer
hacking their own software components to install that routing in their handler,
instead of fiddling with the order in which they do the application init chores.

Well, what about merging the solutions: trap the signal from the library
constructor by default for people relying on #1, AND document the shadow signal
handler for people who can do #2?

-- 
Philippe.


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

* Re: [Xenomai-core] [PATCH 1/1] Use SIGWINCH to trigger priority change in user-space.
  2008-10-19 16:09         ` Philippe Gerum
@ 2008-10-19 16:14           ` Philippe Gerum
  2008-10-19 16:16             ` Gilles Chanteperdrix
  2008-10-19 16:23           ` Gilles Chanteperdrix
  1 sibling, 1 reply; 17+ messages in thread
From: Philippe Gerum @ 2008-10-19 16:14 UTC (permalink / raw)
  To: Gilles Chanteperdrix; +Cc: Jan Kiszka, Xenomai core

Philippe Gerum wrote:
> Gilles Chanteperdrix wrote:
>> Jan Kiszka wrote:
>>> Gilles Chanteperdrix wrote:
>>>> Jan Kiszka wrote:
>>>>> I wonder if we shouldn't switch the signal hooking strategy: So far we
>>>>> install the handler at shadow thread creation time, saving a potentially
>>>>> installed handler of the application for redirection of
>>>>> Xenomai-unrelated events. But that only works if the application
>>>>> installed the handler before it creates the first shadow thread, right?
>>>>> If the app decides to install/change the SIGWINCH handler later on
>>>>> (without taking care of our handler), we will loose.
>>>>>
>>>>> Suggestion: As this is fragile and cannot be solved transparently, lets
>>>>> document this signal requirement of Xenomai, e.g. in all task/thread
>>>>> creation functions of the skins. Tell the user that Xenomai will install
>>>>> a custom SIGWINCH handler and that, if the app wants to override it, it
>>>>> has to make sure to _first_ call into a Xenomai-provided handler and
>>>>> check if that one wants to handle the event. I'm thinking of something
>>>>> like int xeno_sigwinch_handler(int sig, siginfo_t *si, void *ctxt),
>>>>> where the return code is non-zero in case the signal was processed.
>>>>>
>>>>> With this policy in place, we can switch the signal handler installation
>>>>> above to __attribute__ ((construtor)) and save us all the changes
>>>>> regarding sigshadow_install below.
>>>> You mean to push the burden of identifying the signal source and calling
>>>> the proper handler on the user. With the current approach we take care
>>>> of that burden, I think it is better. But I agree that this should be
>>>> documented somewhere.
>>> Nope, the burden will still be Xenomai's, encoded in
>>> xeno_sigwinch_handler. The app will only have to check for its return code.
>> Actually, I have no real preference. Philippe, since you designed the
>> original code, could you tell us what you had in mind?
>>
> 
> My reasoning at that time was that the application folks would not want or even
> be able to change the signal handling code to insert anything Xenomai might
> need, hence the override-then-route-unhandled approach. A typical configuration
> I thought of was GUI-based apps, that would include RT code as well. In that
> case, I don't think that most of us would want to be dragged into rebuilding
> kde/gtk libs, for the sole purpose of routing unhandled SIGSHADOW signals.
> 
> At the same time, I agree with Jan that we should not prevent people who prefer
> hacking their own software components to install that routing in their handler,
> instead of fiddling with the order in which they do the application init chores.
> 
> Well, what about merging the solutions: trap the signal from the library
> constructor by default for people relying on #1,

Mmm, that would not work for components running at late init stage though.

 AND document the shadow signal
> handler for people who can do #2?
> 
... doing so would require to provide some Xenomai-specific marker in the
siginfo struct the application-defined handler could test, but that should be
doable.

-- 
Philippe.


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

* Re: [Xenomai-core] [PATCH 1/1] Use SIGWINCH to trigger priority change in user-space.
  2008-10-19 16:14           ` Philippe Gerum
@ 2008-10-19 16:16             ` Gilles Chanteperdrix
  0 siblings, 0 replies; 17+ messages in thread
From: Gilles Chanteperdrix @ 2008-10-19 16:16 UTC (permalink / raw)
  To: rpm; +Cc: Jan Kiszka, Xenomai core

Philippe Gerum wrote:
> Philippe Gerum wrote:
>> Gilles Chanteperdrix wrote:
>>> Jan Kiszka wrote:
>>>> Gilles Chanteperdrix wrote:
>>>>> Jan Kiszka wrote:
>>>>>> I wonder if we shouldn't switch the signal hooking strategy: So far we
>>>>>> install the handler at shadow thread creation time, saving a potentially
>>>>>> installed handler of the application for redirection of
>>>>>> Xenomai-unrelated events. But that only works if the application
>>>>>> installed the handler before it creates the first shadow thread, right?
>>>>>> If the app decides to install/change the SIGWINCH handler later on
>>>>>> (without taking care of our handler), we will loose.
>>>>>>
>>>>>> Suggestion: As this is fragile and cannot be solved transparently, lets
>>>>>> document this signal requirement of Xenomai, e.g. in all task/thread
>>>>>> creation functions of the skins. Tell the user that Xenomai will install
>>>>>> a custom SIGWINCH handler and that, if the app wants to override it, it
>>>>>> has to make sure to _first_ call into a Xenomai-provided handler and
>>>>>> check if that one wants to handle the event. I'm thinking of something
>>>>>> like int xeno_sigwinch_handler(int sig, siginfo_t *si, void *ctxt),
>>>>>> where the return code is non-zero in case the signal was processed.
>>>>>>
>>>>>> With this policy in place, we can switch the signal handler installation
>>>>>> above to __attribute__ ((construtor)) and save us all the changes
>>>>>> regarding sigshadow_install below.
>>>>> You mean to push the burden of identifying the signal source and calling
>>>>> the proper handler on the user. With the current approach we take care
>>>>> of that burden, I think it is better. But I agree that this should be
>>>>> documented somewhere.
>>>> Nope, the burden will still be Xenomai's, encoded in
>>>> xeno_sigwinch_handler. The app will only have to check for its return code.
>>> Actually, I have no real preference. Philippe, since you designed the
>>> original code, could you tell us what you had in mind?
>>>
>> My reasoning at that time was that the application folks would not want or even
>> be able to change the signal handling code to insert anything Xenomai might
>> need, hence the override-then-route-unhandled approach. A typical configuration
>> I thought of was GUI-based apps, that would include RT code as well. In that
>> case, I don't think that most of us would want to be dragged into rebuilding
>> kde/gtk libs, for the sole purpose of routing unhandled SIGSHADOW signals.
>>
>> At the same time, I agree with Jan that we should not prevent people who prefer
>> hacking their own software components to install that routing in their handler,
>> instead of fiddling with the order in which they do the application init chores.
>>
>> Well, what about merging the solutions: trap the signal from the library
>> constructor by default for people relying on #1,
> 
> Mmm, that would not work for components running at late init stage though.
> 
>  AND document the shadow signal
>> handler for people who can do #2?
>>
> ... doing so would require to provide some Xenomai-specific marker in the
> siginfo struct the application-defined handler could test, but that should be
> doable.

There is already such a marker: Xenomai nucleus use SI_QUEUE whereas the
Linux kernel uses SI_KERNEL, for the siginfo member
which-I-do-not-remember-the-name

-- 
					    Gilles.


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

* Re: [Xenomai-core] [PATCH 1/1] Use SIGWINCH to trigger priority change in user-space.
  2008-10-19 16:09         ` Philippe Gerum
  2008-10-19 16:14           ` Philippe Gerum
@ 2008-10-19 16:23           ` Gilles Chanteperdrix
  2008-10-19 16:37             ` Philippe Gerum
  2008-10-20  8:25             ` Gilles Chanteperdrix
  1 sibling, 2 replies; 17+ messages in thread
From: Gilles Chanteperdrix @ 2008-10-19 16:23 UTC (permalink / raw)
  To: rpm; +Cc: Jan Kiszka, Xenomai core

Philippe Gerum wrote:
> Gilles Chanteperdrix wrote:
>> Jan Kiszka wrote:
>>> Gilles Chanteperdrix wrote:
>>>> Jan Kiszka wrote:
>>>>> I wonder if we shouldn't switch the signal hooking strategy: So far we
>>>>> install the handler at shadow thread creation time, saving a potentially
>>>>> installed handler of the application for redirection of
>>>>> Xenomai-unrelated events. But that only works if the application
>>>>> installed the handler before it creates the first shadow thread, right?
>>>>> If the app decides to install/change the SIGWINCH handler later on
>>>>> (without taking care of our handler), we will loose.
>>>>>
>>>>> Suggestion: As this is fragile and cannot be solved transparently, lets
>>>>> document this signal requirement of Xenomai, e.g. in all task/thread
>>>>> creation functions of the skins. Tell the user that Xenomai will install
>>>>> a custom SIGWINCH handler and that, if the app wants to override it, it
>>>>> has to make sure to _first_ call into a Xenomai-provided handler and
>>>>> check if that one wants to handle the event. I'm thinking of something
>>>>> like int xeno_sigwinch_handler(int sig, siginfo_t *si, void *ctxt),
>>>>> where the return code is non-zero in case the signal was processed.
>>>>>
>>>>> With this policy in place, we can switch the signal handler installation
>>>>> above to __attribute__ ((construtor)) and save us all the changes
>>>>> regarding sigshadow_install below.
>>>> You mean to push the burden of identifying the signal source and calling
>>>> the proper handler on the user. With the current approach we take care
>>>> of that burden, I think it is better. But I agree that this should be
>>>> documented somewhere.
>>> Nope, the burden will still be Xenomai's, encoded in
>>> xeno_sigwinch_handler. The app will only have to check for its return code.
>> Actually, I have no real preference. Philippe, since you designed the
>> original code, could you tell us what you had in mind?
>>
> 
> My reasoning at that time was that the application folks would not want or even
> be able to change the signal handling code to insert anything Xenomai might
> need, hence the override-then-route-unhandled approach. A typical configuration
> I thought of was GUI-based apps, that would include RT code as well. In that
> case, I don't think that most of us would want to be dragged into rebuilding
> kde/gtk libs, for the sole purpose of routing unhandled SIGSHADOW signals.
> 
> At the same time, I agree with Jan that we should not prevent people who prefer
> hacking their own software components to install that routing in their handler,
> instead of fiddling with the order in which they do the application init chores.
> 
> Well, what about merging the solutions: trap the signal from the library
> constructor by default for people relying on #1, AND document the shadow signal
> handler for people who can do #2?

For me, merging the two, would mean keep sigshadow_install called upon
thread creation, but export xeno_sigwinch_handler to be callable from
user signals.

This way, if people install their signal handler before the first thread
creation, they have nothing to worry about. If they install their signal
handler at a later time, they have to take care of calling
xeno_sigwinch_handler and look at its return value.

-- 
					    Gilles.


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

* Re: [Xenomai-core] [PATCH 1/1] Use SIGWINCH to trigger priority change in user-space.
  2008-10-19 16:23           ` Gilles Chanteperdrix
@ 2008-10-19 16:37             ` Philippe Gerum
  2008-10-20  8:25             ` Gilles Chanteperdrix
  1 sibling, 0 replies; 17+ messages in thread
From: Philippe Gerum @ 2008-10-19 16:37 UTC (permalink / raw)
  To: Gilles Chanteperdrix; +Cc: Jan Kiszka, Xenomai core

Gilles Chanteperdrix wrote:
> Philippe Gerum wrote:
>> Gilles Chanteperdrix wrote:
>>> Jan Kiszka wrote:
>>>> Gilles Chanteperdrix wrote:
>>>>> Jan Kiszka wrote:
>>>>>> I wonder if we shouldn't switch the signal hooking strategy: So far we
>>>>>> install the handler at shadow thread creation time, saving a potentially
>>>>>> installed handler of the application for redirection of
>>>>>> Xenomai-unrelated events. But that only works if the application
>>>>>> installed the handler before it creates the first shadow thread, right?
>>>>>> If the app decides to install/change the SIGWINCH handler later on
>>>>>> (without taking care of our handler), we will loose.
>>>>>>
>>>>>> Suggestion: As this is fragile and cannot be solved transparently, lets
>>>>>> document this signal requirement of Xenomai, e.g. in all task/thread
>>>>>> creation functions of the skins. Tell the user that Xenomai will install
>>>>>> a custom SIGWINCH handler and that, if the app wants to override it, it
>>>>>> has to make sure to _first_ call into a Xenomai-provided handler and
>>>>>> check if that one wants to handle the event. I'm thinking of something
>>>>>> like int xeno_sigwinch_handler(int sig, siginfo_t *si, void *ctxt),
>>>>>> where the return code is non-zero in case the signal was processed.
>>>>>>
>>>>>> With this policy in place, we can switch the signal handler installation
>>>>>> above to __attribute__ ((construtor)) and save us all the changes
>>>>>> regarding sigshadow_install below.
>>>>> You mean to push the burden of identifying the signal source and calling
>>>>> the proper handler on the user. With the current approach we take care
>>>>> of that burden, I think it is better. But I agree that this should be
>>>>> documented somewhere.
>>>> Nope, the burden will still be Xenomai's, encoded in
>>>> xeno_sigwinch_handler. The app will only have to check for its return code.
>>> Actually, I have no real preference. Philippe, since you designed the
>>> original code, could you tell us what you had in mind?
>>>
>> My reasoning at that time was that the application folks would not want or even
>> be able to change the signal handling code to insert anything Xenomai might
>> need, hence the override-then-route-unhandled approach. A typical configuration
>> I thought of was GUI-based apps, that would include RT code as well. In that
>> case, I don't think that most of us would want to be dragged into rebuilding
>> kde/gtk libs, for the sole purpose of routing unhandled SIGSHADOW signals.
>>
>> At the same time, I agree with Jan that we should not prevent people who prefer
>> hacking their own software components to install that routing in their handler,
>> instead of fiddling with the order in which they do the application init chores.
>>
>> Well, what about merging the solutions: trap the signal from the library
>> constructor by default for people relying on #1, AND document the shadow signal
>> handler for people who can do #2?
> 
> For me, merging the two, would mean keep sigshadow_install called upon
> thread creation, but export xeno_sigwinch_handler to be callable from
> user signals.
> 
> This way, if people install their signal handler before the first thread
> creation, they have nothing to worry about. If they install their signal
> handler at a later time, they have to take care of calling
> xeno_sigwinch_handler and look at its return value.
> 

Yes, hooking the SIGSHADOW within the lib ctor() would eventually be a bad idea.

-- 
Philippe.


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

* Re: [Xenomai-core] [PATCH 1/1] Use SIGWINCH to trigger priority change in user-space.
  2008-10-19 16:23           ` Gilles Chanteperdrix
  2008-10-19 16:37             ` Philippe Gerum
@ 2008-10-20  8:25             ` Gilles Chanteperdrix
  2008-10-20  8:40               ` Jan Kiszka
  1 sibling, 1 reply; 17+ messages in thread
From: Gilles Chanteperdrix @ 2008-10-20  8:25 UTC (permalink / raw)
  To: rpm; +Cc: Jan Kiszka, Xenomai core

Gilles Chanteperdrix wrote:
> Philippe Gerum wrote:
>> Well, what about merging the solutions: trap the signal from the library
>> constructor by default for people relying on #1, AND document the shadow signal
>> handler for people who can do #2?
> 
> For me, merging the two, would mean keep sigshadow_install called upon
> thread creation, but export xeno_sigwinch_handler to be callable from
> user signals.
> 
> This way, if people install their signal handler before the first thread
> creation, they have nothing to worry about. If they install their signal
> handler at a later time, they have to take care of calling
> xeno_sigwinch_handler and look at its return value.

Jan, do you agree? The main argument is that people may use third-party
libraries such as, say, libreadline or libqt which intercept the
SIGWINCH signal, and that we do not want to recompile these libraries to
run with Xenomai.

-- 
                                                 Gilles.


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

* Re: [Xenomai-core] [PATCH 1/1] Use SIGWINCH to trigger priority change in user-space.
  2008-10-20  8:25             ` Gilles Chanteperdrix
@ 2008-10-20  8:40               ` Jan Kiszka
  2008-10-20 18:57                 ` Gilles Chanteperdrix
  0 siblings, 1 reply; 17+ messages in thread
From: Jan Kiszka @ 2008-10-20  8:40 UTC (permalink / raw)
  To: Gilles Chanteperdrix; +Cc: Xenomai core

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

Gilles Chanteperdrix wrote:
> Gilles Chanteperdrix wrote:
>> Philippe Gerum wrote:
>>> Well, what about merging the solutions: trap the signal from the library
>>> constructor by default for people relying on #1, AND document the shadow signal
>>> handler for people who can do #2?
>> For me, merging the two, would mean keep sigshadow_install called upon
>> thread creation, but export xeno_sigwinch_handler to be callable from
>> user signals.
>>
>> This way, if people install their signal handler before the first thread
>> creation, they have nothing to worry about. If they install their signal
>> handler at a later time, they have to take care of calling
>> xeno_sigwinch_handler and look at its return value.
> 
> Jan, do you agree? The main argument is that people may use third-party
> libraries such as, say, libreadline or libqt which intercept the
> SIGWINCH signal, and that we do not want to recompile these libraries to
> run with Xenomai.

Yes, I'm fine with the current approach + doc + xeno_sigwinch_handler.

Jan


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

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

* Re: [Xenomai-core] [PATCH 1/1] Use SIGWINCH to trigger priority change in user-space.
  2008-10-20  8:40               ` Jan Kiszka
@ 2008-10-20 18:57                 ` Gilles Chanteperdrix
  2008-10-20 19:00                   ` Jan Kiszka
  0 siblings, 1 reply; 17+ messages in thread
From: Gilles Chanteperdrix @ 2008-10-20 18:57 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: Xenomai core

Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> Gilles Chanteperdrix wrote:
>>> Philippe Gerum wrote:
>>>> Well, what about merging the solutions: trap the signal from the library
>>>> constructor by default for people relying on #1, AND document the shadow signal
>>>> handler for people who can do #2?
>>> For me, merging the two, would mean keep sigshadow_install called upon
>>> thread creation, but export xeno_sigwinch_handler to be callable from
>>> user signals.
>>>
>>> This way, if people install their signal handler before the first thread
>>> creation, they have nothing to worry about. If they install their signal
>>> handler at a later time, they have to take care of calling
>>> xeno_sigwinch_handler and look at its return value.
>> Jan, do you agree? The main argument is that people may use third-party
>> libraries such as, say, libreadline or libqt which intercept the
>> SIGWINCH signal, and that we do not want to recompile these libraries to
>> run with Xenomai.
> 
> Yes, I'm fine with the current approach + doc + xeno_sigwinch_handler.

Ok. Since I am going to paste the doc in four different places, I'd 
better get it right from the beginning. Here is my prose:

 * @note: When creating or shadowing a Xenomai thread in user-space, for the
 * first time, Xenomai installs a handler for the SIGWINCH signal. If you had
 * installed a handler before that, it will be automatically called by Xenomai
 * for SIGWINCH signals that it has not sent.
 *
 * If, however, you install a signal handler for SIGWINCH after creating
 * or shadowing the first Xenomai thread, you have to explicitely call the
 * function xeno_sigwinch_handler at the beginning of your signal handler,
 * using its return to know if the signal was in fact an internal signal of
 * Xenomai (in which case it returns 1), or if you should handle the signal (in
 * which case it returns 0). xeno_sigwinch_handler prototype is:
 *
 * <b>int xeno_sigwinch_handler(int sig, siginfo_t *si, void *ctxt);</b>
 *
 * Which means that you should register your handler with sigaction, using the
 * SA_SIGINFO flag, and pass all the arguments you received to
 * xeno_sigwinch_handler.


-- 
					    Gilles.


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

* Re: [Xenomai-core] [PATCH 1/1] Use SIGWINCH to trigger priority change in user-space.
  2008-10-20 18:57                 ` Gilles Chanteperdrix
@ 2008-10-20 19:00                   ` Jan Kiszka
  0 siblings, 0 replies; 17+ messages in thread
From: Jan Kiszka @ 2008-10-20 19:00 UTC (permalink / raw)
  To: Gilles Chanteperdrix; +Cc: Xenomai core

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

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> Gilles Chanteperdrix wrote:
>>> Gilles Chanteperdrix wrote:
>>>> Philippe Gerum wrote:
>>>>> Well, what about merging the solutions: trap the signal from the library
>>>>> constructor by default for people relying on #1, AND document the shadow signal
>>>>> handler for people who can do #2?
>>>> For me, merging the two, would mean keep sigshadow_install called upon
>>>> thread creation, but export xeno_sigwinch_handler to be callable from
>>>> user signals.
>>>>
>>>> This way, if people install their signal handler before the first thread
>>>> creation, they have nothing to worry about. If they install their signal
>>>> handler at a later time, they have to take care of calling
>>>> xeno_sigwinch_handler and look at its return value.
>>> Jan, do you agree? The main argument is that people may use third-party
>>> libraries such as, say, libreadline or libqt which intercept the
>>> SIGWINCH signal, and that we do not want to recompile these libraries to
>>> run with Xenomai.
>> Yes, I'm fine with the current approach + doc + xeno_sigwinch_handler.
> 
> Ok. Since I am going to paste the doc in four different places, I'd 
> better get it right from the beginning. Here is my prose:
> 
>  * @note: When creating or shadowing a Xenomai thread in user-space, for the
>  * first time, Xenomai installs a handler for the SIGWINCH signal. If you had
>  * installed a handler before that, it will be automatically called by Xenomai
>  * for SIGWINCH signals that it has not sent.
>  *
>  * If, however, you install a signal handler for SIGWINCH after creating
>  * or shadowing the first Xenomai thread, you have to explicitely call the

explicitly
(I regularly mistype this as well...)

>  * function xeno_sigwinch_handler at the beginning of your signal handler,
>  * using its return to know if the signal was in fact an internal signal of
>  * Xenomai (in which case it returns 1), or if you should handle the signal (in
>  * which case it returns 0). xeno_sigwinch_handler prototype is:
>  *
>  * <b>int xeno_sigwinch_handler(int sig, siginfo_t *si, void *ctxt);</b>
>  *
>  * Which means that you should register your handler with sigaction, using the
>  * SA_SIGINFO flag, and pass all the arguments you received to
>  * xeno_sigwinch_handler.
> 
> 

Sounds good to me.

Jan


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

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

end of thread, other threads:[~2008-10-20 19:00 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-10-18 14:22 [Xenomai-core] [PATCH 1/1] Use SIGWINCH to trigger priority change in user-space Gilles Chanteperdrix
2008-10-18 14:35 ` Gilles Chanteperdrix
2008-10-18 17:09   ` Philippe Gerum
2008-10-19 15:21   ` Gilles Chanteperdrix
2008-10-19 11:10 ` Jan Kiszka
2008-10-19 11:19   ` Gilles Chanteperdrix
2008-10-19 11:22     ` Jan Kiszka
2008-10-19 11:28       ` Gilles Chanteperdrix
2008-10-19 16:09         ` Philippe Gerum
2008-10-19 16:14           ` Philippe Gerum
2008-10-19 16:16             ` Gilles Chanteperdrix
2008-10-19 16:23           ` Gilles Chanteperdrix
2008-10-19 16:37             ` Philippe Gerum
2008-10-20  8:25             ` Gilles Chanteperdrix
2008-10-20  8:40               ` Jan Kiszka
2008-10-20 18:57                 ` Gilles Chanteperdrix
2008-10-20 19:00                   ` Jan Kiszka

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.