Re: [PATCH -stable] ntp: Correct TAI offset during leap second

[John Stultz <johnstul@us.ibm.com>]

[-- Attachment #1: Type: text/plain, Size: 1706 bytes --]

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


[-- Attachment #2: leaptest.c --]
[-- Type: text/x-csrc, Size: 2076 bytes --]

/* 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;
}