[PATCH 29/30] leon, greth: SMP support by using spin-lock protection

[Sebastian Huber]

On 02/05/17 10:51, Daniel Hellstrom wrote:
>
> On 2017-05-02 10:27, Sebastian Huber wrote:
>>
>>
>> On 02/05/17 10:16, Daniel Hellstrom wrote:
>>> On 2017-05-02 10:00, Sebastian Huber wrote:
>>>> On 02/05/17 09:32, Daniel Hellstrom wrote:
>>>>> On 2017-05-02 07:48, Sebastian Huber wrote:
>>>>>>
>>>>>>
>>>>>> On 13/04/17 21:31, Daniel Hellstrom wrote:
>>>>>>> ---
>>>>>>>   c/src/lib/libbsp/sparc/shared/net/greth.c | 134 
>>>>>>> ++++++++++++++++--------------
>>>>>>>   1 file changed, 71 insertions(+), 63 deletions(-)
>>>>>>>
>>>>>>> diff --git a/c/src/lib/libbsp/sparc/shared/net/greth.c 
>>>>>>> b/c/src/lib/libbsp/sparc/shared/net/greth.c
>>>>>>> index 7726799..9af2de2 100644
>>>>>>> --- a/c/src/lib/libbsp/sparc/shared/net/greth.c
>>>>>>> +++ b/c/src/lib/libbsp/sparc/shared/net/greth.c
>>>>>>> @@ -41,6 +41,18 @@
>>>>>>>   #include <netinet/in.h>
>>>>>>>   #include <netinet/if_ether.h>
>>>>>>>   +#include <rtems/score/isrlock.h> /* spin-lock */
>>>>>>> +
>>>>>>> +/* map via ISR lock: */
>>>>>>> +#define SPIN_DECLARE(lock) ISR_LOCK_MEMBER(lock)
>>>>>>> +#define SPIN_INIT(lock, name) _ISR_lock_Initialize(lock, name)
>>>>>>> +#define SPIN_LOCK(lock, level) _ISR_lock_Acquire_inline(lock, 
>>>>>>> &level)
>>>>>>> +#define SPIN_LOCK_IRQ(lock, level) 
>>>>>>> _ISR_lock_ISR_disable_and_acquire(lock, &level)
>>>>>>> +#define SPIN_UNLOCK(lock, level) _ISR_lock_Release_inline(lock, 
>>>>>>> &level)
>>>>>>> +#define SPIN_UNLOCK_IRQ(lock, level) 
>>>>>>> _ISR_lock_Release_and_ISR_enable(lock, &level)
>>>>>>> +#define SPIN_IRQFLAGS(k) ISR_lock_Context k
>>>>>>> +#define SPIN_ISR_IRQFLAGS(k) SPIN_IRQFLAGS(k)
>>>>>>
>>>>>> Could you please use the rtems_interrupt_lock_*() API and not the 
>>>>>> internal _ISR_lock_*() stuff.
>>>>>
>>>>> Thanks. I think the reason for this may come from that the inline 
>>>>> equivalent. I will add a patch which adds inline macros to the 
>>>>> rtems_interrupt_lock interface instead. I can't see any reason why 
>>>>> that can not be supported?
>>>>
>>>> What do you mean with inline equivalent?
>>> I mean we use _ISR_lock_Acquire_inline(lock, &level) from the ISR to 
>>> avoid an extra function call. But there is only a 
>>> rtems_interrupt_lock_acquire_isr() no 
>>> rtems_interrupt_lock_acquire_INLINE_isr() which we would like to use.
>>
>> Ok, but does this extra function call really matter? If you think so, 
>> then please add the *_inline variants to the rtems_interrupt_lock_* 
>> API (with tests) and document it in the user manual.
> Ok. I will add that patch afterwards, first I will fix our patches to 
> use the correct existing macros. 

Ok. You can commit the patch set from my point of view (with the minor 
fixes).

> On SPARC it would be nice to avoid the extra window SAVE within the ISR.

Since, greth_interrupt() conditionally calls rtems_bsdnet_event_send() 
the greth_interrupt() already uses a SAVE, the SMP lock acquire is a 
tail function.

-- 
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax     : +49 89 189 47 41-09
E-Mail  : sebastian.huber at embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.