I hope final version of patch.
#ifdefs removed and fixed problem with computing HZ (not used, but may
be in future)
Regards,
Honza
Steve,
We talked about it on IRC
Jan Friesse wrote:
Steve,
new patch included.
Steven Dake wrote:
In general the patch looks very good. The issues are as follows:
1)
corosync_timer_add_absolute expects the expire time to be the
nanoseconds since the epoch, or more specifically,
from the time(7) man page:
The Epoch
Unix systems represent time in seconds since the Epoch,
which is
defined as 0:00:00 UTC on the morning of 1 January 1970.
In this case, the expire_time may not be sufficient. We want absolute
timers to match the wall clock time rather then be based upon the offset
from the current time. If that is not the case with your patch, that
needs to be fixed. absolute timers are used by services to expire
timers when a specific wall clock time has passed. So in this case, if
ntp moves the time forward, we want that timer to expire, if its
recorded wall clock time is past the current time, the timer should be
expired.
This is biggest change. I added is_absolute_timer to timerlist_timer. If
this is true -> it's absolute timer and uses old behaviour. If not, it's
relative and uses new behaviour. timerlist_add_absolute sets that
variable to true, and timerlist_add_duration to false.
This is (I hope) how this thing should behave.
2) souce should be spelled source in configure.ac
also the logic to detect monotonic clocks in configure.ac seems complex.
I'd like Jim to have a look at the configure.ac to determine if there is
a better way.
Typo fixed + Jim added to CC.
Jim,
I don't think logic is so complex, but maybe there is really some better
solution, I'm not autotools expert.
3) get_monotonic_timeofday used in pload.c and votequorum.c and
totemsrp.c should be in tlist.h and should be named differently. I'd
choose timerlist_timer_duration_current_get() with same semantics of the
function.
4)
an additional function in tlist would be helpful for calculating the
difference in times, such as timerlist_timer_duration_calculate (tv1,
tv2, tv_out)
Fixed in way point 7 suggest ;)
5)
+ nano_monotonic_hz = ts.tv_sec + 1000000000ULL / (unsigned long
long )ts.
experienced c programmers may be able to read this code if they have a
good understanding of operator precedence. I'd recommend parens to help
clarify to other maintainers the desired order of precedence.
Clarified
6)
+ timer->expire_time= timerlist_nano_monotonic_time() + nano_duration;
missing space
Missing space added
7)
regarding comment 3 and 4, this could be simplified throughout the code
base by making a new function in timerlist called
timerlist_nano_current_get() which retrieves the current nanoseconds
from either the epoch or the monotonic clock source depending on os
support.
this gets rid of all of the timersub junk throughout the code.
Done. This makes some bigger changes in computing values, but I hope it
makes it clearer. As bonus, I added TIMERLIST_?S_IN_?SEC macros, because
writing everywhere values like 1000000000ULL, doesn't look good for me.
8)
after reviewing the changes, I'm not convinced the absolute timers work
as expected by the coroapi.
good work
I think now will.
Regards
-steve
Regards,
Honza
On Tue, 2009-07-21 at 15:42 +0200, Jan Friesse wrote:
Patch should solve problem with ntp and corosync (what it really does)
by using clock_gettime.
I tried to find EVERY appearance of gettimeofday and where it makes
sense (so basically, where this value is used as timer) convert to call
clock_gettime.
Patch modifies configure.ac to detect if clock_gettime and
CLOCK_MONOTONIC exists on system. If not, gettimeofday will be used (so
you will get old behavior).
Comments are welcomed.
Regards,
Honza
_______________________________________________
Openais mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/openais
------------------------------------------------------------------------
_______________________________________________
Openais mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/openais
------------------------------------------------------------------------
_______________________________________________
Openais mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/openais
commit 75ea8266c4821372e1e2422d9e3429445a241dd4
Author: Jan Friesse <[email protected]>
Date: Tue Jul 21 15:33:52 2009 +0200
Support for monotime timer
This patch should solve problems with corosync and ntp, by using
clock_gettime where it make sense.
diff --git a/trunk/exec/timer.h b/trunk/exec/timer.h
index 3df9d36..7d6826a 100644
--- a/trunk/exec/timer.h
+++ b/trunk/exec/timer.h
@@ -43,7 +43,7 @@ extern int corosync_timer_init (
int sched_priority);
extern int corosync_timer_add_duration (
- unsigned long long nanoseconds_in_future,
+ unsigned long long nanosec_duration,
void *data,
void (*timer_fn) (void *data),
corosync_timer_handle *handle);
diff --git a/trunk/exec/tlist.h b/trunk/exec/tlist.h
index 263c8d7..2bad3b6 100644
--- a/trunk/exec/tlist.h
+++ b/trunk/exec/tlist.h
@@ -54,6 +54,13 @@ typedef void * timer_handle;
#define TIMER_HANDLE
#endif
+#define TIMERLIST_MS_IN_SEC 1000ULL
+#define TIMERLIST_US_IN_SEC 1000000ULL
+#define TIMERLIST_NS_IN_SEC 1000000000ULL
+#define TIMERLIST_US_IN_MSEC 1000ULL
+#define TIMERLIST_NS_IN_MSEC 1000000ULL
+#define TIMERLIST_NS_IN_USEC 1000ULL
+
struct timerlist {
struct list_head timer_head;
struct list_head *timer_iter;
@@ -61,7 +68,8 @@ struct timerlist {
struct timerlist_timer {
struct list_head list;
- unsigned long long nano_from_epoch;
+ unsigned long long expire_time;
+ int is_absolute_timer;
void (*timer_fn)(void *data);
void *data;
timer_handle handle_addr;
@@ -78,10 +86,34 @@ static inline unsigned long long timerlist_nano_from_epoch (void)
struct timeval time_from_epoch;
gettimeofday (&time_from_epoch, 0);
- nano_from_epoch = ((time_from_epoch.tv_sec * 1000000000ULL) + (time_from_epoch.tv_usec * 1000ULL));
+ nano_from_epoch = ((time_from_epoch.tv_sec * TIMERLIST_NS_IN_SEC) +
+ (time_from_epoch.tv_usec * TIMERLIST_NS_IN_USEC));
+
return (nano_from_epoch);
}
+static inline unsigned long long timerlist_nano_current_get (void)
+{
+ unsigned long long nano_monotonic;
+ struct timespec ts;
+
+ clock_gettime (CLOCK_MONOTONIC, &ts);
+
+ nano_monotonic = (ts.tv_sec * TIMERLIST_NS_IN_SEC) + (unsigned long long )ts.tv_nsec;
+ return (nano_monotonic);
+}
+
+static inline unsigned long long timerlist_nano_monotonic_hz (void) {
+ unsigned long long nano_monotonic_hz;
+ struct timespec ts;
+
+ clock_getres (CLOCK_MONOTONIC, &ts);
+
+ nano_monotonic_hz = TIMERLIST_NS_IN_SEC / ((ts.tv_sec * TIMERLIST_NS_IN_SEC) + ts.tv_nsec);
+
+ return (nano_monotonic_hz);
+}
+
static inline void timerlist_add (struct timerlist *timerlist, struct timerlist_timer *timer)
{
struct list_head *timer_list = 0;
@@ -95,7 +127,7 @@ static inline void timerlist_add (struct timerlist *timerlist, struct timerlist_
timer_from_list = list_entry (timer_list,
struct timerlist_timer, list);
- if (timer_from_list->nano_from_epoch > timer->nano_from_epoch) {
+ if (timer_from_list->expire_time > timer->expire_time) {
list_add (&timer->list, timer_list->prev);
found = 1;
break; /* for timer iteration */
@@ -120,7 +152,8 @@ static inline int timerlist_add_absolute (struct timerlist *timerlist,
return (-1);
}
- timer->nano_from_epoch = nano_from_epoch;
+ timer->expire_time = nano_from_epoch;
+ timer->is_absolute_timer = 1;
timer->data = data;
timer->timer_fn = timer_fn;
timer->handle_addr = handle;
@@ -144,7 +177,8 @@ static inline int timerlist_add_duration (struct timerlist *timerlist,
return (-1);
}
- timer->nano_from_epoch = timerlist_nano_from_epoch() + nano_duration;
+ timer->expire_time = timerlist_nano_current_get () + nano_duration;
+ timer->is_absolute_timer = 0;
timer->data = data;
timer->timer_fn = timer_fn;
timer->handle_addr = handle;
@@ -177,7 +211,7 @@ static inline unsigned long long timerlist_expire_time (struct timerlist *timerl
{
struct timerlist_timer *timer = (struct timerlist_timer *)_timer_handle;
- return (timer->nano_from_epoch);
+ return (timer->expire_time);
}
static inline void timerlist_pre_dispatch (struct timerlist *timerlist, timer_handle _timer_handle)
@@ -202,7 +236,7 @@ static inline void timerlist_post_dispatch (struct timerlist *timerlist, timer_h
static inline unsigned long long timerlist_msec_duration_to_expire (struct timerlist *timerlist)
{
struct timerlist_timer *timer_from_list;
- volatile unsigned long long nano_from_epoch;
+ volatile unsigned long long current_time;
volatile unsigned long long msec_duration_to_expire;
/*
@@ -215,16 +249,20 @@ static inline unsigned long long timerlist_msec_duration_to_expire (struct timer
timer_from_list = list_entry (timerlist->timer_head.next,
struct timerlist_timer, list);
- nano_from_epoch = timerlist_nano_from_epoch();
+ if (timer_from_list->is_absolute_timer) {
+ current_time = timerlist_nano_from_epoch ();
+ } else {
+ current_time = timerlist_nano_current_get ();
+ }
/*
* timer at head of list is expired, zero msecs required
*/
- if (timer_from_list->nano_from_epoch < nano_from_epoch) {
+ if (timer_from_list->expire_time < current_time) {
return (0);
}
- msec_duration_to_expire = ((timer_from_list->nano_from_epoch - nano_from_epoch) / 1000000ULL) +
+ msec_duration_to_expire = ((timer_from_list->expire_time - current_time) / TIMERLIST_NS_IN_MSEC) +
(1000 / HZ);
return (msec_duration_to_expire);
}
@@ -235,9 +273,12 @@ static inline unsigned long long timerlist_msec_duration_to_expire (struct timer
static inline void timerlist_expire (struct timerlist *timerlist)
{
struct timerlist_timer *timer_from_list;
- unsigned long long nano_from_epoch;
+ unsigned long long current_time_from_epoch;
+ unsigned long long current_monotonic_time;
+ unsigned long long current_time;
- nano_from_epoch = timerlist_nano_from_epoch();
+ current_monotonic_time = timerlist_nano_current_get ();
+ current_time_from_epoch = current_time = timerlist_nano_from_epoch ();
for (timerlist->timer_iter = timerlist->timer_head.next;
timerlist->timer_iter != &timerlist->timer_head;) {
@@ -245,7 +286,9 @@ static inline void timerlist_expire (struct timerlist *timerlist)
timer_from_list = list_entry (timerlist->timer_iter,
struct timerlist_timer, list);
- if (timer_from_list->nano_from_epoch < nano_from_epoch) {
+ current_time = (timer_from_list->is_absolute_timer ? current_time_from_epoch : current_monotonic_time);
+
+ if (timer_from_list->expire_time < current_time) {
timerlist->timer_iter = timerlist->timer_iter->next;
timerlist_pre_dispatch (timerlist, timer_from_list);
diff --git a/trunk/exec/totemsrp.c b/trunk/exec/totemsrp.c
index e40ccfb..e488463 100644
--- a/trunk/exec/totemsrp.c
+++ b/trunk/exec/totemsrp.c
@@ -89,6 +89,7 @@
#include "wthread.h"
#include "crypto.h"
+#include "tlist.h"
#define LOCALHOST_IP inet_addr("127.0.0.1")
#define QUEUE_RTR_ITEMS_SIZE_MAX 256 /* allow 256 retransmit items */
@@ -504,7 +505,7 @@ struct totemsrp_instance {
unsigned int my_cbl;
- struct timeval pause_timestamp;
+ unsigned long long int pause_timestamp;
};
struct message_handlers {
@@ -686,14 +687,12 @@ static unsigned int main_msgs_missing (void)
static int pause_flush (struct totemsrp_instance *instance)
{
- struct timeval now;
uint64_t now_msec;
uint64_t timestamp_msec;
int res = 0;
- gettimeofday (&now, NULL);
- now_msec = ((now.tv_sec * 1000ULL) + (now.tv_usec / 1000ULL));
- timestamp_msec = ((instance->pause_timestamp.tv_sec * 1000ULL) + (instance->pause_timestamp.tv_usec/1000ULL));
+ now_msec = (timerlist_nano_current_get () / TIMERLIST_NS_IN_MSEC);
+ timestamp_msec = instance->pause_timestamp / TIMERLIST_NS_IN_MSEC;
if ((now_msec - timestamp_msec) > (instance->totem_config->token_timeout / 2)) {
log_printf (instance->totemsrp_log_level_notice,
@@ -834,7 +833,7 @@ int totemsrp_initialize (
instance->totemsrp_confchg_fn = confchg_fn;
instance->use_heartbeat = 1;
- gettimeofday (&instance->pause_timestamp, NULL);
+ instance->pause_timestamp = timerlist_nano_current_get ();
if ( totem_config->heartbeat_failures_allowed == 0 ) {
log_printf (instance->totemsrp_log_level_debug,
@@ -1463,7 +1462,7 @@ static void timer_function_pause_timeout (void *data)
{
struct totemsrp_instance *instance = data;
- gettimeofday (&instance->pause_timestamp, NULL);
+ instance->pause_timestamp = timerlist_nano_current_get ();
reset_pause_timeout (instance);
}
@@ -3184,7 +3183,7 @@ static void fcc_token_update (
* Message Handlers
*/
-struct timeval tv_old;
+unsigned long long int tv_old;
/*
* message handler called when TOKEN message type received
*/
@@ -3204,17 +3203,15 @@ static int message_handler_orf_token (
unsigned int last_aru;
#ifdef GIVEINFO
- struct timeval tv_current;
- struct timeval tv_diff;
+ unsigned long long tv_current;
+ unsigned long long tv_diff;
- gettimeofday (&tv_current, NULL);
- timersub (&tv_current, &tv_old, &tv_diff);
- memcpy (&tv_old, &tv_current, sizeof (struct timeval));
+ tv_current = timerlist_nano_current_get ();
+ tv_diff = tv_current - tv_old;
+ tv_old = tv_current;
log_printf (instance->totemsrp_log_level_debug,
- "Time since last token %0.4f ms\n",
- (((float)tv_diff.tv_sec) * 1000) + ((float)tv_diff.tv_usec)
- / 1000.0);
+ "Time since last token %0.4f ms\n", ((float)tv_diff) / 1000000.0);
#endif
#ifdef TEST_DROP_ORF_TOKEN_PERCENTAGE
@@ -3451,12 +3448,12 @@ static int message_handler_orf_token (
token_send (instance, token, forward_token);
#ifdef GIVEINFO
- gettimeofday (&tv_current, NULL);
- timersub (&tv_current, &tv_old, &tv_diff);
- memcpy (&tv_old, &tv_current, sizeof (struct timeval));
+ tv_current = timerlist_nano_current_get ();
+ tv_diff = tv_current - tv_old;
+ tv_old = tv_current;
log_printf (instance->totemsrp_log_level_debug,
"I held %0.4f ms\n",
- ((float)tv_diff.tv_usec) / 1000.0);
+ ((float)tv_diff) / 1000000.0);
#endif
if (instance->memb_state == MEMB_STATE_OPERATIONAL) {
messages_deliver_to_app (instance, 0,
diff --git a/trunk/services/pload.c b/trunk/services/pload.c
index cf82bbc..19bc79a 100644
--- a/trunk/services/pload.c
+++ b/trunk/services/pload.c
@@ -62,6 +62,8 @@
#include <corosync/list.h>
#include <corosync/engine/logsys.h>
+#include "../exec/tlist.h"
+
LOGSYS_DECLARE_SUBSYS ("PLOAD");
enum pload_exec_message_req_types {
@@ -344,9 +346,9 @@ static void message_handler_req_exec_pload_start (
} while (0)
#endif
-struct timeval tv1;
-struct timeval tv2;
-struct timeval tv_elapsed;
+unsigned long long int tv1;
+unsigned long long int tv2;
+unsigned long long int tv_elapsed;
int last_msg_no = 0;
static void message_handler_req_exec_pload_mcast (
@@ -357,19 +359,19 @@ static void message_handler_req_exec_pload_mcast (
last_msg_no = pload_mcast->msg_code;
if (msgs_delivered == 0) {
- gettimeofday (&tv1, NULL);
+ tv1 = timerlist_nano_current_get ();
}
msgs_delivered += 1;
if (msgs_delivered == msgs_wanted) {
- gettimeofday (&tv2, NULL);
- timersub (&tv2, &tv1, &tv_elapsed);
+ tv2 = timerlist_nano_current_get ();
+ tv_elapsed = tv2 - tv1;
printf ("%5d Writes ", msgs_delivered);
printf ("%5d bytes per write ", msg_size);
printf ("%7.3f Seconds runtime ",
- (tv_elapsed.tv_sec + (tv_elapsed.tv_usec / 1000000.0)));
+ (tv_elapsed / 1000000000.0));
printf ("%9.3f TP/s ",
- ((float)msgs_delivered) / (tv_elapsed.tv_sec + (tv_elapsed.tv_usec / 1000000.0)));
+ ((float)msgs_delivered) / (tv_elapsed / 1000000000.0));
printf ("%7.3f MB/s.\n",
- ((float)msgs_delivered) * ((float)msg_size) / ((tv_elapsed.tv_sec + (tv_elapsed.tv_usec / 1000000.0)) * 1000000.0));
+ ((float)msgs_delivered) * ((float)msg_size) / ((tv_elapsed / 1000000000.0)) * 1000000.0);
}
}
diff --git a/trunk/services/votequorum.c b/trunk/services/votequorum.c
index 79855bf..273d50e 100644
--- a/trunk/services/votequorum.c
+++ b/trunk/services/votequorum.c
@@ -68,6 +68,8 @@
#include <corosync/ipc_votequorum.h>
#include <corosync/list.h>
+#include "../exec/tlist.h"
+
#define VOTEQUORUM_MAJOR_VERSION 7
#define VOTEQUORUM_MINOR_VERSION 0
#define VOTEQUORUM_PATCH_VERSION 0
@@ -107,7 +109,7 @@ struct cluster_node {
nodestate_t state;
- struct timeval last_hello; /* Only used for quorum devices */
+ unsigned long long int last_hello; /* Only used for quorum devices */
struct list_head list;
};
@@ -1307,13 +1309,13 @@ static void message_handler_req_lib_votequorum_leaving (void *conn, const void *
static void quorum_device_timer_fn(void *arg)
{
- struct timeval now;
-
ENTER();
if (!quorum_device || quorum_device->state == NODESTATE_DEAD)
return;
- gettimeofday(&now, NULL);
- if (quorum_device->last_hello.tv_sec + quorumdev_poll/1000 < now.tv_sec) {
+
+ if ( (quorum_device->last_hello / TIMERLIST_NS_IN_SEC) + quorumdev_poll/1000 <
+ (timerlist_nano_current_get () / TIMERLIST_NS_IN_SEC)) {
+
quorum_device->state = NODESTATE_DEAD;
log_printf(LOGSYS_LEVEL_INFO, "lost contact with quorum device\n");
recalculate_quorum(0, 0);
@@ -1395,7 +1397,7 @@ static void message_handler_req_lib_votequorum_qdisk_poll (void *conn,
if (quorum_device) {
if (req_lib_votequorum_qdisk_poll->state) {
- gettimeofday(&quorum_device->last_hello, NULL);
+ quorum_device->last_hello = timerlist_nano_current_get ();
if (quorum_device->state == NODESTATE_DEAD) {
quorum_device->state = NODESTATE_MEMBER;
recalculate_quorum(0, 0);
_______________________________________________
Openais mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/openais
