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. Jan -- Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux _______________________________________________ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
