On 05/24/2011 02:23 PM, Jan Kiszka wrote:
> On 2011-05-24 12:41, Gilles Chanteperdrix wrote:
>> On 05/24/2011 12:36 PM, Jan Kiszka wrote:
>>> On 2011-05-24 11:58, Gilles Chanteperdrix wrote:
>>>> On 05/24/2011 11:36 AM, Jan Kiszka wrote:
>>>>> On 2011-05-24 11:32, Gilles Chanteperdrix wrote:
>>>>>> On 05/24/2011 11:13 AM, Jan Kiszka wrote:
>>>>>>> On 2011-05-24 06:31, Gilles Chanteperdrix wrote:
>>>>>>>> On 05/23/2011 03:53 PM, Jan Kiszka wrote:
>>>>>>>>> The following changes since commit
>>>>>>>>> aec30a2543afa18fa7832deee85e187b0faeb1f0:
>>>>>>>>>
>>>>>>>>> xeno-test: fix reference to @XENO_TEST_DIR@ (2011-05-15 21:20:41
>>>>>>>>> +0200)
>>>>>>>>>
>>>>>>>>> are available in the git repository at:
>>>>>>>>> git://git.xenomai.org/xenomai-jki.git for-upstream
>>>>>>>>>
>>>>>>>>> Jan Kiszka (1):
>>>>>>>>> native: Fix msendq fastlock leakage
>>>>>>>>>
>>>>>>>>> include/native/task.h | 5 +++++
>>>>>>>>> ksrc/skins/native/task.c | 13 ++++++-------
>>>>>>>>> 2 files changed, 11 insertions(+), 7 deletions(-)
>>>>>>>>>
>>>>>>>>> ------8<------
>>>>>>>>>
>>>>>>>>> When a native task terminates without going through rt_task_delete, we
>>>>>>>>> leaked the fastlock so far. Fix it by moving the release into the
>>>>>>>>> delete
>>>>>>>>> hook. As the ppd is already invalid at that point, we have to save the
>>>>>>>>> heap address in the task data structure.
>>>>>>>>
>>>>>>>> I Jan, I once worked on a patch to reverse the ppd cleanup order, in
>>>>>>>> order
>>>>>>>> to fix bugs of this kind. Here it comes. I do not remember why I did
>>>>>>>> not
>>>>>>>> commit it, but I guess it was not working well. Could we restart
>>>>>>>> working
>>>>>>>> from this patch?
>>>>>>>>
>>>>>>>> From 038ecf08cd66b3112e0fe277d71d294b8eb83bcc Mon Sep 17 00:00:00 2001
>>>>>>>> From: Gilles Chanteperdrix <[email protected]>
>>>>>>>> Date: Sun, 29 Aug 2010 14:52:08 +0200
>>>>>>>> Subject: [PATCH] nucleus: reverse ppd cleanup order
>>>>>>>>
>>>>>>>> ---
>>>>>>>> ksrc/nucleus/shadow.c | 11 ++++++-----
>>>>>>>> 1 files changed, 6 insertions(+), 5 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/ksrc/nucleus/shadow.c b/ksrc/nucleus/shadow.c
>>>>>>>> index b2d4326..725ae43 100644
>>>>>>>> --- a/ksrc/nucleus/shadow.c
>>>>>>>> +++ b/ksrc/nucleus/shadow.c
>>>>>>>> @@ -556,7 +556,7 @@ static unsigned ppd_lookup_inner(xnqueue_t **pq,
>>>>>>>> }
>>>>>>>> while (holder &&
>>>>>>>> (ppd->key.mm < pkey->mm ||
>>>>>>>> - (ppd->key.mm == pkey->mm && ppd->key.muxid <
>>>>>>>> pkey->muxid)));
>>>>>>>> + (ppd->key.mm == pkey->mm && ppd->key.muxid >
>>>>>>>> pkey->muxid)));
>>>>>>>>
>>>>>>>> if (ppd->key.mm == pkey->mm && ppd->key.muxid == pkey->muxid) {
>>>>>>>> /* found it, return it. */
>>>>>>>> @@ -566,7 +566,7 @@ static unsigned ppd_lookup_inner(xnqueue_t **pq,
>>>>>>>>
>>>>>>>> /* not found, return successor for insertion. */
>>>>>>>> if (ppd->key.mm < pkey->mm ||
>>>>>>>> - (ppd->key.mm == pkey->mm && ppd->key.muxid < pkey->muxid))
>>>>>>>> + (ppd->key.mm == pkey->mm && ppd->key.muxid > pkey->muxid))
>>>>>>>> *pholder = holder ? link2ppd(holder) : NULL;
>>>>>>>> else
>>>>>>>> *pholder = ppd;
>>>>>>>> @@ -589,10 +589,11 @@ static int ppd_insert(xnshadow_ppd_t * holder)
>>>>>>>> }
>>>>>>>>
>>>>>>>> inith(&holder->link);
>>>>>>>> - if (next)
>>>>>>>> + if (next) {
>>>>>>>> insertq(q, &next->link, &holder->link);
>>>>>>>> - else
>>>>>>>> + } else {
>>>>>>>> appendq(q, &holder->link);
>>>>>>>> + }
>>>>>>>> xnlock_put_irqrestore(&nklock, s);
>>>>>>>>
>>>>>>>> return 0;
>>>>>>>> @@ -640,7 +641,7 @@ static inline void ppd_remove_mm(struct mm_struct
>>>>>>>> *mm,
>>>>>>>> xnqueue_t *q;
>>>>>>>> spl_t s;
>>>>>>>>
>>>>>>>> - key.muxid = 0;
>>>>>>>> + key.muxid = ~0UL;
>>>>>>>> key.mm = mm;
>>>>>>>> xnlock_get_irqsave(&nklock, s);
>>>>>>>> ppd_lookup_inner(&q, &ppd, &key);
>>>>>>>
>>>>>>> Unfortunately, that won't help. I think we are forced to clear
>>>>>>> xnshadow_thrptd before calling into xnpod_delete_thread, but we would
>>>>>>> need that for xnshadow_ppd_get (=>xnpod_userspace_p()).
>>>>>>
>>>>>> I remember that now. Even if it worked, when the cleanup handler is
>>>>>> called, current->mm is NULL. We need to do this differently, the sys ppd
>>>>>> should be treated differently and passed to the other ppds cleanup
>>>>>> routines.
>>>>>
>>>>> Do you already have an idea how to get that info to the delete hook
>>>>> function?
>>>>
>>>> Yes. We start by not applying the list reversal patch, then the sys_ppd
>>>> is the first in the list. So, we can, in the function ppd_remove_mm,
>>>> start by removing all the others ppd, then remove the sys ppd (that is
>>>> the first), last. This changes a few signatures in the core code, a lot
>>>> of things in the skin code, but that would be for the better...
>>>
>>> I still don't see how this affects the order we use in
>>> do_taskexit_event, the one that prevents xnsys_get_ppd usage even when
>>> the mm is still present.
>>
>> The idea is to change the cleanup routines not to call xnsys_get_ppd.
>
> ...and use what instead? Sorry, I'm slow today.
The sys_ppd passed as other argument to the cleanup function.
>
> Jan
>
--
Gilles.
_______________________________________________
Xenomai-core mailing list
[email protected]
https://mail.gna.org/listinfo/xenomai-core
