diff mbox

[3/4] S390: Use new __libc_tbegin_retry macro in elision-lock.c.

Message ID 1481032315-12420-3-git-send-email-stli@linux.vnet.ibm.com
State New
Headers show

Commit Message

Stefan Liebler Dec. 6, 2016, 1:51 p.m. UTC 
This patch implements __libc_tbegin_retry macro which is equivalent to
gcc builtin __builtin_tbegin_retry, except the changes which were applied
to __libc_tbegin in the previous patch.

If tbegin aborts with _HTM_TBEGIN_TRANSIENT.  Then this macros restores
the fpc, fprs and automatically retries up to retry_cnt tbegins.
Further saving of the state is omitted as it is already saved in the
first round.  Before retrying a further transaction, the
transaction-abort-assist instruction is used to support the cpu.

This macro is now used in function __lll_lock_elision.

ChangeLog:

	* sysdeps/unix/sysv/linux/s390/htm.h(__libc_tbegin_retry): New macro.
	* sysdeps/unix/sysv/linux/s390/elision-lock.c (__lll_lock_elision):
	Use __libc_tbegin_retry macro.
---
 sysdeps/unix/sysv/linux/s390/elision-lock.c | 50 ++++++++++++++---------------
 sysdeps/unix/sysv/linux/s390/htm.h          | 36 +++++++++++++++++++--
 2 files changed, 58 insertions(+), 28 deletions(-)

Comments

Torvald Riegel Jan. 10, 2017, 4:53 p.m. UTC | #1 
On Tue, 2016-12-06 at 14:51 +0100, Stefan Liebler wrote:
> This patch implements __libc_tbegin_retry macro which is equivalent to
> gcc builtin __builtin_tbegin_retry, except the changes which were applied
> to __libc_tbegin in the previous patch.
> 
> If tbegin aborts with _HTM_TBEGIN_TRANSIENT.  Then this macros restores
> the fpc, fprs and automatically retries up to retry_cnt tbegins.
> Further saving of the state is omitted as it is already saved in the
> first round.  Before retrying a further transaction, the
> transaction-abort-assist instruction is used to support the cpu.

This looks okay to me on the surface of it, but I don't know anything
about s390 asm.

> Use __libc_tbegin_retry macro.
> ---
>  sysdeps/unix/sysv/linux/s390/elision-lock.c | 50 ++++++++++++++---------------
>  sysdeps/unix/sysv/linux/s390/htm.h          | 36 +++++++++++++++++++--
>  2 files changed, 58 insertions(+), 28 deletions(-)
> 
> diff --git a/sysdeps/unix/sysv/linux/s390/elision-lock.c b/sysdeps/unix/sysv/linux/s390/elision-lock.c
> index 48cc3db..3dd7fbc 100644
> --- a/sysdeps/unix/sysv/linux/s390/elision-lock.c
> +++ b/sysdeps/unix/sysv/linux/s390/elision-lock.c
> @@ -60,17 +60,16 @@ __lll_lock_elision (int *futex, short *adapt_count, EXTRAARG int private)
>        goto use_lock;
>      }
>  
> -  int try_tbegin;
> -  for (try_tbegin = aconf.try_tbegin;
> -       try_tbegin > 0;
> -       try_tbegin--)
> +  if (aconf.try_tbegin > 0)
>      {
> -      int status;
> -      if (__builtin_expect
> -	  ((status = __libc_tbegin ((void *) 0)) == _HTM_TBEGIN_STARTED, 1))
> +      int status = __libc_tbegin_retry ((void *) 0, aconf.try_tbegin - 1);

Maybe add a comment that the macro that reminds the reader that the
macro expects a retry count, not how often a transaction should be
tried.

> +      if (__builtin_expect (status == _HTM_TBEGIN_STARTED,
> +			    _HTM_TBEGIN_STARTED))
>  	{
> -	  if (*futex == 0)
> +	  if (__builtin_expect (*futex == 0, 1))
> +	    /* Lock was free.  Return to user code in a transaction.  */
>  	    return 0;
> +
>  	  /* Lock was busy.  Fall back to normal locking.  */
>  	  if (__builtin_expect (__libc_tx_nesting_depth (), 1))
>  	    {
> @@ -81,7 +80,6 @@ __lll_lock_elision (int *futex, short *adapt_count, EXTRAARG int private)
>  		 See above for why relaxed MO is sufficient.  */
>  	      if (aconf.skip_lock_busy > 0)
>  		atomic_store_relaxed (adapt_count, aconf.skip_lock_busy);
> -	      goto use_lock;
>  	    }
>  	  else /* nesting depth is > 1 */
>  	    {
> @@ -99,28 +97,28 @@ __lll_lock_elision (int *futex, short *adapt_count, EXTRAARG int private)
>  	      __libc_tabort (_HTM_FIRST_USER_ABORT_CODE | 1);
>  	    }
>  	}
> +      else if (status != _HTM_TBEGIN_TRANSIENT)
> +	{
> +	  /* A persistent abort (cc 1 or 3) indicates that a retry is
> +	     probably futile.  Use the normal locking now and for the
> +	     next couple of calls.
> +	     Be careful to avoid writing to the lock.  See above for why
> +	     relaxed MO is sufficient.  */
> +	  if (aconf.skip_lock_internal_abort > 0)
> +	    atomic_store_relaxed (adapt_count,
> +				  aconf.skip_lock_internal_abort);
> +	}
>        else
>  	{
> -	  if (status != _HTM_TBEGIN_TRANSIENT)
> -	    {
> -	      /* A persistent abort (cc 1 or 3) indicates that a retry is
> -		 probably futile.  Use the normal locking now and for the
> -		 next couple of calls.
> -		 Be careful to avoid writing to the lock.  See above for why
> -		 relaxed MO is sufficient.  */
> -	      if (aconf.skip_lock_internal_abort > 0)
> -		atomic_store_relaxed (adapt_count,
> -				      aconf.skip_lock_internal_abort);
> -	      goto use_lock;
> -	    }
> +	  /* Same logic as above, but for for a number of temporary failures in
> +	     a row.  */

for for

> +	  if (aconf.skip_lock_out_of_tbegin_retries > 0)
> +	    atomic_store_relaxed (adapt_count,
> +				  aconf.skip_lock_out_of_tbegin_retries);
>  	}
>      }
>  
> -  /* Same logic as above, but for for a number of temporary failures in a
> -     row.  See above for why relaxed MO is sufficient.  */
> -  if (aconf.skip_lock_out_of_tbegin_retries > 0 && aconf.try_tbegin > 0)
> -    atomic_store_relaxed (adapt_count, aconf.skip_lock_out_of_tbegin_retries);
> -
>    use_lock:
> +  /* Use normal locking as fallback path if transaction does not succeed.  */

