From patchwork Fri Feb 9 14:18:58 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Matteo Italia X-Patchwork-Id: 1897080 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=server2.sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=patchwork.ozlabs.org) Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4TWbbN69wNz23h4 for ; Sat, 10 Feb 2024 01:19:59 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 0D6403858416 for ; Fri, 9 Feb 2024 14:19:57 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from smtp3-g21.free.fr (smtp3-g21.free.fr [212.27.42.3]) by sourceware.org (Postfix) with ESMTPS id 3AB9D3858C35 for ; Fri, 9 Feb 2024 14:19:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 3AB9D3858C35 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=mitalia.net Authentication-Results: sourceware.org; spf=fail smtp.mailfrom=mitalia.net ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 3AB9D3858C35 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=212.27.42.3 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1707488381; cv=none; b=CuBU3rOI4jz5TVoxHp39BIL6d8cSJIUcRdXBFZtPEVHzRAHUHNlkeiMCzWzCC7KpQ2rwQwafc3gfTOQztNP5xgYAnGz7tnEsxi1PDxip9T56P7zvSF+cXbmypAunFNBxYA3pFmtqn/+vQ5Ywm0/hfqmpWLRl8PWf3NBzaRTfPY4= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1707488381; c=relaxed/simple; bh=/ONCr/xcVmlnP9aBplunEG547wRcxLIk8I1hot60OG0=; h=From:To:Subject:Date:Message-Id:MIME-Version; b=jAf2qNk4NHdIv9v7EBYwrgMTejRbvCLMcIiHNzkHbfhGVGV4uAwIvuCTRg+VGuDgalsdmUNUfpaOr2wyTogTrTGQlgPwes4Q9McxRzLjLLn2jWDaoWoPpbG8ORpFfjqzqtElxktA1u7tt/LfTfGL5vvbuOHslUcjGOGizYe1MJA= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from teokubuntu (unknown [IPv6:2a01:e11:100c:8530:f1a6:ffc:eb58:b8a6]) by smtp3-g21.free.fr (Postfix) with ESMTPS id 6574C13F89A; Fri, 9 Feb 2024 15:19:34 +0100 (CET) Received: from localhost ([127.0.0.1] helo=localhost.localdomain) by teokubuntu with esmtp (Exim 4.95) (envelope-from ) id 1rYRj2-000JaI-TV; Fri, 09 Feb 2024 15:19:33 +0100 From: Matteo Italia To: gcc-patches@gcc.gnu.org Cc: Matteo Italia , 10walls@gmail.com, ebotcazou@adacore.com Subject: [PATCH] libgcc: fix Win32 CV abnormal spurious wakeups in timed wait [PR113850] Date: Fri, 9 Feb 2024 15:18:58 +0100 Message-Id: <20240209141858.75189-1-matteo@mitalia.net> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 X-Spam-Status: No, score=-13.2 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_SOFTFAIL, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org The Win32 threading model uses __gthr_win32_abs_to_rel_time to convert the timespec used in gthreads to specify the absolute time for end of the condition variables timed wait to a milliseconds value relative to "now" to pass to the Win32 SleepConditionVariableCS function. Unfortunately, the conversion is incorrect, as, due to a typo, it returns the relative time _in seconds_, so SleepConditionVariableCS receives a timeout value 1000 times shorter than it should be, resulting in a huge amount of spurious wakeups in calls such as std::condition_variable::wait_for or wait_until. This can be demonstrated easily by this sample program: ``` int main() { std::condition_variable cv; std::mutex mx; bool pass = false; auto thread_fn = [&](bool timed) { int wakeups = 0; using sc = std::chrono::system_clock; auto before = sc::now(); std::unique_lock ml(mx); if (timed) { cv.wait_for(ml, std::chrono::seconds(2), [&]{ ++wakeups; return pass; }); } else { cv.wait(ml, [&]{ ++wakeups; return pass; }); } printf("pass: %d; wakeups: %d; elapsed: %d ms\n", pass, wakeups, int((sc::now() - before) / std::chrono::milliseconds(1))); pass = false; }; { // timed wait, let expire std::thread t(thread_fn, true); t.join(); } { // timed wait, wake up explicitly after 1 second std::thread t(thread_fn, true); std::this_thread::sleep_for(std::chrono::seconds(1)); { std::unique_lock ml(mx); pass = true; } cv.notify_all(); t.join(); } { // non-timed wait, wake up explicitly after 1 second std::thread t(thread_fn, false); std::this_thread::sleep_for(std::chrono::seconds(1)); { std::unique_lock ml(mx); pass = true; } cv.notify_all(); t.join(); } return 0; } ``` On builds that other threading models (e.g. POSIX on Linux, or winpthreads or MCF on Win32) the output is something like ``` pass: 0; wakeups: 2; elapsed: 2000 ms pass: 1; wakeups: 2; elapsed: 991 ms pass: 1; wakeups: 2; elapsed: 996 ms ``` while on the affected builds it's more like ``` pass: 0; wakeups: 1418; elapsed: 2000 ms pass: 1; wakeups: 479; elapsed: 988 ms pass: 1; wakeups: 2; elapsed: 992 ms ``` (notice the huge number of wakeups). This commit fixes the conversion, adjusting the final division by NSEC100_PER_SEC to use NSEC100_PER_MSEC instead (already defined in the file and not used in any other place, so the problem here was probably a typo or some rebase/refactoring mishap). libgcc/ChangeLog: * config/i386/gthr-win32-cond.c (__gthr_win32_abs_to_rel_time): fix absolute timespec to relative milliseconds count conversion (it incorrectly returned seconds instead of milliseconds); this avoids spurious wakeups in __gthr_win32_cond_timedwait --- libgcc/config/i386/gthr-win32-cond.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libgcc/config/i386/gthr-win32-cond.c b/libgcc/config/i386/gthr-win32-cond.c index ecb007a54b2..650c448f286 100644 --- a/libgcc/config/i386/gthr-win32-cond.c +++ b/libgcc/config/i386/gthr-win32-cond.c @@ -78,7 +78,7 @@ __gthr_win32_abs_to_rel_time (const __gthread_time_t *abs_time) if (abs_time_nsec100 < now.nsec100) return 0; - return (DWORD) CEIL_DIV (abs_time_nsec100 - now.nsec100, NSEC100_PER_SEC); + return (DWORD) CEIL_DIV (abs_time_nsec100 - now.nsec100, NSEC100_PER_MSEC); } /* Check the sizes of the local version of the Win32 types. */