4.6 Barrier Patch for Review

[Thomas Doerfler]

Hello Joel,

I think Pavel's changes are right. Additionally I am not sure about the 
Thread_disable_dispatch stuff:

In the following routine I think the memory_barrier should be behind the 
increment and before the decrement of disable_level. This will make 
sure, that critical accesses are bracketed in the secured state.

Your version:
----------------
RTEMS_INLINE_ROUTINE void _Thread_Disable_dispatch( void )
  {
+  RTEMS_COMPILER_MEMORY_BARRIER();
    _Thread_Dispatch_disable_level += 1;
  }
...
  RTEMS_INLINE_ROUTINE void _Thread_Unnest_dispatch( void )
  {
    _Thread_Dispatch_disable_level -= 1;
+  RTEMS_COMPILER_MEMORY_BARRIER();
  }
...
  #define _Thread_Unnest_dispatch()  \
-  _Thread_Dispatch_disable_level -= 1
+  do { \
+    _Thread_Dispatch_disable_level -= 1; \
+    RTEMS_COMPILER_MEMORY_BARRIER(); \
+  } while (0)

----------------
My recommendation:
----------------
RTEMS_INLINE_ROUTINE void _Thread_Disable_dispatch( void )
  {
    _Thread_Dispatch_disable_level += 1;
!!!+  RTEMS_COMPILER_MEMORY_BARRIER();
  }
...
  RTEMS_INLINE_ROUTINE void _Thread_Unnest_dispatch( void )
  {
!!!+  RTEMS_COMPILER_MEMORY_BARRIER();
    _Thread_Dispatch_disable_level -= 1;
  }
...
  #define _Thread_Unnest_dispatch()  \
-  _Thread_Dispatch_disable_level -= 1
+  do { \
!!!+    RTEMS_COMPILER_MEMORY_BARRIER(); \
+    _Thread_Dispatch_disable_level -= 1; \
+  } while (0)

----------------

wkr,
Thomas.

Pavel Pisa wrote:
> On Wednesday 01 March 2006 23:08, Joel Sherrill wrote:
> 
>> #define _ISR_Enable( _level ) \
>>-        _CPU_ISR_Enable( _level )
>>+  do { \
>>+    _CPU_ISR_Enable( _level ); \
>>+    RTEMS_COMPILER_MEMORY_BARRIER(); \
>>+  } while (0)
> 
> 
> Hello Joel,
> 
> I think, that barrier and enable order has to
> follow original Till Straumann's ordering,
> else there is allmost neglectable but real chance,
> that GCC moves something from protected region
> before _ISR_Enable() after interrupt enable
> instruction, because barrier is in the fact
> after it and blocking only move of the code
> after whole _ISR_Enable() construct.
> 
> #define _ISR_Enable( _level ) \
> do { \
>    RTEMS_COMPILER_MEMORY_BARRIER(); \
>    _CPU_ISR_Enable( _level ); \
>  } while (0)
> 
> Same for _ISR_Flash()
> 
> #define _ISR_Flash( _level ) \
>   do { \
>     RTEMS_COMPILER_MEMORY_BARRIER(); \
>     _CPU_ISR_Flash( _level ); \
>     RTEMS_COMPILER_MEMORY_BARRIER(); \
>   } while(0)
> 
> Again fault is non-probable but possible in theoretically constructed case.
> The two subsequent "calls" to RTEMS_COMPILER_MEMORY_BARRIER()
> has zero cost in reality, because the first one already forces
> moves of all data and they do not represent any instruction.
> 
> Best wishes
> 
>                 Pavel Pisa


-- 
--------------------------------------------
IMD Ingenieurbuero fuer Microcomputertechnik
Thomas Doerfler           Herbststrasse 8
D-82178 Puchheim          Germany
email:    Thomas.Doerfler at imd-systems.de
PGP public key available at:
      http://www.imd-systems.de/pgpkey_en.html