the transaction

>    return LLL_LOCK ((*futex), private);
>  }
diff mbox

Patch

diff --git a/sysdeps/unix/sysv/linux/s390/elision-lock.c b/sysdeps/unix/sysv/linux/s390/elision-lock.c
index 48cc3db..3dd7fbc 100644
--- a/sysdeps/unix/sysv/linux/s390/elision-lock.c
+++ b/sysdeps/unix/sysv/linux/s390/elision-lock.c
@@ -60,17 +60,16 @@  __lll_lock_elision (int *futex, short *adapt_count, EXTRAARG int private)
       goto use_lock;
     }
 
-  int try_tbegin;
-  for (try_tbegin = aconf.try_tbegin;
-       try_tbegin > 0;
-       try_tbegin--)
+  if (aconf.try_tbegin > 0)
     {
-      int status;
-      if (__builtin_expect
-	  ((status = __libc_tbegin ((void *) 0)) == _HTM_TBEGIN_STARTED, 1))
+      int status = __libc_tbegin_retry ((void *) 0, aconf.try_tbegin - 1);
+      if (__builtin_expect (status == _HTM_TBEGIN_STARTED,
+			    _HTM_TBEGIN_STARTED))
 	{
-	  if (*futex == 0)
+	  if (__builtin_expect (*futex == 0, 1))
+	    /* Lock was free.  Return to user code in a transaction.  */
 	    return 0;
+
 	  /* Lock was busy.  Fall back to normal locking.  */
 	  if (__builtin_expect (__libc_tx_nesting_depth (), 1))
 	    {
@@ -81,7 +80,6 @@  __lll_lock_elision (int *futex, short *adapt_count, EXTRAARG int private)
 		 See above for why relaxed MO is sufficient.  */
 	      if (aconf.skip_lock_busy > 0)
 		atomic_store_relaxed (adapt_count, aconf.skip_lock_busy);
-	      goto use_lock;
 	    }
 	  else /* nesting depth is > 1 */
 	    {
@@ -99,28 +97,28 @@  __lll_lock_elision (int *futex, short *adapt_count, EXTRAARG int private)
 	      __libc_tabort (_HTM_FIRST_USER_ABORT_CODE | 1);
 	    }
 	}
+      else if (status != _HTM_TBEGIN_TRANSIENT)
+	{
+	  /* A persistent abort (cc 1 or 3) indicates that a retry is
+	     probably futile.  Use the normal locking now and for the
+	     next couple of calls.
+	     Be careful to avoid writing to the lock.  See above for why
+	     relaxed MO is sufficient.  */
+	  if (aconf.skip_lock_internal_abort > 0)
+	    atomic_store_relaxed (adapt_count,
+				  aconf.skip_lock_internal_abort);
+	}
       else
 	{
-	  if (status != _HTM_TBEGIN_TRANSIENT)
-	    {
-	      /* A persistent abort (cc 1 or 3) indicates that a retry is
-		 probably futile.  Use the normal locking now and for the
-		 next couple of calls.
-		 Be careful to avoid writing to the lock.  See above for why
-		 relaxed MO is sufficient.  */
-	      if (aconf.skip_lock_internal_abort > 0)
-		atomic_store_relaxed (adapt_count,
-				      aconf.skip_lock_internal_abort);
-	      goto use_lock;
-	    }
+	  /* Same logic as above, but for for a number of temporary failures in
+	     a row.  */
+	  if (aconf.skip_lock_out_of_tbegin_retries > 0)
+	    atomic_store_relaxed (adapt_count,
+				  aconf.skip_lock_out_of_tbegin_retries);
 	}
     }
 
