From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 66367 invoked by alias); 12 Jan 2018 13:18:08 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 66316 invoked by uid 89); 12 Jan 2018 13:18:07 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.9 required=5.0 tests=BAYES_00,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=no version=3.3.2 spammy= X-Spam-User: qpsmtpd, 2 recipients X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 12 Jan 2018 13:18:06 +0000 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D6469A0B21; Fri, 12 Jan 2018 13:18:04 +0000 (UTC) Received: from ovpn-118-71.ams2.redhat.com (unknown [10.36.118.71]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 93B6C798A9; Fri, 12 Jan 2018 13:18:03 +0000 (UTC) Message-ID: <1515763080.4439.129.camel@redhat.com> Subject: Re: [PATCH 2/5] libstdc++ futex: Use FUTEX_CLOCK_REALTIME for wait From: Torvald Riegel To: Mike Crowe Cc: Jonathan Wakely , libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Date: Fri, 12 Jan 2018 13:21:00 -0000 In-Reply-To: <20180109175419.aamg44vdbizhdoka@mcrowe.com> References: <20180107205532.13138-1-mac@mcrowe.com> <20180107205532.13138-3-mac@mcrowe.com> <20180109135053.GB5527@redhat.com> <20180109175419.aamg44vdbizhdoka@mcrowe.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2018-01/txt/msg01047.txt.bz2 On Tue, 2018-01-09 at 17:54 +0000, Mike Crowe wrote: > On Tuesday 09 January 2018 at 13:50:54 +0000, Jonathan Wakely wrote: > > On 07/01/18 20:55 +0000, Mike Crowe wrote: > > > The futex system call supports waiting for an absolute time if > > > FUTEX_WAIT_BITSET is used rather than FUTEX_WAIT. Doing so provides two > > > benefits: > > > > > > 1. The call to gettimeofday is not required in order to calculate a > > > relative timeout. > > > > > > 2. If someone changes the system clock during the wait then the futex > > > timeout will correctly expire earlier or later. Currently that only > > > happens if the clock is changed prior to the call to gettimeofday. > > > > > > According to futex(2), support for FUTEX_CLOCK_REALTIME was added in the > > > v2.6.28 Linux kernel and FUTEX_WAIT_BITSET was added in v2.6.25. There is > > > no attempt to detect the kernel version and fall back to the previous > > > method. > > > > I don't think we can require a specific kernel version just for this. > > What is the minimum kernel version that libstdc++ requires? Since it's > already relying on NPTL it can't go back earlier than v2.6, but I suppose > that's a while before v2.6.28. I'm not aware of any choice regarding this, but Jonathan will know for sure. Generally, I think choosing a minium kernel version might be helpful, in particular if we want to optimize more often specifically for Linux environments; this may become more worthwhile in the future, for example when we look at new C++ features such as parallel algorithms, or upcoming executors. The gthreads abstraction may is a nice goal, but we can benefit a lot from knowing what the underlying platform really is. Another option might be to require a minimum glibc version on Linux, and build libstdc++ for that. That would yield a minimum kernel version as well, and we may can make use of other things in return such as syscall wrappers. > > What happens if you use those bits on an older kernel, will there be > > an ENOSYS error? Because that would allow us to try the new form, and > > fallback to the old. > > Glibc's nptl-init.c calls > > futex(.., FUTEX_WAIT_BITSET | FUTEX_CLOCK_REALTIME | FUTEX_PRIVATE_FLAG, ..) > > and sets __have_futex_clock_realtime based on whether it gets ENOSYS back > or not so it looks like it is possible to determine whether support is > available. > > The only such check I can find in libstdc++ is in filesystem/std-ops.cc > fs::do_copy_file which can try sendfile(2) on each invocation but then > automatically falls back to copying by steam. In that case, the overhead of > the potentially-failing sendfile system call is small compared to copying > the file. > > Doing such a check in _M_futex_wait_until means one system call if > FUTEX_CLOCK_REALTIME is supported, but three system calls if it is not > supported. If we found a way to cache the answer in a thread-safe and cheap > manner then this could be avoided. > > Do you think it's worth trying to cache whether FUTEX_CLOCK_REALTIME is > available? It may be worth caching that, given how simple this can be: Just add a global atomic variable whose initial value means trying the most recent syscall op, and set that to some other value that indicates an older kernel. Then check the value before attempting the syscall. Can all be relaxed atomic accesses because it's essentially just a best-effort optimization. Performance-wise, the trade-off is between an additional atomic load for new kernels vs. one additional syscall for older kernels. Given that we need the fallback for old kernels anyway unless we select a minimum version, I guess doing the caching makes sense. The syscalls are on the slow paths anyway.