All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michal Hocko <mhocko@kernel.org>
To: LKML <linux-kernel@vger.kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	"H. Peter Anvin" <hpa@zytor.com>,
	"David S. Miller" <davem@davemloft.net>,
	Tony Luck <tony.luck@intel.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Chris Zankel <chris@zankel.net>,
	Max Filippov <jcmvbkbc@gmail.com>,
	x86@kernel.org, linux-alpha@vger.kernel.org,
	linux-ia64@vger.kernel.org, linux-s390@vger.kernel.org,
	linux-sh@vger.kernel.org, sparclinux@vger.kernel.org,
	linux-xtensa@linux-xtensa.org, linux-arch@vger.kernel.org,
	Michal Hocko <mhocko@suse.com>
Subject: [RFC 10/12] x86, rwsem: simplify __down_write
Date: Tue, 02 Feb 2016 20:19:27 +0000	[thread overview]
Message-ID: <1454444369-2146-11-git-send-email-mhocko@kernel.org> (raw)
In-Reply-To: <1454444369-2146-1-git-send-email-mhocko@kernel.org>

From: Michal Hocko <mhocko@suse.com>

x86 implementation of __down_write is using inline asm to optimize the
code flow. This however requires that it has go over an additional hop
for the slow path call_rwsem_down_write_failed which has to
save_common_regs/restore_common_regs to preserve the calling convention.
This, however doesn't add much because the fast path only saves one
register push/pop (rdx) when compared to the generic implementation:

Before:
0000000000000019 <down_write>:
  19:   e8 00 00 00 00          callq  1e <down_write+0x5>
  1e:   55                      push   %rbp
  1f:   48 ba 01 00 00 00 ff    movabs $0xffffffff00000001,%rdx
  26:   ff ff ff
  29:   48 89 f8                mov    %rdi,%rax
  2c:   48 89 e5                mov    %rsp,%rbp
  2f:   f0 48 0f c1 10          lock xadd %rdx,(%rax)
  34:   85 d2                   test   %edx,%edx
  36:   74 05                   je     3d <down_write+0x24>
  38:   e8 00 00 00 00          callq  3d <down_write+0x24>
  3d:   65 48 8b 04 25 00 00    mov    %gs:0x0,%rax
  44:   00 00
  46:   5d                      pop    %rbp
  47:   48 89 47 38             mov    %rax,0x38(%rdi)
  4b:   c3                      retq

After:
0000000000000019 <down_write>:
  19:   e8 00 00 00 00          callq  1e <down_write+0x5>
  1e:   55                      push   %rbp
  1f:   48 b8 01 00 00 00 ff    movabs $0xffffffff00000001,%rax
  26:   ff ff ff
  29:   48 89 e5                mov    %rsp,%rbp
  2c:   53                      push   %rbx
  2d:   48 89 fb                mov    %rdi,%rbx
  30:   f0 48 0f c1 07          lock xadd %rax,(%rdi)
  35:   48 85 c0                test   %rax,%rax
  38:   74 05                   je     3f <down_write+0x26>
  3a:   e8 00 00 00 00          callq  3f <down_write+0x26>
  3f:   65 48 8b 04 25 00 00    mov    %gs:0x0,%rax
  46:   00 00
  48:   48 89 43 38             mov    %rax,0x38(%rbx)
  4c:   5b                      pop    %rbx
  4d:   5d                      pop    %rbp
  4e:   c3                      retq

This doesn't seem to justify the code obfuscation and complexity. Use
the generic implementation instead.

Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 arch/x86/include/asm/rwsem.h | 17 +++++------------
 arch/x86/lib/rwsem.S         |  9 ---------
 2 files changed, 5 insertions(+), 21 deletions(-)

