[Xenomai] rtdm_mutex_init on forge

[Xenomai] rtdm_mutex_init on forge

[Matthias Schneider]

Hi all,

I have a question about rtdm_mutex_init in forge. BUG_ON is 
triggered each time I create an rtdm_mutex. This seems to be 
due to a check in xnsynch_init, while apparently some 
implementation is still missing in rtdm_mutex_init(): 

void rtdm_mutex_init(rtdm_mutex_t *mutex)
{
        spl_t s;

        /* Make atomic for re-initialisation support */
        xnlock_get_irqsave(&nklock, s);

        xnsynch_init(&mutex->synch_base,
                     XNSYNCH_PRIO | XNSYNCH_PIP | XNSYNCH_OWNER, NULL);

        xnlock_put_irqrestore(&nklock, s);
}


void xnsynch_init(struct xnsynch *synch, int flags, atomic_long_t *fastlock)
{
        if (flags & XNSYNCH_PIP)
                flags |= XNSYNCH_PRIO | XNSYNCH_OWNER;  /* Obviously... */

        synch->status = flags & ~XNSYNCH_CLAIMED;
        synch->owner = NULL;
        synch->cleanup = NULL;  /* Only works for PIP-enabled objects. */
        synch->wprio = -1;
        INIT_LIST_HEAD(&synch->pendq);

        if (flags & XNSYNCH_OWNER) {
                BUG_ON(fastlock == NULL); // <---------- Obviously this is
                                          //             always occurs for an 
                                          //             rtdm_mutex.
                synch->fastlock = fastlock;
                atomic_long_set(fastlock, XN_NO_HANDLE);
        } else
                synch->fastlock = NULL;
}

At the moment I am not sure if the atomic_long_t counter needs be added to 
rtdm_mutex or the flags need to be changed. Thanks for your help,

Matthias

Re: [Xenomai] rtdm_mutex_init on forge

[Philippe Gerum]

On 05/29/2014 05:42 PM, Matthias Schneider wrote:
> Hi all,
>
> I have a question about rtdm_mutex_init in forge. BUG_ON is
> triggered each time I create an rtdm_mutex. This seems to be
> due to a check in xnsynch_init, while apparently some
> implementation is still missing in rtdm_mutex_init():
>
> void rtdm_mutex_init(rtdm_mutex_t *mutex)
> {
>          spl_t s;
>
>          /* Make atomic for re-initialisation support */
>          xnlock_get_irqsave(&nklock, s);
>
>          xnsynch_init(&mutex->synch_base,
>                       XNSYNCH_PRIO | XNSYNCH_PIP | XNSYNCH_OWNER, NULL);
>
>          xnlock_put_irqrestore(&nklock, s);
> }
>
>
> void xnsynch_init(struct xnsynch *synch, int flags, atomic_long_t *fastlock)
> {
>          if (flags & XNSYNCH_PIP)
>                  flags |= XNSYNCH_PRIO | XNSYNCH_OWNER;  /* Obviously... */
>
>          synch->status = flags & ~XNSYNCH_CLAIMED;
>          synch->owner = NULL;
>          synch->cleanup = NULL;  /* Only works for PIP-enabled objects. */
>          synch->wprio = -1;
>          INIT_LIST_HEAD(&synch->pendq);
>
>          if (flags & XNSYNCH_OWNER) {
>                  BUG_ON(fastlock == NULL); // <---------- Obviously this is
>                                            //             always occurs for an
>                                            //             rtdm_mutex.
>                  synch->fastlock = fastlock;
>                  atomic_long_set(fastlock, XN_NO_HANDLE);
>          } else
>                  synch->fastlock = NULL;
> }
>
> At the moment I am not sure if the atomic_long_t counter needs be added to
> rtdm_mutex or the flags need to be changed. Thanks for your help,
>

RTDM mutexes cannot and do not have to provide fast locking memory, the 
assertion I have added to xnsynch_init() is broken. Will fix.
-- 
Philippe.

Re: [Xenomai] rtdm_mutex_init on forge

[Philippe Gerum]

