Re: [PATCH] xnsynch_requeue_sleeper: Return early when !XNSYNCH_PRIO

On 13.04.26 22:11, Richard Weinberger wrote:
> ----- Ursprüngliche Mail -----
>> Von: "Jan Kiszka" <[email protected]>
>> But XNSYNCH_PI != XNSYNCH_PRIO. PI implies PRIO, but if we enter under
>> prio-ceiling conditions, we won't bail out here, and we won't get the
>> BUG_ON anymore.
>>
>> Please check if you use prio-ceiling mutex instead what the behavior is.
>> Add that to your test case as well.
> 
> With a prio-ceiling mutex I'm unable to trigger that code path.
> 
> This is my new (hopefully proper) test case:
> https://gitlab.com/Xenomai/legacy/xenomai-hacker-space/-/commit/b35c9b37ab4beec5084aa8303d91fec2bf3b4eba
> 

Right, a prio adjustment of the lock-holder that is blocked on xddp will 
not take place with PP - prio adjustments happen already on lock entry. 
But your changes would still cause regressions to 
xnsynch_requeue_sleeper.

Let's step back: In the PI case, the lock holder will be on a FIFO queue 
while waiting for messages. So it may not be woken up first if there are 
multiple waiters - that is something we probably cannot change as we 
have no information about other waiters in that queue and their urgency.

Once woken up, my next question would be if the right prio is used from 
there on until the lock is released. That should be checked by test 
case.

Now to clarify why your changes is not yet correct: A thread is blocked 
on a PP mutex while holding a PI one. If someone tries to get the PI 
lock while have a high prio than the PP mutex, the necessary propagation 
of this prio to lock owner will no longer work. I'm not sure if we have 
such a test case in Xenomai, we definitely should, and this one would 
reveal the regression.

Maybe this is already enough and correct:

diff --git a/kernel/cobalt/synch.c b/kernel/cobalt/synch.c
index 70c05c1f49..33ef8ef183 100644
--- a/kernel/cobalt/synch.c
+++ b/kernel/cobalt/synch.c
@@ -963,7 +963,9 @@ void xnsynch_requeue_sleeper(struct xnthread *thread)
 	struct xnsynch *synch = thread->wchan;
 	struct xnthread *owner;
 
-	XENO_BUG_ON(COBALT, !(synch->status & XNSYNCH_PRIO));
+	/* Nothing to do if waiting in a FIFO queue */
+	if ((synch->status & XNSYNCH_PRIO) == 0)
+		return;
 
 	/*
 	 * Update the position in the pend queue of a thread waiting


Jan

-- 
Siemens AG, Foundational Technologies
Linux Expert Center