On 16/06/16 08:09, Sebastian Huber wrote:
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.
[...]
From IRC log:
[19:44] <gedare> sebhub: Does it make sense to you to redefine
Watchdog_Interval as a struct containing a union for the interval and an
enum for the Clock "Discipline" (relative/absolute)?
[19:45] <gedare> ^the union for uint32_t relative, uint64_t absolute--or
else we make all intervals 64-bit
No, I would not change Watchdog_Interval. We should keep the ticks based
services as is. The problem is that _Thread_queue_Enqueue_critical()
doesn't support absolute timeouts currently. I would do something like
the attached.
We have
void _Thread_queue_Enqueue_critical(
Thread_queue_Queue *queue,
const Thread_queue_Operations *operations,
Thread_Control *the_thread,
States_Control state,
Thread_queue_Context *queue_context
)
{
Per_CPU_Control *cpu_self;
bool success;
#if defined(RTEMS_MULTIPROCESSING)
if ( _Thread_MP_Is_receive( the_thread ) &&
the_thread->receive_packet ) {
the_thread = _Thread_MP_Allocate_proxy( state );
}
#endif
_Thread_Lock_set( the_thread, &queue->Lock );
the_thread->Wait.return_code = STATUS_SUCCESSFUL;
_Thread_Wait_set_queue( the_thread, queue );
_Thread_Wait_set_operations( the_thread, operations );
( *operations->enqueue )( queue, the_thread );
Thus, we would have to store the timeout related parameters to the stack
anyway, since ( *operations->enqueue )( queue, the_thread ) is a
function call which cannot be optimized way (except maybe through
link-time optimization). So, there is no overhead due to the use of the
queue context.
--
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.
>From 62fa4b43ecdf560c6f5f1e397e25c06ded5bfc63 Mon Sep 17 00:00:00 2001
From: Sebastian Huber <sebastian.hu...@embedded-brains.de>
Date: Tue, 21 Jun 2016 07:45:19 +0200
Subject: [PATCH] score: _Thread_queue_Enqueue_critical()
Support absolute timeouts in _Thread_queue_Enqueue_critical().
---
cpukit/score/include/rtems/score/threadq.h | 5 +++++
cpukit/score/include/rtems/score/threadqimpl.h | 28 ++++++++++++++++++--------
cpukit/score/include/rtems/score/watchdog.h | 20 +++++++++++-------
cpukit/score/src/threadqenqueue.c | 27 +++++++++++++++++--------
4 files changed, 57 insertions(+), 23 deletions(-)
diff --git a/cpukit/score/include/rtems/score/threadq.h b/cpukit/score/include/rtems/score/threadq.h
index a4e5292..f0ebf13 100644
--- a/cpukit/score/include/rtems/score/threadq.h
+++ b/cpukit/score/include/rtems/score/threadq.h
@@ -24,6 +24,7 @@
#include <rtems/score/object.h>
#include <rtems/score/priority.h>
#include <rtems/score/rbtree.h>
+#include <rtems/score/watchdog.h>
#ifdef __cplusplus
extern "C" {
@@ -78,6 +79,10 @@ typedef struct {
*/
uint32_t expected_thread_dispatch_disable_level;
+ Watchdog_Dicipline timeout_discipline;
+
+ uint64_t timeout_interval;
+
/**
* @brief Callout to unblock the thread in case it is actually a thread
* proxy.
diff --git a/cpukit/score/include/rtems/score/threadqimpl.h b/cpukit/score/include/rtems/score/threadqimpl.h
index 73d4de2..48cb9d6 100644
--- a/cpukit/score/include/rtems/score/threadqimpl.h
+++ b/cpukit/score/include/rtems/score/threadqimpl.h
@@ -62,16 +62,34 @@ RTEMS_INLINE_ROUTINE void _Thread_queue_Context_initialize(
Thread_queue_Context *queue_context
)
{
+ queue_context->timeout_discipline = WATCHDOG_NO_TIMEOUT;
+
#if defined(RTEMS_DEBUG)
queue_context->expected_thread_dispatch_disable_level = 0xdeadbeef;
#if defined(RTEMS_MULTIPROCESSING)
queue_context->mp_callout = NULL;
#endif
-#else
- (void) queue_context;
#endif
}
+RTEMS_INLINE_ROUTINE void _Thread_queue_Context_set_relative_timeout(
+ Thread_queue_Context *queue_context,
+ Watchdog_Interval timeout
+)
+{
+ queue_context->timeout_discipline = WATCHDOG_RELATIVE;
+ queue_context->timeout_interval = timeout;
+}
+
+RTEMS_INLINE_ROUTINE void _Thread_queue_Context_set_absolute_timeout(
+ Thread_queue_Context *queue_context,
+ uint64_t timeout
+)
+{
+ queue_context->timeout_discipline = WATCHDOG_ABSOLUTE;
+ queue_context->timeout_interval = timeout;
+}
+
/**
* @brief Sets the expected thread dispatch disable level in the thread queue
* context.
@@ -348,8 +366,6 @@ Thread_Control *_Thread_queue_Do_dequeue(
* MUTEX_TQ_OPERATIONS,
* executing,
* STATES_WAITING_FOR_MUTEX,
- * WATCHDOG_NO_TIMEOUT,
- * 0,
* &queue_context
* );
* }
@@ -360,8 +376,6 @@ Thread_Control *_Thread_queue_Do_dequeue(
* @param[in] operations The thread queue operations.
* @param[in] the_thread The thread to enqueue.
* @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] queue_context The thread queue context of the lock acquire.
*/
void _Thread_queue_Enqueue_critical(
@@ -369,7 +383,6 @@ void _Thread_queue_Enqueue_critical(
const Thread_queue_Operations *operations,
Thread_Control *the_thread,
States_Control state,
- Watchdog_Interval timeout,
Thread_queue_Context *queue_context
);
@@ -382,7 +395,6 @@ RTEMS_INLINE_ROUTINE void _Thread_queue_Enqueue(
const Thread_queue_Operations *operations,
Thread_Control *the_thread,
States_Control state,
- Watchdog_Interval timeout,
uint32_t expected_level
)
{
diff --git a/cpukit/score/include/rtems/score/watchdog.h b/cpukit/score/include/rtems/score/watchdog.h
index c582dbd..1bf7d5c 100644
--- a/cpukit/score/include/rtems/score/watchdog.h
+++ b/cpukit/score/include/rtems/score/watchdog.h
@@ -67,13 +67,19 @@ typedef void Watchdog_Service_routine;
typedef Watchdog_Service_routine
( *Watchdog_Service_routine_entry )( Watchdog_Control * );
-/**
- * @brief The constant for indefinite wait.
- *
- * This is the constant for indefinite wait. It is actually an
- * illegal interval.
- */
-#define WATCHDOG_NO_TIMEOUT 0
+typedef struct {
+ /**
+ * @brief The constant for indefinite wait.
+ *
+ * This is the constant for indefinite wait. It is actually an
+ * illegal interval.
+ */
+ WATCHDOG_NO_TIMEOUT,
+
+ WATCHDOG_RELATIVE,
+
+ WATCHDOG_ABSOLUTE
+} Watchdog_Dicipline;
/**
* @brief The watchdog header to manage scheduled watchdogs.
diff --git a/cpukit/score/src/threadqenqueue.c b/cpukit/score/src/threadqenqueue.c
index 8bd1905..cbec7c7 100644
--- a/cpukit/score/src/threadqenqueue.c
+++ b/cpukit/score/src/threadqenqueue.c
@@ -39,7 +39,6 @@ void _Thread_queue_Enqueue_critical(
const Thread_queue_Operations *operations,
Thread_Control *the_thread,
States_Control state,
- Watchdog_Interval timeout,
Thread_queue_Context *queue_context
)
{
@@ -83,13 +82,25 @@ void _Thread_queue_Enqueue_critical(
/*
* If the thread wants to timeout, then schedule its timer.
*/
- if ( timeout != WATCHDOG_NO_TIMEOUT ) {
- _Thread_Timer_insert_relative(
- the_thread,
- cpu_self,
- _Thread_Timeout,
- timeout
- );
+ switch ( queue_context->timeout_discipline ) {
+ case WATCHDOG_RELATIVE:
+ _Thread_Timer_insert_relative(
+ the_thread,
+ cpu_self,
+ _Thread_Timeout,
+ (Watchdog_Interval) queue_context->timeout_interval
+ );
+ break;
+ case WATCHDOG_ABSOLUTE:
+ _Thread_Timer_insert_absolute(
+ the_thread,
+ cpu_self,
+ _Thread_Timeout,
+ queue_context->timeout_interval
+ );
+ break;
+ default:
+ break;
}
/*
--
1.8.4.5
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
