Re: Bug in arch/i386/lib/delay.c file, delay_loop function

["Jiri Hladky" <hladky.jiri@googlemail.com>]

[-- Attachment #1: Type: text/plain, Size: 1349 bytes --]

Hi Ingo, hi Martin!

As Martin correctly pointed out, my code will fail when loops=0. So I
have added a fix for this.

The patch file is attached:

diff -up arch/x86/lib/delay_32.c-orig arch/x86/lib/delay_32.c > delay_32.c_patch

I'm attaching also both kernel source codes (delay_32.c-orig and
delay_32.c) for reference.

Thanks
Jiri

On Sun, Jun 1, 2008 at 7:05 PM, Martin Mares <mj@ucw.cz> wrote:
> Hi!
>
>> when trying to understand how Bogomips are implemented I have found
>> bug in arch/i386/lib/delay.c file, delay_loop function
> [...]
>
>> The function fails for loops > 2^31+1. It because SF is set when dec
>> returns numbers > 2^31
>>
>> The fix is to use jnz instruction instead of jns (and add one decl
>> instruction to the end to have exactly the same number of loops as in
>> original version):
>
> It is a long time since I have hacked that file, but you should definitely
> make sure that the function is never called with a zero argument. In such
> case, the original version made just a single pass, but your version
> makes 2^32 of them.
>
>                                Have a nice fortnight
> --
> Martin `MJ' Mares                          <mj@ucw.cz>   http://mj.ucw.cz/
> Faculty of Math and Physics, Charles University, Prague, Czech Rep., Earth
> Homo homini lupus, frater fratri lupior, bohemus bohemo lupissimus.
>

[-- Attachment #2: delay_32.c_patch --]
[-- Type: application/octet-stream, Size: 1205 bytes --]

--- arch/x86/lib/delay_32.c-orig	2008-05-31 23:44:34.000000000 +0200
+++ arch/x86/lib/delay_32.c	2008-06-02 11:55:50.000000000 +0200
@@ -3,6 +3,7 @@
  *
  *	Copyright (C) 1993 Linus Torvalds
  *	Copyright (C) 1997 Martin Mares <mj@atrey.karlin.mff.cuni.cz>
+ *	Copyright (C) 2008 Jiri Hladky <hladky _dot_ jiri _at_ gmail _dot_ com>
  *
  *	The __delay function must _NOT_ be inlined as its execution time
  *	depends wildly on alignment on many x86 processors. The additional
@@ -28,16 +29,32 @@
 /* simple loop based delay: */
 static void delay_loop(unsigned long loops)
 {
-	int d0;
-
-	__asm__ __volatile__(
-		"\tjmp 1f\n"
-		".align 16\n"
-		"1:\tjmp 2f\n"
-		".align 16\n"
-		"2:\tdecl %0\n\tjns 2b"
-		:"=&a" (d0)
-		:"0" (loops));
+/*
+ * Translates to
+#APP
+	test %eax,%eax
+	jz 3f
+	jmp 1f
+.align 16
+1:      jmp 2f
+.align 16
+2:      decl %eax
+        jnz 2b
+3:			decl %eax
+#NO_APP
+*/
+__asm__ __volatile__(
+       "test %0,%0\n"
+       "\tjz 3f\n"
+       "\tjmp 1f\n"
+       ".align 16\n"
+       "1:\tjmp 2f\n"
+       ".align 16\n"
+       "2:\tdecl %0\n\tjnz 2b\n"
+       "3:\tdecl %0"
+       :/*we don't need output */
+       :"a" (loops));
+}
 }
 
 /* TSC based delay: */

[-- Attachment #3: linux-2.6.25.4-delay_32.c_patch.tar.bz2 --]
[-- Type: application/x-bzip2, Size: 2061 bytes --]