-  /* Same logic as above, but for for a number of temporary failures in a
-     row.  See above for why relaxed MO is sufficient.  */
-  if (aconf.skip_lock_out_of_tbegin_retries > 0 && aconf.try_tbegin > 0)
-    atomic_store_relaxed (adapt_count, aconf.skip_lock_out_of_tbegin_retries);
-
   use_lock:
+  /* Use normal locking as fallback path if transaction does not succeed.  */
   return LLL_LOCK ((*futex), private);
 }
diff --git a/sysdeps/unix/sysv/linux/s390/htm.h b/sysdeps/unix/sysv/linux/s390/htm.h
index 6b4e8f4..aa9d01a 100644
--- a/sysdeps/unix/sysv/linux/s390/htm.h
+++ b/sysdeps/unix/sysv/linux/s390/htm.h
@@ -69,7 +69,36 @@ 
    started.  Thus the user of the tbegin macros in this header file has to
    compile the file / function with -msoft-float.  It prevents gcc from using
    fprs / vrs.  */
-#define __libc_tbegin(tdb)						\
+#define __libc_tbegin(tdb) __libc_tbegin_base(tdb,,,)
+
+#define __libc_tbegin_retry_output_regs , [R_TX_CNT] "+&d" (__tx_cnt)
+#define __libc_tbegin_retry_input_regs(retry_cnt) , [R_RETRY] "d" (retry_cnt)
+#define __libc_tbegin_retry_abort_path_insn				\
+  /* If tbegin returned _HTM_TBEGIN_TRANSIENT, retry immediately so	\
+     that max tbegin_cnt transactions are tried.  Otherwise return and	\
+     let the caller of this macro do the fallback path.  */		\
+  "   jnh 1f\n\t" /* cc 1/3: jump to fallback path.  */			\
+  /* tbegin returned _HTM_TBEGIN_TRANSIENT: retry with transaction.  */ \
+  "   crje %[R_TX_CNT], %[R_RETRY], 1f\n\t" /* Reached max retries?  */	\
+  "   ahi %[R_TX_CNT], 1\n\t"						\
+  "   ppa %[R_TX_CNT], 0, 1\n\t" /* Transaction-Abort Assist.  */	\
+  "   j 2b\n\t" /* Loop to tbegin.  */
+
+/* Same as __libc_tbegin except if tbegin aborts with _HTM_TBEGIN_TRANSIENT.
+   Then this macros restores the fpc, fprs and automatically retries up to
+   retry_cnt tbegins.  Further saving of the state is omitted as it is already
+   saved.  This macro calls tbegin at most as retry_cnt + 1 times.  */
+#define __libc_tbegin_retry(tdb, retry_cnt)				\
+  ({ int __ret;								\
+    int __tx_cnt = 0;							\
+    __ret = __libc_tbegin_base(tdb,					\
+			       __libc_tbegin_retry_abort_path_insn,	\
+			       __libc_tbegin_retry_output_regs,		\
+			       __libc_tbegin_retry_input_regs(retry_cnt)); \
+    __ret;								\
+  })
+
+#define __libc_tbegin_base(tdb, abort_path_insn, output_regs, input_regs) \
   ({ int __ret;								\
      int __fpc;								\
      char __fprs[TX_FPRS_BYTES];					\
@@ -95,7 +124,7 @@ 
 			      again and result in a core dump wich does	\
 			      now show at tbegin but the real executed	\
 			      instruction.  */				\
-			   "   tbegin 0, 0xFF0E\n\t"			\
+			   "2: tbegin 0, 0xFF0E\n\t"			\
 			   /* Branch away in abort case (this is the	\
 			      prefered sequence.  See PoP in chapter 5	\
 			      Transactional-Execution Facility		\
@@ -111,11 +140,14 @@ 
 			   "   srl %[R_RET], 28\n\t"			\
 			   "   sfpc %[R_FPC]\n\t"			\
 			   TX_RESTORE_FPRS				\
+			   abort_path_insn				\
 			   "1:\n\t"					\
 			   ".machine pop\n"				\
 			   : [R_RET] "=&d" (__ret),			\
 			     [R_FPC] "=&d" (__fpc)			\
+			     output_regs				\
 			   : [R_FPRS] "a" (__fprs)			\
+			     input_regs					\
 			   : "cc", "memory");				\
      __ret;								\
      })