4.6 Barrier Patch for Review

[Eric Norum]

Looks good.  A minor question, though.  Given that the barrier is a  
single asm statement, is it really necessary to encapsulate it in a  
'do { } while(0)' block?

In the interests of simplicity, would the following be better?
#ifdef __GNUC__
  #define RTEMS_COMPILER_MEMORY_BARRIER() asm volatile("" ::: "memory")
#else
  #define RTEMS_COMPILER_MEMORY_BARRIER()
#endif


On Mar 2, 2006, at 8:53 AM, Joel Sherrill wrote:

> Round 2 attached.  How does this one look?
>
> --joel
> Till Straumann wrote:
>
>> I agree with Eric, Thomas and Pavel.
>>
>> The barriers must prevent stuff from
>> being moved out of the protected section:
>>
>>    protection enable (isr_disable or thread_dispatch_disable++)
>>    barrier
>>       protected code
>>    barrier
>>    protection disable (isr_enable or thread_enable_dispatch)
>>
>> -- Till
>>
>
> 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 14:52:09 -0000
> @@ -102,6 +102,20 @@
>  #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() \
> +    do { \
> +      asm volatile("" ::: "memory"); \
> +    } while (0)
> +#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 14:52:09 -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 14:52:09  
> -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();
>  }
> @@ -224,6 +226,7 @@
>  RTEMS_INLINE_ROUTINE void _Thread_Unnest_dispatch( void )
>  {
>    _Thread_Dispatch_disable_level -= 1;
> +  RTEMS_COMPILER_MEMORY_BARRIER();
>  }
>
>  /*PAGE
> 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 14:52:09  
> -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
>   *

-- 
Eric Norum <norume at aps.anl.gov>
Advanced Photon Source
Argonne National Laboratory
(630) 252-4793