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, >>>>> 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 >> _______________________________________________ >> Openais mailing list >> [email protected] >> https://lists.linux-foundation.org/mailman/listinfo/openais >
commit b7cb8efc0eb146649f8d09c9d59c540179e83bb3 Author: Jan Friesse <[email protected]> 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_CLOCK static 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; +} +#endif static inline void timerlist_add (struct timerlist *timerlist, struct timerlist_timer *timer) {
_______________________________________________ Openais mailing list [email protected] https://lists.linux-foundation.org/mailman/listinfo/openais