diff --git a/arch/x86/include/asm/rwsem.h b/arch/x86/include/asm/rwsem.h
index d79a218675bc..1b5e89b3643d 100644
--- a/arch/x86/include/asm/rwsem.h
+++ b/arch/x86/include/asm/rwsem.h
@@ -102,18 +102,11 @@ static inline int __down_read_trylock(struct rw_semaphore *sem)
 static inline void __down_write(struct rw_semaphore *sem)
 {
 	long tmp;
-	asm volatile("# beginning down_write\n\t"
-		     LOCK_PREFIX "  xadd      %1,(%2)\n\t"
-		     /* adds 0xffff0001, returns the old value */
-		     "  test " __ASM_SEL(%w1,%k1) "," __ASM_SEL(%w1,%k1) "\n\t"
-		     /* was the active mask 0 before? */
-		     "  jz        1f\n"
-		     "  call call_rwsem_down_write_failed\n"
-		     "1:\n"
-		     "# ending down_write"
-		     : "+m" (sem->count), "=d" (tmp)
-		     : "a" (sem), "1" (RWSEM_ACTIVE_WRITE_BIAS)
-		     : "memory", "cc");
+
+	tmp = atomic_long_add_return(RWSEM_ACTIVE_WRITE_BIAS,
+				     (atomic_long_t *)&sem->count);
+	if (unlikely(tmp != RWSEM_ACTIVE_WRITE_BIAS))
+		rwsem_down_write_failed(sem);
 }
 
 /*
diff --git a/arch/x86/lib/rwsem.S b/arch/x86/lib/rwsem.S
index 40027db99140..ea5c7c177483 100644
--- a/arch/x86/lib/rwsem.S
+++ b/arch/x86/lib/rwsem.S
@@ -57,7 +57,6 @@
  * is also the input argument to these helpers)
  *
  * The following can clobber %rdx because the asm clobbers it:
- *   call_rwsem_down_write_failed
  *   call_rwsem_wake
  * but %rdi, %rsi, %rcx, %r8-r11 always need saving.
  */
@@ -93,14 +92,6 @@ ENTRY(call_rwsem_down_read_failed)
 	ret
 ENDPROC(call_rwsem_down_read_failed)
 
-ENTRY(call_rwsem_down_write_failed)
-	save_common_regs
-	movq %rax,%rdi
-	call rwsem_down_write_failed
-	restore_common_regs
-	ret
-ENDPROC(call_rwsem_down_write_failed)
-
 ENTRY(call_rwsem_wake)
 	/* do nothing if still outstanding active readers */
 	__ASM_HALF_SIZE(dec) %__ASM_HALF_REG(dx)
-- 
2.7.0


WARNING: multiple messages have this Message-ID (diff)
From: Michal Hocko <mhocko@kernel.org>
To: LKML <linux-kernel@vger.kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	"H. Peter Anvin" <hpa@zytor.com>,
	"David S. Miller" <davem@davemloft.net>,
	Tony Luck <tony.luck@intel.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Chris Zankel <chris@zankel.net>,
	Max Filippov <jcmvbkbc@gmail.com>,
	x86@kernel.org, linux-alpha@vger.kernel.org,
	linux-ia64@vger.kernel.org, linux-s390@vger.kernel.org,
	linux-sh@vger.kernel.org, sparclinux@vger.kernel.org,
	linux-xtensa@linux-xtensa.org, linux-arch@vger.kernel.org,
	Michal Hocko <mhocko@suse.com>
Subject: [RFC 10/12] x86, rwsem: simplify __down_write
Date: Tue,  2 Feb 2016 21:19:27 +0100	[thread overview]
Message-ID: <1454444369-2146-11-git-send-email-mhocko@kernel.org> (raw)
In-Reply-To: <1454444369-2146-1-git-send-email-mhocko@kernel.org>

From: Michal Hocko <mhocko@suse.com>

x86 implementation of __down_write is using inline asm to optimize the
code flow. This however requires that it has go over an additional hop
for the slow path call_rwsem_down_write_failed which has to
save_common_regs/restore_common_regs to preserve the calling convention.
This, however doesn't add much because the fast path only saves one
register push/pop (rdx) when compared to the generic implementation:

Before:
0000000000000019 <down_write>:
  19:   e8 00 00 00 00          callq  1e <down_write+0x5>
  1e:   55                      push   %rbp
  1f:   48 ba 01 00 00 00 ff    movabs $0xffffffff00000001,%rdx
  26:   ff ff ff
  29:   48 89 f8                mov    %rdi,%rax
  2c:   48 89 e5                mov    %rsp,%rbp
  2f:   f0 48 0f c1 10          lock xadd %rdx,(%rax)
  34:   85 d2                   test   %edx,%edx
  36:   74 05                   je     3d <down_write+0x24>
  38:   e8 00 00 00 00          callq  3d <down_write+0x24>
  3d:   65 48 8b 04 25 00 00    mov    %gs:0x0,%rax
  44:   00 00
  46:   5d                      pop    %rbp
  47:   48 89 47 38             mov    %rax,0x38(%rdi)
  4b:   c3                      retq

