4.6 Barrier Patch for Review

[Ian Caddy]

Sorry, ignore my last email.

The barrier should be after the disable and before the enable.

I haven't had my first coffee yet, and am obviously not thinking 
straight.  Sorry for the noise.

The patch is correct as last stands as far as I can see.

regards,

Ian C.


Ian Caddy wrote:
> Hi,
> 
> I thought we had also agree to put the barrier *before* the disable_isr, 
> this is the same as Thomas's request for moving the barrier to *before* 
> the _Thread_Unnest_dispatch.
> 
> regards,
> 
> Ian Caddy
> 
> 
> Joel Sherrill wrote:
>> Thomas Doerfler wrote:
>>
>>> Joel,
>>>
>>> nearly, but no. (sorry).
>>>
>>
>> That's what the review request was for. :)
>>
>>> I think the _Thread_Unnest_dispatch function still does it the wrong 
>>> way round:
>>
>> This should be fixed along with Eric Norum's suggestion about not 
>> needing the
>> do/while in the BARRIER macro.
>>
>> Did Mattias and Elena get to play in the snow this week?  We peeked in
>> on the webcam in Marienplatz and it looked quite snowy a few days ago.
>>
>> --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    2 Mar 2006 16:21:30 -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    2 Mar 2006 16:21:30 -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    2 Mar 2006 16:21:30 
>> -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    2 Mar 2006 16:21:30 
>> -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 { \
>> +    _Thread_Dispatch_disable_level -= 1; \
>> +    RTEMS_COMPILER_MEMORY_BARRIER(); \
>> +  } while (0)
>>  
>>  /*PAGE
>>   *
> 

-- 
Ian Caddy
Goanna Technologies Pty Ltd
+61 8 9221 1860