gmane.linux.kernel

john stultz | 20 Aug 04:27

[PATCH] fix adjtimex() freq return error

Martin Ziegler reported a bug in adjtimex(), where odd positive 
frequencies are reduced by 1 while negative values are correct.

See http://bugzilla.kernel.org/show_bug.cgi?id=11370 for details

The issue was introduced in the following commit that increased 
time_freq to a 64bit value and increased its resolution:

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=074b3b87941c99bc0ce35385b5817924b1ed0c23;hp=eea83d896e318bda54be2d2770d2c5d6668d11db

The problem is how we convert from our internal high res value back to
userland. Instead of dividing, we shift, then mult, then shift again,
causing the slight error seen in the bug.

I don't think adjtimex() is a terribly hot path, so this patch simply
uses div_s64() to convert back to the external resolution and removes 
the unused shift defines to resolve the issue.

Signed-off-by: John Stultz <johnstul <at> us.ibm.com>

Index: 2.6-git/kernel/time/ntp.c
===================================================================
--- 2.6-git.orig/kernel/time/ntp.c
+++ 2.6-git/kernel/time/ntp.c
@@ -402,9 +402,7 @@ int do_adjtimex(struct timex *txc)
 		if (!(time_status & STA_NANO))
 			txc->offset /= NSEC_PER_USEC;
 	}
-	txc->freq	   = shift_right((s32)(time_freq >> PPM_SCALE_INV_SHIFT) *
-					 (s64)PPM_SCALE_INV,

(Continue reading)

headers

Andrew Morton | 20 Aug 05:47

Re: [PATCH] fix adjtimex() freq return error

On Tue, 19 Aug 2008 19:29:06 -0700 john stultz <johnstul <at> us.ibm.com> wrote:

> Martin Ziegler reported a bug in adjtimex(), where odd positive 
> frequencies are reduced by 1 while negative values are correct.
> 
> See http://bugzilla.kernel.org/show_bug.cgi?id=11370 for details
> 
> The issue was introduced in the following commit that increased 
> time_freq to a 64bit value and increased its resolution:
> 
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=074b3b87941c99bc0ce35385b5817924b1ed0c23;hp=eea83d896e318bda54be2d2770d2c5d6668d11db
> 
> The problem is how we convert from our internal high res value back to
> userland. Instead of dividing, we shift, then mult, then shift again,
> causing the slight error seen in the bug.
> 
> I don't think adjtimex() is a terribly hot path, so this patch simply
> uses div_s64() to convert back to the external resolution and removes 
> the unused shift defines to resolve the issue.
> 
> Signed-off-by: John Stultz <johnstul <at> us.ibm.com>
> 
> Index: 2.6-git/kernel/time/ntp.c
> ===================================================================
> --- 2.6-git.orig/kernel/time/ntp.c
> +++ 2.6-git/kernel/time/ntp.c
> @@ -402,9 +402,7 @@ int do_adjtimex(struct timex *txc)
>  		if (!(time_status & STA_NANO))
>  			txc->offset /= NSEC_PER_USEC;
>  	}

(Continue reading)

headers

Roman Zippel | 20 Aug 14:02

Re: [PATCH] fix adjtimex() freq return error

Hi,

On Tue, 19 Aug 2008, Andrew Morton wrote:

> On Tue, 19 Aug 2008 19:29:06 -0700 john stultz <johnstul <at> us.ibm.com> wrote:
> 
> > Martin Ziegler reported a bug in adjtimex(), where odd positive 
> > frequencies are reduced by 1 while negative values are correct.

To put this in perspective one should also know that 1 here means an error 
of 2^-16ppm or less than 0.02nsec.
When I wrote this code, I noticed that the rounding isn't perfect, but 
OTOH the error is so small that it can't have any practical relevance.
AFAIC the old code is fine as is.

bye, Roman

headers

Roman Zippel | 21 Aug 05:07

Re: [PATCH] fix adjtimex() freq return error

Hi,

On Tue, 19 Aug 2008, Andrew Morton wrote:

> This fix caused Roman's
> ntp-fix-adj_offset_ss_read-bug-and-do_adjtimex-cleanup.patch (which I
> have queued for when Thomas wakes up) to toss a reject, but fixing that
> seemed pretty simple.  I think.  The scripts/kconfig/conf segfaults are
> being a bit of a PITA at present.

Below is a better patch on top of the above patch, note that this not a 
bug fix and thus not an issue for stable.

bye, Roman

[PATCH] improve adjtimex frequency rounding

Change PPM_SCALE_INV_SHIFT so that it doesn't throw away any input bits
(19 is the amount of the factor 2 in PPM_SCALE), the output frequency
can then be calculated back to its input value, as the inverse divide
produce a slightly larger value, which is then correctly rounded by the
final shift.

Signed-off-by: Roman Zippel <zippel <at> linux-m68k.org>

---
 include/linux/timex.h |    2 +-
 kernel/time/ntp.c     |    5 ++---
 2 files changed, 3 insertions(+), 4 deletions(-)

(Continue reading)