After:
0000000000000019 <down_write>:
  19:   e8 00 00 00 00          callq  1e <down_write+0x5>
  1e:   55                      push   %rbp
  1f:   48 b8 01 00 00 00 ff    movabs $0xffffffff00000001,%rax
  26:   ff ff ff
  29:   48 89 e5                mov    %rsp,%rbp
  2c:   53                      push   %rbx
  2d:   48 89 fb                mov    %rdi,%rbx
  30:   f0 48 0f c1 07          lock xadd %rax,(%rdi)
  35:   48 85 c0                test   %rax,%rax
  38:   74 05                   je     3f <down_write+0x26>
  3a:   e8 00 00 00 00          callq  3f <down_write+0x26>
  3f:   65 48 8b 04 25 00 00    mov    %gs:0x0,%rax
  46:   00 00
  48:   48 89 43 38             mov    %rax,0x38(%rbx)
  4c:   5b                      pop    %rbx
  4d:   5d                      pop    %rbp
  4e:   c3                      retq

This doesn't seem to justify the code obfuscation and complexity. Use
the generic implementation instead.

Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 arch/x86/include/asm/rwsem.h | 17 +++++------------
 arch/x86/lib/rwsem.S         |  9 ---------
 2 files changed, 5 insertions(+), 21 deletions(-)

diff --git a/arch/x86/include/asm/rwsem.h b/arch/x86/include/asm/rwsem.h
index d79a218675bc..1b5e89b3643d 100644
--- a/arch/x86/include/asm/rwsem.h
+++ b/arch/x86/include/asm/rwsem.h
@@ -102,18 +102,11 @@ static inline int __down_read_trylock(struct rw_semaphore *sem)
 static inline void __down_write(struct rw_semaphore *sem)
 {
 	long tmp;
-	asm volatile("# beginning down_write\n\t"
-		     LOCK_PREFIX "  xadd      %1,(%2)\n\t"
-		     /* adds 0xffff0001, returns the old value */
-		     "  test " __ASM_SEL(%w1,%k1) "," __ASM_SEL(%w1,%k1) "\n\t"
-		     /* was the active mask 0 before? */
-		     "  jz        1f\n"
-		     "  call call_rwsem_down_write_failed\n"
-		     "1:\n"
-		     "# ending down_write"
-		     : "+m" (sem->count), "=d" (tmp)
-		     : "a" (sem), "1" (RWSEM_ACTIVE_WRITE_BIAS)
-		     : "memory", "cc");
+
+	tmp = atomic_long_add_return(RWSEM_ACTIVE_WRITE_BIAS,
+				     (atomic_long_t *)&sem->count);
+	if (unlikely(tmp != RWSEM_ACTIVE_WRITE_BIAS))
+		rwsem_down_write_failed(sem);
 }
 
 /*
diff --git a/arch/x86/lib/rwsem.S b/arch/x86/lib/rwsem.S
index 40027db99140..ea5c7c177483 100644
--- a/arch/x86/lib/rwsem.S
+++ b/arch/x86/lib/rwsem.S
@@ -57,7 +57,6 @@
  * is also the input argument to these helpers)
  *
  * The following can clobber %rdx because the asm clobbers it:
- *   call_rwsem_down_write_failed
  *   call_rwsem_wake
  * but %rdi, %rsi, %rcx, %r8-r11 always need saving.
  */
@@ -93,14 +92,6 @@ ENTRY(call_rwsem_down_read_failed)
 	ret
 ENDPROC(call_rwsem_down_read_failed)
 
-ENTRY(call_rwsem_down_write_failed)
-	save_common_regs
-	movq %rax,%rdi
-	call rwsem_down_write_failed
-	restore_common_regs
-	ret
-ENDPROC(call_rwsem_down_write_failed)
-
 ENTRY(call_rwsem_wake)
 	/* do nothing if still outstanding active readers */
 	__ASM_HALF_SIZE(dec) %__ASM_HALF_REG(dx)
