Re: [BUG] nanosleep() granularity bumps up in 2.5.64 (was: [PATCH] settimeofday() not synchronised with gettimeofday())

Re: [BUG] nanosleep() granularity bumps up in 2.5.64 (was: [PATCH] settimeofday() not synchronised with gettimeofday())

[Eric Piel]

David Mosberger wrote:
> Are you running ntp?
Yes (I hadn't noticed it) but it was not connected to any server and
disabling it doesn't change the results.
 

> 
> On 2.5:
> 
>         $ time sleep 16
>         real    0m16.002s
>         user    0m0.001s
>         sys     0m0.002s
> 
>         $ time sleep 16.02
>         real    0m25.189s
>         user    0m0.000s
>         sys     0m0.001s
> 
> So clearly something very strange is going on.  My suspicion is that
> the bug was introduced back when x86 switched from 100Hz to 1000Hz
> ticks, but that's just a guess.  Eric, would you be
> able/willing/interested to look into this?
Sure, I aim at porting the high resolution timers but any annoying bug
related to the time can be interesting to remove.

Coincidently Vita has just reported a bug on the lkml which, after a
closer look, seems to be the same:
>   When playing with select() timeout values I found that granularity
> of nanosleep() in 2.5.64 kernel bumps to 256 msec. Trying to get finer
> granularity it ends up sleeping to the next multiple of 256 msec

>From what I understand their is a bug in the timers that causes a big
granularity. The case of Vita is a very good example. Also, after 16s it
seems the granuality (slowly?!) jumps from 1/64th s to 16s! :
sleep requested	 time obtained
14.000000000     14.006201744
15.000000000     15.006647110
16.000000000     16.007089615
17.000000000     18.742679596
18.000000000     32.014190674
19.000000000     32.014190674
20.000000000     32.014190674

I think lines like that from patch-2.5.64 are very suspicious to be
related to the bug:
+	base->timer_jiffies = INITIAL_JIFFIES;
+	base->tv1.index = INITIAL_JIFFIES & TVR_MASK;
+	base->tv2.index = (INITIAL_JIFFIES >> TVR_BITS) & TVN_MASK;
+	base->tv3.index = (INITIAL_JIFFIES >> (TVR_BITS+TVN_BITS)) & TVN_MASK;
+	base->tv4.index = (INITIAL_JIFFIES >> (TVR_BITS+2*TVN_BITS)) &
TVN_MASK;
+	base->tv5.index = (INITIAL_JIFFIES >> (TVR_BITS+3*TVN_BITS)) &
TVN_MASK;

Any idea/sugestion/patch is welcomed. Whatever, I will try to fix this
as soon as I'm back from my week end :-)

	Eric

Re: [Linux-ia64] Re: [BUG] nanosleep() granularity bumps up in 2.5.64 (was: [PATCH] settimeofday() not synchronised with gettimeofday())

[Matthew Wilcox]

On Fri, Mar 14, 2003 at 03:34:36PM +0100, Eric Piel wrote:
> I think lines like that from patch-2.5.64 are very suspicious to be
> related to the bug:
> +	base->timer_jiffies = INITIAL_JIFFIES;
> +	base->tv1.index = INITIAL_JIFFIES & TVR_MASK;
> +	base->tv2.index = (INITIAL_JIFFIES >> TVR_BITS) & TVN_MASK;
> +	base->tv3.index = (INITIAL_JIFFIES >> (TVR_BITS+TVN_BITS)) & TVN_MASK;
> +	base->tv4.index = (INITIAL_JIFFIES >> (TVR_BITS+2*TVN_BITS)) &
> TVN_MASK;
> +	base->tv5.index = (INITIAL_JIFFIES >> (TVR_BITS+3*TVN_BITS)) &
> TVN_MASK;

No, I don't think so.  Those lines are for starting `jiffies' at a very
high number so we spot jiffie-wrap bugs early on.

-- 
"It's not Hollywood.  War is real, war is primarily not about defeat or
victory, it is about death.  I've seen thousands and thousands of dead bodies.
Do you think I want to have an academic debate on this subject?" -- Robert Fisk

Re: [Linux-ia64] Re: [BUG] nanosleep() granularity bumps up in 2.5.64 (was: [PATCH] settimeofday() not synchronised with gettimeofday())

[David Mosberger]

>>>>> On Fri, 14 Mar 2003 15:34:36 +0100, Eric Piel <Eric.Piel@Bull.Net> said:

  Eric> Sure, I aim at porting the high resolution timers but any
  Eric> annoying bug related to the time can be interesting to remove.

Great!  It would be great to have someone who can focus on that.

  Eric> Any idea/sugestion/patch is welcomed. Whatever, I will try to
  Eric> fix this as soon as I'm back from my week end :-)

Does the x86 show the same behavior?  That would be a useful start to
narrow down the problem.

	--david

Re: [Linux-ia64] Re: [BUG] nanosleep() granularity bumps up in 2.5.64 (was: [PATCH] settimeofday() not synchronised with gettimeofday())

[Vitezslav Samel]

On Fri, Mar 14, 2003 at 02:48:59PM +0000, Matthew Wilcox wrote:
> On Fri, Mar 14, 2003 at 03:34:36PM +0100, Eric Piel wrote:
> > I think lines like that from patch-2.5.64 are very suspicious to be
> > related to the bug:
> > +	base->timer_jiffies = INITIAL_JIFFIES;
> > +	base->tv1.index = INITIAL_JIFFIES & TVR_MASK;
> > +	base->tv2.index = (INITIAL_JIFFIES >> TVR_BITS) & TVN_MASK;
> > +	base->tv3.index = (INITIAL_JIFFIES >> (TVR_BITS+TVN_BITS)) & TVN_MASK;
> > +	base->tv4.index = (INITIAL_JIFFIES >> (TVR_BITS+2*TVN_BITS)) &
> > TVN_MASK;
> > +	base->tv5.index = (INITIAL_JIFFIES >> (TVR_BITS+3*TVN_BITS)) &
> > TVN_MASK;
> 
> No, I don't think so.  Those lines are for starting `jiffies' at a very
> high number so we spot jiffie-wrap bugs early on.

  The nanosleep() bug narrowed down to 2.5.63-bk2. That's version, the "initial
jiffies" patch went in. And yes, it's on i686 machine.

	Cheers,
		Vita

Re: [BUG] nanosleep() granularity bumps up in 2.5.64

[Tim Schmielau]

On Mon, 17 Mar 2003, Vitezslav Samel wrote:

> On Fri, Mar 14, 2003 at 02:48:59PM +0000, Matthew Wilcox wrote:
> > On Fri, Mar 14, 2003 at 03:34:36PM +0100, Eric Piel wrote:
> > > I think lines like that from patch-2.5.64 are very suspicious to be
> > > related to the bug:
> > > +	base->timer_jiffies = INITIAL_JIFFIES;
> > > +	base->tv1.index = INITIAL_JIFFIES & TVR_MASK;
> > > +	base->tv2.index = (INITIAL_JIFFIES >> TVR_BITS) & TVN_MASK;
> > > +	base->tv3.index = (INITIAL_JIFFIES >> (TVR_BITS+TVN_BITS)) & TVN_MASK;
> > > +	base->tv4.index = (INITIAL_JIFFIES >> (TVR_BITS+2*TVN_BITS)) &
> > > TVN_MASK;
> > > +	base->tv5.index = (INITIAL_JIFFIES >> (TVR_BITS+3*TVN_BITS)) &
> > > TVN_MASK;
> >
> > No, I don't think so.  Those lines are for starting `jiffies' at a very
> > high number so we spot jiffie-wrap bugs early on.
>
>   The nanosleep() bug narrowed down to 2.5.63-bk2. That's version, the "initial
> jiffies" patch went in. And yes, it's on i686 machine.

You can easily check whether it's connected with this change by setting
INITIAL_JIFFIES to zero. This should exactly recover the previous
situation.
I.e., something like the following (untested, hand-crafted) patch:

--- linux-2.5.64/include/linux/time.h
+++ linux-2.5.64/include/linux/time.h
@@ -28,7 +28,7 @@
  * Have the 32 bit jiffies value wrap 5 minutes after boot
  * so jiffies wrap bugs show up earlier.
  */
- #define INITIAL_JIFFIES ((unsigned int) (-300*HZ))
+ #define INITIAL_JIFFIES 0

 /*
  * Change timeval to jiffies, trying to avoid the


Tim

[BUG & WORKAROUND] nanosleep() granularity bumps up in 2.5.64

[Tim Schmielau]

On Mon, 17 Mar 2003, Tim Schmielau wrote:

> On Mon, 17 Mar 2003, Vitezslav Samel wrote:
>
> >   The nanosleep() bug narrowed down to 2.5.63-bk2. That's version, the "initial
> > jiffies" patch went in. And yes, it's on i686 machine.
>
> You can easily check whether it's connected with this change by setting
> INITIAL_JIFFIES to zero. This should exactly recover the previous
> situation.

OK. I've done the test myself and I plead guilty. As a temporary
workaround you can apply the following patch:


--- linux-2.5.64/include/linux/time.h.orig	Wed Mar  5 04:29:24 2003
+++ linux-2.5.64/include/linux/time.h	Mon Mar 17 20:31:06 2003
@@ -31,7 +31,7 @@
  * Have the 32 bit jiffies value wrap 5 minutes after boot
  * so jiffies wrap bugs show up earlier.
  */
-#define INITIAL_JIFFIES ((unsigned int) (-300*HZ))
+#define INITIAL_JIFFIES 0

 /*
  * Change timeval to jiffies, trying to avoid the


Still, after half an hour of glancing at the code I can't see my mistake.
I've re-checked that the problem does not occur with the original "initial
jiffies" patch for 2.4. So I must have missed a (subtle?) difference
between 2.4 and 2.5 when I did the forward-port.

Sorry,
Tim

[PATCH] fix nanosleep() granularity bumps

[Tim Schmielau]

On Mon, 17 Mar 2003, Tim Schmielau wrote:

> I've re-checked that the problem does not occur with the original "initial
> jiffies" patch for 2.4.

Stupid me - 2.4 with the patch has the same problem, it just takes 10x as
long to show up, a range that was not covered by the testcase.


The actual problem is that I didn't fully understand the, well,
'elaborated' way of counting jiffies in the timer cascade:

When starting out with timer_jiffies=0, the timer cascade is
(unneccessarily) triggered on the first timer interrupt, incrementing all
the higher indices.
When starting with any other initial jiffies value, we miss that and end
up with all higher indices being off by one.

Fix is as follows:

--- linux-2.5.65/kernel/timer.c	Wed Mar  5 04:29:33 2003
+++ linux-2.5.65-jfix/kernel/timer.c	Tue Mar 18 09:40:13 2003
@@ -1183,10 +1183,14 @@

 	base->timer_jiffies = INITIAL_JIFFIES;
 	base->tv1.index = INITIAL_JIFFIES & TVR_MASK;
-	base->tv2.index = (INITIAL_JIFFIES >> TVR_BITS) & TVN_MASK;
-	base->tv3.index = (INITIAL_JIFFIES >> (TVR_BITS+TVN_BITS)) & TVN_MASK;
-	base->tv4.index = (INITIAL_JIFFIES >> (TVR_BITS+2*TVN_BITS)) & TVN_MASK;
-	base->tv5.index = (INITIAL_JIFFIES >> (TVR_BITS+3*TVN_BITS)) & TVN_MASK;
+	base->tv2.index = ((INITIAL_JIFFIES >> TVR_BITS)
+	                   + (INITIAL_JIFFIES!=0)) & TVN_MASK;
+	base->tv3.index = ((INITIAL_JIFFIES >> (TVR_BITS+TVN_BITS))
+	                   + (INITIAL_JIFFIES!=0)) & TVN_MASK;
+	base->tv4.index = ((INITIAL_JIFFIES >> (TVR_BITS+2*TVN_BITS))
+	                   + (INITIAL_JIFFIES!=0)) & TVN_MASK;
+	base->tv5.index = ((INITIAL_JIFFIES >> (TVR_BITS+3*TVN_BITS))
+	                   + (INITIAL_JIFFIES!=0)) & TVN_MASK;
 }

 static int __devinit timer_cpu_notify(struct notifier_block *self,


Sorry for all the trouble,
Tim

Re: [PATCH] fix nanosleep() granularity bumps

[Finn Arne Gangstad]

On Tue, Mar 18, 2003 at 10:05:56AM +0100, Tim Schmielau wrote:
> On Mon, 17 Mar 2003, Tim Schmielau wrote:
> 
> > I've re-checked that the problem does not occur with the original "initial
> > jiffies" patch for 2.4.
> 
> Stupid me - 2.4 with the patch has the same problem, it just takes 10x as
> long to show up, a range that was not covered by the testcase.
> 
> 
> The actual problem is that I didn't fully understand the, well,
> 'elaborated' way of counting jiffies in the timer cascade:
> 
> When starting out with timer_jiffies=0, the timer cascade is
> (unneccessarily) triggered on the first timer interrupt, incrementing all
> the higher indices.
> When starting with any other initial jiffies value, we miss that and end
> up with all higher indices being off by one.

Suggest the attached patch as a fix instead - easier to understand I
think and works for every possible start value. This is what I made for
Andrea Arcangeli many years ago...

diff -ur linux-2.5.65/kernel/timer.c linux-2.5.65-new/kernel/timer.c
--- linux-2.5.65/kernel/timer.c	2003-03-17 22:44:41.000000000 +0100
+++ linux-2.5.65-new/kernel/timer.c	2003-03-24 12:57:31.000000000 +0100
@@ -1182,11 +1182,14 @@
 		INIT_LIST_HEAD(base->tv1.vec + j);
 
 	base->timer_jiffies = INITIAL_JIFFIES;
-	base->tv1.index = INITIAL_JIFFIES & TVR_MASK;
-	base->tv2.index = (INITIAL_JIFFIES >> TVR_BITS) & TVN_MASK;
-	base->tv3.index = (INITIAL_JIFFIES >> (TVR_BITS+TVN_BITS)) & TVN_MASK;
-	base->tv4.index = (INITIAL_JIFFIES >> (TVR_BITS+2*TVN_BITS)) & TVN_MASK;
-	base->tv5.index = (INITIAL_JIFFIES >> (TVR_BITS+3*TVN_BITS)) & TVN_MASK;
+	base->tv1.index = (1 + (INITIAL_JIFFIES - 1)) & TVR_MASK;
+	base->tv2.index = (1 + ((INITIAL_JIFFIES - 1) >> TVR_BITS)) & TVN_MASK;
+	base->tv3.index = (1 + ((INITIAL_JIFFIES - 1)
+				>> (TVR_BITS + TVN_BITS))) & TVN_MASK;
+	base->tv4.index = (1 + ((INITIAL_JIFFIES - 1)
+				>> (TVR_BITS + 2 * TVN_BITS))) & TVN_MASK;
+	base->tv5.index = (1 + ((INITIAL_JIFFIES - 1)
+				>> (TVR_BITS + 3 * TVN_BITS))) & TVN_MASK;
 }
 	
 static int __devinit timer_cpu_notify(struct notifier_block *self, 


- Finn Arne

Re: [PATCH] fix nanosleep() granularity bumps

[Tim Schmielau]

> On Tue, Mar 18, 2003 at 10:05:56AM +0100, Tim Schmielau wrote:
[...]
> Suggest the attached patch as a fix instead - easier to understand I
> think and works for every possible start value. This is what I made for
> Andrea Arcangeli many years ago...
>
> diff -ur linux-2.5.65/kernel/timer.c linux-2.5.65-new/kernel/timer.c
> --- linux-2.5.65/kernel/timer.c	2003-03-17 22:44:41.000000000 +0100
> +++ linux-2.5.65-new/kernel/timer.c	2003-03-24 12:57:31.000000000 +0100
> @@ -1182,11 +1182,14 @@
>  		INIT_LIST_HEAD(base->tv1.vec + j);
>
>  	base->timer_jiffies = INITIAL_JIFFIES;
> -	base->tv1.index = INITIAL_JIFFIES & TVR_MASK;
> -	base->tv2.index = (INITIAL_JIFFIES >> TVR_BITS) & TVN_MASK;
> -	base->tv3.index = (INITIAL_JIFFIES >> (TVR_BITS+TVN_BITS)) & TVN_MASK;
> -	base->tv4.index = (INITIAL_JIFFIES >> (TVR_BITS+2*TVN_BITS)) & TVN_MASK;
> -	base->tv5.index = (INITIAL_JIFFIES >> (TVR_BITS+3*TVN_BITS)) & TVN_MASK;
> +	base->tv1.index = (1 + (INITIAL_JIFFIES - 1)) & TVR_MASK;
> +	base->tv2.index = (1 + ((INITIAL_JIFFIES - 1) >> TVR_BITS)) & TVN_MASK;
> +	base->tv3.index = (1 + ((INITIAL_JIFFIES - 1)
> +				>> (TVR_BITS + TVN_BITS))) & TVN_MASK;
> +	base->tv4.index = (1 + ((INITIAL_JIFFIES - 1)
> +				>> (TVR_BITS + 2 * TVN_BITS))) & TVN_MASK;
> +	base->tv5.index = (1 + ((INITIAL_JIFFIES - 1)
> +				>> (TVR_BITS + 3 * TVN_BITS))) & TVN_MASK;
>  }
>
>  static int __devinit timer_cpu_notify(struct notifier_block *self,

Too late - the whole tv[1-5].index duplication is gone already after a
cleanup done by George Anzinger.

Tim