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.hu...@embedded-brains.de PGP : Public key available on request. Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG. _______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel