Use SIGWINCH to trigger priority change in user-space.

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.

Quoted 6 lines · Gilles Chanteperdrix

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.

Quoted 6 lines · Gilles Chanteperdrix

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.

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
===================================================================

nucleus/thread.h +1 -0

@@ -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
===================================================================

asm-generic/syscall.h +6 -1

@@ -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
===================================================================

nucleus/shadow.h +1 -0

@@ -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
===================================================================

nucleus/shadow.c +26 -3

@@ -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

nucleus/shadow.c +8 -8

-	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
===================================================================

nucleus/synch.c +2 -0

@@ -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
===================================================================

nucleus/pod.c +8 -4

@@ -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
===================================================================

asm-generic/bits/sigshadow.h +62 -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);
+

Index: src/skins/posix/thread.c
===================================================================

skins/posix/thread.c +3 -13

@@ -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
===================================================================

skins/native/task.c +3 -14

@@ -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
===================================================================

skins/vxworks/taskLib.c +2 -13

@@ -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
===================================================================

skins/psos+/task.c +3 -15

@@ -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
===================================================================

skins/vrtx/task.c +2 -13

@@ -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
===================================================================

skins/uitron/task.c +2 -12

@@ -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,

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.

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?

Quoted 17 lines · Jan Kiszka

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.

Quoted 3 lines

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.

Quoted 7 lines

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.

Quoted 21 lines · Gilles Chanteperdrix

Quoted 20 lines · Jan Kiszka

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.

Quoted 4 lines

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.

Quoted 26 lines · Jan Kiszka

Quoted 25 lines · Gilles Chanteperdrix

Quoted 17 lines · Jan Kiszka

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.

Quoted 3 lines

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.

Quoted 2 lines

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?

Quoted 23 lines · Gilles Chanteperdrix

Quoted 22 lines · Jan Kiszka

Quoted 21 lines · Gilles Chanteperdrix

Quoted 20 lines · Jan Kiszka

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.

Quoted 9 lines

Quoted 6 lines

Quoted 4 lines

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,

handler for people who can do #2?

Quoted 40 lines · Philippe Gerum

Quoted 23 lines · Gilles Chanteperdrix

Quoted 22 lines · Jan Kiszka

Quoted 21 lines · Gilles Chanteperdrix

Quoted 20 lines · Jan Kiszka

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.

Quoted 9 lines

Quoted 6 lines

Quoted 4 lines

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.

Quoted 6 lines

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

Quoted 2 lines

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.

Quoted 23 lines · Gilles Chanteperdrix

Quoted 22 lines · Jan Kiszka

Quoted 21 lines · Gilles Chanteperdrix

Quoted 20 lines · Jan Kiszka

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.

Quoted 9 lines

Quoted 6 lines

Quoted 4 lines

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?

Quoted 40 lines · Philippe Gerum

Quoted 23 lines · Gilles Chanteperdrix

Quoted 22 lines · Jan Kiszka

Quoted 21 lines · Gilles Chanteperdrix

Quoted 20 lines · Jan Kiszka

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.

Quoted 9 lines

Quoted 6 lines

Quoted 4 lines

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.

Quoted 7 lines

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.

Quoted 3 lines · Philippe Gerum

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.

Quoted 4 lines · Gilles Chanteperdrix

Quoted 3 lines · Philippe Gerum

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?

Quoted 8 lines

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.

Quoted 13 lines · Gilles Chanteperdrix

Quoted 12 lines · Gilles Chanteperdrix

Quoted 3 lines · Philippe Gerum

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.

Quoted 4 lines

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.

Quoted 14 lines · Jan Kiszka

Quoted 13 lines · Gilles Chanteperdrix

Quoted 12 lines · Gilles Chanteperdrix

Quoted 3 lines · Philippe Gerum

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.

Quoted 5 lines

Quoted 4 lines

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.