From: Heiko Carstens <heiko.carstens@de.ibm.com>
To: Linus Torvalds <torvalds@linux-foundation.org>,
Tony Luck <tony.luck@intel.com>
Cc: linux-kernel@vger.kernel.org
Subject: [PATCH] lockref: remove cpu_relax() again
Date: Thu, 5 Sep 2013 15:18:14 +0200 [thread overview]
Message-ID: <20130905131814.GA24274@osiris> (raw)
d472d9d9 "lockref: Relax in cmpxchg loop" added a cpu_relax() call to the
CMPXCHG_LOOP() macro. However to me it seems to be wrong since it is very
likely that the next round will succeed (or the loop will be left).
Even worse: cpu_relax() is very expensive on s390, since it means yield
"my virtual cpu to the hypervisor". So we are talking of several 1000 cycles.
In fact some measurements show the bad impact of the cpu_relax() call on
s390 using Linus' test case that "stats()" like mad:
Without converting s390 to lockref:
Total loops: 81236173
After converting s390 to lockref:
Total loops: 31896802
After converting s390 to lockref but with removed cpu_relax() call:
Total loops: 86242190
So the cpu_relax() call completely contradicts the intention of
CONFIG_CMPXCHG_LOCKREF at least on s390.
*If* however the cpu_relax() makes sense on other platforms maybe we could
add something like we have already with "arch_mutex_cpu_relax()":
include/linux/mutex.h:
#ifndef CONFIG_HAVE_ARCH_MUTEX_CPU_RELAX
#define arch_mutex_cpu_relax() cpu_relax()
#endif
arch/s390/include/asm/mutex.h:
#define arch_mutex_cpu_relax() barrier()
Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---
lib/lockref.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/lib/lockref.c b/lib/lockref.c
index 9d76f40..7819c2d 100644
--- a/lib/lockref.c
+++ b/lib/lockref.c
@@ -19,7 +19,6 @@
if (likely(old.lock_count == prev.lock_count)) { \
SUCCESS; \
} \
- cpu_relax(); \
} \
} while (0)
--
1.8.3.4
next reply other threads:[~2013-09-05 13:18 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-09-05 13:18 Heiko Carstens [this message]
2013-09-05 14:13 ` [PATCH] lockref: remove cpu_relax() again Heiko Carstens
2013-09-05 14:48 ` Luck, Tony
2013-09-05 15:31 ` Linus Torvalds
2013-09-05 17:35 ` Luck, Tony
2013-09-05 17:53 ` Linus Torvalds
2013-09-05 18:57 ` Luck, Tony
2013-09-05 19:21 ` Linus Torvalds
2013-09-05 19:45 ` Luck, Tony
2013-09-05 19:50 ` Linus Torvalds
2013-09-05 19:56 ` Luck, Tony
2013-09-06 18:36 ` Tony Luck
2013-09-05 17:54 ` Waiman Long
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=20130905131814.GA24274@osiris \
--to=heiko.carstens@de.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=tony.luck@intel.com \
--cc=torvalds@linux-foundation.org \
/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