Re: [PATCH] Make the kernel NTP code hand 64-bit *unsigned* values to do_div()

Previous thread: [PATCH [RT] 01/14] spinlocks: fix preemption feature when PREEMPT_RT is enabled by Gregory Haskins on Thursday, February 21, 2008 - 11:26 am. (56 messages)

Next thread: [PATCH] NOMMU: is_vmalloc_addr() won't compile if !MMU by David Howells on Thursday, February 21, 2008 - 11:58 am. (1 message)
To: <torvalds@...>, <akpm@...>, <zippel@...>
Cc: <linux-kernel@...>, <dhowells@...>
Date: Thursday, February 21, 2008 - 11:54 am

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

--

To: David Howells <dhowells@...>
Cc: <torvalds@...>, <akpm@...>, <linux-kernel@...>
Date: Monday, February 25, 2008 - 9:55 pm

Hi,

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;
+
+ up...

To: Roman Zippel <zippel@...>
Cc: <dhowells@...>, <torvalds@...>, <akpm@...>, <linux-kernel@...>
Date: Monday, February 25, 2008 - 10:01 pm

Yeah, that's a better plan.

David
--

To: David Howells <dhowells@...>
Cc: <torvalds@...>, <akpm@...>, <zippel@...>, <linux-kernel@...>
Date: Thursday, February 21, 2008 - 12:38 pm

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

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."
--

To: Andreas Schwab <schwab@...>
Cc: <dhowells@...>, <torvalds@...>, <akpm@...>, <zippel@...>, <linux-kernel@...>
Date: Thursday, February 21, 2008 - 12:46 pm

Good point.

David
--

Previous thread: [PATCH [RT] 01/14] spinlocks: fix preemption feature when PREEMPT_RT is enabled by Gregory Haskins on Thursday, February 21, 2008 - 11:26 am. (56 messages)

Next thread: [PATCH] NOMMU: is_vmalloc_addr() won't compile if !MMU by David Howells on Thursday, February 21, 2008 - 11:58 am. (1 message)