From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx.kolabnow.com (mx.kolabnow.com [95.128.36.42]) by sourceware.org (Postfix) with ESMTPS id BB77C3896C2F; Thu, 22 Apr 2021 14:40:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org BB77C3896C2F Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=appliantology.com Authentication-Results: sourceware.org; spf=none smtp.mailfrom=rodgert@appliantology.com Received: from localhost (unknown [127.0.0.1]) by ext-mx-out001.mykolab.com (Postfix) with ESMTP id 00697DB4; Thu, 22 Apr 2021 16:40:14 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kolabnow.com; h= message-id:references:in-reply-to:subject:subject:from:from:date :date:content-transfer-encoding:content-type:content-type :mime-version:received:received:received; s=dkim20160331; t= 1619102414; x=1620916815; bh=PvaXyFTz9AUHzeVf6+MEerU1MUvD8+r6K2I FyXCY9DI=; b=Vi6w1P12pTPhKLCdEwWY8sKz3JpPR8ZcJ5+jf9JXWU+X+15ihv3 9qXl57H3vBPTKPoP72H6KNWtXQ7ymjgJN0adLYpLdAqP4aLM2HjRFxvChemDZ+ge VgOZAI7YM10NdY5tQZ4pXsPii5TyVGMDcHTC25wC4HY3eAxghlU8+TOJTiHkrTtZ 6Ijf3KKFB5WX3hVcg9SJPxBtFeOQRZg7kgnuCDwa2ZG519pOi8EOMU+yyNHPCfu2 NQf2tPrlmGuMQJSgxCAQZkSVuAOMIeGUwJro9oNLV4llvnT3UrUe1Ox0v/GOWtsQ Ik/DeEadAh34fSdpGDqI5lgMVfpuqmIa8lno/YgnEeK29ExQXrSvVEQ61a8JCtyO 0+9oa2Unw41GtnCtoEI2EvOZamI1uCw23jp+yfPSH/gH0J2Owr+ygEOeIVjO4ung A6O0d4w/23zNdoAR2IhjWgwyNhDHd6nM+91OHGmsjM8LSFlmKc5Vv1hddarBqP2V m2TNdkVhQ0LomacSHu4usUwuSlwlI7tlWPS8H9KK1+WKZxieNs7bwFsHr7ANnjGr mgP4kHGot1idW1cSK1W1k884sluPt/G87wf74v9+ttvhP3n3t3yyCA5TXz0hNE17 uuREeJboAHYoL7Nmep8deQETTzvyNKy0OUNEaUWj9Zz0X6991EzX0SLA= X-Virus-Scanned: amavisd-new at mykolab.com X-Spam-Score: -1.9 X-Spam-Level: X-Spam-Status: No, score=-11.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, GIT_PATCH_0, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.2 Received: from mx.kolabnow.com ([127.0.0.1]) by localhost (ext-mx-out001.mykolab.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id qhVwVv3nqmhh; Thu, 22 Apr 2021 16:40:14 +0200 (CEST) Received: from int-mx002.mykolab.com (unknown [10.9.13.2]) by ext-mx-out001.mykolab.com (Postfix) with ESMTPS id 0DD67A85; Thu, 22 Apr 2021 16:40:13 +0200 (CEST) Received: from int-subm002.mykolab.com (unknown [10.9.37.2]) by int-mx002.mykolab.com (Postfix) with ESMTPS id B401425B6; Thu, 22 Apr 2021 16:40:08 +0200 (CEST) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Thu, 22 Apr 2021 07:40:05 -0700 From: Thomas Rodgers To: Jonathan Wakely Cc: gcc-patches@gcc.gnu.org, libstdc++@gcc.gnu.org, trodgers@redhat.com, Thomas Rodgers Subject: Re: [PATCH] [libstdc++] Fix "bare" notifications dropped by waiters check In-Reply-To: <20210422092307.GB3008@redhat.com> References: <20210422012942.420359-1-rodgert@appliantology.com> <20210422092307.GB3008@redhat.com> Message-ID: <3017d98951f1ad71a4601c2ebf28aaf8@appliantology.com> X-Sender: rodgert@appliantology.com X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libstdc++@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libstdc++ mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 22 Apr 2021 14:40:20 -0000 On 2021-04-22 02:23, Jonathan Wakely wrote: > On 21/04/21 18:29 -0700, Thomas Rodgers wrote: > >> From: Thomas Rodgers >> >> NOTE - This patch also needs to be backported to gcc-11 in order for >> semaphore release() to work correctly on non-futex platforms. >> >> Tested sparc-sun-solaris2.11 >> >> For types that track whether or not there extant waiters (e.g. >> semaphore) internally, the __atomic_notify_address_bare() call was >> introduced to avoid the overhead of loading the atomic count of >> waiters. For platforms that don't have Futex, however, there was >> still a check for waiters, and seeing that there are none (because >> in the bare case, the count is not incremented), the notification >> is dropped. This commit addresses that case. >> >> libstdc++-v3/ChangeLog: >> * include/bits/atomic_wait.h: Always notify waiters in the >> in the case of 'bare' address notification. > > Repeated text: "in the in the" > >> --- >> libstdc++-v3/include/bits/atomic_wait.h | 12 ++++++------ >> 1 file changed, 6 insertions(+), 6 deletions(-) >> >> diff --git a/libstdc++-v3/include/bits/atomic_wait.h >> b/libstdc++-v3/include/bits/atomic_wait.h >> index 0ac5575190c..984ed70f16c 100644 >> --- a/libstdc++-v3/include/bits/atomic_wait.h >> +++ b/libstdc++-v3/include/bits/atomic_wait.h >> @@ -226,9 +226,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >> } >> >> void >> - _M_notify(const __platform_wait_t* __addr, bool __all) noexcept >> + _M_notify(const __platform_wait_t* __addr, bool __all, bool >> __bare) noexcept >> { >> - if (!_M_waiting()) >> + if (!(__bare || _M_waiting())) > > Maybe it's just me, but I would find (!__base && !__waiting) to be a > clearer expression of the logic here. > > i.e. "return if the wait is not bare and there are no waiters" > rather than "return if the wait is not either bare or has waiters". > The latter makes me take a second to grok. > As discussed on IRC, I went back and forth on this a couple of times and neither option seemed particularly great to me. I started down the path of propagating the std::true_type/std::false_type bits that the RAII wrapper type knows about and making this a compile time choice, but felt the change was too big to make this late in the release cycle. I will revisit this for stage1 (though ideally all of this will be moved into the .so for GCC12 and partially rewritten as part of that process anyway). > The patch is OK either way though, with the ChangeLog typo fix. > >> return; >> >> #ifdef _GLIBCXX_HAVE_PLATFORM_WAIT >> @@ -304,11 +304,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >> } >> >> void >> - _M_notify(bool __all) >> + _M_notify(bool __all, bool __bare = false) >> { >> if (_M_addr == &_M_w._M_ver) >> __atomic_fetch_add(_M_addr, 1, __ATOMIC_ACQ_REL); >> - _M_w._M_notify(_M_addr, __all); >> + _M_w._M_notify(_M_addr, __all, __bare); >> } >> >> template> @@ -452,7 +452,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >> __atomic_notify_address(const _Tp* __addr, bool __all) noexcept >> { >> __detail::__bare_wait __w(__addr); >> - __w._M_notify(__all); >> + __w._M_notify(__all, true); >> } >> >> // This call is to be used by atomic types which track contention >> externally >> @@ -464,7 +464,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION >> __detail::__platform_notify(__addr, __all); >> #else >> __detail::__bare_wait __w(__addr); >> - __w._M_notify(__all); >> + __w._M_notify(__all, true); >> #endif >> } >> _GLIBCXX_END_NAMESPACE_VERSION >> -- 2.30.2 Tested sparc-sun-solaris2.11. Committed to master, backported to release/gcc-11.