From patchwork Tue Jul 25 21:32:23 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Torvald Riegel X-Patchwork-Id: 793660 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=sourceware.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=libc-alpha-return-82415-incoming=patchwork.ozlabs.org@sourceware.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.b="NqOwxjyj"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3xHBKf4Dllz9s65 for ; Wed, 26 Jul 2017 07:32:38 +1000 (AEST) 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:cc:date :content-type:mime-version; q=dns; s=default; b=uecgf82Kzbpt5NXw 2TT/Ik8YCQJf94T2kMskDxBaty5+iB/6XnBps0qoo3roB3Wz5vVXRXAMXt4Gr8Dw B49RJcNnN9OZZJnWCh7EBNDtymobH3U3wou2ieMtXL0mnWrRduJbukkr+kij4Jzp gkcBbHtIQOQEvHhON+kuM4oiokE= 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:cc:date :content-type:mime-version; s=default; bh=ekaRdCLkiwYpEtXxAX5REl YDFCw=; b=NqOwxjyjLxEuTyZr8mmQvx7oKy1gq0odHiGzhyfqdPIMm7n0eXfWu1 YQDXJ7m0RuqMhigUSYL3HizTCcPS+J4fCd8s7C6l4mzuqYZo9S49UvVrv0jzlvCK FpCbEpO7/jWA/hcW5wHyh9ljzNtaCUeZBID7pWFkR8loNcaAR/tNc= Received: (qmail 23088 invoked by alias); 25 Jul 2017 21:32:30 -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 21618 invoked by uid 89); 25 Jul 2017 21:32:30 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=acquisition, Hx-languages-length:5510 X-HELO: mx1.redhat.com DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 5FE8564077 Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=triegel@redhat.com Message-ID: <1501018343.30433.50.camel@redhat.com> Subject: [PATCH] [BZ #21778] Fix oversight in robust mutex lock acquisition. From: Torvald Riegel To: GLIBC Devel Cc: Carlos O'Donell , siddhesh Porarekar Date: Tue, 25 Jul 2017 23:32:23 +0200 Mime-Version: 1.0 65810f0ef05e8c9e333f17a44e77808b163ca298 fixed a robust mutex bug but introduced BZ 21778: if the CAS used to try to acquire a lock fails, the expected value is not updated, which breaks other cases in the lock acquisition loop. The fix is to simply update the expected value with the value returned by the CAS, which ensures that behavior is as if the first case with the CAS never happened (if the CAS fails). This is a regression introduced in the last release, so it would be good to get this included in this release. I'll likely be AFK on Thursday, so please just commit this once it has been approved. Tested on x86_64-linux. [BZ 21778] * nptl/pthread_mutex_timedlock.c (__pthread_mutex_timedlock): Update oldval if the CAS fails. * nptl/pthread_mutex_lock.c (__pthread_mutex_lock_full): Likewise. * nptl/tst-mutex7.c (ROBUST, DELAY_NSEC, ROUNDS, N): New. (tf, do_test): Use them. * nptl/tst-mutex7robust.c: New file. * nptl/Makefile (tests): Add new test. commit 77e7d626fa6d7f8800c9658e65e43d13c5cdc81d Author: Torvald Riegel Date: Mon Jul 24 22:53:38 2017 +0200 Fix oversight in robust mutex lock acquisition. 65810f0ef05e8c9e333f17a44e77808b163ca298 fixed a robust mutex bug but introduced BZ 21778: if the CAS used to try to acquire a lock fails, the expected value is not updated, which breaks other cases in the lock acquisition loop. The fix is to simply update the expected value with the value returned by the CAS, which ensures that behavior is as if the first case with the CAS never happened. 2017-07-25 Torvald Riegel [BZ 21778] * nptl/pthread_mutex_timedlock.c (__pthread_mutex_timedlock): Update oldval if the CAS fails. * nptl/pthread_mutex_lock.c (__pthread_mutex_lock_full): Likewise. * nptl/tst-mutex7.c (ROBUST, DELAY_NSEC, ROUNDS, N): New. (tf, do_test): Use them. * nptl/tst-mutex7robust.c: New file. * nptl/Makefile (tests): Add new test. diff --git a/nptl/Makefile b/nptl/Makefile index dd01994..bca09bf 100644 --- a/nptl/Makefile +++ b/nptl/Makefile @@ -230,7 +230,7 @@ LDLIBS-tst-thread_local1 = -lstdc++ tests = tst-attr1 tst-attr2 tst-attr3 tst-default-attr \ tst-mutex1 tst-mutex2 tst-mutex3 tst-mutex4 tst-mutex5 tst-mutex6 \ - tst-mutex7 tst-mutex9 tst-mutex5a tst-mutex7a \ + tst-mutex7 tst-mutex9 tst-mutex5a tst-mutex7a tst-mutex7robust \ tst-mutexpi1 tst-mutexpi2 tst-mutexpi3 tst-mutexpi4 tst-mutexpi5 \ tst-mutexpi5a tst-mutexpi6 tst-mutexpi7 tst-mutexpi7a \ tst-mutexpi9 \ diff --git a/nptl/pthread_mutex_lock.c b/nptl/pthread_mutex_lock.c index b76475b..8c48503 100644 --- a/nptl/pthread_mutex_lock.c +++ b/nptl/pthread_mutex_lock.c @@ -197,11 +197,14 @@ __pthread_mutex_lock_full (pthread_mutex_t *mutex) { /* Try to acquire the lock through a CAS from 0 (not acquired) to our TID | assume_other_futex_waiters. */ - if (__glibc_likely ((oldval == 0) - && (atomic_compare_and_exchange_bool_acq - (&mutex->__data.__lock, - id | assume_other_futex_waiters, 0) == 0))) - break; + if (__glibc_likely (oldval == 0)) + { + oldval + = atomic_compare_and_exchange_val_acq (&mutex->__data.__lock, + id | assume_other_futex_waiters, 0); + if (__glibc_likely (oldval == 0)) + break; + } if ((oldval & FUTEX_OWNER_DIED) != 0) { diff --git a/nptl/pthread_mutex_timedlock.c b/nptl/pthread_mutex_timedlock.c index be53381..d5ec314 100644 --- a/nptl/pthread_mutex_timedlock.c +++ b/nptl/pthread_mutex_timedlock.c @@ -154,11 +154,14 @@ __pthread_mutex_timedlock (pthread_mutex_t *mutex, { /* Try to acquire the lock through a CAS from 0 (not acquired) to our TID | assume_other_futex_waiters. */ - if (__glibc_likely ((oldval == 0) - && (atomic_compare_and_exchange_bool_acq - (&mutex->__data.__lock, - id | assume_other_futex_waiters, 0) == 0))) - break; + if (__glibc_likely (oldval == 0)) + { + oldval + = atomic_compare_and_exchange_val_acq (&mutex->__data.__lock, + id | assume_other_futex_waiters, 0); + if (__glibc_likely (oldval == 0)) + break; + } if ((oldval & FUTEX_OWNER_DIED) != 0) { diff --git a/nptl/tst-mutex7.c b/nptl/tst-mutex7.c index a11afdb..7990409 100644 --- a/nptl/tst-mutex7.c +++ b/nptl/tst-mutex7.c @@ -26,13 +26,22 @@ #ifndef TYPE # define TYPE PTHREAD_MUTEX_DEFAULT #endif - +#ifndef ROBUST +# define ROBUST PTHREAD_MUTEX_STALLED +#endif +#ifndef DELAY_NSEC +# define DELAY_NSEC 11000 +#endif +#ifndef ROUNDS +# define ROUNDS 1000 +#endif +#ifndef N +# define N 100 +#endif static pthread_mutex_t lock; -#define ROUNDS 1000 -#define N 100 static void * @@ -40,7 +49,7 @@ tf (void *arg) { int nr = (long int) arg; int cnt; - struct timespec ts = { .tv_sec = 0, .tv_nsec = 11000 }; + struct timespec ts = { .tv_sec = 0, .tv_nsec = DELAY_NSEC }; for (cnt = 0; cnt < ROUNDS; ++cnt) { @@ -56,7 +65,8 @@ tf (void *arg) return (void *) 1l; } - nanosleep (&ts, NULL); + if ((ts.tv_sec > 0) || (ts.tv_nsec > 0)) + nanosleep (&ts, NULL); } return NULL; @@ -80,6 +90,12 @@ do_test (void) exit (1); } + if (pthread_mutexattr_setrobust (&a, ROBUST) != 0) + { + puts ("mutexattr_setrobust failed"); + exit (1); + } + #ifdef ENABLE_PI if (pthread_mutexattr_setprotocol (&a, PTHREAD_PRIO_INHERIT) != 0) { diff --git a/nptl/tst-mutex7robust.c b/nptl/tst-mutex7robust.c new file mode 100644 index 0000000..73eb66d --- /dev/null +++ b/nptl/tst-mutex7robust.c @@ -0,0 +1,6 @@ +#define TYPE PTHREAD_MUTEX_NORMAL +#define ROBUST PTHREAD_MUTEX_ROBUST +#define DELAY_NSEC 0 +#define ROUNDS 1000 +#define N 32 +#include "tst-mutex7.c"