From patchwork Fri Dec 23 20:36:21 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Torvald Riegel X-Patchwork-Id: 88978 Delivered-To: patch@linaro.org Received: by 10.140.20.101 with SMTP id 92csp3498920qgi; Fri, 23 Dec 2016 12:36:48 -0800 (PST) X-Received: by 10.99.185.1 with SMTP id z1mr28538260pge.179.1482525408428; Fri, 23 Dec 2016 12:36:48 -0800 (PST) Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id b35si11933078plh.329.2016.12.23.12.36.48 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 23 Dec 2016 12:36:48 -0800 (PST) Received-SPF: pass (google.com: domain of libc-alpha-return-76314-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Authentication-Results: mx.google.com; dkim=pass header.i=@sourceware.org; spf=pass (google.com: domain of libc-alpha-return-76314-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=libc-alpha-return-76314-patch=linaro.org@sourceware.org DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:message-id:subject:from:to:date:in-reply-to :references:content-type:mime-version; q=dns; s=default; b=BI0eI EwG+a5iblUlUbvx60ZYNIgd8DjsvP137WIRcPhxU/ZXC6TdmLKkhDrtmiM5f9a9Y nsVRxzn83IDgehxid0NmQXehv1fXjC85xRt6ATMvzETgbY+9OpvosMop0ibp/0FB gFl8yo6FvDSM607gS36SpysIxFpGIh2fE2CqDE= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:message-id:subject:from:to:date:in-reply-to :references:content-type:mime-version; s=default; bh=jj3Ol8P3LJR gfoSntcy/z/qlXyk=; b=FsWg5s3i3n4NhWeAzhv2hVedbiTwbH/Wc0zUZl+xSeB kFGelSfQvfkS4KWZtBSgqVJtx4XWnWsDSw1Lr3nrGRG9dDjmpgFHmt5x7nMUURmO g2nIhhNj7EBBJlXIYzzZVwP8jr4swSQWF5WzxXiSNF4bUwNZXbwKLgTxaa8AZHtM = Received: (qmail 109004 invoked by alias); 23 Dec 2016 20:36:37 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 108988 invoked by uid 89); 23 Dec 2016 20:36:35 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-5.0 required=5.0 tests=BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=acquisition, unlocked, sk:get_cur, Unlocked X-HELO: mx1.redhat.com Message-ID: <1482525381.14990.834.camel@redhat.com> Subject: Re: [PATCH 2/2] New pthread rwlock that is more scalable. From: Torvald Riegel To: GLIBC Devel Date: Fri, 23 Dec 2016 21:36:21 +0100 In-Reply-To: <1469655868.19224.34.camel@localhost.localdomain> References: <1469655533.19224.27.camel@localhost.localdomain> <1469655868.19224.34.camel@localhost.localdomain> Mime-Version: 1.0 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 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