From patchwork Sun Oct 27 15:46:26 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mike Crowe X-Patchwork-Id: 1185047 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-511865-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=quarantine dis=none) header.from=mcrowe.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="KlueNX5z"; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=mcrowe.com header.i=@mcrowe.com header.b="YtLm9wOO"; 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 471MkH1zknz9sP4 for ; Mon, 28 Oct 2019 02:50:15 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; q=dns; s=default; b=hjc E/MHQaQ0ayPx3ApOLoPvn8ZN2ZHirE3Mka6lDrbqKSoI5gkJ5SCPeMUfWv5P/cVc BzSsJtvfS3W0PnSZbeRqANKzoQYXumYecep8TntZ0Nbycd7bEzO5G+whK+BrxHTx xIH7FLKWxqB7IAp3LFXbVGtf3R0LFIwVHnT12A5Y= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; s=default; bh=7swj9fOYg d1/ZJwHzmQymFmtj80=; b=KlueNX5zKJ2P2ytDWqczSsnYfQhEyv3PfNipOrazH /ULVDZ74tvs22OFj4Qn4EPkMFYvdKjza7m9m079uuOKRfBWvsCowpBljZp5C8aBh ZIaEvVa3vhVIQ1OhPHgXLQartgv1n7Ees1SMURjon1DScBn3lTHL+q2SLjHEjmuW Ts= Received: (qmail 111350 invoked by alias); 27 Oct 2019 15:50:07 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 111338 invoked by uid 89); 27 Oct 2019 15:50:07 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-23.3 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_SOFTFAIL autolearn=ham version=3.3.1 spammy=XXX, HX-detected-operating-system:timestamps, libsupc X-HELO: eggs.gnu.org Received: from eggs.gnu.org (HELO eggs.gnu.org) (209.51.188.92) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 27 Oct 2019 15:50:04 +0000 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1iOknp-0005pe-32 for gcc-patches@gcc.gnu.org; Sun, 27 Oct 2019 11:50:02 -0400 Received: from avasout03.plus.net ([84.93.230.244]:59627) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1iOkno-0005nB-Qz for gcc-patches@gcc.gnu.org; Sun, 27 Oct 2019 11:50:01 -0400 Received: from deneb ([80.229.24.9]) by smtp with ESMTP id OkkiieIUYtvkXOkkjihKUF; Sun, 27 Oct 2019 15:46:58 +0000 X-Clacks-Overhead: "GNU Terry Pratchett" X-CM-Score: 0.00 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=mcrowe.com; s=20191005; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To: Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To:Content-Type:Content-ID: Content-Description; bh=373Y0alntjcQZAM4gL2b6ml6f3bB/FxCfTftbnR1HjY=; b=YtLm9 wOO1R62LqL8AFrbl/C02AarAMq9v67S5LC4ucs4VXjYRDgLSNsXrFYhKIi8leqzzIVHy7/Z9rGpfR +e5YqUy0DuxUM9dvzp3MuVvzw+ovaC2loIORwOUu/LeEJuZOXR9DYBVMeHWFqyb7M10fcawzpGxn6 6QSQYO2K1WPzJoDREJ5AiNupKCI9rvppWJ5l5H1ryXyqkSv6Njso2pb9NT7QfeaH6YVRoHygBIEiL 8PLb/5Z7J23kDUU1FlMj2IZJ5pW2KXd2kw/4d18IiG9OJ0qypDfqImU2vQpCqL4yv8h7JuywIs+AS pvcDqihtZgsSP5Kz+PMrtQcK3NbIw==; Received: from mac by deneb with local (Exim 4.92) (envelope-from ) id 1iOkkg-0005QX-KX; Sun, 27 Oct 2019 15:46:46 +0000 From: Mike Crowe To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Cc: Mike Crowe Subject: [PATCH v4 3/7] libstdc++ futex: Support waiting on std::chrono::steady_clock directly Date: Sun, 27 Oct 2019 15:46:26 +0000 Message-Id: In-Reply-To: References: MIME-Version: 1.0 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x (no timestamps) [generic] [fuzzy] X-Received-From: 84.93.230.244 The user-visible effect of this change is for std::future::wait_until to use CLOCK_MONOTONIC when passed a timeout of std::chrono::steady_clock type. This makes it immune to any changes made to the system clock CLOCK_REALTIME. Add an overload of __atomic_futex_unsigned::_M_load_and_text_until_impl that accepts a std::chrono::steady_clock, and correctly passes this through to __atomic_futex_unsigned_base::_M_futex_wait_until_steady which uses CLOCK_MONOTONIC for the timeout within the futex system call. These functions are mostly just copies of the std::chrono::system_clock versions with small tweaks. Prior to this commit, a std::chrono::steady timeout would be converted via std::chrono::system_clock which risks reducing or increasing the timeout if someone changes CLOCK_REALTIME whilst the wait is happening. (The commit immediately prior to this one increases the window of opportunity for that from a short period during the calculation of a relative timeout, to the entire duration of the wait.) FUTEX_WAIT_BITSET was added in kernel v2.6.25. If futex reports ENOSYS to indicate that this operation is not supported then the code falls back to using clock_gettime(2) to calculate a relative time to wait for. I believe that I've added this functionality in a way that it doesn't break ABI compatibility, but that has made it more verbose and less type safe. I believe that it would be better to maintain the timeout as an instance of the correct clock type all the way down to a single _M_futex_wait_until function with an overload for each clock. The current scheme of separating out the seconds and nanoseconds early risks accidentally calling the wait function for the wrong clock. Unfortunately, doing this would break code that compiled against the old header. * libstdc++-v3/config/abi/pre/gnu.ver: Update for addition of __atomic_futex_unsigned_base::_M_futex_wait_until_steady. * libstdc++-v3/include/bits/atomic_futex.h (__atomic_futex_unsigned_base): Add comments to clarify that _M_futex_wait_until _M_load_and_test_until use CLOCK_REALTIME. Declare new _M_futex_wait_until_steady and _M_load_and_text_until_steady methods that use CLOCK_MONOTONIC. Add _M_load_and_test_until_impl and _M_load_when_equal_until overloads that accept a steady_clock time_point and use these new methods. * libstdc++-v3/src/c++11/futex.cc: Include headers required for clock_gettime. Add futex_clock_monotonic_flag constant to tell futex to use CLOCK_MONOTONIC to match the existing futex_clock_realtime_flag. Add futex_clock_monotonic_unavailable to store the result of trying to use CLOCK_MONOTONIC. (__atomic_futex_unsigned_base::_M_futex_wait_until_steady): Add new variant of _M_futex_wait_until that uses CLOCK_MONOTONIC to support waiting using steady_clock. --- libstdc++-v3/config/abi/pre/gnu.ver | 10 +-- libstdc++-v3/include/bits/atomic_futex.h | 67 +++++++++++++++++++- libstdc++-v3/src/c++11/futex.cc | 82 +++++++++++++++++++++++++- 3 files changed, 154 insertions(+), 5 deletions(-) diff --git a/libstdc++-v3/config/abi/pre/gnu.ver b/libstdc++-v3/config/abi/pre/gnu.ver index e61fbe0..26492bc 100644 --- a/libstdc++-v3/config/abi/pre/gnu.ver +++ b/libstdc++-v3/config/abi/pre/gnu.ver @@ -1916,10 +1916,9 @@ GLIBCXX_3.4.21 { _ZNSt7codecvtID[is]c*; _ZT[ISV]St7codecvtID[is]c*E; - extern "C++" - { - std::__atomic_futex_unsigned_base*; - }; + # std::__atomic_futex_unsigned_base members + _ZNSt28__atomic_futex_unsigned_base19_M_futex_notify_all*; + _ZNSt28__atomic_futex_unsigned_base19_M_futex_wait_until*; # codecvt_utf8 etc. _ZNKSt19__codecvt_utf8_base*; @@ -2291,6 +2290,9 @@ GLIBCXX_3.4.28 { _ZNSt12__shared_ptrINSt10filesystem28recursive_directory_iterator10_Dir_stackELN9__gnu_cxx12_Lock_policyE[012]EEC2EOS5_; _ZNSt12__shared_ptrINSt10filesystem7__cxx1128recursive_directory_iterator10_Dir_stackELN9__gnu_cxx12_Lock_policyE[012]EEC2EOS6_; + # std::__atomic_futex_unsigned_base::_M_futex_wait_until_steady + _ZNSt28__atomic_futex_unsigned_base26_M_futex_wait_until_steady*; + } GLIBCXX_3.4.27; # Symbols in the support library (libsupc++) have their own tag. diff --git a/libstdc++-v3/include/bits/atomic_futex.h b/libstdc++-v3/include/bits/atomic_futex.h index 36fda38..6d28d28 100644 --- a/libstdc++-v3/include/bits/atomic_futex.h +++ b/libstdc++-v3/include/bits/atomic_futex.h @@ -52,11 +52,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION #if defined(_GLIBCXX_HAVE_LINUX_FUTEX) && ATOMIC_INT_LOCK_FREE > 1 struct __atomic_futex_unsigned_base { - // Returns false iff a timeout occurred. + // __s and __ns are measured against CLOCK_REALTIME. Returns false + // iff a timeout occurred. bool _M_futex_wait_until(unsigned *__addr, unsigned __val, bool __has_timeout, chrono::seconds __s, chrono::nanoseconds __ns); + // __s and __ns are measured against CLOCK_MONOTONIC. Returns + // false iff a timeout occurred. + bool + _M_futex_wait_until_steady(unsigned *__addr, unsigned __val, + bool __has_timeout, chrono::seconds __s, chrono::nanoseconds __ns); + // This can be executed after the object has been destroyed. static void _M_futex_notify_all(unsigned* __addr); }; @@ -86,6 +93,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // value if equal is false. // The assumed value is the caller's assumption about the current value // when making the call. + // __s and __ns are measured against CLOCK_REALTIME. unsigned _M_load_and_test_until(unsigned __assumed, unsigned __operand, bool __equal, memory_order __mo, bool __has_timeout, @@ -110,6 +118,36 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION } } + // If a timeout occurs, returns a current value after the timeout; + // otherwise, returns the operand's value if equal is true or a different + // value if equal is false. + // The assumed value is the caller's assumption about the current value + // when making the call. + // __s and __ns are measured against CLOCK_MONOTONIC. + unsigned + _M_load_and_test_until_steady(unsigned __assumed, unsigned __operand, + bool __equal, memory_order __mo, bool __has_timeout, + chrono::seconds __s, chrono::nanoseconds __ns) + { + for (;;) + { + // Don't bother checking the value again because we expect the caller + // to have done it recently. + // memory_order_relaxed is sufficient because we can rely on just the + // modification order (store_notify uses an atomic RMW operation too), + // and the futex syscalls synchronize between themselves. + _M_data.fetch_or(_Waiter_bit, memory_order_relaxed); + bool __ret = _M_futex_wait_until_steady((unsigned*)(void*)&_M_data, + __assumed | _Waiter_bit, + __has_timeout, __s, __ns); + // Fetch the current value after waiting (clears _Waiter_bit). + __assumed = _M_load(__mo); + if (!__ret || ((__operand == __assumed) == __equal)) + return __assumed; + // TODO adapt wait time + } + } + // Returns the operand's value if equal is true or a different value if // equal is false. // The assumed value is the caller's assumption about the current value @@ -140,6 +178,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION true, __s.time_since_epoch(), __ns); } + template + unsigned + _M_load_and_test_until_impl(unsigned __assumed, unsigned __operand, + bool __equal, memory_order __mo, + const chrono::time_point& __atime) + { + auto __s = chrono::time_point_cast(__atime); + auto __ns = chrono::duration_cast(__atime - __s); + // XXX correct? + return _M_load_and_test_until_steady(__assumed, __operand, __equal, __mo, + true, __s.time_since_epoch(), __ns); + } + public: _GLIBCXX_ALWAYS_INLINE unsigned @@ -200,6 +251,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION return (__i & ~_Waiter_bit) == __val; } + // Returns false iff a timeout occurred. + template + _GLIBCXX_ALWAYS_INLINE bool + _M_load_when_equal_until(unsigned __val, memory_order __mo, + const chrono::time_point& __atime) + { + unsigned __i = _M_load(__mo); + if ((__i & ~_Waiter_bit) == __val) + return true; + // TODO Spin-wait first. Ignore effect on timeout. + __i = _M_load_and_test_until_impl(__i, __val, true, __mo, __atime); + return (__i & ~_Waiter_bit) == __val; + } + _GLIBCXX_ALWAYS_INLINE void _M_store_notify_all(unsigned __val, memory_order __mo) { diff --git a/libstdc++-v3/src/c++11/futex.cc b/libstdc++-v3/src/c++11/futex.cc index 38ae448..1df84bb 100644 --- a/libstdc++-v3/src/c++11/futex.cc +++ b/libstdc++-v3/src/c++11/futex.cc @@ -33,9 +33,15 @@ #include #include +#ifdef _GLIBCXX_USE_CLOCK_GETTIME_SYSCALL +#include +#include +#endif + // Constants for the wait/wake futex syscall operations const unsigned futex_wait_op = 0; const unsigned futex_wait_bitset_op = 9; +const unsigned futex_clock_monotonic_flag = 0; const unsigned futex_clock_realtime_flag = 256; const unsigned futex_bitset_match_any = ~0; const unsigned futex_wake_op = 1; @@ -43,6 +49,7 @@ const unsigned futex_wake_op = 1; namespace { std::atomic futex_clock_realtime_unavailable; + std::atomic futex_clock_monotonic_unavailable; } namespace std _GLIBCXX_VISIBILITY(default) @@ -121,6 +128,81 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION } } + bool + __atomic_futex_unsigned_base::_M_futex_wait_until_steady(unsigned *__addr, + unsigned __val, + bool __has_timeout, chrono::seconds __s, chrono::nanoseconds __ns) + { + if (!__has_timeout) + { + // Ignore whether we actually succeeded to block because at worst, + // we will fall back to spin-waiting. The only thing we could do + // here on errors is abort. + int ret __attribute__((unused)); + ret = syscall (SYS_futex, __addr, futex_wait_op, __val, nullptr); + __glibcxx_assert(ret == 0 || errno == EINTR || errno == EAGAIN); + return true; + } + else + { + if (!futex_clock_monotonic_unavailable.load(std::memory_order_relaxed)) + { + struct timespec rt; + rt.tv_sec = __s.count(); + rt.tv_nsec = __ns.count(); + + if (syscall (SYS_futex, __addr, + futex_wait_bitset_op | futex_clock_monotonic_flag, + __val, &rt, nullptr, futex_bitset_match_any) == -1) + { + __glibcxx_assert(errno == EINTR || errno == EAGAIN + || errno == ETIMEDOUT || errno == ENOSYS); + if (errno == ETIMEDOUT) + return false; + else if (errno == ENOSYS) + { + futex_clock_monotonic_unavailable.store(true, + std::memory_order_relaxed); + // Fall through to legacy implementation if the system + // call is unavailable. + } + else + return true; + } + } + + // We only get to here if futex_clock_monotonic_unavailable was + // true or has just been set to true. + struct timespec ts; +#ifdef _GLIBCXX_USE_CLOCK_GETTIME_SYSCALL + syscall(SYS_clock_gettime, CLOCK_MONOTONIC, &ts); +#else + clock_gettime(CLOCK_MONOTONIC, &ts); +#endif + // Convert the absolute timeout value to a relative timeout + struct timespec rt; + rt.tv_sec = __s.count() - ts.tv_sec; + rt.tv_nsec = __ns.count() - ts.tv_nsec; + if (rt.tv_nsec < 0) + { + rt.tv_nsec += 1000000000; + --rt.tv_sec; + } + // Did we already time out? + if (rt.tv_sec < 0) + return false; + + if (syscall (SYS_futex, __addr, futex_wait_op, __val, &rt) == -1) + { + __glibcxx_assert(errno == EINTR || errno == EAGAIN + || errno == ETIMEDOUT); + if (errno == ETIMEDOUT) + return false; + } + return true; + } + } + void __atomic_futex_unsigned_base::_M_futex_notify_all(unsigned* __addr) {