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.
And by the way, I think that with the mutex/xnsynch shared owner, we can
implement stealing without a syscall.
--
Gilles.
_______________________________________________
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core
