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

Reply via email to