Was last Barrier Patch OK?
Till Straumann
strauman at slac.stanford.edu
Tue Mar 7 21:23:02 UTC 2006
Till Straumann wrote:
> Thomas Doerfler wrote:
>
>> Joel,
>>
>> how often will I look at the patch and find bugs again, sigh!
>
>
> Darn! Had missed that one,too. How long will it take 4 people to
> get this right???!!! It's a *#@$ JOKE
>
No - I hadn't missed it. This was a newly introduced one.
Joel, I guess you have to come up with a new 'final' version
and we'll have to double-check everything again...
reminder - it needs to be
apply protection
BARRIER
critical section
BARRIER
remove protection
sigh.
T.
> ;-)
> T.
>
>>
>> In the macro _Thread_Disable_dispatch(), the memory barrier seems to
>> be misplaced.
>>
>> I have corrected it (hopefully) and nothing else in the attached patch.
>>
>> So the temperature in Alabama is almost similar to munich, it seems.
>> we got half a meter of snow this weekend, this is the absolute record
>> at least for a century. And it just starts snowing again =8>
>>
>> wkr,
>> Thomas.
>>
>>
>> Joel Sherrill wrote:
>>
>>> Thomas Doerfler (nt) wrote:
>>>
>>>> -----BEGIN PGP SIGNED MESSAGE-----
>>>> Hash: SHA1
>>>>
>>>> Till,
>>>>
>>>> ups, you got it, I actually missed that one when I checked the patch.
>>>>
>>>> Joel, can you fix it (AFTER! the camping trip with your boys)?
>>>>
>>>> wkr,
>>>> Thomas.
>>>>
>>>>
>>>>
>>>> Till Straumann schrieb:
>>>>
>>>>
>>>>> Sorry, NO, it's still *not* OK.
>>>>>
>>>>> - the '.inl' version of _Thread_Unnest_dispatch()
>>>>> has the barrier after the decrement operation instead
>>>>> of before.
>>>>>
>>>>> Looks like you could use some vacation...
>>>>>
>>>>> ;-)
>>>>>
>>>>> -- T.
>>>>>
>>>
>>>
>>>
>>>
>>> Yeah and time to recover from it afterwards. :) We were near Auburn
>>> Alabama and it got down
>>> below freezing Friday night. The days were warm and sunny while the
>>> nights were cold. Combine
>>> that with the 4 hour drive there and I didn't get much sleep the
>>> first night. I will barely have time to
>>> catch up on my sleep before doing another trip to Montgomery this
>>> weekend. One of my sons is on
>>> the school Science Olympiad team which won the regional competition
>>> and is going to state!
>>>
>>> Anyway I think I have this all fixed in the attached patch.
>>> critical section enter
>>> barrier
>>> protected operation
>>> barrier
>>> critical section exit
>>>
>>> One other thought. Can gcc move the critical section enter earlier
>>> and exit later to include more
>>> in the critical section?
>>> --joel
>>>
>>>
>>> ------------------------------------------------------------------------
>>>
>>> Index: cpukit/score/include/rtems/system.h
>>> ===================================================================
>>> RCS file: /usr1/CVS/rtems/cpukit/score/include/rtems/system.h,v
>>> retrieving revision 1.32.2.5
>>> diff -u -r1.32.2.5 system.h
>>> --- cpukit/score/include/rtems/system.h 14 Jan 2005 05:23:21
>>> -0000 1.32.2.5
>>> +++ cpukit/score/include/rtems/system.h 7 Mar 2006 12:14:04 -0000
>>> @@ -102,6 +102,17 @@
>>> #endif
>>>
>>> /*
>>> + * The following macro is a compiler specific way to ensure that
>>> memory
>>> + * writes are not reordered around certian points. This
>>> specifically can
>>> + * impact interrupt disable and thread dispatching critical sections.
>>> + */
>>> +#ifdef __GNUC__
>>> + #define RTEMS_COMPILER_MEMORY_BARRIER() asm volatile("" ::: "memory")
>>> +#else
>>> + #define RTEMS_COMPILER_MEMORY_BARRIER()
>>> +#endif
>>> +
>>> +/*
>>> * The following are used by the POSIX implementation to catch bad
>>> paths.
>>> */
>>>
>>> Index: cpukit/score/include/rtems/score/isr.h
>>> ===================================================================
>>> RCS file: /usr1/CVS/rtems/cpukit/score/include/rtems/score/isr.h,v
>>> retrieving revision 1.17.2.1
>>> diff -u -r1.17.2.1 isr.h
>>> --- cpukit/score/include/rtems/score/isr.h 4 Sep 2003 18:55:05
>>> -0000 1.17.2.1
>>> +++ cpukit/score/include/rtems/score/isr.h 7 Mar 2006 12:14:04 -0000
>>> @@ -111,7 +111,10 @@
>>> */
>>>
>>> #define _ISR_Disable( _level ) \
>>> - _CPU_ISR_Disable( _level )
>>> + do { \
>>> + _CPU_ISR_Disable( _level ); \
>>> + RTEMS_COMPILER_MEMORY_BARRIER(); \
>>> + } while (0)
>>>
>>> /*
>>> * _ISR_Enable
>>> @@ -124,7 +127,10 @@
>>> */
>>>
>>> #define _ISR_Enable( _level ) \
>>> - _CPU_ISR_Enable( _level )
>>> + do { \
>>> + RTEMS_COMPILER_MEMORY_BARRIER(); \
>>> + _CPU_ISR_Enable( _level ); \
>>> + } while (0)
>>>
>>> /*
>>> * _ISR_Flash
>>> @@ -144,7 +150,11 @@
>>> */
>>>
>>> #define _ISR_Flash( _level ) \
>>> - _CPU_ISR_Flash( _level )
>>> + do { \
>>> + RTEMS_COMPILER_MEMORY_BARRIER(); \
>>> + _CPU_ISR_Flash( _level ); \
>>> + RTEMS_COMPILER_MEMORY_BARRIER(); \
>>> + } while (0)
>>>
>>> /*
>>> * _ISR_Install_vector
>>> Index: cpukit/score/inline/rtems/score/thread.inl
>>> ===================================================================
>>> RCS file: /usr1/CVS/rtems/cpukit/score/inline/rtems/score/thread.inl,v
>>> retrieving revision 1.17.2.2
>>> diff -u -r1.17.2.2 thread.inl
>>> --- cpukit/score/inline/rtems/score/thread.inl 15 Sep 2003
>>> 02:12:47 -0000 1.17.2.2
>>> +++ cpukit/score/inline/rtems/score/thread.inl 7 Mar 2006 12:14:04
>>> -0000
>>> @@ -184,6 +184,7 @@
>>> RTEMS_INLINE_ROUTINE void _Thread_Disable_dispatch( void )
>>> {
>>> _Thread_Dispatch_disable_level += 1;
>>> + RTEMS_COMPILER_MEMORY_BARRIER();
>>> }
>>>
>>> /*PAGE
>>> @@ -201,6 +202,7 @@
>>> #if ( CPU_INLINE_ENABLE_DISPATCH == TRUE )
>>> RTEMS_INLINE_ROUTINE void _Thread_Enable_dispatch()
>>> {
>>> + RTEMS_COMPILER_MEMORY_BARRIER();
>>> if ( (--_Thread_Dispatch_disable_level) == 0 )
>>> _Thread_Dispatch();
>>> }
>>> @@ -223,6 +225,7 @@
>>>
>>> RTEMS_INLINE_ROUTINE void _Thread_Unnest_dispatch( void )
>>> {
>>> + RTEMS_COMPILER_MEMORY_BARRIER();
>>> _Thread_Dispatch_disable_level -= 1;
>>> }
>>>
>>> Index: cpukit/score/macros/rtems/score/thread.inl
>>> ===================================================================
>>> RCS file: /usr1/CVS/rtems/cpukit/score/macros/rtems/score/thread.inl,v
>>> retrieving revision 1.12.2.1
>>> diff -u -r1.12.2.1 thread.inl
>>> --- cpukit/score/macros/rtems/score/thread.inl 4 Sep 2003 18:55:06
>>> -0000 1.12.2.1
>>> +++ cpukit/score/macros/rtems/score/thread.inl 7 Mar 2006 12:14:04
>>> -0000
>>> @@ -126,7 +126,10 @@
>>> */
>>>
>>> #define _Thread_Disable_dispatch() \
>>> - _Thread_Dispatch_disable_level += 1
>>> + do { \
>>> + RTEMS_COMPILER_MEMORY_BARRIER(); \
>>> + _Thread_Dispatch_disable_level += 1; \
>>> + } while (0)
>>>
>>> /*PAGE
>>> *
>>> @@ -136,9 +139,11 @@
>>>
>>> #if ( CPU_INLINE_ENABLE_DISPATCH == TRUE )
>>> #define _Thread_Enable_dispatch() \
>>> - { if ( (--_Thread_Dispatch_disable_level) == 0 ) \
>>> - _Thread_Dispatch(); \
>>> - }
>>> + do { \
>>> + RTEMS_COMPILER_MEMORY_BARRIER(); \
>>> + if ( (--_Thread_Dispatch_disable_level) == 0 ) \
>>> + _Thread_Dispatch(); \
>>> + } while (0)
>>> #endif
>>>
>>> #if ( CPU_INLINE_ENABLE_DISPATCH == FALSE )
>>> @@ -152,7 +157,10 @@
>>> */
>>>
>>> #define _Thread_Unnest_dispatch() \
>>> - _Thread_Dispatch_disable_level -= 1
>>> + do { \
>>> + RTEMS_COMPILER_MEMORY_BARRIER(); \
>>> + _Thread_Dispatch_disable_level -= 1; \
>>> + } while (0)
>>>
>>> /*PAGE
>>> *
>>
>>
>>
>>
>>
>> ------------------------------------------------------------------------
>>
>> Index: cpukit/score/include/rtems/system.h
>> ===================================================================
>> RCS file: /usr1/CVS/rtems/cpukit/score/include/rtems/system.h,v
>> retrieving revision 1.32.2.5
>> diff -u -r1.32.2.5 system.h
>> --- cpukit/score/include/rtems/system.h 14 Jan 2005 05:23:21
>> -0000 1.32.2.5
>> +++ cpukit/score/include/rtems/system.h 7 Mar 2006 12:14:04 -0000
>> @@ -102,6 +102,17 @@
>> #endif
>>
>> /*
>> + * The following macro is a compiler specific way to ensure that memory
>> + * writes are not reordered around certian points. This
>> specifically can
>> + * impact interrupt disable and thread dispatching critical sections.
>> + */
>> +#ifdef __GNUC__
>> + #define RTEMS_COMPILER_MEMORY_BARRIER() asm volatile("" ::: "memory")
>> +#else
>> + #define RTEMS_COMPILER_MEMORY_BARRIER()
>> +#endif
>> +
>> +/*
>> * The following are used by the POSIX implementation to catch bad
>> paths.
>> */
>>
>> Index: cpukit/score/include/rtems/score/isr.h
>> ===================================================================
>> RCS file: /usr1/CVS/rtems/cpukit/score/include/rtems/score/isr.h,v
>> retrieving revision 1.17.2.1
>> diff -u -r1.17.2.1 isr.h
>> --- cpukit/score/include/rtems/score/isr.h 4 Sep 2003 18:55:05
>> -0000 1.17.2.1
>> +++ cpukit/score/include/rtems/score/isr.h 7 Mar 2006 12:14:04 -0000
>> @@ -111,7 +111,10 @@
>> */
>>
>> #define _ISR_Disable( _level ) \
>> - _CPU_ISR_Disable( _level )
>> + do { \
>> + _CPU_ISR_Disable( _level ); \
>> + RTEMS_COMPILER_MEMORY_BARRIER(); \
>> + } while (0)
>>
>> /*
>> * _ISR_Enable
>> @@ -124,7 +127,10 @@
>> */
>>
>> #define _ISR_Enable( _level ) \
>> - _CPU_ISR_Enable( _level )
>> + do { \
>> + RTEMS_COMPILER_MEMORY_BARRIER(); \
>> + _CPU_ISR_Enable( _level ); \
>> + } while (0)
>>
>> /*
>> * _ISR_Flash
>> @@ -144,7 +150,11 @@
>> */
>>
>> #define _ISR_Flash( _level ) \
>> - _CPU_ISR_Flash( _level )
>> + do { \
>> + RTEMS_COMPILER_MEMORY_BARRIER(); \
>> + _CPU_ISR_Flash( _level ); \
>> + RTEMS_COMPILER_MEMORY_BARRIER(); \
>> + } while (0)
>>
>> /*
>> * _ISR_Install_vector
>> Index: cpukit/score/inline/rtems/score/thread.inl
>> ===================================================================
>> RCS file: /usr1/CVS/rtems/cpukit/score/inline/rtems/score/thread.inl,v
>> retrieving revision 1.17.2.2
>> diff -u -r1.17.2.2 thread.inl
>> --- cpukit/score/inline/rtems/score/thread.inl 15 Sep 2003 02:12:47
>> -0000 1.17.2.2
>> +++ cpukit/score/inline/rtems/score/thread.inl 7 Mar 2006 12:14:04
>> -0000
>> @@ -184,6 +184,7 @@
>> RTEMS_INLINE_ROUTINE void _Thread_Disable_dispatch( void )
>> {
>> _Thread_Dispatch_disable_level += 1;
>> + RTEMS_COMPILER_MEMORY_BARRIER();
>> }
>>
>> /*PAGE
>> @@ -201,6 +202,7 @@
>> #if ( CPU_INLINE_ENABLE_DISPATCH == TRUE )
>> RTEMS_INLINE_ROUTINE void _Thread_Enable_dispatch()
>> {
>> + RTEMS_COMPILER_MEMORY_BARRIER();
>> if ( (--_Thread_Dispatch_disable_level) == 0 )
>> _Thread_Dispatch();
>> }
>> @@ -223,6 +225,7 @@
>>
>> RTEMS_INLINE_ROUTINE void _Thread_Unnest_dispatch( void )
>> {
>> + RTEMS_COMPILER_MEMORY_BARRIER();
>> _Thread_Dispatch_disable_level -= 1;
>> }
>>
>> Index: cpukit/score/macros/rtems/score/thread.inl
>> ===================================================================
>> RCS file: /usr1/CVS/rtems/cpukit/score/macros/rtems/score/thread.inl,v
>> retrieving revision 1.12.2.1
>> diff -u -r1.12.2.1 thread.inl
>> --- cpukit/score/macros/rtems/score/thread.inl 4 Sep 2003 18:55:06
>> -0000 1.12.2.1
>> +++ cpukit/score/macros/rtems/score/thread.inl 7 Mar 2006 12:14:04
>> -0000
>> @@ -126,7 +126,10 @@
>> */
>>
>> #define _Thread_Disable_dispatch() \
>> - _Thread_Dispatch_disable_level += 1
>> + do { \
>> + _Thread_Dispatch_disable_level += 1; \
>> + RTEMS_COMPILER_MEMORY_BARRIER(); \
>> + } while (0)
>>
>> /*PAGE
>> *
>> @@ -136,9 +139,11 @@
>>
>> #if ( CPU_INLINE_ENABLE_DISPATCH == TRUE )
>> #define _Thread_Enable_dispatch() \
>> - { if ( (--_Thread_Dispatch_disable_level) == 0 ) \
>> - _Thread_Dispatch(); \
>> - }
>> + do { \
>> + RTEMS_COMPILER_MEMORY_BARRIER(); \
>> + if ( (--_Thread_Dispatch_disable_level) == 0 ) \
>> + _Thread_Dispatch(); \
>> + } while (0)
>> #endif
>>
>> #if ( CPU_INLINE_ENABLE_DISPATCH == FALSE )
>> @@ -152,7 +157,10 @@
>> */
>>
>> #define _Thread_Unnest_dispatch() \
>> - _Thread_Dispatch_disable_level -= 1
>> + do { \
>> + RTEMS_COMPILER_MEMORY_BARRIER(); \
>> + _Thread_Dispatch_disable_level -= 1; \
>> + } while (0)
>>
>> /*PAGE
>> *
>
>
More information about the users
mailing list