Re: [PATCH] time: Add locking to xtime access in get_seconds()

[john stultz]

On Thu, 2011-05-05 at 19:57 +0200, Andi Kleen wrote:
> > I suspect the reason this hasn't been triggered on x86 or power6 is due
> > to compiler or processor optimizations reordering the assignment to in
> > effect make it atomic. Or maybe the timing window to see the issue is
> > harder to observe?
> 
> On x86 all aligned stores are atomic. So I don't see how this 
> could be a problem ever.

No no. The issue was with the fact that in update_xtime_cache we modify
xtime_cache twice (once setting it possibly backwards to xtime, then
adding in the nsec offset).

Since get_seconds does no locking, this issue should be visible
anywhere, as long as you manage to hit the race window between the first
assignment and the second.

However, in the testing, the issue only showed up on P7, but not P6 or
x86.

My guess was that the code:

xtime_cache.sec = xtime.sec
xtime_cache.nsec = xtime.nsec
xtime_cache.sec = xtime_cache.sec 
		 + div(xtime_cache.nsec + nsec, NSEC_PER_SEC, &rem);
xtime_cache.nsec = rem

Was getting rearranged to:

xtime_cache.sec = xtime.sec 
		+ div(xtime.nsec + nsec, NSEC_PER_SEC, &rem);
xtime_cache.nsec = rem


Which makes the xtime_cache.sec update atomic.

But its just a guess.

thanks
-john


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/