Hello Gedare,
On Wednesday 13 of July 2016 19:39:45 Gedare Bloom wrote:
> Gedare Bloom (5):
> cpukit: Add and use Watchdog_Discipline.
> posix: add clock_nanosleep and tests
> cpukit/rtems: fix return type mismatch for _TOD_To_seconds
> posix: refactor cond wait support to defer abstime conversion
> posix: cond_timedwait remember and use clock from condattr
it is great that RTEMS would support clock_nanosleep.
But there are some remarks for future considerations at least.
The first, I think there is real error when clock_nanosleep
is called with CLOCK_MONOTONIC and TIMER_ABSTIME.
+++ b/cpukit/posix/src/nanosleep.c
+ * High Resolution Sleep with Specifiable Clock, IEEE Std 1003.1, 2001
+ */
+int clock_nanosleep(
+ clockid_t clock_id,
+ int flags,
+ const struct timespec *rqtp,
+ struct timespec *rmtp
+)
+{
+ int err = 0;
+ if ( clock_id == CLOCK_REALTIME || clock_id == CLOCK_MONOTONIC ) {
+ if ( flags & TIMER_ABSTIME ) {
+ err = nanosleep_helper(rqtp, rmtp, WATCHDOG_ABSOLUTE);
+ } else {
+ err = nanosleep_helper(rqtp, rmtp, WATCHDOG_RELATIVE);
+ }
+ } else {
+ err = ENOTSUP;
+ }
+ return err;
+}
You use same path for CLOCK_MONOTONIC and CLOCK_REALTIME but if user
calls clock_gettime first with CLOCK_MONOTONIC, it returns timespec
value as zero based uptime. If that value is later used
as absolute timeout base for WATCHDOG_ABSOLUTE clock_nanosleep,
then it is incorrectly treated as absolute CLOCK_REALTIME time
Conversion in static inline nanosleep_helper() does not distinquis
these
ticks = _Timespec_To_ticks( rqtp );
But clock_gettime does
rtems/cpukit/posix/src/clockgettime.c
int clock_gettime(
clockid_t clock_id,
struct timespec *tp
)
{
if ( !tp )
rtems_set_errno_and_return_minus_one( EINVAL );
if ( clock_id == CLOCK_REALTIME ) {
_TOD_Get_as_timespec(tp);
return 0;
}
#ifdef CLOCK_MONOTONIC
if ( clock_id == CLOCK_MONOTONIC ) {
_TOD_Get_zero_based_uptime_as_timespec( tp );
return 0;
}
#endif
There is even more problematic and complex background
behind. The CLOCK_MONOTONIC are forbidden to be directly
adjusted by POSIX, only their increments scale can be tuned
if NTP or some other reference time source is used.
On the other hand CLOCK_REALTIME can be forcibly updated
to match world time at moment when discrepancy is found.
If you want to sleep ten clock till 8 AM, then if time
is CLOCK_REALTIME adjusted then actual sleep time
is changes. But moving base is disaster for control
tasks where precise timing has to be followed because
clock adjustment can lead to burst of sampling activities
or to stuck of control for example for one hour.
Core RTEMS Watchod infrastructure seems to define two
RB tree time (priority) queues to support both these
cases.
cpukit/score/include/rtems/score/percpu.h
typedef enum {
/**
* @brief Index for relative per-CPU watchdog header.
*
* The reference time point for this header is current ticks value
* during insert. Time is measured in clock ticks.
*/
PER_CPU_WATCHDOG_RELATIVE,
/**
* @brief Index for absolute per-CPU watchdog header.
*
* The reference time point for this header is the POSIX Epoch. Time is
* measured in nanoseconds since POSIX Epoch.
*/
PER_CPU_WATCHDOG_ABSOLUTE,
/**
* @brief Count of per-CPU watchdog headers.
*/
PER_CPU_WATCHDOG_COUNT
} Per_CPU_Watchdog_index;
It is little complicated that each is in different time scale
but support is there. But if I do not miss something,
your clock_nanosleep() does not distinguish between queues.
There are four cases for clock_nanosleep() and if only
two queues are used then they should be used next way
CLOCK_REALTIME && TIMER_ABSTIME = 0
should use PER_CPU_WATCHDOG_RELATIVE queue
value should be added to
_TOD_Get_zero_based_uptime_as_timespec( ¤t_time );
and converted to ticks or converted the first
and then added to _Watchdog_Ticks_since_boot added
CLOCK_REALTIME && TIMER_ABSTIME = 1
should use PER_CPU_WATCHDOG_RELATIVE queue
value needs to be only converted to ticks
CLOCK_MONOTONIC && TIMER_ABSTIME = 0
should use PER_CPU_WATCHDOG_ABSOLUTE queue
value has to be added to value retrieved by
_TOD_Get_as_timespec( ¤t_time );
getnanouptime(struct timespec *tsp)
with nanoseconds field overflow correction then converted
somewhere to uint64_t packed timespec value by
_Watchdog_Ticks_from_timespec
CLOCK_MONOTONIC && TIMER_ABSTIME = 1
should use PER_CPU_WATCHDOG_ABSOLUTE queue
value needs only to be repacked by _Watchdog_Ticks_from_timespec
But I am not sure if the queues are really intended that way,
one as monotonic and the second as realtime or if they are there
only to spedup/eliminate complete conversion from timespec to ticks.
But if there is option to set and adjust CLOCK_REALTIME there has to
be some other/completely separated queue for monotonic time based
operations.
The overflow of 64-bit ticks and 34 bit for seconds packed timespec
format is not probable but I would like to see support for infinite
operation there even if it cost halving range of the most distant
timeouts. It would worth to define descriptive type for uint64_t
expire which can be in ticks or packed timespec. Something like
typedef uint64_t Watchdog_expire_time_t;
It has to be unsigned to allow cyclic arithmetics.
There should exists another type for intervals
typedef int64_t Watchdog_expire_interval_t;
Then there should exists something like
int _Watchdog_expire_compare(Watchdog_expire_time_t a,
Watchdog_expire_time_t b)
{
Watchdog_expire_interval_t diff;
diff = a - b;
if (diff < 0)
return -1;
if (diff > 0)
return 1;
return 0;
}
But that can worse optimization. But
if ( expire < parent_watchdog->expire )
could be replaced at least by
if ( (Watchdog_expire_interval_t)(expire - parent_watchdog->expire) < 0 ) {
in the next construct
struct Watchdog_Control {
...
/** @brief This field is the expiration time point. */
uint64_t expire;
rtems/cpukit/score/src/watchdoginsert.c
void _Watchdog_Insert(
Watchdog_Header *header,
Watchdog_Control *the_watchdog,
uint64_t expire
)
if ( expire < parent_watchdog->expire ) {
link = _RBTree_Left_reference( parent );
} else {
link = _RBTree_Right_reference( parent );
new_first = old_first;
}
Generally, I am not 100% sure about mapping to actual RTEMS code
but the concept of realtime and monotonic time is critical
for control applications.
Best wishes,
Pavel
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
