diff mbox

[2/2] pthread_once: Use futex wrappers with error checking.

Message ID 1417726635.22797.50.camel@triegel.csb
State Superseded
Headers

Commit Message

Torvald Riegel Dec. 4, 2014, 8:57 p.m. UTC 
  This uses the new glibc-internal futex wrappers from Patch 1/2 in
pthread_once.

Tested on x86 and x86_64.


2014-12-05  Torvald Riegel  <triegel@redhat.com>

	* nptl/pthread_once.c (clear_once_control): Use futex wrappers with
	error checking.
	(__pthread_once_slow): Likewise.

Comments

Roland McGrath Dec. 5, 2014, 12:36 a.m. UTC | #1 
> -	      /* 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.


Thanks,
Roland
diff mbox

Patch

commit d9cf9ffe8b1df575a3d9c9a8fd08e5b7463a1c38
Author: Torvald Riegel <triegel@redhat.com>
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..61ccd13 100644
--- a/nptl/pthread_once.c
+++ b/nptl/pthread_once.c
@@ -17,7 +17,7 @@ 
    <http://www.gnu.org/licenses/>.  */
 
 #include "pthreadP.h"
-#include <lowlevellock.h>
+#include <futex-internal.h>
 #include <atomic.h>
 
 
@@ -35,7 +35,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 (once_control, INT_MAX, FUTEX_PRIVATE);
 }
 
 
@@ -100,8 +100,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, -EWOULDBLOCK, -EINTR) need to be handled the
+		 same.  */
+	      futex_wait (once_control, newval, FUTEX_PRIVATE);
 	      continue;
 	    }
 	}
@@ -122,7 +125,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 (once_control, INT_MAX, FUTEX_PRIVATE);
       break;
     }