On 2014/4/15 16:18, Dave Martin wrote:
On Mon, Apr 14, 2014 at 12:03:09PM -0400, Nicolas Pitre wrote:
On Mon, 14 Apr 2014, Dave Martin wrote:
On Fri, Apr 11, 2014 at 06:16:24PM +0800, Lu Xiangyu wrote:
From: Xiangyu Lu <[email protected]>
In big-endian systems, "%1" get the most significant part of the value, cause
the instruction to get the wrong result.
When viewing ftrace record in big-endian ARM systems, we found that
the timestamp errors:
swapper-0 [001] 1325.970000: 0:120:R ==> [001] 16:120:R events/1
events/1-16 [001] 1325.970000: 16:120:S ==> [001] 0:120:R swapper
swapper-0 [000] 1325.1000000: 0:120:R + [000] 15:120:R events/0
swapper-0 [000] 1325.1000000: 0:120:R ==> [000] 15:120:R events/0
swapper-0 [000] 1326.030000: 0:120:R + [000] 1150:120:R sshd
swapper-0 [000] 1326.030000: 0:120:R ==> [000] 1150:120:R sshd
When viewed ftrace records, it will call the do_div(n, base) function, which
achieved arch/arm/include/asm/div64.h in. When n = 10000000, base = 1000000, in
do_div(n, base) will execute "umull %Q0, %R0, %1, %Q2".
Cc: <[email protected]> # 2.6.20+
Signed-off-by: Alex Wu <[email protected]>
Signed-off-by: Xiangyu Lu <[email protected]>
---
arch/arm/include/asm/div64.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/include/asm/div64.h b/arch/arm/include/asm/div64.h
index 191ada6..662c7bd 100644
--- a/arch/arm/include/asm/div64.h
+++ b/arch/arm/include/asm/div64.h
@@ -156,7 +156,7 @@
/* Select the best insn combination to perform the */ \
/* actual __m * __n / (__p << 64) operation. */ \
if (!__c) { \
- asm ( "umull %Q0, %R0, %1, %Q2\n\t" \
+ asm ( "umull %Q0, %R0, %Q1, %Q2\n\t" \
This looks plausible: these if() clauses are all concerned with
multiplying the low parts of __m and __n together, and this seems
to be the only 64-bit asm operand reference where Q or R is suspiciously
missing: so it looks likely that "Q" is required here for consistency.
My understanding of the details of this code are limited: do you have
a simple test case to demonstrate the error and the fix?
No need -- it is indeed wrong on big endian and has been so for the last
7.5 years.
OK, well with that sanity-check on my reasoning I'm happy to:
Reviewed-by: Dave Martin <[email protected]>
I suggest you go ahead and send it to Russell's patch system.
Cheers
---Dave
.
OK, thanks.
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
