Fwd: [PATCH 1/2] score: Add and use Watchdog_Clock for clock types.

Sebastian Huber sebastian.huber at embedded-brains.de
Thu Jun 16 06:09:45 UTC 2016



On 16/06/16 01:19, Gedare Bloom wrote:
> For review, the below patch failed to get through the spam filter..
>
> Clock types may be WATCHDOG_RELATIVE or WATCHDOG_ABSOLUTE
> and they relate to the PER_CPU_WATCHDOG_RELATIVE/ABSOLUTE.
>
> updates #2732
> ---
>   cpukit/posix/src/condwaitsupp.c                 |  1 +
>   cpukit/posix/src/nanosleep.c                    |  1 +
>   cpukit/posix/src/sigtimedwait.c                 |  1 +
>   cpukit/rtems/src/eventseize.c                   |  5 ++--
>   cpukit/rtems/src/regiongetsegment.c             |  1 +
>   cpukit/rtems/src/taskwakeafter.c                |  5 ++--
>   cpukit/rtems/src/taskwakewhen.c                 |  5 ++--
>   cpukit/score/include/rtems/score/coresemimpl.h  |  1 +
>   cpukit/score/include/rtems/score/threadimpl.h   | 31 +++++++------------------
>   cpukit/score/include/rtems/score/threadqimpl.h  |  5 ++++
>   cpukit/score/include/rtems/score/watchdog.h     | 23 ++++++++++++++++++
>   cpukit/score/include/rtems/score/watchdogimpl.h |  2 +-
>   cpukit/score/src/condition.c                    |  1 +
>   cpukit/score/src/corebarrierwait.c              |  1 +
>   cpukit/score/src/coremsgseize.c                 |  1 +
>   cpukit/score/src/coremsgsubmit.c                |  1 +
>   cpukit/score/src/coremutexseize.c               |  3 +++
>   cpukit/score/src/corerwlockobtainread.c         |  1 +
>   cpukit/score/src/corerwlockobtainwrite.c        |  1 +
>   cpukit/score/src/futex.c                        |  1 +
>   cpukit/score/src/mpci.c                         |  1 +
>   cpukit/score/src/mutex.c                        |  1 +
>   cpukit/score/src/semaphore.c                    |  1 +
>   cpukit/score/src/threadqenqueue.c               |  6 +++--
>   cpukit/score/src/threadrestart.c                |  1 +
>   25 files changed, 70 insertions(+), 31 deletions(-)
[...]
> diff --git a/cpukit/score/include/rtems/score/threadimpl.h
> b/cpukit/score/include/rtems/score/threadimpl.h
> index 164773a..520a194 100644
> --- a/cpukit/score/include/rtems/score/threadimpl.h
> +++ b/cpukit/score/include/rtems/score/threadimpl.h
> @@ -1461,38 +1461,25 @@ RTEMS_INLINE_ROUTINE void _Thread_Timer_initialize(
>     _Watchdog_Preinitialize( &timer->Watchdog, cpu );
>   }
>
> -RTEMS_INLINE_ROUTINE void _Thread_Timer_insert_relative(
> +RTEMS_INLINE_ROUTINE void _Thread_Timer_insert(
>     Thread_Control                 *the_thread,
>     Per_CPU_Control                *cpu,
>     Watchdog_Service_routine_entry  routine,
> -  Watchdog_Interval               ticks
> +  uint64_t                        ticks,
> +  Watchdog_Clock                  clock
>   )
>   {
>     ISR_lock_Context lock_context;
>
>     _ISR_lock_ISR_disable_and_acquire( &the_thread->Timer.Lock, &lock_context );
>
> -  the_thread->Timer.header = &cpu->Watchdog.Header[
> PER_CPU_WATCHDOG_RELATIVE ];
> +  the_thread->Timer.header = &cpu->Watchdog.Header[ clock ];
>     the_thread->Timer.Watchdog.routine = routine;
> -  _Watchdog_Per_CPU_insert_relative( &the_thread->Timer.Watchdog, cpu, ticks );
> -
> -  _ISR_lock_Release_and_ISR_enable( &the_thread->Timer.Lock, &lock_context );
> -}
> -
> -RTEMS_INLINE_ROUTINE void _Thread_Timer_insert_absolute(
> -  Thread_Control                 *the_thread,
> -  Per_CPU_Control                *cpu,
> -  Watchdog_Service_routine_entry  routine,
> -  uint64_t                        expire
> -)
> -{
> -  ISR_lock_Context lock_context;
> -
> -  _ISR_lock_ISR_disable_and_acquire( &the_thread->Timer.Lock, &lock_context );
> -
> -  the_thread->Timer.header = &cpu->Watchdog.Header[
> PER_CPU_WATCHDOG_ABSOLUTE ];
> -  the_thread->Timer.Watchdog.routine = routine;
> -  _Watchdog_Per_CPU_insert_absolute( &the_thread->Timer.Watchdog,
> cpu, expire );
> +  if ( clock == WATCHDOG_RELATIVE ) {
> +    _Watchdog_Per_CPU_insert_relative( &the_thread->Timer.Watchdog,
> cpu, ticks );
> +  } else {
> +    _Watchdog_Per_CPU_insert_absolute( &the_thread->Timer.Watchdog,
> cpu, ticks );
> +  }

I would add an _Watchdog_Per_CPU_insert() to avoid the double ISR lock 
acquire/release code.

>
>     _ISR_lock_Release_and_ISR_enable( &the_thread->Timer.Lock, &lock_context );
>   }
> diff --git a/cpukit/score/include/rtems/score/threadqimpl.h
> b/cpukit/score/include/rtems/score/threadqimpl.h
> index 73d4de2..f59a334 100644
> --- a/cpukit/score/include/rtems/score/threadqimpl.h
> +++ b/cpukit/score/include/rtems/score/threadqimpl.h
> @@ -349,6 +349,7 @@ Thread_Control *_Thread_queue_Do_dequeue(
>    *       executing,
>    *       STATES_WAITING_FOR_MUTEX,
>    *       WATCHDOG_NO_TIMEOUT,
> + *       WATCHDOG_RELATIVE,
>    *       0,
>    *       &queue_context
>    *     );
> @@ -362,6 +363,7 @@ Thread_Control *_Thread_queue_Do_dequeue(
>    * @param[in] state The new state of the thread.
>    * @param[in] timeout Interval to wait.  Use WATCHDOG_NO_TIMEOUT to block
>    * potentially forever.
> + * @param[in] clock The kind of clock used for the interval.
>    * @param[in] queue_context The thread queue context of the lock acquire.
>    */
>   void _Thread_queue_Enqueue_critical(
> @@ -370,6 +372,7 @@ void _Thread_queue_Enqueue_critical(
>     Thread_Control                *the_thread,
>     States_Control                 state,
>     Watchdog_Interval              timeout,
> +  Watchdog_Clock                 clock,
>     Thread_queue_Context          *queue_context
>   );

We already have a lot of parameters for the 
_Thread_queue_Enqueue_critical(). I would rather move the timeout 
parameters to the queue_context, since this is more efficient.

The Watchdog_Interval is 32-bit, thus not suitable for the absolute 
timeouts.

A value of 0 is valid for absolute timeouts, see what happens if you get 
that wrong

https://www.wired.com/2016/02/dont-set-your-iphone-back-to-1970-no-matter-what/

So, I would move the WATCHDOG_NO_TIMEOUT to your new Watchdog_Clock.

[...]

-- 
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax     : +49 89 189 47 41-09
E-Mail  : sebastian.huber at embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.




More information about the devel mailing list