Steve, patch should fulfill that case. regards, Honza
Steven Dake napsal(a):
_POSIX_MONOTONIC_CLOCK could be defined to -1 which would pass this ifdef test but indicates the feature is not available on the platform. In general the concept is good though. Regards -steve On Thu, 2009-07-30 at 11:51 +0200, Jan Friesse wrote:Steve, what do you think about following patch. No autotools integration, rather one simple #ifdef and warning on compilation. Regards, Honza Steven Dake wrote:After more looking, there is a sysconf test for this (_POSIX_TIMERS). I'm not sure how to do that test inside autotools though. Regards -steve On Wed, 2009-07-29 at 18:33 -0700, Steven Dake wrote:Honza, After more testing, we need the special casing. MacOSX 10.5.7 (latest version) doesn't support clock_gettime. Regards -steve On Mon, 2009-07-27 at 12:13 +0200, Jan Friesse wrote:Committed revision 2373 Steven Dake napsal(a):Patch looks good for commit Regards -steve On Thu, 2009-07-23 at 10:09 +0200, Jan Friesse wrote:I hope final version of patch.#ifdefs removed and fixed problem with computing HZ (not used, but may be in future)Regards, HonzaSteve, 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 thenanoseconds 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.Clarified6) + timer->expire_time= timerlist_nano_monotonic_time() + nano_duration; missing spaceMissing space added7) 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 workI think now will.Regards -steveRegards, HonzaOn 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 Openais@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/openais------------------------------------------------------------------------ _______________________________________________ Openais mailing list Openais@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/openais------------------------------------------------------------------------ _______________________________________________ Openais mailing list Openais@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/openais_______________________________________________ Openais mailing list Openais@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/openaisplain text document attachment (corosync-monotonic_timer-and-masox.patch) commit b7cb8efc0eb146649f8d09c9d59c540179e83bb3 Author: Jan Friesse <jfrie...@redhat.com> Date: Thu Jul 30 11:47:57 2009 +0200 Support for systems where clock_gettime is not available diff --git a/trunk/exec/tlist.h b/trunk/exec/tlist.h index 2bad3b6..a87850d 100644 --- a/trunk/exec/tlist.h +++ b/trunk/exec/tlist.h @@ -92,6 +92,7 @@ static inline unsigned long long timerlist_nano_from_epoch (void) return (nano_from_epoch); }+#ifdef _POSIX_MONOTONIC_CLOCKstatic inline unsigned long long timerlist_nano_current_get (void) { unsigned long long nano_monotonic; @@ -113,6 +114,17 @@ static inline unsigned long long timerlist_nano_monotonic_hz (void) {return (nano_monotonic_hz);} +#else +#warning "Your system doesn't support monotonic timer. gettimeofday will be used" +static inline unsigned long long timerlist_nano_current_get (void) +{ + return (timerlist_nano_from_epoch ()); +} + +static inline unsigned long long timerlist_nano_monotonic_hz (void) { + return HZ; +} +#endifstatic inline void timerlist_add (struct timerlist *timerlist, struct timerlist_timer *timer){
commit d0010d387051f08bd20a1e95af1f28df37a8d03b
Author: Jan Friesse <jfrie...@redhat.com>
Date: Thu Jul 30 11:47:57 2009 +0200
Support for systems where clock_gettime is not available
diff --git a/trunk/exec/tlist.h b/trunk/exec/tlist.h
index 2bad3b6..7b568ee 100644
--- a/trunk/exec/tlist.h
+++ b/trunk/exec/tlist.h
@@ -42,6 +42,7 @@
#include <errno.h>
#include <string.h>
#include <sys/param.h>
+#include <unistd.h>
#include <corosync/list.h>
@@ -92,6 +93,7 @@ static inline unsigned long long timerlist_nano_from_epoch (void)
return (nano_from_epoch);
}
+#if defined _POSIX_MONOTONIC_CLOCK && _POSIX_MONOTONIC_CLOCK >= 0
static inline unsigned long long timerlist_nano_current_get (void)
{
unsigned long long nano_monotonic;
@@ -113,6 +115,17 @@ static inline unsigned long long timerlist_nano_monotonic_hz (void) {
return (nano_monotonic_hz);
}
+#else
+#warning "Your system doesn't support monotonic timer. gettimeofday will be used"
+static inline unsigned long long timerlist_nano_current_get (void)
+{
+ return (timerlist_nano_from_epoch ());
+}
+
+static inline unsigned long long timerlist_nano_monotonic_hz (void) {
+ return HZ;
+}
+#endif
static inline void timerlist_add (struct timerlist *timerlist, struct timerlist_timer *timer)
{
_______________________________________________ Openais mailing list Openais@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/openais
