[PATCH] rtems: Add rtems_interrupt_local_disable|enable()

Gedare Bloom gedare at gwu.edu
Sat Jun 20 18:06:07 UTC 2015


Quick skim looks good.

On Fri, Jun 19, 2015 at 5:02 PM, Sebastian Huber
<sebastian.huber at embedded-brains.de> wrote:
> Add rtems_interrupt_local_disable|enable() as suggested by Pavel Pisa to
> emphasize that interrupts are only disabled on the current processor.
> Do not define the rtems_interrupt_disable|enable|flash() macros and
> functions on SMP configurations since they don't ensure system wide
> mutual exclusion.
> ---
>  c/src/lib/libbsp/shared/bootcard.c      |   2 +-
>  cpukit/rtems/include/rtems/rtems/intr.h |  38 ++++++++
>  cpukit/rtems/src/intrbody.c             |   4 +
>  doc/user/intr.t                         |  94 ++++++++++++++++++
>  testsuites/libtests/heapwalk/init.c     |   4 +-
>  testsuites/samples/Makefile.am          |   3 +
>  testsuites/samples/configure.ac         |   2 +
>  testsuites/sptests/sp37/init.c          | 164 ++++++++++++++++++--------------
>  testsuites/sptests/sp40/init.c          |   6 +-
>  testsuites/tmtests/tm26/task1.c         |   9 +-
>  10 files changed, 244 insertions(+), 82 deletions(-)
>
> diff --git a/c/src/lib/libbsp/shared/bootcard.c b/c/src/lib/libbsp/shared/bootcard.c
> index 4f91aa9..0ee5e12 100644
> --- a/c/src/lib/libbsp/shared/bootcard.c
> +++ b/c/src/lib/libbsp/shared/bootcard.c
> @@ -72,7 +72,7 @@ void boot_card(
>     *  Make sure interrupts are disabled.
>     */
>    (void) bsp_isr_level;
> -  rtems_interrupt_disable( bsp_isr_level );
> +  rtems_interrupt_local_disable( bsp_isr_level );
>
>    bsp_boot_cmdline = cmdline;
>
> diff --git a/cpukit/rtems/include/rtems/rtems/intr.h b/cpukit/rtems/include/rtems/rtems/intr.h
> index 259120f..d084959 100644
> --- a/cpukit/rtems/include/rtems/rtems/intr.h
> +++ b/cpukit/rtems/include/rtems/rtems/intr.h
> @@ -89,10 +89,15 @@ rtems_status_code rtems_interrupt_catch(
>  );
>  #endif
>
> +#if !defined(RTEMS_SMP)
> +
>  /**
>   *  @brief Disable RTEMS Interrupt
>   *
>   *  @note The interrupt level shall be of type @ref rtems_interrupt_level.
> + *
> + *  This macro is only available on uni-processor configurations.  The macro
> + *  rtems_interrupt_local_disable() is available on all configurations.
>   */
>  #define rtems_interrupt_disable( _isr_cookie ) \
>      _ISR_Disable(_isr_cookie)
> @@ -101,6 +106,9 @@ rtems_status_code rtems_interrupt_catch(
>   *  @brief Enable RTEMS Interrupt
>   *
>   *  @note The interrupt level shall be of type @ref rtems_interrupt_level.
> + *
> + *  This macro is only available on uni-processor configurations.  The macro
> + *  rtems_interrupt_local_enable() is available on all configurations.
>   */
>  #define rtems_interrupt_enable( _isr_cookie ) \
>      _ISR_Enable(_isr_cookie)
> @@ -109,10 +117,40 @@ rtems_status_code rtems_interrupt_catch(
>   *  @brief Flash RTEMS Interrupt
>   *
>   *  @note The interrupt level shall be of type @ref rtems_interrupt_level.
> + *
> + *  This macro is only available on uni-processor configurations.  The macro
> + *  rtems_interrupt_local_disable() and rtems_interrupt_local_enable() is
> + *  available on all configurations.
>   */
>  #define rtems_interrupt_flash( _isr_cookie ) \
>      _ISR_Flash(_isr_cookie)
>
> +#endif /* RTEMS_SMP */
> +
> +/**
> + * @brief This macro disables the interrupts on the current processor.
> + *
> + * On SMP configurations this will not ensure system wide mutual exclusion.
> + * Use interrupt locks instead.
> + *
> + * @param[in] _isr_cookie The previous interrupt level is returned.  The type
> + *   of this variable must be rtems_interrupt_level.
> + *
> + * @see rtems_interrupt_local_enable().
> + */
> +#define rtems_interrupt_local_disable( _isr_cookie ) \
> +  _ISR_Disable_without_giant( _isr_cookie )
> +
> +/**
> + * @brief This macro restores the previous interrupt level on the current
> + * processor.
> + *
> + * @param[in] _isr_cookie The previous interrupt level returned by
> + *   rtems_interrupt_local_disable().
> + */
> +#define rtems_interrupt_local_enable( _isr_cookie ) \
> +  _ISR_Enable_without_giant( _isr_cookie )
> +
>  /**
>   *  @brief RTEMS Interrupt Is in Progress
>   *
> diff --git a/cpukit/rtems/src/intrbody.c b/cpukit/rtems/src/intrbody.c
> index 6b37eb2..deb0dc0 100644
> --- a/cpukit/rtems/src/intrbody.c
> +++ b/cpukit/rtems/src/intrbody.c
> @@ -23,6 +23,8 @@
>  #include <rtems/score/isr.h>
>  #include <rtems/rtems/intr.h>
>
> +#if !defined(RTEMS_SMP)
> +
>  /*
>   *  Undefine all of these is normally a macro and we want a real body in
>   *  the library for other language bindings.
> @@ -70,3 +72,5 @@ bool rtems_interrupt_is_in_progress( void )
>  {
>    return _ISR_Is_in_progress();
>  }
> +
> +#endif /* RTEMS_SMP */
> diff --git a/doc/user/intr.t b/doc/user/intr.t
> index 6cb6a26..cf106f3 100644
> --- a/doc/user/intr.t
> +++ b/doc/user/intr.t
> @@ -21,6 +21,8 @@ directive:
>  @item @code{@value{DIRPREFIX}interrupt_disable} - Disable Interrupts
>  @item @code{@value{DIRPREFIX}interrupt_enable} - Enable Interrupts
>  @item @code{@value{DIRPREFIX}interrupt_flash} - Flash Interrupt
> + at item @code{@value{DIRPREFIX}interrupt_local_disable} - Disable Interrupts on Current Processor
> + at item @code{@value{DIRPREFIX}interrupt_local_enable} - Enable Interrupts on Current Processor
>  @item @code{@value{DIRPREFIX}interrupt_lock_initialize} - Initialize an ISR Lock
>  @item @code{@value{DIRPREFIX}interrupt_lock_acquire} - Acquire an ISR Lock
>  @item @code{@value{DIRPREFIX}interrupt_lock_release} - Release an ISR Lock
> @@ -397,6 +399,10 @@ This directive will not cause the calling task to be preempted.
>  parameter.}
>  @end ifset
>
> +This directive is only available on uni-processor configurations.  The
> +directive @code{@value{DIRPREFIX}interrupt_local_disable} is available on all
> +configurations.
> +
>  @c
>  @c
>  @c
> @@ -441,6 +447,9 @@ and will be enabled when this directive returns to the caller.
>
>  This directive will not cause the calling task to be preempted.
>
> +This directive is only available on uni-processor configurations.  The
> +directive @code{@value{DIRPREFIX}interrupt_local_enable} is available on all
> +configurations.
>
>  @c
>  @c
> @@ -486,6 +495,91 @@ and will be redisabled when this directive returns to the caller.
>
>  This directive will not cause the calling task to be preempted.
>
> +This directive is only available on uni-processor configurations.  The
> +directives @code{@value{DIRPREFIX}interrupt_local_disable} and
> + at code{@value{DIRPREFIX}interrupt_local_enable} is available on all
> +configurations.
> +
> + at c
> + at c
> + at c
> + at page
> + at subsection INTERRUPT_LOCAL_DISABLE - Disable Interrupts on Current Processor
> +
> + at cindex disable interrupts
> +
> + at subheading CALLING SEQUENCE:
> +
> + at ifset is-C
> + at findex rtems_interrupt_local_disable
> + at example
> +void rtems_interrupt_local_disable(
> +  rtems_interrupt_level  level
> +);
> +
> +/* this is implemented as a macro and sets level as a side-effect */
> + at end example
> + at end ifset
> +
> + at subheading DIRECTIVE STATUS CODES:
> +
> +NONE
> +
> + at subheading DESCRIPTION:
> +
> +This directive disables all maskable interrupts and returns
> +the previous @code{level}.  A later invocation of the
> + at code{@value{DIRPREFIX}interrupt_local_enable} directive should be used to
> +restore the interrupt level.
> +
> + at subheading NOTES:
> +
> +This directive will not cause the calling task to be preempted.
> +
> + at ifset is-C
> + at b{This directive is implemented as a macro which modifies the @code{level}
> +parameter.}
> + at end ifset
> +
> +On SMP configurations this will not ensure system wide mutual exclusion.  Use
> +interrupt locks instead.
> +
> + at c
> + at c
> + at c
> + at page
> + at subsection INTERRUPT_LOCAL_ENABLE - Enable Interrupts on Current Processor
> +
> + at cindex enable interrupts
> +
> + at subheading CALLING SEQUENCE:
> +
> + at ifset is-C
> + at findex rtems_interrupt_local_enable
> + at example
> +void rtems_interrupt_local_enable(
> +  rtems_interrupt_level  level
> +);
> + at end example
> + at end ifset
> +
> + at subheading DIRECTIVE STATUS CODES:
> +
> +NONE
> +
> + at subheading DESCRIPTION:
> +
> +This directive enables maskable interrupts to the @code{level}
> +which was returned by a previous call to
> + at code{@value{DIRPREFIX}interrupt_local_disable}.
> +Immediately prior to invoking this directive, maskable interrupts should
> +be disabled by a call to @code{@value{DIRPREFIX}interrupt_local_disable}
> +and will be enabled when this directive returns to the caller.
> +
> + at subheading NOTES:
> +
> +This directive will not cause the calling task to be preempted.
> +
>  @c
>  @c
>  @c
> diff --git a/testsuites/libtests/heapwalk/init.c b/testsuites/libtests/heapwalk/init.c
> index 80073e8..3bd6c91 100644
> --- a/testsuites/libtests/heapwalk/init.c
> +++ b/testsuites/libtests/heapwalk/init.c
> @@ -94,12 +94,12 @@ static void test_system_not_up(void)
>
>    puts( "start with a system state != SYSTEM_STATE_UP" );
>
> -  rtems_interrupt_disable( level );
> +  rtems_interrupt_local_disable( level );
>    System_state_Codes state = _System_state_Get();
>    _System_state_Set( SYSTEM_STATE_TERMINATED );
>    test_call_heap_walk( true );
>    _System_state_Set( state );
> -  rtems_interrupt_enable( level );
> +  rtems_interrupt_local_enable( level );
>  }
>
>  static void test_check_control(void)
> diff --git a/testsuites/samples/Makefile.am b/testsuites/samples/Makefile.am
> index 08455d3..374617b 100644
> --- a/testsuites/samples/Makefile.am
> +++ b/testsuites/samples/Makefile.am
> @@ -18,8 +18,11 @@ endif
>  if NETTESTS
>  ## loopback tests a network loopback interface
>  _SUBDIRS += loopback
> +if HAS_SMP
> +else
>  _SUBDIRS += pppd
>  endif
> +endif
>
>  include $(top_srcdir)/../automake/test-subdirs.am
>  include $(top_srcdir)/../automake/local.am
> diff --git a/testsuites/samples/configure.ac b/testsuites/samples/configure.ac
> index e6f12d0..91a3661 100644
> --- a/testsuites/samples/configure.ac
> +++ b/testsuites/samples/configure.ac
> @@ -26,6 +26,7 @@ RTEMS_CHECK_CUSTOM_BSP(RTEMS_BSP)
>  RTEMS_CHECK_CPUOPTS([RTEMS_MULTIPROCESSING])
>  RTEMS_CHECK_CXX(RTEMS_BSP)
>  RTEMS_CHECK_CPUOPTS([RTEMS_NETWORKING])
> +RTEMS_CHECK_CPUOPTS([RTEMS_SMP])
>
>  CXXTESTS=$HAS_CPLUSPLUS
>  AS_IF([test $HAS_CPLUSPLUS = yes],[
> @@ -52,6 +53,7 @@ AS_IF([test $HAS_CPLUSPLUS = yes],[
>  AM_CONDITIONAL([CXXTESTS],[test $CXXTESTS = "yes"])
>  AM_CONDITIONAL(NETTESTS,test "$rtems_cv_RTEMS_NETWORKING" = "yes")
>  AM_CONDITIONAL(MPTESTS,test "$rtems_cv_RTEMS_MULTIPROCESSING" = "yes")
> +AM_CONDITIONAL(HAS_SMP,test "$rtems_cv_RTEMS_SMP" = "yes")
>
>  # FIXME: We should get rid of this. It's a cludge.
>  AC_CHECK_SIZEOF([time_t])
> diff --git a/testsuites/sptests/sp37/init.c b/testsuites/sptests/sp37/init.c
> index 0846491..a963a30 100644
> --- a/testsuites/sptests/sp37/init.c
> +++ b/testsuites/sptests/sp37/init.c
> @@ -243,8 +243,7 @@ static void test_clock_tick_functions( void )
>    rtems_interrupt_level level;
>    Watchdog_Interval saved_ticks;
>
> -  _Thread_Disable_dispatch();
> -  rtems_interrupt_disable( level );
> +  rtems_interrupt_local_disable( level );
>
>    saved_ticks = _Watchdog_Ticks_since_boot;
>
> @@ -287,16 +286,19 @@ static void test_clock_tick_functions( void )
>
>    _Watchdog_Ticks_since_boot = saved_ticks;
>
> -  rtems_interrupt_enable( level );
> -  _Thread_Enable_dispatch();
> +  rtems_interrupt_local_enable( level );
>  }
>
>  void test_interrupt_inline(void)
>  {
>    rtems_interrupt_level level;
> +  rtems_interrupt_level level_1;
>    rtems_mode            level_mode_body;
>    rtems_mode            level_mode_macro;
>    bool                  in_isr;
> +  uint32_t              isr_level_0;
> +  uint32_t              isr_level_1;
> +  uint32_t              isr_level_2;
>
>    puts( "interrupt is in progress (use body)" );
>    in_isr = rtems_interrupt_is_in_progress();
> @@ -305,20 +307,33 @@ void test_interrupt_inline(void)
>      rtems_test_exit( 0 );
>    }
>
> +#if !defined(RTEMS_SMP)
>    puts( "interrupt disable (use inline)" );
> -  _Thread_Disable_dispatch();
>    rtems_interrupt_disable( level );
> -  _Thread_Enable_dispatch();
>
>    puts( "interrupt flash (use inline)" );
> -  _Thread_Disable_dispatch();
>    rtems_interrupt_flash( level );
> -  _Thread_Enable_dispatch();
>
>    puts( "interrupt enable (use inline)" );
> -  _Thread_Disable_dispatch();
>    rtems_interrupt_enable( level );
> -  _Thread_Enable_dispatch();
> +#endif /* RTEMS_SMP */
> +
> +  isr_level_0 = _ISR_Get_level();
> +  rtems_test_assert( isr_level_0 == 0 );
> +
> +  rtems_interrupt_local_disable( level );
> +  isr_level_1 = _ISR_Get_level();
> +  rtems_test_assert( isr_level_1 != isr_level_0 );
> +
> +  rtems_interrupt_local_disable( level_1 );
> +  isr_level_2 = _ISR_Get_level();
> +  rtems_test_assert( isr_level_2 == isr_level_1 );
> +
> +  rtems_interrupt_local_enable( level_1 );
> +  rtems_test_assert( _ISR_Get_level() == isr_level_1 );
> +
> +  rtems_interrupt_local_enable( level );
> +  rtems_test_assert( _ISR_Get_level() == isr_level_0 );
>
>    puts( "interrupt level mode (use inline)" );
>    level_mode_body = rtems_interrupt_level_body( level );
> @@ -328,7 +343,10 @@ void test_interrupt_inline(void)
>    }
>  }
>
> +#if !defined(RTEMS_SMP)
>  volatile int isr_in_progress_body;
> +#endif
> +
>  volatile int isr_in_progress_inline;
>
>  void check_isr_in_progress_inline(void)
> @@ -336,25 +354,6 @@ void check_isr_in_progress_inline(void)
>    isr_in_progress_inline = rtems_interrupt_is_in_progress() ? 1 : 2;
>  }
>
> -#undef rtems_interrupt_disable
> -extern rtems_interrupt_level rtems_interrupt_disable(void);
> -#undef rtems_interrupt_enable
> -extern void rtems_interrupt_enable(rtems_interrupt_level previous_level);
> -#undef rtems_interrupt_flash
> -extern void rtems_interrupt_flash(rtems_interrupt_level previous_level);
> -#undef rtems_interrupt_is_in_progress
> -extern bool rtems_interrupt_is_in_progress(void);
> -
> -rtems_timer_service_routine test_isr_in_progress(
> -  rtems_id  timer,
> -  void     *arg
> -)
> -{
> -  check_isr_in_progress_inline();
> -
> -  isr_in_progress_body = rtems_interrupt_is_in_progress() ? 1 : 2;
> -}
> -
>  void check_isr_worked(
>    char *s,
>    int   result
> @@ -429,16 +428,72 @@ rtems_timer_service_routine test_unblock_task(
>    directive_failed( status, "rtems_task_resume" );
>  }
>
> +#undef rtems_interrupt_disable
> +extern rtems_interrupt_level rtems_interrupt_disable(void);
> +#undef rtems_interrupt_enable
> +extern void rtems_interrupt_enable(rtems_interrupt_level previous_level);
> +#undef rtems_interrupt_flash
> +extern void rtems_interrupt_flash(rtems_interrupt_level previous_level);
> +#undef rtems_interrupt_is_in_progress
> +extern bool rtems_interrupt_is_in_progress(void);
> +
> +static void test_interrupt_body(void)
> +{
> +#if !defined(RTEMS_SMP)
> +  rtems_interrupt_level level;
> +  rtems_mode            level_mode_body;
> +  rtems_mode            level_mode_macro;
> +  bool                  in_isr;
> +
> +  puts( "interrupt disable (use body)" );
> +  level = rtems_interrupt_disable();
> +
> +  puts( "interrupt disable (use body)" );
> +  level = rtems_interrupt_disable();
> +
> +  puts( "interrupt flash (use body)" );
> +  rtems_interrupt_flash( level );
> +
> +  puts( "interrupt enable (use body)" );
> +  rtems_interrupt_enable( level );
> +
> +  puts( "interrupt level mode (use body)" );
> +  level_mode_body = rtems_interrupt_level_body( level );
> +  level_mode_macro = RTEMS_INTERRUPT_LEVEL(level);
> +  if ( level_mode_macro == level_mode_body ) {
> +    puts("test seems to work");
> +  }
> +
> +  /*
> +   *  Test interrupt bodies
> +   */
> +  puts( "interrupt is in progress (use body)" );
> +  in_isr = rtems_interrupt_is_in_progress();
> +  if ( in_isr ) {
> +    puts( "interrupt reported to be is in progress (body)" );
> +    rtems_test_exit( 0 );
> +  }
> +#endif /* RTEMS_SMP */
> +}
> +
> +rtems_timer_service_routine test_isr_in_progress(
> +  rtems_id  timer,
> +  void     *arg
> +)
> +{
> +  check_isr_in_progress_inline();
> +
> +#if !defined(RTEMS_SMP)
> +  isr_in_progress_body = rtems_interrupt_is_in_progress() ? 1 : 2;
> +#endif
> +}
> +
>  rtems_task Init(
>    rtems_task_argument argument
>  )
>  {
>    rtems_time_of_day     time;
>    rtems_status_code     status;
> -  rtems_interrupt_level level;
> -  rtems_mode            level_mode_body;
> -  rtems_mode            level_mode_macro;
> -  bool                  in_isr;
>    rtems_id              timer;
>    int                   i;
>
> @@ -523,47 +578,8 @@ rtems_task Init(
>         break;
>    }
>
> -  /*
> -   *  Test interrupt inline versions
> -   */
>    test_interrupt_inline();
> -
> -  /*
> -   *  Test interrupt bodies
> -   */
> -  puts( "interrupt is in progress (use body)" );
> -  in_isr = rtems_interrupt_is_in_progress();
> -  if ( in_isr ) {
> -    puts( "interrupt reported to be is in progress (body)" );
> -    rtems_test_exit( 0 );
> -  }
> -
> -  puts( "interrupt disable (use body)" );
> -  _Thread_Disable_dispatch();
> -  level = rtems_interrupt_disable();
> -  _Thread_Enable_dispatch();
> -
> -  puts( "interrupt disable (use body)" );
> -  _Thread_Disable_dispatch();
> -  level = rtems_interrupt_disable();
> -  _Thread_Enable_dispatch();
> -
> -  puts( "interrupt flash (use body)" );
> -  _Thread_Disable_dispatch();
> -  rtems_interrupt_flash( level );
> -  _Thread_Enable_dispatch();
> -
> -  puts( "interrupt enable (use body)" );
> -  _Thread_Disable_dispatch();
> -  rtems_interrupt_enable( level );
> -  _Thread_Enable_dispatch();
> -
> -  puts( "interrupt level mode (use body)" );
> -  level_mode_body = rtems_interrupt_level_body( level );
> -  level_mode_macro = RTEMS_INTERRUPT_LEVEL(level);
> -  if ( level_mode_macro == level_mode_body ) {
> -    puts("test seems to work");
> -  }
> +  test_interrupt_body();
>
>    /*
>     * Test ISR in progress from actual ISR
> @@ -576,7 +592,9 @@ rtems_task Init(
>
>    check_isr_worked( "inline", isr_in_progress_inline );
>
> +#if !defined(RTEMS_SMP)
>    check_isr_worked( "body", isr_in_progress_body );
> +#endif
>
>    TEST_END();
>    rtems_test_exit( 0 );
> diff --git a/testsuites/sptests/sp40/init.c b/testsuites/sptests/sp40/init.c
> index 5b0ad7d..d3b547b 100644
> --- a/testsuites/sptests/sp40/init.c
> +++ b/testsuites/sptests/sp40/init.c
> @@ -42,16 +42,14 @@ static rtems_driver_address_table test_driver = {
>
>  #define test_interrupt_context_enter( level ) \
>    do { \
> -    _Thread_Disable_dispatch(); \
> -    rtems_interrupt_disable( level ); \
> +    rtems_interrupt_local_disable( level ); \
>      ++_ISR_Nest_level; \
>    } while (0)
>
>  #define test_interrupt_context_leave( level ) \
>    do { \
>      --_ISR_Nest_level; \
> -    rtems_interrupt_enable( level ); \
> -    _Thread_Enable_dispatch(); \
> +    rtems_interrupt_local_enable( level ); \
>    } while (0)
>
>  rtems_task Init(
> diff --git a/testsuites/tmtests/tm26/task1.c b/testsuites/tmtests/tm26/task1.c
> index ee66218..5b19c3d 100644
> --- a/testsuites/tmtests/tm26/task1.c
> +++ b/testsuites/tmtests/tm26/task1.c
> @@ -341,15 +341,20 @@ rtems_task High_task(
>    _Thread_Disable_dispatch();
>
>    benchmark_timer_initialize();
> -    rtems_interrupt_disable( level );
> +    rtems_interrupt_local_disable( level );
>    isr_disable_time = benchmark_timer_read();
>
>    benchmark_timer_initialize();
> +#if defined(RTEMS_SMP)
> +    rtems_interrupt_local_enable( level );
> +    rtems_interrupt_local_disable( level );
> +#else
>      rtems_interrupt_flash( level );
> +#endif
>    isr_flash_time = benchmark_timer_read();
>
>    benchmark_timer_initialize();
> -    rtems_interrupt_enable( level );
> +    rtems_interrupt_local_enable( level );
>    isr_enable_time = benchmark_timer_read();
>
>    _Thread_Enable_dispatch();
> --
> 1.8.1.4
>
> _______________________________________________
> devel mailing list
> devel at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel



More information about the devel mailing list