Re: [PATCH 2/2] New pthread rwlock that is more scalable.

[Torvald Riegel <triegel at redhat dot com>]

On Wed, 2016-07-27 at 23:44 +0200, Torvald Riegel wrote:
> This replaces the pthread rwlock with a new implementation that uses a
> more scalable algorithm (primarily through not using a critical section
> anymore to make state changes).  The fast path for rdlock acquisition
> and release is now basically a single atomic read-modify write or CAS
> and a few branches.  See nptl/pthread_rwlock_common.c for details.

Attached is a follow-up patch that adapts the pretty printers added in
the meantime to work with the new rwlock.  For ease-of-review, this is a
separate patch.  I'll commit it as one patch, of course.

commit 8839c1956594113d1f19bfe3dd13b1f4c01273ea
Author: Torvald Riegel <triegel@redhat.com>
Date:   Fri Dec 23 21:30:59 2016 +0100

    rwlock: Add pretty printers support.

diff --git a/nptl/nptl-printers.py b/nptl/nptl-printers.py
index e402f23..dddbb06 100644
--- a/nptl/nptl-printers.py
+++ b/nptl/nptl-printers.py
@@ -474,12 +474,10 @@ class RWLockPrinter(object):
         """
 
         data = rwlock['__data']
-        self.readers = data['__nr_readers']
-        self.queued_readers = data['__nr_readers_queued']
-        self.queued_writers = data['__nr_writers_queued']
-        self.writer_id = data['__writer']
+        self.readers = data['__readers']
+        self.cur_writer = data['__cur_writer']
         self.shared = data['__shared']
-        self.prefers_writers = data['__flags']
+        self.flags = data['__flags']
         self.values = []
         self.read_values()
 
@@ -512,20 +510,19 @@ class RWLockPrinter(object):
     def read_status(self):
         """Read the status of the rwlock."""
 
-        # Right now pthread_rwlock_destroy doesn't do anything, so there's no
-        # way to check if an rwlock is destroyed.
-
-        if self.writer_id:
-            self.values.append(('Status', 'Locked (Write)'))
-            self.values.append(('Writer ID', self.writer_id))
-        elif self.readers:
-            self.values.append(('Status', 'Locked (Read)'))
-            self.values.append(('Readers', self.readers))
+        if self.readers & PTHREAD_RWLOCK_WRPHASE:
+            if self.readers & PTHREAD_RWLOCK_WRLOCKED:
+                self.values.append(('Status', 'Acquired (Write)'))
+                self.values.append(('Writer ID', self.cur_writer))
+            else:
+                self.values.append(('Status', 'Not acquired'))
         else:
-            self.values.append(('Status', 'Unlocked'))
-
-        self.values.append(('Queued readers', self.queued_readers))
-        self.values.append(('Queued writers', self.queued_writers))
+            r = self.readers >> PTHREAD_RWLOCK_READER_SHIFT
+            if r > 0:
+                self.values.append(('Status', 'Acquired (Read)'))
+                self.values.append(('Readers', r))
+            else:
+                self.values.append(('Status', 'Not acquired'))
 
     def read_attributes(self):
         """Read the attributes of the rwlock."""
@@ -535,10 +532,12 @@ class RWLockPrinter(object):
         else:
             self.values.append(('Shared', 'No'))
 
-        if self.prefers_writers:
+        if self.flags == PTHREAD_RWLOCK_PREFER_READER_NP:
+            self.values.append(('Prefers', 'Readers'))
+        elif self.flags == PTHREAD_RWLOCK_PREFER_WRITER_NP:
             self.values.append(('Prefers', 'Writers'))
         else:
-            self.values.append(('Prefers', 'Readers'))
+            self.values.append(('Prefers', 'Writers no recursive readers'))
 
 class RWLockAttributesPrinter(object):
     """Pretty printer for pthread_rwlockattr_t.
@@ -599,13 +598,12 @@ class RWLockAttributesPrinter(object):
             # PTHREAD_PROCESS_PRIVATE
             self.values.append(('Shared', 'No'))
 
-        if (rwlock_type == PTHREAD_RWLOCK_PREFER_READER_NP or
-            rwlock_type == PTHREAD_RWLOCK_PREFER_WRITER_NP):
-            # This is a known bug.  Using PTHREAD_RWLOCK_PREFER_WRITER_NP will
-            # still make the rwlock prefer readers.
+        if rwlock_type == PTHREAD_RWLOCK_PREFER_READER_NP:
             self.values.append(('Prefers', 'Readers'))