On 05/29/2014 06:19 PM, Philippe Gerum wrote:
> On 05/29/2014 05:42 PM, Matthias Schneider wrote:
>> Hi all,
>>
>> I have a question about rtdm_mutex_init in forge. BUG_ON is
>> triggered each time I create an rtdm_mutex. This seems to be
>> due to a check in xnsynch_init, while apparently some
>> implementation is still missing in rtdm_mutex_init():
>>
>> void rtdm_mutex_init(rtdm_mutex_t *mutex)
>> {
>>          spl_t s;
>>
>>          /* Make atomic for re-initialisation support */
>>          xnlock_get_irqsave(&nklock, s);
>>
>>          xnsynch_init(&mutex->synch_base,
>>                       XNSYNCH_PRIO | XNSYNCH_PIP | XNSYNCH_OWNER, NULL);
>>
>>          xnlock_put_irqrestore(&nklock, s);
>> }
>>
>>
>> void xnsynch_init(struct xnsynch *synch, int flags, atomic_long_t
>> *fastlock)
>> {
>>          if (flags & XNSYNCH_PIP)
>>                  flags |= XNSYNCH_PRIO | XNSYNCH_OWNER;  /*
>> Obviously... */
>>
>>          synch->status = flags & ~XNSYNCH_CLAIMED;
>>          synch->owner = NULL;
>>          synch->cleanup = NULL;  /* Only works for PIP-enabled
>> objects. */
>>          synch->wprio = -1;
>>          INIT_LIST_HEAD(&synch->pendq);
>>
>>          if (flags & XNSYNCH_OWNER) {
>>                  BUG_ON(fastlock == NULL); // <---------- Obviously
>> this is
>>                                            //             always
>> occurs for an
>>                                            //             rtdm_mutex.
>>                  synch->fastlock = fastlock;
>>                  atomic_long_set(fastlock, XN_NO_HANDLE);
>>          } else
>>                  synch->fastlock = NULL;
>> }
>>
>> At the moment I am not sure if the atomic_long_t counter needs be
>> added to
>> rtdm_mutex or the flags need to be changed. Thanks for your help,
>>
>
> RTDM mutexes cannot and do not have to provide fast locking memory, the
> assertion I have added to xnsynch_init() is broken. Will fix.

Yes, the intent was to acknowledge the fact that we know how to 
implement fast locking on all supported arch, rtdm was overlooked. So 
this spurious assertion builds on the wrong assumption that a NULL 
fastlock cannot happen with priority inversion protected locks.

-- 
Philippe.

Re: [Xenomai] rtdm_mutex_init on forge

[Gilles Chanteperdrix]

On 05/29/2014 06:30 PM, Philippe Gerum wrote:
> On 05/29/2014 06:19 PM, Philippe Gerum wrote:
>> On 05/29/2014 05:42 PM, Matthias Schneider wrote:
>>> Hi all,
>>>
>>> I have a question about rtdm_mutex_init in forge. BUG_ON is
>>> triggered each time I create an rtdm_mutex. This seems to be
>>> due to a check in xnsynch_init, while apparently some
>>> implementation is still missing in rtdm_mutex_init():
>>>
>>> void rtdm_mutex_init(rtdm_mutex_t *mutex)
>>> {
>>>          spl_t s;
>>>
>>>          /* Make atomic for re-initialisation support */
>>>          xnlock_get_irqsave(&nklock, s);
>>>
>>>          xnsynch_init(&mutex->synch_base,
>>>                       XNSYNCH_PRIO | XNSYNCH_PIP | XNSYNCH_OWNER, NULL);
>>>
>>>          xnlock_put_irqrestore(&nklock, s);
>>> }
>>>
>>>
>>> void xnsynch_init(struct xnsynch *synch, int flags, atomic_long_t
>>> *fastlock)
>>> {
>>>          if (flags & XNSYNCH_PIP)
>>>                  flags |= XNSYNCH_PRIO | XNSYNCH_OWNER;  /*
>>> Obviously... */
>>>
>>>          synch->status = flags & ~XNSYNCH_CLAIMED;
>>>          synch->owner = NULL;
>>>          synch->cleanup = NULL;  /* Only works for PIP-enabled
>>> objects. */
>>>          synch->wprio = -1;
>>>          INIT_LIST_HEAD(&synch->pendq);
>>>
>>>          if (flags & XNSYNCH_OWNER) {
>>>                  BUG_ON(fastlock == NULL); // <---------- Obviously
>>> this is
>>>                                            //             always
>>> occurs for an
>>>                                            //             rtdm_mutex.
>>>                  synch->fastlock = fastlock;
>>>                  atomic_long_set(fastlock, XN_NO_HANDLE);
>>>          } else
>>>                  synch->fastlock = NULL;
>>> }
>>>
>>> At the moment I am not sure if the atomic_long_t counter needs be
>>> added to
>>> rtdm_mutex or the flags need to be changed. Thanks for your help,
>>>
>>
>> RTDM mutexes cannot and do not have to provide fast locking memory, the
>> assertion I have added to xnsynch_init() is broken. Will fix.
> 
> Yes, the intent was to acknowledge the fact that we know how to 
> implement fast locking on all supported arch, rtdm was overlooked. So 
> this spurious assertion builds on the wrong assumption that a NULL 
> fastlock cannot happen with priority inversion protected locks.
> 

Adding fastlock to RTDM mutexes looks possible, and would create a
lockless fast patch for them as well.

-- 
                                                                Gilles.

Re: [Xenomai] rtdm_mutex_init on forge

[Philippe Gerum]

