Messages in this thread | | | Date | Mon, 18 Jun 2012 11:20:17 -0700 | From | John Stultz <> | Subject | Re: [PATCH -stable] ntp: Correct TAI offset during leap second |
| |
On 06/18/2012 06:55 AM, Ben Hutchings wrote: > On Sun, 2012-06-17 at 19:34 +0200, Richard Cochran wrote: >> On Sun, Jun 17, 2012 at 11:47:51AM -0500, Jonathan Nieder wrote: >>> Ben Hutchings wrote: >>> >>> 6b43ae8a619d (ntp: Fix leap-second hrtimer livelock) sounds important, >>> but the patch depends on bd3312681f69 (ntp: Add ntp_lock to replace >>> xtime_locking) which does not have a commit message explaining its >>> purpose (and that patch in turn depends on ea7cf49a7633). > If I understand the commit message for 6b43ae8a619d correctly, the > livelock results from ntp_lock and xtime_lock being acquired in opposite > orders in two threads. Which means it wasn't possible before ntp_lock > was introduced in bd3312681f69. Yes, I think Ben is right that before the ntp_lock split the potential deadlock couldn't happen.
>>> John, is that bug present in 3.2.y and 3.0.y, too? Any hints for >>> fixing it? >> It looks like incrementing the TAI offset was wrong even before >> >> 6b43ae8a ntp: Fix leap-second hrtimer livelock v3.4-rc1~44^2~9 >> >> The offset should change upon entering state OOP, so something like >> the following (untested) patch should fix it for 3.2.9. > [...] > > It looks like this patch just changes the offset reported by adjtimex() > during an inserted second; is that right?
Yep. It just makes sure the TAI offset is adjusted at the same point that the leapsecond is inserted (as opposed to a second late).
> > Other than that, is 3.2.y likely to be OK? Is there a good way to test > that in advance; does > <http://codemonkey.org.uk/2012/06/15/testing-leap-code/> look > reasonable? Attached is a simple leap second test you can play with.
thanks -john
/* Leap second test * by: john stultz (johnstul@us.ibm.com) * (C) Copyright IBM 2012 * Licensed under the GPL */
#include <stdio.h> #include <time.h> #include <sys/time.h> #include <sys/timex.h>
#define CALLS_PER_LOOP 64 #define NSEC_PER_SEC 1000000000ULL
/* returns 1 if a <= b, 0 otherwise */ static inline int in_order(struct timespec a, struct timespec b) { if(a.tv_sec < b.tv_sec) return 1; if(a.tv_sec > b.tv_sec) return 0; if(a.tv_nsec > b.tv_nsec) return 0; return 1; }
int main(void) { struct timeval tv; struct timex tx; struct timespec list[CALLS_PER_LOOP]; int i, inconsistent; int clock_type = CLOCK_REALTIME; long now, then;
/* Get the current time */ gettimeofday(&tv, NULL);
/* Calculate the next leap second */ tv.tv_sec += 86400 - tv.tv_sec % 86400;
/* Set the time to be 10 seconds from that time */ tv.tv_sec -= 10; settimeofday(&tv, NULL);
/* Set the leap second insert flag */ tx.modes = ADJ_STATUS; tx.status = STA_INS; adjtimex(&tx);
clock_gettime(clock_type, &list[0]); now = then = list[0].tv_sec; while(now - then < 30){ inconsistent = 0;
/* Fill list */ for(i=0; i < CALLS_PER_LOOP; i++) clock_gettime(clock_type, &list[i]);
/* Check for inconsistencies */ for(i=0; i < CALLS_PER_LOOP-1; i++) if(!in_order(list[i],list[i+1])) inconsistent = i;
/* display inconsistency */ if(inconsistent){ unsigned long long delta; for(i=0; i < CALLS_PER_LOOP; i++){ if(i == inconsistent) printf("--------------------\n"); printf("%lu:%lu\n",list[i].tv_sec, list[i].tv_nsec); if(i == inconsistent + 1 ) printf("--------------------\n"); } delta = list[inconsistent].tv_sec*NSEC_PER_SEC; delta += list[inconsistent].tv_nsec; delta -= list[inconsistent+1].tv_sec*NSEC_PER_SEC; delta -= list[inconsistent+1].tv_nsec; printf("Delta: %llu ns\n", delta); fflush(0); break; } now = list[0].tv_sec; }
/* clear TIME_WAIT */ tx.modes = ADJ_STATUS; tx.status = 0; adjtimex(&tx);
return 0; }
| |