-        elif rwlock_type == PTHREAD_RWLOCK_PREFER_WRITER_NONRECURSIVE_NP:
+        elif rwlock_type == PTHREAD_RWLOCK_PREFER_WRITER_NP:
             self.values.append(('Prefers', 'Writers'))
+        else:
+            self.values.append(('Prefers', 'Writers no recursive readers'))
 
 def register(objfile):
     """Register the pretty printers within the given objfile."""
diff --git a/nptl/nptl_lock_constants.pysym b/nptl/nptl_lock_constants.pysym
index 303ec61..3251d2e 100644
--- a/nptl/nptl_lock_constants.pysym
+++ b/nptl/nptl_lock_constants.pysym
@@ -70,6 +70,11 @@ PTHREAD_RWLOCK_PREFER_READER_NP
 PTHREAD_RWLOCK_PREFER_WRITER_NP
 PTHREAD_RWLOCK_PREFER_WRITER_NONRECURSIVE_NP
 
+-- Rwlock
+PTHREAD_RWLOCK_WRPHASE
+PTHREAD_RWLOCK_WRLOCKED
+PTHREAD_RWLOCK_READER_SHIFT
+
 -- 'Shared' attribute values
 PTHREAD_PROCESS_PRIVATE
 PTHREAD_PROCESS_SHARED
diff --git a/nptl/test-rwlock-printers.py b/nptl/test-rwlock-printers.py
index b972fa6..bc58be3 100644
--- a/nptl/test-rwlock-printers.py
+++ b/nptl/test-rwlock-printers.py
@@ -35,9 +35,9 @@ try:
 
     break_at(test_source, 'Test locking (reader)')
     continue_cmd() # Go to test_locking_reader
-    test_printer(var, to_string, {'Status': 'Unlocked'})
+    test_printer(var, to_string, {'Status': 'Not acquired'})
     next_cmd()
-    test_printer(var, to_string, {'Status': r'Locked \(Read\)', 'Readers': '1'})
+    test_printer(var, to_string, {'Status': r'Acquired \(Read\)', 'Readers': '1'})
     next_cmd()
     test_printer(var, to_string, {'Readers': '2'})
     next_cmd()
@@ -45,10 +45,10 @@ try:
 
     break_at(test_source, 'Test locking (writer)')
     continue_cmd() # Go to test_locking_writer
-    test_printer(var, to_string, {'Status': 'Unlocked'})
+    test_printer(var, to_string, {'Status': 'Not acquired'})
     next_cmd()
     thread_id = get_current_thread_lwpid()
-    test_printer(var, to_string, {'Status': r'Locked \(Write\)',
+    test_printer(var, to_string, {'Status': r'Acquired \(Write\)',
                                   'Writer ID': thread_id})
 
     continue_cmd() # Exit
diff --git a/nptl/test-rwlockattr-printers.c b/nptl/test-rwlockattr-printers.c
index d12facf..c79956b 100644
--- a/nptl/test-rwlockattr-printers.c
+++ b/nptl/test-rwlockattr-printers.c
@@ -75,6 +75,8 @@ test_setkind_np (pthread_rwlock_t *rwlock, pthread_rwlockattr_t *attr)
 
   if (SET_KIND (attr, PTHREAD_RWLOCK_PREFER_READER_NP) == 0 /* Set kind.  */
       && rwlock_reinit (rwlock, attr) == PASS
+      && SET_KIND (attr, PTHREAD_RWLOCK_PREFER_WRITER_NP) == 0
+      && rwlock_reinit (rwlock, attr) == PASS
       && SET_KIND (attr, PTHREAD_RWLOCK_PREFER_WRITER_NONRECURSIVE_NP) == 0
       && rwlock_reinit (rwlock, attr) == PASS)
     result = PASS;
diff --git a/nptl/test-rwlockattr-printers.py b/nptl/test-rwlockattr-printers.py
index 1ca2dc6..aa4f964 100644
--- a/nptl/test-rwlockattr-printers.py
+++ b/nptl/test-rwlockattr-printers.py
@@ -46,6 +46,9 @@ try:
     next_cmd(2)
     test_printer(rwlock_var, rwlock_to_string, {'Prefers': 'Writers'})
     test_printer(attr_var, attr_to_string, {'Prefers': 'Writers'})
+    next_cmd(2)
+    test_printer(rwlock_var, rwlock_to_string, {'Prefers': 'Writers no recursive readers'})
+    test_printer(attr_var, attr_to_string, {'Prefers': 'Writers no recursive readers'})
 
     break_at(test_source, 'Set shared')
     continue_cmd() # Go to test_setpshared