Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> Jan Kiszka wrote:
>>> Gilles Chanteperdrix wrote:
>>>> Jan Kiszka wrote:
>>>>> OTOH, we can safe some text size which is precious as well. So I'm
>>>>> convinced to go your way (with a modification):
>>>> My approach sucks: we get a silly atomic_cmpxchg if the mutex is already
>>>> claimed, which is as least as much a common case as a currently
>>>> unclaimed mutex. Need to think a bit. But I think a good solution is to
>>>> re-read only if the mutex has been seen as already claimed.
>>> That makes no difference as then we will go through the cmpxchg path anyway.
>>>
>>> There is _no_ way around re-reading under nklock, all we can avoid is
>>> atomic cmpxchg in the case of >1 waiters. But that would come at the
>>> price of more complexity for all waiter.
>>>
>>> However, let's find some solution. I bet things will look different
>>> again when we start fiddling with a generic lock + the additional bit to
>>> replace XNWAKEN.
>> What I meant is that if the claimed bit is already set, we can avoid the
>> cmpxchg altogether, which was the intent of the original code. So I propose
>> the following version:
>>
>> if(test_claimed(owner))
>> owner = xnarch_atomic_intptr_get(mutex->owner);
>
> This version lacks a test for owner == 0 here, otherwise you re-create
> my old bug that bite me today.
Ok, so jumping in the middle of the loop is the best thing to do then.
>
>> while(!test_claimed(owner)) {
>> old = xnarch_atomic_intptr_cmpxchg(mutex->owner,
>> owner, set_claimed(owner,
>> 1));
>> if (likely(old == owner))
>> break;
>> if (old == NULL) {
>> /* Owner called fast mutex_unlock
>> (on another cpu) */
>> xnlock_put_irqrestore(&nklock, s);
>> goto retry_lock;
>> }
>> owner = old;
>> }
>>
>> The compiler rearranges things correctly (at least on ARM), and avoids the
>> redundant test.
>>
>
> My latest concern remains: Is all this additional complexity, are all
> these conditional branches and the text size increase worth the effort?
> How much cycles or micoseconds would be gain on the most suffering
> architecture?
Since the else and the while are folded together and the loop becomes
post-tested, and with the help of conditional instructions, I do not
think there is much more conditional branch (we need to to jump over
the while loop anyway). However avoiding the cmpxchg is a real gain,
since it is implemented as irq save/irq restore on an old arm.
if(test_claimed(owner)) {
old = xnarch_atomic_intptr_get(mutex->owner);
goto test_old;
}
while(!test_claimed(owner)) {
old = xnarch_atomic_intptr_cmpxchg(mutex->owner,
owner, set_claimed(owner,
1));
if (likely(old == owner))
break;
test_old:
if (old == NULL) {
/* Owner called fast mutex_unlock
(on another cpu) */
xnlock_put_irqrestore(&nklock, s);
goto retry_lock;
}
owner = old;
}
--
Gilles.
_______________________________________________
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core
