From patchwork Fri Dec 5 18:37:03 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Torvald Riegel X-Patchwork-Id: 4090 Received: (qmail 9501 invoked by alias); 5 Dec 2014 18:37:15 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 9489 invoked by uid 89); 5 Dec 2014 18:37:14 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.0 required=5.0 tests=AWL, BAYES_00, SPF_HELO_PASS, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Subject: Re: [PATCH 2/2] pthread_once: Use futex wrappers with error checking. From: Torvald Riegel To: Roland McGrath Cc: GLIBC Devel In-Reply-To: <20141205003647.854CA2C3A73@topped-with-meat.com> References: <1417726487.22797.48.camel@triegel.csb> <1417726635.22797.50.camel@triegel.csb> <20141205003647.854CA2C3A73@topped-with-meat.com> Date: Fri, 05 Dec 2014 19:37:03 +0100 Message-ID: <1417804623.22797.106.camel@triegel.csb> Mime-Version: 1.0 On Thu, 2014-12-04 at 16:36 -0800, Roland McGrath wrote: > > - /* Same generation, some other thread was faster. Wait. */ > > - lll_futex_wait (once_control, newval, LLL_PRIVATE); > > + /* Same generation, some other thread was faster. Wait and > > + retry. Ignore the return value because all possible > > + values (0, -EWOULDBLOCK, -EINTR) need to be handled the > > + same. */ > > Two spaces between sentences, even when it was wrong before. > Always mention EAGAIN rather than EWOULDBLOCK. > > Use ignore_value rather than only a comment, to be more explicit about a > correct case of ignoring errors. Then we can use warn_unused_result on all > the new inlines, and turn up cases where failing to check for errors is > dubious (we have many today). > > Aside from those nits, this change is fine and good once we've settled on > the new internal API details. Updated patch attached. If there's no objection, and once we have agreed on a Patch 1/2 that can be committed, I will commit this one as well. commit 5858b0ec69ff188310c049ec27b102303c38b7b3 Author: Torvald Riegel Date: Thu Dec 4 14:29:27 2014 +0100 pthread_once: Use futex wrappers with error checking. diff --git a/nptl/pthread_once.c b/nptl/pthread_once.c index 9bb82e9..b5d3963 100644 --- a/nptl/pthread_once.c +++ b/nptl/pthread_once.c @@ -17,8 +17,9 @@ . */ #include "pthreadP.h" -#include +#include #include +#include unsigned long int __fork_generation attribute_hidden; @@ -35,7 +36,7 @@ clear_once_control (void *arg) get interrupted (see __pthread_once), so all we need to relay to other threads is the state being reset again. */ atomic_store_relaxed (once_control, 0); - lll_futex_wake (once_control, INT_MAX, LLL_PRIVATE); + futex_wake ((unsigned int *)once_control, INT_MAX, FUTEX_PRIVATE); } @@ -100,8 +101,11 @@ __pthread_once_slow (pthread_once_t *once_control, void (*init_routine) (void)) is set and __PTHREAD_ONCE_DONE is not. */ if (val == newval) { - /* Same generation, some other thread was faster. Wait. */ - lll_futex_wait (once_control, newval, LLL_PRIVATE); + /* Same generation, some other thread was faster. Wait and + retry. Ignore the return value because all possible + values (0, EAGAIN, EINTR) need to be handled the same. */ + ignore_value (futex_wait ((unsigned int *)once_control, + (unsigned)newval, FUTEX_PRIVATE)); continue; } } @@ -122,7 +126,7 @@ __pthread_once_slow (pthread_once_t *once_control, void (*init_routine) (void)) atomic_store_release (once_control, __PTHREAD_ONCE_DONE); /* Wake up all other threads. */ - lll_futex_wake (once_control, INT_MAX, LLL_PRIVATE); + futex_wake ((unsigned int *)once_control, INT_MAX, FUTEX_PRIVATE); break; }