Re: [PATCH] pthread_once: Clean up constants.

[Torvald Riegel <triegel at redhat dot com>]

On Fri, 2013-10-11 at 23:27 +0300, Torvald Riegel wrote:
> This is part c) of
> https://sourceware.org/bugzilla/show_bug.cgi?id=15215: Replace magic
> numbers with name constants.  Also updates the documentation.
> Applies on top of the pthread_once unification patch I sent recently.
> 
> OK?

This slipped through.  This is the last part of BZ 15215 (together witht
the removal of the x86 custom variants that slipped through as well).
Attached is an updated patch, which I'll commit after two days if nobody
objects.

2014-10-20  Torvald Riegel  <triegel@redhat.com>

	[BZ #15215]
	* nptl/pthreadP.h (__PTHREAD_ONCE_INPROGRESS, __PTHREAD_ONCE_DONE,
	__PTHREAD_ONCE_FORK_GEN_INCR): New.
	* sysdeps/nptl/fork.c (__libc_fork): Use them.
	* nptl/pthread_once.c (__pthread_once): Likewise.
	Update comments.

commit 58ce36df021f8d55fc60b7879c067f5ff9b34866
Author: Torvald Riegel <triegel@redhat.com>
Date:   Fri Oct 11 18:58:04 2013 +0300

    pthread_once: Clean up constants.
    
    [BZ #15215] This just gives a name to the integer constants being used.

diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
index d4415ba..3aa24c2 100644
--- a/nptl/pthreadP.h
+++ b/nptl/pthreadP.h
@@ -161,6 +161,12 @@ enum
 #define FUTEX_TID_MASK		0x3fffffff
 
 
+/* pthread_once definitions.  See __pthread_once for how these are used.  */
+#define __PTHREAD_ONCE_INPROGRESS	1
+#define __PTHREAD_ONCE_DONE		2
+#define __PTHREAD_ONCE_FORK_GEN_INCR	4
+
+
 /* Internal variables.  */
 
 
diff --git a/nptl/pthread_once.c b/nptl/pthread_once.c
index 10c01d6..595bd7e 100644
--- a/nptl/pthread_once.c
+++ b/nptl/pthread_once.c
@@ -40,8 +40,11 @@ clear_once_control (void *arg)
 
 
 /* This is similar to a lock implementation, but we distinguish between three
-   states: not yet initialized (0), initialization finished (2), and
-   initialization in progress (__fork_generation | 1).  If in the first state,
+   states: not yet initialized (0), initialization in progress
+   (__fork_generation | __PTHREAD_ONCE_INPROGRESS), and initialization
+   finished (__PTHREAD_ONCE_DONE); __fork_generation does not use the bits
+   that are used for __PTHREAD_ONCE_INPROGRESS and __PTHREAD_ONCE_DONE (which
+   is what __PTHREAD_ONCE_FORK_GEN_INCR is used for).  If in the first state,
    threads will try to run the initialization by moving to the second state;
    the first thread to do so via a CAS on once_control runs init_routine,
    other threads block.
@@ -66,14 +69,14 @@ __pthread_once (once_control, init_routine)
       int oldval, val, newval;
 
       /* We need acquire memory order for this load because if the value
-         signals that initialization has finished, we need to be see any
+         signals that initialization has finished, we need to see any
          data modifications done during initialization.  */
       val = *once_control;
       atomic_read_barrier();
       do
 	{
 	  /* Check if the initialization has already been done.  */
-	  if (__glibc_likely ((val & 2) != 0))
+	  if (__glibc_likely ((val & __PTHREAD_ONCE_DONE) != 0))
 	    return 0;
 
 	  oldval = val;
@@ -82,7 +85,7 @@ __pthread_once (once_control, init_routine)
 	     generation because it's immutable in a particular process, and
 	     forked child processes start with a single thread that modified
 	     the generation.  */
-	  newval = __fork_generation | 1;
+	  newval = __fork_generation | __PTHREAD_ONCE_INPROGRESS;
 	  /* We need acquire memory order here for the same reason as for the
 	     load from once_control above.  */
 	  val = atomic_compare_and_exchange_val_acq (once_control, newval,
@@ -91,11 +94,11 @@ __pthread_once (once_control, init_routine)
       while (__glibc_unlikely (val != oldval));
 
       /* Check if another thread already runs the initializer.	*/
-      if ((oldval & 1) != 0)
+      if ((oldval & __PTHREAD_ONCE_INPROGRESS) != 0)
 	{
 	  /* Check whether the initializer execution was interrupted by a
-	     fork.  We know that for both values, bit 0 is set and bit 1 is
-	     not.  */
+	     fork.  We know that for both values, __PTHREAD_ONCE_INPROGRESS
+	     is set and __PTHREAD_ONCE_DONE is not.  */
 	  if (oldval == newval)
 	    {
 	      /* Same generation, some other thread was faster. Wait.  */
@@ -118,7 +121,7 @@ __pthread_once (once_control, init_routine)
          release memory order here because we need to synchronize with other
          threads that want to use the initialized data.  */
       atomic_write_barrier();
-      *once_control = 2;
+      *once_control = __PTHREAD_ONCE_DONE;
 
       /* Wake up all other threads.  */
       lll_futex_wake (once_control, INT_MAX, LLL_PRIVATE);
diff --git a/sysdeps/nptl/fork.c b/sysdeps/nptl/fork.c
index 511533a..a7dafa8 100644
--- a/sysdeps/nptl/fork.c
+++ b/sysdeps/nptl/fork.c
@@ -141,8 +141,9 @@ __libc_fork (void)
 
       assert (THREAD_GETMEM (self, tid) != ppid);
 
+      /* See __pthread_once.  */
       if (__fork_generation_pointer != NULL)
-	*__fork_generation_pointer += 4;
+	*__fork_generation_pointer += __PTHREAD_ONCE_FORK_GEN_INCR;
 
       /* Adjust the PID field for the new process.  */
       THREAD_SETMEM (self, pid, THREAD_GETMEM (self, tid));