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. Jan PS: Handle leak fixed, #4156, was SVN-only, I caused it. -- 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
