Re: [Xenomai-core] Policy switching and XNOTHER maintenance

2011-09-16 Thread Gilles Chanteperdrix
On 09/11/2011 04:29 PM, Jan Kiszka wrote:
 On 2011-09-11 16:24, Gilles Chanteperdrix wrote:
 On 09/11/2011 12:50 PM, Jan Kiszka wrote:
 Hi all,

 just looked into the hrescnt issue again, specifically the corner case
 of a shadow thread switching from real-time policy to SCHED_OTHER.

 Doing this while holding a mutex looks invalid.
 
 Looking at POSIX e.g., is there anything in the spec that makes this
 invalid? If the kernel preserves or established proper priority
 boosting, I do not see what could break in principle.
 
 It is nothing I would design into some app, but we should somehow handle
 it (doc update or code adjustments).
 
 If we do not do it, the current code is valid.
 
 Except for its dependency on XNOTHER which is not updated on RT-NORMAL
 transitions.

The fact that this update did not take place made the code work. No 
negative rescnt could happen with that code.

Anyway, here is a patch to allow switching back from RT to NORMAL, but 
send a SIGDEBUG to a thread attempting to release a mutex while its 
counter is already 0. We end up avoiding a big chunk of code that would 
have been useful for a really strange corner case.

diff --git a/examples/native/sigdebug.c b/examples/native/sigdebug.c
index e20714b..7b840f9 100644
--- a/examples/native/sigdebug.c
+++ b/examples/native/sigdebug.c
@@ -34,6 +34,8 @@ static const char *reason_str[] = {
 [SIGDEBUG_MIGRATE_PRIOINV] = affected by priority inversion,
 [SIGDEBUG_NOMLOCK] = missing mlockall,
 [SIGDEBUG_WATCHDOG] = runaway thread,
+[SIGDEBUG_SCHED_CHANGE_W_MX] =
+switched to SCHED_OTHER policy with mutex held,
 };
 
 void warn_upon_switch(int sig, siginfo_t *si, void *context)
