From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id 256E93894428 for ; Thu, 22 Apr 2021 09:23:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 256E93894428 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-368-XnkV9Mj_MoyCHx40yFah4g-1; Thu, 22 Apr 2021 05:23:09 -0400 X-MC-Unique: XnkV9Mj_MoyCHx40yFah4g-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id E45C418397A3; Thu, 22 Apr 2021 09:23:08 +0000 (UTC) Received: from localhost (unknown [10.33.36.164]) by smtp.corp.redhat.com (Postfix) with ESMTP id 662A860BE5; Thu, 22 Apr 2021 09:23:08 +0000 (UTC) Date: Thu, 22 Apr 2021 10:23:07 +0100 From: Jonathan Wakely To: Thomas Rodgers 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 Message-ID: <20210422092307.GB3008@redhat.com> References: <20210422012942.420359-1-rodgert@appliantology.com> MIME-Version: 1.0 In-Reply-To: <20210422012942.420359-1-rodgert@appliantology.com> X-Clacks-Overhead: GNU Terry Pratchett X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline X-Spam-Status: No, score=-14.0 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 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 09:23:13 -0000 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. 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 >