From patchwork Tue Aug 5 15:43:03 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bernie Ogden X-Patchwork-Id: 34958 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-oi0-f71.google.com (mail-oi0-f71.google.com [209.85.218.71]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 84633202A1 for ; Tue, 5 Aug 2014 15:43:27 +0000 (UTC) Received: by mail-oi0-f71.google.com with SMTP id e131sf3977963oig.6 for ; Tue, 05 Aug 2014 08:43:25 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:mailing-list:precedence:list-id :list-unsubscribe:list-subscribe:list-archive:list-post:list-help :sender:delivered-to:from:subject:message-id:date:to:mime-version :x-original-sender:x-original-authentication-results:content-type :content-transfer-encoding; bh=4PpB39HMk9S1lXm2u3rV7uL57UdhAkgxsu3lQQAZAfk=; b=A8COh7OEqQvhIUlKUA68FDl+qC0bqnVpTX7B5UCxjiBOGxHxnbDJNa0OYGL/TCBstJ +ZemusWBH7Zt1BPqRNraMd11fNH8599tNzt+EyJ0tChPvsHHY8ysZvhwTqLYIgyYQkgv kTz8VNfBcH1G3jZD3uZZRKpxlIhXP+3th8Yob4g89p08uG5IURXAQtxrEWqVqdR0sHRX zMuz4+cjfdvi0p6FDijsHVFEio/kIdgJTPc0pdQB8K2Yf5ciFMcNp/KW7dIzTuf+MzkR DzJNd/QIEv1pccJc6hx6OJK481DfgsCm6fCvWT7xpquyJw1tW/kBrW9la3JMuXw71/6N iN6Q== X-Gm-Message-State: ALoCoQl9XfbQ9EvoTyA8IPJ9AGdOqkXTOkuNB+EBTMcd1CSyPFp5RzTbEg1fMgCGX4zFMjfBZDXk X-Received: by 10.42.188.84 with SMTP id cz20mr2849429icb.1.1407253405739; Tue, 05 Aug 2014 08:43:25 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.140.86.80 with SMTP id o74ls379560qgd.48.gmail; Tue, 05 Aug 2014 08:43:25 -0700 (PDT) X-Received: by 10.221.68.135 with SMTP id xy7mr2654421vcb.65.1407253405616; Tue, 05 Aug 2014 08:43:25 -0700 (PDT) Received: from mail-vc0-x22c.google.com (mail-vc0-x22c.google.com [2607:f8b0:400c:c03::22c]) by mx.google.com with ESMTPS id xv9si1391613vdb.0.2014.08.05.08.43.25 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Tue, 05 Aug 2014 08:43:25 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 2607:f8b0:400c:c03::22c as permitted sender) client-ip=2607:f8b0:400c:c03::22c; Received: by mail-vc0-f172.google.com with SMTP id im17so1860533vcb.3 for ; Tue, 05 Aug 2014 08:43:25 -0700 (PDT) X-Received: by 10.220.97.5 with SMTP id j5mr4712592vcn.16.1407253405495; Tue, 05 Aug 2014 08:43:25 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.221.37.5 with SMTP id tc5csp399583vcb; Tue, 5 Aug 2014 08:43:24 -0700 (PDT) X-Received: by 10.70.102.130 with SMTP id fo2mr5258812pdb.68.1407253404534; Tue, 05 Aug 2014 08:43:24 -0700 (PDT) Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id fy14si1190918pdb.120.2014.08.05.08.43.24 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 05 Aug 2014 08:43:24 -0700 (PDT) Received-SPF: pass (google.com: domain of libc-alpha-return-52078-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Received: (qmail 30286 invoked by alias); 5 Aug 2014 15:43:14 -0000 Mailing-List: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org Precedence: list 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 30275 invoked by uid 89); 5 Aug 2014 15:43:14 -0000 X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-we0-f182.google.com X-Received: by 10.194.185.113 with SMTP id fb17mr6900746wjc.117.1407253389437; Tue, 05 Aug 2014 08:43:09 -0700 (PDT) From: Bernard Ogden Subject: [PATCH][BZ #16892] Check value of futex before updating in __lll_timedlock Message-Id: <1D01BCB7-8B02-415F-9244-58C15296B799@linaro.org> Date: Tue, 5 Aug 2014 16:43:03 +0100 To: "GNU C. Library" Mime-Version: 1.0 (Mac OS X Mail 7.3 \(1878.6\)) X-Original-Sender: bernie.ogden@linaro.org X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 2607:f8b0:400c:c03::22c as permitted sender) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org; dkim=pass header.i=@sourceware.org X-Google-Group-Id: 836684582541 __lll_timedlock in generic lowlevellock.h sets futex to 1 without checking that it is already 0. This can create 2 performance problems (analysis by Carlos O'Donell): 1) Up to N threads can fail to sleep when they ought to have done, where N is the number of threads expecting futex==2. For example: * T1 calls __lll_timedlock setting futex to 1 and taking the lock. * T2 calls __lll_timedlock setting futex to 1 but does not take the lock. * T2 calls __lll_timedlock_wait and sets the futex to 2 and does not gain the lock. * T3 calls __lll_timedlock setting futex to 1 but does not take the lock. * T2 calls lll_futex_time_wait but fails with -EWOULDBLOCK because T3 reset futex to 1. -> One inflight thread (T2), and one spurious failed futex wait syscall. * T2 again sets the futex to 2 and does not gain the lock. * ... T2 and T3 go on to call futex wait syscall and both sleep. 2) __lll_unlock only wakes if futex was > 1 prior to release. Thus it can happen that __lll_timedlock keeps setting futex from 2 to 1 just prior to __lll_unlock calls, preventing waiters from being awoken. This certainly affects m68k, arm and aarch64 - sh may also be affected but it's a little harder to tell as its written in asm. We fix this by changing an atomic_exchange_acq to atomic_compare_and_exchange_bool_acq. This bug previously affected arm, aarch64, m68k and sh/sh4. Since mips switched to the generic lowlevellock.h, it is also affected. Applying this patch will fix arm, aarch64 and mips. m68k and sh would need to switch to the generic header to get the fix. Change is pretty simple, but has been built and tested on arm. OK? 2014-08-05 Bernard Ogden [BZ #16892] * sysdeps/nptl/lowlevellock.h: Use atomic_compare_and_exchange_bool_acq rather than atomic_exchange_acq in __lll_timedlock. diff --git a/sysdeps/nptl/lowlevellock.h b/sysdeps/nptl/lowlevellock.h index 548a9c8..06ccde5 100644 --- a/sysdeps/nptl/lowlevellock.h +++ b/sysdeps/nptl/lowlevellock.h @@ -93,7 +93,8 @@ extern int __lll_robust_timedlock_wait (int *futex, const struct timespec *, int *__futex = (futex); \ int __val = 0; \ \ - if (__glibc_unlikely (atomic_exchange_acq (__futex, 1))) \ + if (__glibc_unlikely \ + (atomic_compare_and_exchange_bool_acq (__futex, 1, 0))) \ __val = __lll_timedlock_wait (__futex, abstime, private); \ __val; \ })