mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Andreas Mohr <andi@rhlx01.fht-esslingen.de>
To: lkml <linux-kernel@vger.kernel.org>
Cc: john stultz <johnstul@us.ibm.com>, Dominik Brodowski <linux@brodo.de>
Subject: delay_tsc(): inefficient delay loop (2.6.16-mm1)
Date: Fri, 24 Mar 2006 18:04:36 +0100	[thread overview]
Message-ID: <20060324170436.GA1568@rhlx01.fht-esslingen.de> (raw)

Hello all,

I discovered that the delay loop used there (which simply found a new place
in 2.6.16-mm1; it existed much longer) is inefficient,
since it does an unnecessary subtraction *in the main loop* which also hits
asm code:

00000024 <delay_tsc>:
  24:   53                      push   %ebx
  25:   89 c3                   mov    %eax,%ebx
  27:   0f 31                   rdtsc
  29:   89 c1                   mov    %eax,%ecx
  2b:   f3 90                   pause
  2d:   0f 31                   rdtsc
  2f:   29 c8                   sub    %ecx,%eax
  31:   39 d8                   cmp    %ebx,%eax
  33:   72 f6                   jb     2b <delay_tsc+0x7>
  35:   5b                      pop    %ebx
  36:   c3                      ret

With such a patch:

--- linux-2.6.16-mm1/arch/i386/lib/delay.c.orig	2006-03-23 12:52:45.000000000 +0100
+++ linux-2.6.16-mm1/arch/i386/lib/delay.c	2006-03-24 12:53:01.000000000 +0100
@@ -44,10 +44,12 @@
 	unsigned long bclock, now;
 
 	rdtscl(bclock);
+	/* offset with bclock to have very simple comparison below */
+	loops += bclock;
 	do {
 		rep_nop();
 		rdtscl(now);
-	} while ((now-bclock) < loops);
+	} while (now < loops);
 }
 
 /*

the result is:

00000024 <delay_tsc>:
  24:   89 c1                   mov    %eax,%ecx
  26:   0f 31                   rdtsc
  28:   01 c1                   add    %eax,%ecx
  2a:   f3 90                   pause
  2c:   0f 31                   rdtsc
  2e:   39 c8                   cmp    %ecx,%eax
  30:   72 f8                   jb     2a <delay_tsc+0x6>
  32:   c3                      ret

Improvement: no unnecessary stuff after having hit the timer target value
(read: faster), better power saving, 4 bytes less opcodes.

The patch above could be considered weird since it fiddles with "loops"
directly. A possibly cleaner way would be to introduce a new variable
end = bclock + loops.

Comments? Anything that I'm missing here as to why it hasn't been done that
way before?

If this holds water then I'll submit a final patch soon.

Thanks!

Andreas Mohr

             reply	other threads:[~2006-03-24 17:04 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-03-24 17:04 Andreas Mohr [this message]
2006-03-24 17:22 ` Ray Lee
2006-03-24 17:41   ` Andreas Mohr
2006-03-24 17:37 ` Edgar Toernig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20060324170436.GA1568@rhlx01.fht-esslingen.de \
    --to=andi@rhlx01.fht-esslingen.de \
    --cc=johnstul@us.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@brodo.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®