diff --git a/include/asm-generic/syscall.h b/include/asm-generic/syscall.h
index a9cdc87..5b6c764 100644
--- a/include/asm-generic/syscall.h
+++ b/include/asm-generic/syscall.h
@@ -61,6 +61,7 @@ typedef struct xnsysinfo {
 #define SIGDEBUG_MIGRATE_PRIOINV   4
 #define SIGDEBUG_NOMLOCK   5
 #define SIGDEBUG_WATCHDOG  6
+#define SIGDEBUG_SCHED_CHANGE_W_MX  7
 
 #ifdef __KERNEL__
 
diff --git a/include/nucleus/sched-idle.h b/include/nucleus/sched-idle.h
index 6399a17..3cc7185 100644
--- a/include/nucleus/sched-idle.h
+++ b/include/nucleus/sched-idle.h
@@ -39,6 +39,7 @@ extern struct xnsched_class xnsched_class_idle;
 static inline void __xnsched_idle_setparam(struct xnthread *thread,
   const union xnsched_policy_param *p)
 {
+   xnthread_set_state(thread, XNOTHER);
thread-cprio = p-idle.prio;
 }
 
diff --git a/include/nucleus/sched-rt.h b/include/nucleus/sched-rt.h
index 71f655c..d17b058 100644
--- a/include/nucleus/sched-rt.h
+++ b/include/nucleus/sched-rt.h
@@ -86,6 +86,10 @@ static inline void __xnsched_rt_setparam(struct xnthread 
*thread,
 const union xnsched_policy_param *p)
 {
thread-cprio = p-rt.prio;
+   if (thread-cprio)
+   xnthread_clear_state(thread, XNOTHER);
+   else
+   xnthread_set_state(thread, XNOTHER);
 }
 
 static inline void __xnsched_rt_getparam(struct xnthread *thread,
diff --git a/ksrc/nucleus/pod.c b/ksrc/nucleus/pod.c
index 9a02e80..d1f 100644
--- a/ksrc/nucleus/pod.c
+++ b/ksrc/nucleus/pod.c
@@ -1896,16 +1896,6 @@ int __xnpod_set_thread_schedparam(struct xnthread 
*thread,
xnsched_putback(thread);
 
 #ifdef CONFIG_XENO_OPT_PERVASIVE
-   /*
-* A non-real-time shadow may upgrade to real-time FIFO
-* scheduling, but the latter may never downgrade to
-* SCHED_NORMAL Xenomai-wise. In the valid case, we clear
-* XNOTHER to reflect the change. Note that we keep handling
-* non real-time shadow specifics in higher code layers, not
-* to pollute the core scheduler with peculiarities.
-*/
-   if (sched_class == xnsched_class_rt  sched_param-rt.prio  0)
-   xnthread_clear_state(thread, XNOTHER);
if (propagate) {
if (xnthread_test_state(thread, XNRELAX))
xnshadow_renice(thread);
diff --git a/ksrc/nucleus/sched-sporadic.c b/ksrc/nucleus/sched-sporadic.c
index fd37c21..587be4a 100644
--- a/ksrc/nucleus/sched-sporadic.c
+++ b/ksrc/nucleus/sched-sporadic.c
@@ -258,6 +258,7 @@ static void xnsched_sporadic_setparam(struct xnthread 
*thread,
}
}
 
+   xnthread_clear_state(thread, XNOTHER);
thread-cprio = p-pss.current_prio;
 }
 
diff --git a/ksrc/nucleus/sched-tp.c b/ksrc/nucleus/sched-tp.c
index 43a548e..2423e81 100644
--- a/ksrc/nucleus/sched-tp.c
+++ b/ksrc/nucleus/sched-tp.c
@@ -100,6 +100,7 @@ static void xnsched_tp_setparam(struct xnthread *thread,
 {
struct xnsched *sched = thread-sched;
 
+   xnthread_clear_state(thread, XNOTHER);
thread-tps = sched-tp.partitions[p-tp.ptid];
thread-cprio = p-tp.prio;
 }
diff --git a/ksrc/nucleus/synch.c b/ksrc/nucleus/synch.c
index 

Re: [Xenomai-core] Policy switching and XNOTHER maintenance

2011-09-16 Thread Gilles Chanteperdrix
On 09/16/2011 10:13 PM, Gilles Chanteperdrix wrote:
 On 09/11/2011 04:29 PM, Jan Kiszka wrote:
 On 2011-09-11 16:24, Gilles Chanteperdrix wrote:
 On 09/11/2011 12:50 PM, Jan Kiszka wrote:
 Hi all,

 just looked into the hrescnt issue again, specifically the corner case
 of a shadow thread switching from real-time policy to SCHED_OTHER.

 Doing this while holding a mutex looks invalid.

 Looking at POSIX e.g., is there anything in the spec that makes this
 invalid? If the kernel preserves or established proper priority
 boosting, I do not see what could break in principle.

 It is nothing I would design into some app, but we should somehow handle
 it (doc update or code adjustments).

 If we do not do it, the current code is valid.

 Except for its dependency on XNOTHER which is not updated on RT-NORMAL
 transitions.
 
 The fact that this update did not take place made the code work. No 
 negative rescnt could happen with that code.
 
 Anyway, here is a patch to allow switching back from RT to NORMAL, but 
 send a SIGDEBUG to a thread attempting to release a mutex while its 
 counter is already 0. We end up avoiding a big chunk of code that would 
 have been useful for a really strange corner case.
 

Here comes version 2:
diff --git a/include/nucleus/sched-idle.h b/include/nucleus/sched-idle.h
index 6399a17..417170f 100644
--- a/include/nucleus/sched-idle.h
+++ b/include/nucleus/sched-idle.h
@@ -39,6 +39,8 @@ extern struct xnsched_class xnsched_class_idle;
 static inline void __xnsched_idle_setparam(struct xnthread *thread,
   const union xnsched_policy_param *p)
 {
+   if (xnthread_test_state(thread, XNSHADOW))
+   xnthread_clear_state(thread, XNOTHER);
thread-cprio = p-idle.prio;
 }
 
diff --git a/include/nucleus/sched-rt.h b/include/nucleus/sched-rt.h
index 71f655c..cc1cefa 100644
--- a/include/nucleus/sched-rt.h
+++ b/include/nucleus/sched-rt.h
@@ -86,6 +86,12 @@ static inline void __xnsched_rt_setparam(struct xnthread 
*thread,
 const union xnsched_policy_param *p)
 {
thread-cprio = p-rt.prio;
+   if (xnthread_test_state(thread, XNSHADOW)) {
+   if (thread-cprio)
+   xnthread_clear_state(thread, XNOTHER);
+   else
+   xnthread_set_state(thread, XNOTHER);
+   }
 }
 
 static inline void __xnsched_rt_getparam(struct xnthread *thread,
diff --git a/ksrc/nucleus/pod.c b/ksrc/nucleus/pod.c
index 9a02e80..d1f 100644
--- a/ksrc/nucleus/pod.c
+++ b/ksrc/nucleus/pod.c
@@ -1896,16 +1896,6 @@ int __xnpod_set_thread_schedparam(struct xnthread 
*thread,
xnsched_putback(thread);
 
 #ifdef CONFIG_XENO_OPT_PERVASIVE
-   /*
-* A non-real-time shadow may upgrade to real-time FIFO
-* scheduling, but the latter may never downgrade to
-* SCHED_NORMAL Xenomai-wise. In the valid case, we clear
-* XNOTHER to reflect the change. Note that we keep handling
-* non real-time shadow specifics in higher code layers, not
-* to pollute the core scheduler with peculiarities.
-*/
-   if (sched_class == xnsched_class_rt  sched_param-rt.prio  0)
-   xnthread_clear_state(thread, XNOTHER);
if (propagate) {
if (xnthread_test_state(thread, XNRELAX))
xnshadow_renice(thread);
diff --git a/ksrc/nucleus/sched-sporadic.c b/ksrc/nucleus/sched-sporadic.c
index fd37c21..ffc9bab 100644
--- a/ksrc/nucleus/sched-sporadic.c
+++ b/ksrc/nucleus/sched-sporadic.c
@@ -258,6 +258,8 @@ static void xnsched_sporadic_setparam(struct xnthread 
*thread,
}
}
 
+   if (xnthread_test_state(thread, XNSHADOW))
+   xnthread_clear_state(thread, XNOTHER);
thread-cprio = p-pss.current_prio;
 }
 
diff --git a/ksrc/nucleus/sched-tp.c b/ksrc/nucleus/sched-tp.c
index 43a548e..a2af1d3 100644
--- a/ksrc/nucleus/sched-tp.c
+++ b/ksrc/nucleus/sched-tp.c
@@ -100,6 +100,8 @@ static void xnsched_tp_setparam(struct xnthread *thread,
 {
struct xnsched *sched = thread-sched;
 
+   if (xnthread_test_state(thread, XNSHADOW))
+   xnthread_clear_state(thread, XNOTHER);
thread-tps = sched-tp.partitions[p-tp.ptid];
thread-cprio = p-tp.prio;
 }
diff --git a/ksrc/nucleus/synch.c b/ksrc/nucleus/synch.c
index b956e46..47bc0c5 100644
--- a/ksrc/nucleus/synch.c
+++ b/ksrc/nucleus/synch.c
@@ -684,9 +684,13 @@ xnsynch_release_thread(struct xnsynch *synch, struct 
xnthread *lastowner)
 
XENO_BUGON(NUCLEUS, !testbits(synch-status, XNSYNCH_OWNER));
 
-   if (xnthread_test_state(lastowner, XNOTHER))
-   xnthread_dec_rescnt(lastowner);
-   XENO_BUGON(NUCLEUS, xnthread_get_rescnt(lastowner)  0);
+   if (xnthread_test_state(lastowner, XNOTHER)) {
+   if (xnthread_get_rescnt(lastowner) == 0)
+   xnshadow_send_sig(lastowner, SIGDEBUG,
+