[Bug nptl/28164] New: nptl: use relaxed MO read to avoid expensive memory for adaptive mutex lock

[Bug nptl/28164] New: nptl: use relaxed MO read to avoid expensive memory for adaptive mutex lock

[wangxuszcn at foxmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28164

            Bug ID: 28164
           Summary: nptl: use relaxed MO read to avoid expensive memory
                    for adaptive mutex lock
           Product: glibc
           Version: unspecified
            Status: UNCONFIRMED
          Severity: enhancement
          Priority: P2
         Component: nptl
          Assignee: unassigned at sourceware dot org
          Reporter: wangxuszcn at foxmail dot com
                CC: drepper.fsp at gmail dot com
  Target Milestone: ---

Created attachment 13575
  --> https://sourceware.org/bugzilla/attachment.cgi?id=13575&action=edit
nptl-use-relaxed-MO-read-to-avoid-expensive-memory-s

>From ff3f9aa65f9b4e62b641166bb205428b8f2ec3af Mon Sep 17 00:00:00 2001
From: Wang Xu <wangxuszcn@foxmail.com>
Date: Mon, 2 Aug 2021 18:20:52 +0800
Subject: [PATCH]nptl: use relaxed MO read to avoid expensive memory for
adaptive mutex lock

 synchronizations Going straight back to cmpxchg is not a good idea on many
 targets as that will force expensive memory synchronizations among processors
 and penalize other running threads. There is no technical reason for throwing
 in a CAS every now and then, and so far we have no evidence that it can
 improve performance. If that would be the case, we have to adjust other
 spin-waiting loops elsewhere, too! Thus we use relaxed MO reads until we
 observe the lock to not be acquired anymore.

Signed-off-by: Wang Xu <wangxuszcn@foxmail.com>
---
 nptl/pthread_mutex_lock.c | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/nptl/pthread_mutex_lock.c b/nptl/pthread_mutex_lock.c
index c9e438e..0a0c774 100644
--- a/nptl/pthread_mutex_lock.c
+++ b/nptl/pthread_mutex_lock.c
@@ -136,15 +136,30 @@ PTHREAD_MUTEX_LOCK (pthread_mutex_t *mutex)
                             mutex->__data.__spins * 2 + 10);
          do
            {
-             if (cnt++ >= max_cnt)
-               {
-                 LLL_MUTEX_LOCK (mutex);
-                 break;
-               }
-             atomic_spin_nop ();
+            /* The lock is contended and we need to wait.  Going straight back
+              to cmpxchg is not a good idea on many targets as that will force
+              expensive memory synchronizations among processors and penalize
+              other running threads.
+              There is no technical reason for throwing in a CAS every now
+              and then, and so far we have no evidence that it can improve
+              performance.
+              If that would be the case, we have to adjust other spin-waiting
+              loops elsewhere, too!
+              Thus we use relaxed MO reads until we observe the lock to not be
+              acquired anymore.  */
+              do
+               {
+                 if (cnt++ >= max_cnt)
+                 {
+                   LLL_MUTEX_LOCK (mutex);
+                   goto unlock;
+                 }
+                 atomic_spin_nop ();
+               }
+             while (atomic_load_relaxed (&mutex->__data.__lock) != 0);
            }
          while (LLL_MUTEX_TRYLOCK (mutex) != 0);
-
+unlock:
          mutex->__data.__spins += (cnt - mutex->__data.__spins) / 8;
        }
       assert (mutex->__data.__owner == 0);
-- 
1.8.5.6

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug nptl/28164] nptl: use relaxed MO read to avoid expensive memory for adaptive mutex lock

[wangxuszcn at foxmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28164

--- Comment #1 from wangxu <wangxuszcn at foxmail dot com> ---
commit d672a98a1af106bd68deb15576710cd61363f7a6 realize this, but have a bug,

"https://sourceware.org/bugzilla/show_bug.cgi?id=28670" will fix this:

In commit d672a98a1af106bd68deb15576710cd61363f7a6, we want to do an atomic
load  
instead of CAS. As a matter of fact, d672a98a1af106bd68deb15576710cd61363f7a6
will execute both LLL_MUTEX_READ_LOCK() and LLL_MUTEX_TRYLOCK().
So the following patch try to sovle it.
diff --git a/nptl/pthread_mutex_lock.c b/nptl/pthread_mutex_lock.c
index 5a49763..1d7a80b 100644
--- a/nptl/pthread_mutex_lock.c
+++ b/nptl/pthread_mutex_lock.c
@@ -140,6 +140,7 @@ PTHREAD_MUTEX_LOCK (pthread_mutex_t *mutex)
                             mutex->__data.__spins * 2 + 10);
          do
            {
+again:
              if (cnt++ >= max_cnt)
                {
                  LLL_MUTEX_LOCK (mutex);
@@ -147,7 +148,7 @@ PTHREAD_MUTEX_LOCK (pthread_mutex_t *mutex)
                }
              atomic_spin_nop ();
              if (LLL_MUTEX_READ_LOCK (mutex) != 0)
-           continue;
+         goto;
            }
          while (LLL_MUTEX_TRYLOCK (mutex) != 0);
 unlock:

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug nptl/28164] nptl: use relaxed MO read to avoid expensive memory for adaptive mutex lock

[wangxuszcn at foxmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28164

--- Comment #2 from wangxu <wangxuszcn at foxmail dot com> ---
*** Bug 28670 has been marked as a duplicate of this bug. ***

-- 
You are receiving this mail because:
You are on the CC list for the bug.