Re: [Xenomai] Prio ceiling mutex issues

[Philippe Gerum <rpm@xenomai.org>]

On 05/09/2016 09:03 PM, Jan Kiszka wrote:
> Hi Philippe,
> 
> we ran into at least one bug of the current prio ceiling support for
> synch objects: on xnsynch_release, we only reschedule if we woken up a
> new owner. But we also have to reschedule after dropping the ceiling
> prio as some higher prio thread may be waiting.
> 
> For that, I would propose to alter the return type and semantic of
> xnsynch_release from "pointer to new owner" into "bool, true if
> reschedule is needed". It seems, no caller of xnsynch_release makes a
> non-boolean use of the return value anyway. And then we can simply
> return true on "synch->status & XNSYNCH_PP".

I agree, but we need to keep the next owner check in the boolean status. Something like this would do I believe:

diff --git a/kernel/cobalt/synch.c b/kernel/cobalt/synch.c
index 976261d..f84cb6e 100644
--- a/kernel/cobalt/synch.c
+++ b/kernel/cobalt/synch.c
@@ -913,10 +913,10 @@ static struct xnthread *transfer_ownership(struct xnsynch *synch,
  *
  * @coretags{primary-only, might-switch}
  */
-struct xnthread *xnsynch_release(struct xnsynch *synch,
-				 struct xnthread *curr)
+static bool xnsynch_release(struct xnsynch *synch,
+			    struct xnthread *curr)
 {
-	struct xnthread *nextowner = NULL;
+	bool resched = false;
 	xnhandle_t currh, h;
 	atomic_t *lockp;
 	spl_t s;
@@ -946,16 +946,18 @@ struct xnthread *xnsynch_release(struct xnsynch *synch,
 	h = atomic_cmpxchg(lockp, currh, XN_NO_HANDLE);
 	if ((h & ~XNSYNCH_FLCEIL) != currh)
 		/* FLCLAIM set, synch is contended. */
-		nextowner = transfer_ownership(synch, curr);
+		resched = transfer_ownership(synch, curr) != NULL;
 	else if (h != currh)	/* FLCEIL set, FLCLAIM clear. */
 		atomic_set(lockp, XN_NO_HANDLE);
 
-	if (synch->status & XNSYNCH_PP)
+	if (synch->status & XNSYNCH_PP) {
 		clear_pp_boost(synch, curr);
+		resched = true;
+	}
 
 	xnlock_put_irqrestore(&nklock, s);
 
-	return nextowner;
+	return resched;
 }
 EXPORT_SYMBOL_GPL(xnsynch_release);
 

-- 
Philippe.