-- 
2.7.0

  parent reply	other threads:[~2016-02-02 20:19 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-02 20:19 [RFC 0/12] introduce down_write_killable for rw_semaphore Michal Hocko
2016-02-02 20:19 ` Michal Hocko
2016-02-02 20:19 ` [RFC 01/12] locking, rwsem: get rid of __down_write_nested Michal Hocko
2016-02-02 20:19   ` Michal Hocko
2016-02-02 20:19 ` [RFC 02/12] locking, rwsem: drop explicit memory barriers Michal Hocko
2016-02-02 20:19   ` Michal Hocko
2016-02-02 20:19 ` [RFC 03/12] locking, rwsem: introduce basis for down_write_killable Michal Hocko
2016-02-02 20:19   ` Michal Hocko
2016-02-02 20:19 ` [RFC 04/12] alpha, rwsem: provide __down_write_killable Michal Hocko
2016-02-02 20:19   ` Michal Hocko
2016-02-02 20:19 ` [RFC 05/12] ia64, " Michal Hocko
2016-02-02 20:19   ` Michal Hocko
2016-02-02 20:19 ` [RFC 06/12] s390, " Michal Hocko
2016-02-02 20:19   ` Michal Hocko
2016-02-02 20:19 ` [RFC 07/12] sh, " Michal Hocko
2016-02-02 20:19   ` Michal Hocko
2016-02-03 11:19   ` Sergei Shtylyov
2016-02-03 11:19     ` Sergei Shtylyov
2016-02-03 12:11     ` Michal Hocko
2016-02-03 12:11       ` Michal Hocko
2016-02-02 20:19 ` [RFC 08/12] sparc, " Michal Hocko
2016-02-02 20:19   ` Michal Hocko
2016-02-02 20:19 ` [RFC 09/12] xtensa, " Michal Hocko
2016-02-02 20:19   ` Michal Hocko
2016-02-02 20:19 ` Michal Hocko [this message]
2016-02-02 20:19   ` [RFC 10/12] x86, rwsem: simplify __down_write Michal Hocko
2016-02-03  8:10   ` Ingo Molnar
2016-02-03  8:10     ` Ingo Molnar
2016-02-03 12:10     ` Michal Hocko
2016-02-03 12:10       ` Michal Hocko
2016-06-03 16:13     ` Peter Zijlstra
2016-06-03 16:13       ` Peter Zijlstra
2016-06-03 22:34       ` Peter Zijlstra
2016-06-03 22:34         ` Peter Zijlstra
2016-06-09 14:40       ` David Howells
2016-06-09 14:40         ` David Howells
2016-06-09 17:36         ` Peter Zijlstra
2016-06-09 17:36           ` Peter Zijlstra
2016-06-10 16:39           ` Paul E. McKenney
2016-06-10 16:39             ` Paul E. McKenney
2016-02-02 20:19 ` [RFC 11/12] x86, rwsem: provide __down_write_killable Michal Hocko
2016-02-02 20:19   ` Michal Hocko
2016-02-17 16:41   ` [RFC 11/12 v1] " Michal Hocko
2016-02-17 16:41     ` Michal Hocko
2016-02-17 16:41     ` Michal Hocko
2016-02-17 16:41     ` Michal Hocko
2016-02-02 20:19 ` [RFC 12/12] locking, rwsem: provide down_write_killable Michal Hocko
2016-02-02 20:19   ` Michal Hocko
2016-02-19 12:15 ` [RFC 0/12] introduce down_write_killable for rw_semaphore Michal Hocko
2016-02-19 12:15   ` Michal Hocko
2016-03-09 12:18 ` Ingo Molnar
2016-03-09 12:18   ` Ingo Molnar
2016-03-09 12:56   ` Michal Hocko
2016-03-09 12:56     ` Michal Hocko
2016-03-09 13:17     ` Ingo Molnar
2016-03-09 13:17       ` Ingo Molnar
2016-03-09 13:28       ` Michal Hocko
2016-03-09 13:28         ` Michal Hocko
2016-03-09 13:43         ` Ingo Molnar
2016-03-09 13:43           ` Ingo Molnar
2016-03-09 14:41           ` Michal Hocko
2016-03-09 14:41             ` Michal Hocko
2016-03-10 10:24             ` Ingo Molnar
2016-03-10 10:24               ` Ingo Molnar

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=1454444369-2146-11-git-send-email-mhocko@kernel.org \
    --to=mhocko@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=chris@zankel.net \
    --cc=davem@davemloft.net \
    --cc=hpa@zytor.com \
    --cc=jcmvbkbc@gmail.com \
    --cc=linux-alpha@vger.kernel.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-ia64@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=linux-sh@vger.kernel.org \
    --cc=linux-xtensa@linux-xtensa.org \
    --cc=mhocko@suse.com \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=sparclinux@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --cc=tony.luck@intel.com \
    --cc=x86@kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.