LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] Make the kernel NTP code hand 64-bit *unsigned* values to do_div()
@ 2008-02-21 15:54 David Howells
  2008-02-21 16:38 ` Andreas Schwab
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: David Howells @ 2008-02-21 15:54 UTC (permalink / raw)
  To: torvalds, akpm, zippel; +Cc: linux-kernel, dhowells

From: David Howells <dhowells@redhat.com>

The kernel NTP code shouldn't hand 64-bit *signed* values to do_div().  Make it
instead hand 64-bit unsigned values.  This gets rid of a couple of warnings.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 kernel/time/ntp.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)


diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index c88b591..3bead00 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -342,13 +342,15 @@ int do_adjtimex(struct timex *txc)
 		    freq_adj = shift_right(freq_adj, time_constant * 2 +
 					   (SHIFT_PLL + 2) * 2 - SHIFT_NSEC);
 		    if (mtemp >= MINSEC && (time_status & STA_FLL || mtemp > MAXSEC)) {
+			u64 utemp64;
 			temp64 = time_offset << (SHIFT_NSEC - SHIFT_FLL);
 			if (time_offset < 0) {
-			    temp64 = -temp64;
-			    do_div(temp64, mtemp);
+			    utemp64 = -temp64;
+			    do_div(utemp64, mtemp);
 			    freq_adj -= temp64;
 			} else {
-			    do_div(temp64, mtemp);
+			    utemp64 = temp64;
+			    do_div(utemp64, mtemp);
 			    freq_adj += temp64;
 			}
 		    }


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Make the kernel NTP code hand 64-bit *unsigned* values to do_div()
  2008-02-21 15:54 [PATCH] Make the kernel NTP code hand 64-bit *unsigned* values to do_div() David Howells
@ 2008-02-21 16:38 ` Andreas Schwab
  2008-02-21 16:46 ` David Howells
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Andreas Schwab @ 2008-02-21 16:38 UTC (permalink / raw)
  To: David Howells; +Cc: torvalds, akpm, zippel, linux-kernel

David Howells <dhowells@redhat.com> writes:

>  			if (time_offset < 0) {
> -			    temp64 = -temp64;
> -			    do_div(temp64, mtemp);
> +			    utemp64 = -temp64;
> +			    do_div(utemp64, mtemp);
>  			    freq_adj -= temp64;

do_div previously modified temp64 by side effect, now it no longer does
that.

>  			} else {
> -			    do_div(temp64, mtemp);
> +			    utemp64 = temp64;
> +			    do_div(utemp64, mtemp);
>  			    freq_adj += temp64;

Same.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Make the kernel NTP code hand 64-bit *unsigned* values to do_div()
  2008-02-21 15:54 [PATCH] Make the kernel NTP code hand 64-bit *unsigned* values to do_div() David Howells
  2008-02-21 16:38 ` Andreas Schwab