On 05/29/2014 06:34 PM, Gilles Chanteperdrix wrote:
> On 05/29/2014 06:30 PM, Philippe Gerum wrote:
>> On 05/29/2014 06:19 PM, Philippe Gerum wrote:
>>> On 05/29/2014 05:42 PM, Matthias Schneider wrote:
>>>> Hi all,
>>>>
>>>> I have a question about rtdm_mutex_init in forge. BUG_ON is
>>>> triggered each time I create an rtdm_mutex. This seems to be
>>>> due to a check in xnsynch_init, while apparently some
>>>> implementation is still missing in rtdm_mutex_init():
>>>>
>>>> void rtdm_mutex_init(rtdm_mutex_t *mutex)
>>>> {
>>>>           spl_t s;
>>>>
>>>>           /* Make atomic for re-initialisation support */
>>>>           xnlock_get_irqsave(&nklock, s);
>>>>
>>>>           xnsynch_init(&mutex->synch_base,
>>>>                        XNSYNCH_PRIO | XNSYNCH_PIP | XNSYNCH_OWNER, NULL);
>>>>
>>>>           xnlock_put_irqrestore(&nklock, s);
>>>> }
>>>>
>>>>
>>>> void xnsynch_init(struct xnsynch *synch, int flags, atomic_long_t
>>>> *fastlock)
>>>> {
>>>>           if (flags & XNSYNCH_PIP)
>>>>                   flags |= XNSYNCH_PRIO | XNSYNCH_OWNER;  /*
>>>> Obviously... */
>>>>
>>>>           synch->status = flags & ~XNSYNCH_CLAIMED;
>>>>           synch->owner = NULL;
>>>>           synch->cleanup = NULL;  /* Only works for PIP-enabled
>>>> objects. */
>>>>           synch->wprio = -1;
>>>>           INIT_LIST_HEAD(&synch->pendq);
>>>>
>>>>           if (flags & XNSYNCH_OWNER) {
>>>>                   BUG_ON(fastlock == NULL); // <---------- Obviously
>>>> this is
>>>>                                             //             always
>>>> occurs for an
>>>>                                             //             rtdm_mutex.
>>>>                   synch->fastlock = fastlock;
>>>>                   atomic_long_set(fastlock, XN_NO_HANDLE);
>>>>           } else
>>>>                   synch->fastlock = NULL;
>>>> }
>>>>
>>>> At the moment I am not sure if the atomic_long_t counter needs be
>>>> added to
>>>> rtdm_mutex or the flags need to be changed. Thanks for your help,
>>>>
>>>
>>> RTDM mutexes cannot and do not have to provide fast locking memory, the
>>> assertion I have added to xnsynch_init() is broken. Will fix.
>>
>> Yes, the intent was to acknowledge the fact that we know how to
>> implement fast locking on all supported arch, rtdm was overlooked. So
>> this spurious assertion builds on the wrong assumption that a NULL
>> fastlock cannot happen with priority inversion protected locks.
>>
>
> Adding fastlock to RTDM mutexes looks possible, and would create a
> lockless fast patch for them as well.
>

yes, this is the way I intend to fix this if possible (did not see any 
showstopper yet): make the fastlock a mandatory feature including for 
rtdm. The fastlock area could just live in the rtdm mutex descriptor.

-- 
Philippe.

Re: [Xenomai] rtdm_mutex_init on forge

[Philippe Gerum]

On 05/29/2014 05:42 PM, Matthias Schneider wrote:
> Hi all,
>
> I have a question about rtdm_mutex_init in forge. BUG_ON is
> triggered each time I create an rtdm_mutex. This seems to be
> due to a check in xnsynch_init, while apparently some
> implementation is still missing in rtdm_mutex_init():
>

Could you please give a try to this patch? It implements fast locking 
for RTDM mutexes, which should fix the issue:

http://git.xenomai.org/xenomai-forge.git/commit/?h=next&id=c00754093d283247695356d416608daab0979786

TIA,

-- 
Philippe.

Re: [Xenomai] rtdm_mutex_init on forge

[Matthias Schneider]

----- Original Message -----

> From: Philippe Gerum <rpm@xenomai.org>
> To: Matthias Schneider <ma30002000@yahoo.de>; "xenomai@xenomai.org" <xenomai@xenomai.org>
> Cc: 
> Sent: Thursday, May 29, 2014 9:37 PM
> Subject: Re: [Xenomai] rtdm_mutex_init on forge
> 
> On 05/29/2014 05:42 PM, Matthias Schneider wrote:
> 
>>  Hi all,
>> 
>>  I have a question about rtdm_mutex_init in forge. BUG_ON is
>>  triggered each time I create an rtdm_mutex. This seems to be
>>  due to a check in xnsynch_init, while apparently some
>>  implementation is still missing in rtdm_mutex_init():
>> 
> 
> Could you please give a try to this patch? It implements fast locking 
> for RTDM mutexes, which should fix the issue:
> 
> http://git.xenomai.org/xenomai-forge.git/commit/?h=next&id=c00754093d283247695356d416608daab0979786
> 
> TIA,
> 
> -- 
> Philippe.
> 
Again, thanks for the fix. It seems I am now possible to lock
and unlock an rtdm mutex without causing exceptions. However, 
I have not confirmed that it is actually guaranteeing mutual 
exclusivity - I suppose that this was already verified previously
by some kind of unit test.

Regards,
Matthias