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. -- Gilles. _______________________________________________ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core