@ 2008-02-21 16:46 ` David Howells
  2008-02-26  1:55 ` Roman Zippel
  2008-02-26  2:01 ` David Howells
  3 siblings, 0 replies; 5+ messages in thread
From: David Howells @ 2008-02-21 16:46 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: dhowells, torvalds, akpm, zippel, linux-kernel

Andreas Schwab <schwab@suse.de> wrote:

> do_div previously modified temp64 by side effect, now it no longer does
> that.

Good point.

David

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Make the kernel NTP code hand 64-bit *unsigned* values to do_div()
  2008-02-21 15:54 [PATCH] Make the kernel NTP code hand 64-bit *unsigned* values to do_div() David Howells
  2008-02-21 16:38 ` Andreas Schwab
  2008-02-21 16:46 ` David Howells
@ 2008-02-26  1:55 ` Roman Zippel
  2008-02-26  2:01 ` David Howells
  3 siblings, 0 replies; 5+ messages in thread
From: Roman Zippel @ 2008-02-26  1:55 UTC (permalink / raw)
  To: David Howells; +Cc: torvalds, akpm, linux-kernel

Hi,

On Thu, 21 Feb 2008, David Howells wrote:

> The kernel NTP code shouldn't hand 64-bit *signed* values to do_div().  Make it
> instead hand 64-bit unsigned values.  This gets rid of a couple of warnings.

I would actually prefer to introduce an explicit API for signed 64 
divides to get rid of the temps completely, something like below.
Right now it uses do_div as fallback. When all archs are converted, do_div 
can be single compatibility define and perhaps we can get rid of it
completely.
Bonus feature: implement the x86 version without the asm casts allowing 
gcc to generate better code.

bye, Roman

---
 include/asm-generic/div64.h |   14 ++++++++++++++
 include/asm-i386/div64.h    |   20 ++++++++++++++++++++
 include/linux/calc64.h      |   28 ++++++++++++++++++++++++++++
 kernel/time.c               |   26 +++++++-------------------
 kernel/time/ntp.c           |   21 +++++----------------
 lib/div64.c                 |   21 ++++++++++++++++++++-
 6 files changed, 94 insertions(+), 36 deletions(-)

Index: linux-2.6/include/asm-generic/div64.h
===================================================================
--- linux-2.6.orig/include/asm-generic/div64.h
+++ linux-2.6/include/asm-generic/div64.h
@@ -35,6 +35,20 @@ static inline uint64_t div64_64(uint64_t
 	return dividend / divisor;
 }
 
+static inline u64 div_u64_rem(u64 dividend, u32 divisor, u32 *remainder)
+{
+	*remainder = dividend % divisor;
+	return dividend / divisor;
+}
+#define div_u64_rem	div_u64_rem
+
+static inline s64 div_s64_rem(s64 dividend, s32 divisor, s32 *remainder)
+{
+	*remainder = dividend % divisor;
+	return dividend / divisor;
+}
+#define div_s64_rem	div_s64_rem
+
 #elif BITS_PER_LONG == 32
 
 extern uint32_t __div64_32(uint64_t *dividend, uint32_t divisor);
Index: linux-2.6/include/asm-i386/div64.h
===================================================================
--- linux-2.6.orig/include/asm-i386/div64.h
+++ linux-2.6/include/asm-i386/div64.h
@@ -48,5 +48,25 @@ div_ll_X_l_rem(long long divs, long div,
 
 }
 
+static inline u64 div_u64_rem(u64 dividend, u32 divisor, u32 *remainder)
+{
+	union {
+		u64 v64;
+		u32 v32[2];
+	} d = { dividend };
+	u32 upper;
+
+	upper = d.v32[1];
+	if (upper) {
+		upper = d.v32[1] % divisor;
+		d.v32[1] = d.v32[1] / divisor;
+	}
+	asm ("divl %2" : "=a" (d.v32[0]), "=d" (*remainder) :
+		"rm" (divisor), "0" (d.v32[0]), "1" (upper));
+	return d.v64;
+}
+#define div_u64_rem	div_u64_rem
+
 extern uint64_t div64_64(uint64_t dividend, uint64_t divisor);
+
 #endif
Index: linux-2.6/include/linux/calc64.h
===================================================================
--- linux-2.6.orig/include/linux/calc64.h
+++ linux-2.6/include/linux/calc64.h
@@ -46,4 +46,32 @@ static inline long div_long_long_rem_sig
 	return res;
 }
 
+#ifndef div_u64_rem
+static inline u64 div_u64_rem(u64 dividend, u32 divisor, u32 *remainder)
+{
+	*remainder = do_div(dividend, divisor);
+	return dividend;
+}
+#endif
+
+#ifndef div_u64
+static inline u64 div_u64(u64 dividend, u32 divisor)
+{
+	u32 remainder;
+	return div_u64_rem(dividend, divisor, &remainder);
+}
+#endif
+
+#ifndef div_s64_rem
+extern s64 div_s64_rem(s64 dividend, s32 divisor, s32 *remainder);
+#endif
+
+#ifndef div_s64
+static inline s64 div_s64(s64 dividend, s32 divisor)
+{
+	s32 remainder;
+	return div_s64_rem(dividend, divisor, &remainder);
+}
+#endif
+
 #endif
Index: linux-2.6/kernel/time.c
===================================================================
--- linux-2.6.orig/kernel/time.c
+++ linux-2.6/kernel/time.c
@@ -661,9 +661,7 @@ clock_t jiffies_to_clock_t(long x)
 #if (TICK_NSEC % (NSEC_PER_SEC / USER_HZ)) == 0
 	return x / (HZ / USER_HZ);
 #else
-	u64 tmp = (u64)x * TICK_NSEC;
-	do_div(tmp, (NSEC_PER_SEC / USER_HZ));
-	return (long)tmp;
+	return div_u64((u64)x * TICK_NSEC, NSEC_PER_SEC / USER_HZ);
 #endif
 }
 EXPORT_SYMBOL(jiffies_to_clock_t);
@@ -675,16 +673,12 @@ unsigned long clock_t_to_jiffies(unsigne
 		return ~0UL;
 	return x * (HZ / USER_HZ);
 #else
-	u64 jif;
-
 	/* Don't worry about loss of precision here .. */
 	if (x >= ~0UL / HZ * USER_HZ)
 		return ~0UL;
 
 	/* .. but do try to contain it here */
-	jif = x * (u64) HZ;
-	do_div(jif, USER_HZ);
-	return jif;
+	return div_u64((u64)x * HZ, USER_HZ);
 #endif
 }
 EXPORT_SYMBOL(clock_t_to_jiffies);
@@ -692,17 +686,15 @@ EXPORT_SYMBOL(clock_t_to_jiffies);
 u64 jiffies_64_to_clock_t(u64 x)
 {
 #if (TICK_NSEC % (NSEC_PER_SEC / USER_HZ)) == 0
-	do_div(x, HZ / USER_HZ);
+	return div_u64(x, HZ / USER_HZ);
 #else
 	/*
 	 * There are better ways that don't overflow early,
 	 * but even this doesn't overflow in hundreds of years
 	 * in 64 bits, so..
 	 */
-	x *= TICK_NSEC;
-	do_div(x, (NSEC_PER_SEC / USER_HZ));
+	return div_u64(x * TICK_NSEC, NSEC_PER_SEC / USER_HZ);
 #endif
-	return x;
 }
 
 EXPORT_SYMBOL(jiffies_64_to_clock_t);
@@ -710,21 +702,17 @@ EXPORT_SYMBOL(jiffies_64_to_clock_t);
 u64 nsec_to_clock_t(u64 x)
 {
 #if (NSEC_PER_SEC % USER_HZ) == 0
-	do_div(x, (NSEC_PER_SEC / USER_HZ));
+	return div_u64(x, NSEC_PER_SEC / USER_HZ);
 #elif (USER_HZ % 512) == 0
-	x *= USER_HZ/512;
-	do_div(x, (NSEC_PER_SEC / 512));
+	return div_u64(x * USER_HZ / 512, NSEC_PER_SEC / 512);
 #else
 	/*
          * max relative error 5.7e-8 (1.8s per year) for USER_HZ <= 1024,
          * overflow after 64.99 years.
          * exact for HZ=60, 72, 90, 120, 144, 180, 300, 600, 900, ...
          */
-	x *= 9;
-	do_div(x, (unsigned long)((9ull * NSEC_PER_SEC + (USER_HZ/2)) /
-				  USER_HZ));
+	return div_u64(x * 9, (9ull * NSEC_PER_SEC + (USER_HZ / 2)) / USER_HZ);
 #endif
-	return x;
 }
 
 #if (BITS_PER_LONG < 64)
Index: linux-2.6/kernel/time/ntp.c
===================================================================
--- linux-2.6.orig/kernel/time/ntp.c
+++ linux-2.6/kernel/time/ntp.c
@@ -53,10 +53,8 @@ static void ntp_update_frequency(void)
 
 	tick_length_base = second_length;
 
-	do_div(second_length, HZ);
-	tick_nsec = second_length >> TICK_LENGTH_SHIFT;
-
-	do_div(tick_length_base, NTP_INTERVAL_FREQ);
+	tick_nsec = div_u64(second_length, HZ) >> TICK_LENGTH_SHIFT;
+	tick_length_base = div_u64(tick_length_base, NTP_INTERVAL_FREQ);
 }
 
 /**
@@ -197,7 +195,7 @@ void __attribute__ ((weak)) notify_arch_
 int do_adjtimex(struct timex *txc)
 {
 	long mtemp, save_adjust, rem;
-	s64 freq_adj, temp64;
+	s64 freq_adj;
 	int result;
 
 	/* In order to modify anything, you gotta be super-user! */
@@ -300,17 +298,8 @@ int do_adjtimex(struct timex *txc)
 		    freq_adj = time_offset * mtemp;
 		    freq_adj = shift_right(freq_adj, time_constant * 2 +
 					   (SHIFT_PLL + 2) * 2 - SHIFT_NSEC);
-		    if (mtemp >= MINSEC && (time_status & STA_FLL || mtemp > MAXSEC)) {
-			temp64 = time_offset << (SHIFT_NSEC - SHIFT_FLL);
-			if (time_offset < 0) {
-			    temp64 = -temp64;
-			    do_div(temp64, mtemp);
-			    freq_adj -= temp64;
-			} else {
-			    do_div(temp64, mtemp);
-			    freq_adj += temp64;
-			}
-		    }
+		    if (mtemp >= MINSEC && (time_status & STA_FLL || mtemp > MAXSEC))
+			freq_adj += div_s64(time_offset << (SHIFT_NSEC - SHIFT_FLL), mtemp);
 		    freq_adj += time_freq;
 		    freq_adj = min(freq_adj, (s64)MAXFREQ_NSEC);
 		    time_freq = max(freq_adj, (s64)-MAXFREQ_NSEC);
Index: linux-2.6/lib/div64.c
===================================================================
--- linux-2.6.orig/lib/div64.c
+++ linux-2.6/lib/div64.c
@@ -18,7 +18,7 @@
 
 #include <linux/types.h>
 #include <linux/module.h>
-#include <asm/div64.h>
+#include <linux/calc64.h>
 
 /* Not needed on 64bit architectures */
 #if BITS_PER_LONG == 32
@@ -78,4 +78,23 @@ uint64_t div64_64(uint64_t dividend, uin
 }
 EXPORT_SYMBOL(div64_64);
 
+#ifndef div_s64_rem
+s64 div_s64_rem(s64 dividend, s32 divisor, s32 *remainder)
+{
+	u64 quotient;
+
+	if (dividend < 0) {
+		quotient = div_u64_rem(-dividend, abs(divisor), (u32 *)remainder);
+		*remainder = -*remainder;
+		if (divisor > 0)
+			quotient = -quotient;
+	} else {
+		quotient = div_u64_rem(dividend, abs(divisor), (u32 *)remainder);
+		if (divisor < 0)
+			quotient = -quotient;
+	}
+	return quotient;
+}
+#endif
+
 #endif /* BITS_PER_LONG == 32 */

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Make the kernel NTP code hand 64-bit *unsigned* values to do_div()
  2008-02-21 15:54 [PATCH] Make the kernel NTP code hand 64-bit *unsigned* values to do_div() David Howells
                   ` (2 preceding siblings ...)
  2008-02-26  1:55 ` Roman Zippel
@ 2008-02-26  2:01 ` David Howells
  3 siblings, 0 replies; 5+ messages in thread
From: David Howells @ 2008-02-26  2:01 UTC (permalink / raw)
  To: Roman Zippel; +Cc: dhowells, torvalds, akpm, linux-kernel

Roman Zippel <zippel@linux-m68k.org> wrote:

> I would actually prefer to introduce an explicit API for signed 64 
> divides to get rid of the temps completely

Yeah, that's a better plan.

David

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2008-02-26  2:02 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-02-21 15:54 [PATCH] Make the kernel NTP code hand 64-bit *unsigned* values to do_div() David Howells
2008-02-21 16:38 ` Andreas Schwab
2008-02-21 16:46 ` David Howells
2008-02-26  1:55 ` Roman Zippel
2008-02-26  2:01 ` David Howells

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).