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