Philippe Gerum wrote:
> Gilles Chanteperdrix wrote:
>> Jan Kiszka wrote:
>>> Gilles Chanteperdrix wrote:
>>>> Jan Kiszka wrote:
>>>>> Gilles Chanteperdrix wrote:
>>>>>> Jan Kiszka wrote:
>>>>>>> Gilles Chanteperdrix wrote:
>>>>>>>> Philippe Gerum wrote:
>>>>>>>>> Gilles Chanteperdrix wrote:
>>>>>>>>>> Jan Kiszka wrote:
>>>>>>>>>>> Gilles Chanteperdrix wrote:
>>>>>>>>>>>> Jan Kiszka wrote:
>>>>>>>>>>> ...
>>>>>>>>>>>>> I think I'm getting closer to the issue. Our actual problem comes 
>>>>>>>>>>>>> from
>>>>>>>>>>>>> the fact that the xnsynch_owner is easily out of sync with the 
>>>>>>>>>>>>> real
>>>>>>>>>>>>> owner, it even sometimes points to a former owner:
>>>>>>>>>>>>> Thread A releases a mutex on which thread B pends. It wakes up B,
>>>>>>>>>>>>> causing it to become the new xnsynch owner, and clears the 
>>>>>>>>>>>>> claimed bit
>>>>>>>>>>>>> as there are no further sleepers. B returns, and when it wants to
>>>>>>>>>>>>> release the mutex, it does this happily in user space because 
>>>>>>>>>>>>> claimed is
>>>>>>>>>>>>> not set. Now the fast lock variable is 'unlocked', while xnsynch 
>>>>>>>>>>>>> still
>>>>>>>>>>>>> reports B being the owner. This is no problem as the next time two
>>>>>>>>>>>>> threads fight over this lock the waiter will simply overwrite the
>>>>>>>>>>>>> xnsynch_owner before it falls asleep. But this "trick" doesn't 
>>>>>>>>>>>>> work for
>>>>>>>>>>>>> waiters that have been robbed. They will spin inside 
>>>>>>>>>>>>> xnsynch_sleep_on
>>>>>>>>>>>>> and stumble over this inconsistency.
>>>>>>>>>>>>> I have two approaches in mind now: First one is something like
>>>>>>>>>>>>> XNSYNCH_STEALNOINFORM, i.e. causing xnsynch_sleep_on to not set 
>>>>>>>>>>>>> XNROBBED
>>>>>>>>>>>>> so that the robbed thread spins one level higher in the skin code 
>>>>>>>>>>>>> -
>>>>>>>>>>>>> which would have to be extended a bit.
>>>>>>>>>>>> No, the stealing is the xnsynch job.
>>>>>>>>>>>>> Option two is to clear xnsynch_owner once a new owner is about to 
>>>>>>>>>>>>> return
>>>>>>>>>>>>> from kernel with the lock held while there are no more 
>>>>>>>>>>>>> xnsynch_sleepers.
>>>>>>>>>>>>> That should work with even less changes and save us one syscall 
>>>>>>>>>>>>> in the
>>>>>>>>>>>>> robbed case. Need to think about it more, though.
>>>>>>>>>>>> In fact the only time when the owner is required to be in sync is 
>>>>>>>>>>>> when
>>>>>>>>>>>> PIP occurs, and this is guaranteed to work, because when PIP is 
>>>>>>>>>>>> needed a
>>>>>>>>>>>> syscall is emitted anyway. To the extent that xnsynch does not even
>>>>>>>>>>>> track the owner on non PIP synch (which is why the posix skin 
>>>>>>>>>>>> originally
>>>>>>>>>>>>  forcibly set the synch owner, and it was simply kept to get the 
>>>>>>>>>>>> fastsem
>>>>>>>>>>>> stuff working).
>>>>>>>>>>>> Ok. And what about the idea of the xnsynch bit to tell him "hey, 
>>>>>>>>>>>> the
>>>>>>>>>>>> owner is tracked in the upper layer, go there to find it".
>>>>>>>>>>> I'm yet having difficulties to imagine how this should look like 
>>>>>>>>>>> when
>>>>>>>>>>> it's implemented. Would it be simpler than my second idea?
>>>>>>>>>>> Anyway, here is a patch (on top of my handle-based lock series) for 
>>>>>>>>>>> the
>>>>>>>>>>> approach that clears xnsynch_owner when there are no waiters. At 
>>>>>>>>>>> least
>>>>>>>>>>> it causes no regression based on your test, but I haven't checked 
>>>>>>>>>>> lock
>>>>>>>>>>> stealing yet. In theory, everything still appears to be fine to me. 
>>>>>>>>>>> This
>>>>>>>>>>> approach basically restores the state we find when some thread just
>>>>>>>>>>> acquired the lock in user space.
>>>>>>>>>> Yes, I did not think about the stealing when writing my test, but I
>>>>>>>>>> think it could be a good idea to add it to the test, especially if 
>>>>>>>>>> you
>>>>>>>>>> want to port the test to the native API.
>>>>>>>>>> I let Philippe decide here. He is the one who did the stealing stuff 
>>>>>>>>>> and
>>>>>>>>>> probably knows better.
>>>>>>>>> Currently, the xnsynch strongly couples PIP and ownership, which 
>>>>>>>>> seems to impede
>>>>>>>>> your various proposals. I would suggest to decouple that: the basic 
>>>>>>>>> property of
>>>>>>>>> some xnsynch that we may want to handle is exclusiveness, then 
>>>>>>>>> dynamic priority
>>>>>>>>> inheritance is another property, that could stack its own semantics 
>>>>>>>>> on top of
>>>>>>>>> exclusiveness.
>>>>>>>>> XNSYNCH_EXCLUSIVE would cover all ownership-related actions, 
>>>>>>>>> XNSYNCH_PIP would
>>>>>>>>> simply add dynamic priority management. Non exclusive object would 
>>>>>>>>> not require
>>>>>>>>> any xnsynch_set_owner() handling.
>>>>>>>>> Just to give a clear signal here: I will happily consider any change 
>>>>>>>>> to the
>>>>>>>>> xnsynch object that may ease the implementation of fast ownership 
>>>>>>>>> handling (i.e.
>>>>>>>>> userland-to-userland transfer). The only thing is that such code is 
>>>>>>>>> very much
>>>>>>>>> prone to regressions, so a testsuite must come with core changes in 
>>>>>>>>> that area,
>>>>>>>>> but I guess you know that already.
>>>>>>>> Ok. I think unit_mutex.c is a good start. It only lacks testing
>>>>>>>> XNROBBED.
>>>>>>> My colleague sent me an extension. It's native-only so far, but it
>>>>>>> already pointed out a bug in my try-acquire implementation that should
>>>>>>> be present in posix as well (trylock must go through the slow path).
>>>>>> I do not see why. If mutex lock can lock without a syscall, the same
>>>>>> goes for trylock.
>>>>> Lock stealing requires the slow path.
>>>> Ah ok. I thought you mean that trylock had to go systematically through
>>>> syscall.
>>>> As for lock stealing, I already said that it was not tested in the
>>>> current test.
>>> In fact, that bug is also present the current native skin (SVN, 2.4.x
>>> should suffer as well). Probably also current 2.4.x posix is affected,
>>> at least from a first glance.
>> Well, yes, now that you mention it, calling xnsynch_sleep_on does not
>> seem obvious for the implementation of a trylock.
> Don't even try calling xnsynch_sleep_on with a nonblocking-type timeout.

Works nicely today (using (XN_NONBLOCK, XN_RELATIVE)) due to early
timeout checks in xntimer_start. You just have to translate XNTIMEO into
EWOULDBLOCK on return.


Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

Xenomai-core mailing list

Reply via email to