From patchwork Fri Jan 20 12:57:44 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Galvan X-Patchwork-Id: 18961 Received: (qmail 28260 invoked by alias); 20 Jan 2017 12:58:04 -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 26260 invoked by uid 89); 20 Jan 2017 12:57:51 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-HELO: mail-yb0-f194.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id; bh=/c3Dio0k/+1L9plYoxDIDItcuyqIesEeC1kVPfkT0vA=; b=XwzXzAIv6ozoM3eoOB+3901crWbxQsPDjD7SJ7yDNCAJheH6ex16MpwLtxB8vyrlDK +L8Ur6T53xVqwEm3OlVmtt6dafZZUjiRvEAYlD3iQLr155MlbeZw8DrK/aDbqLr60pQz nto6R1nCJfcREYt+wfym51vV8IlpRA4gyx0pFycO0HqOxrLoJGUtRvD5fPKmLxyYMe83 6p3s7uyRkm6Fe8UWfyDFuWNozfp3v24LP5hXTe1ZRdxdrkpCUjvUErCGyI60naKbIUIb 8JFJP/QdMRA3cRgMQoLnz2SHQmLHBFPuaxgbkfoInAFfZnrmJhH5tkPxgsmDt330wlLE S4xA== X-Gm-Message-State: AIkVDXJpizSxuRMHdxRAmd8YR39c6xlbLU7WT7sL5FuWot4Ct+e55bwCFNdQUWRqkw5WtA== X-Received: by 10.55.153.198 with SMTP id b189mr12404648qke.79.1484917069908; Fri, 20 Jan 2017 04:57:49 -0800 (PST) From: Martin Galvan To: libc-alpha@sourceware.org, siddhesh@sourceware.org, triegel@redhat.com Subject: [PING][PATCH v2] Fix mutex pretty printer test and pretty printer output Date: Fri, 20 Jan 2017 09:57:44 -0300 Message-Id: <1484917064-5206-1-git-send-email-omgalvan.86@gmail.com> This is a new version of the patch Torvald posted here: https://www.sourceware.org/ml/libc-alpha/2017-01/msg00169.html This fixes the mutex pretty printer so that, if the owner ID isn't recorded (such as in the current lock elision implementation), "Owner ID" will be shown as "Unknown" instead of 0. It also changes the mutex printer output so that it says "Acquired" instead of "Locked". The mutex tests are updated accordingly. In addition, I've added a paragraph to the "Known issues" section of the printers README explaining that the printer output isn't guaranteed to cover every detail, as we discussed with Torvald in that same thread. I don't have write access, so somebody should commit this for me. Thanks. ChangeLog: 2017-01-14 Martin Galvan * README.pretty-printers (Known issues): Warn about printers not always covering everything. * nptl/nptl-printers.py (MutexPrinter): Change output. * nptl/test-mutex-printers.py: Fix test and adapt to changed output. --- README.pretty-printers | 7 ++++++- nptl/nptl-printers.py | 33 ++++++++++++++++++++------------- nptl/test-mutex-printers.py | 17 ++++++++++------- 3 files changed, 36 insertions(+), 21 deletions(-) -- 2.7.4 diff --git a/README.pretty-printers b/README.pretty-printers index a2536ca..2522cb8 100644 --- a/README.pretty-printers +++ b/README.pretty-printers @@ -29,7 +29,7 @@ However, with a pretty printer gdb will output something like this: (gdb) print mutex $1 = pthread_mutex_t = { Type = Normal, - Status = Unlocked, + Status = Not acquired, Robust = No, Shared = No, Protocol = Priority protect, @@ -145,6 +145,11 @@ any changes to the target code must also update the corresponding printers. On the plus side, the printer code itself may serve as a kind of documentation for the target code. +* There's no guarantee that the information the pretty printers provide is +complete, i.e. some details might be left off. For example, the pthread_mutex_t +printers won't report whether a thread is spin-waiting in an attempt to acquire +the mutex. + * Older versions of the gdb Python API have a bug where gdb.RegexpCollectionPrettyPrinter would not be able to get a value's real type if it was typedef'd. This would cause gdb to ignore the pretty printers for diff --git a/nptl/nptl-printers.py b/nptl/nptl-printers.py index 9d67865..572a25c 100644 --- a/nptl/nptl-printers.py +++ b/nptl/nptl-printers.py @@ -101,9 +101,9 @@ class MutexPrinter(object): def read_status(self): """Read the mutex's status. - For architectures which support lock elision, this method reads - whether the mutex appears as locked in memory (i.e. it may show it as - unlocked even after calling pthread_mutex_lock). + Architectures that support lock elision might not record the mutex owner + ID in the __owner field. In that case, the owner will be reported as + "Unknown". """ if self.kind == PTHREAD_MUTEX_DESTROYED: @@ -124,13 +124,14 @@ class MutexPrinter(object): """ if self.lock == PTHREAD_MUTEX_UNLOCKED: - self.values.append(('Status', 'Unlocked')) + self.values.append(('Status', 'Not acquired')) else: if self.lock & FUTEX_WAITERS: - self.values.append(('Status', 'Locked, possibly with waiters')) + self.values.append(('Status', + 'Acquired, possibly with waiters')) else: self.values.append(('Status', - 'Locked, possibly with no waiters')) + 'Acquired, possibly with no waiters')) if self.lock & FUTEX_OWNER_DIED: self.values.append(('Owner ID', '%d (dead)' % self.owner)) @@ -147,7 +148,7 @@ class MutexPrinter(object): def read_status_no_robust(self): """Read the status of a non-robust mutex. - Read info on whether the mutex is locked, if it may have waiters + Read info on whether the mutex is acquired, if it may have waiters and its owner (if any). """ @@ -157,7 +158,7 @@ class MutexPrinter(object): lock_value &= ~(PTHREAD_MUTEX_PRIO_CEILING_MASK) if lock_value == PTHREAD_MUTEX_UNLOCKED: - self.values.append(('Status', 'Unlocked')) + self.values.append(('Status', 'Not acquired')) else: if self.kind & PTHREAD_MUTEX_PRIO_INHERIT_NP: waiters = self.lock & FUTEX_WAITERS @@ -168,12 +169,18 @@ class MutexPrinter(object): owner = self.owner if waiters: - self.values.append(('Status', 'Locked, possibly with waiters')) + self.values.append(('Status', + 'Acquired, possibly with waiters')) else: self.values.append(('Status', - 'Locked, possibly with no waiters')) + 'Acquired, possibly with no waiters')) - self.values.append(('Owner ID', owner)) + if self.owner != 0: + self.values.append(('Owner ID', owner)) + else: + # Owner isn't recorded, probably because lock elision + # is enabled. + self.values.append(('Owner ID', 'Unknown')) def read_attributes(self): """Read the mutex's attributes.""" @@ -208,14 +215,14 @@ class MutexPrinter(object): def read_misc_info(self): """Read miscellaneous info on the mutex. - For now this reads the number of times a recursive mutex was locked + For now this reads the number of times a recursive mutex was acquired by the same thread. """ mutex_type = self.kind & PTHREAD_MUTEX_KIND_MASK if mutex_type == PTHREAD_MUTEX_RECURSIVE and self.count > 1: - self.values.append(('Times locked recursively', self.count)) + self.values.append(('Times acquired by the owner', self.count)) class MutexAttributesPrinter(object): """Pretty printer for pthread_mutexattr_t. diff --git a/nptl/test-mutex-printers.py b/nptl/test-mutex-printers.py index 23f16b0..687a32c 100644 --- a/nptl/test-mutex-printers.py +++ b/nptl/test-mutex-printers.py @@ -39,15 +39,18 @@ try: break_at(test_source, 'Test status (non-robust)') continue_cmd() # Go to test_status_no_robust - 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': 'Locked, possibly with no waiters', - 'Owner ID': thread_id}) + # Owner ID might be reported either as the thread ID or as "Unknown" + # (if e.g. lock elision is enabled). + test_printer(var, to_string, + {'Status': 'Acquired, possibly with no waiters', + 'Owner ID': r'({0}|Unknown)'.format(thread_id)}) break_at(test_source, 'Test status (robust)') continue_cmd() # Go to test_status_robust - test_printer(var, to_string, {'Status': 'Unlocked'}) + test_printer(var, to_string, {'Status': 'Not acquired'}) # We'll now test the robust mutex locking states. We'll create a new # thread that will lock a robust mutex and exit without unlocking it. @@ -75,15 +78,15 @@ try: test_printer(var, to_string, {'Owner ID': thread_id, 'State protected by this mutex': 'Inconsistent'}) next_cmd() - test_printer(var, to_string, {'Status': 'Unlocked', + test_printer(var, to_string, {'Status': 'Not acquired', 'State protected by this mutex': 'Not recoverable'}) set_scheduler_locking(False) break_at(test_source, 'Test recursive locks') continue_cmd() # Go to test_recursive_locks - test_printer(var, to_string, {'Times locked recursively': '2'}) + test_printer(var, to_string, {'Times acquired by the owner': '2'}) next_cmd() - test_printer(var, to_string, {'Times locked recursively': '3'}) + test_printer(var, to_string, {'Times acquired by the owner': '3'}) continue_cmd() # Exit except (NoLineError, pexpect.TIMEOUT) as exception: