[PATCH 2/3] nptl: Move cancel state out of cancelhandling

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

The thread cancellation state is not accessed concurrently internally
neither the pthread interface allows changing the state of a different
thread than its own.

The code is also simplified: the CANCELLATION_P is replaced with a
internal pthread_testcancel call and the CANCELSTATE_BIT{MASK} is
removed.

Checked on x86_64-linux-gnu.
---
 nptl/allocatestack.c          |  1 +
 nptl/cancellation.c           |  3 ++-
 nptl/cleanup_defer.c          |  2 +-
 nptl/cleanup_defer_compat.c   |  2 +-
 nptl/descr.h                  | 13 +++++--------
 nptl/pthreadP.h               | 12 ------------
 nptl/pthread_cancel.c         |  3 ++-
 nptl/pthread_join_common.c    |  5 ++++-
 nptl/pthread_setcancelstate.c | 36 +++--------------------------------
 nptl/pthread_setcanceltype.c  |  3 ++-
 nptl/pthread_testcancel.c     | 11 ++++++++++-
 11 files changed, 31 insertions(+), 60 deletions(-)

diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c
index c94980c21c..d9174274c2 100644
--- a/nptl/allocatestack.c
+++ b/nptl/allocatestack.c
@@ -232,6 +232,7 @@ get_cached_stack (size_t *sizep, void **memp)
 
   /* Cancellation handling is back to the default.  */
   result->cancelhandling = 0;
+  result->cancelstate = PTHREAD_CANCEL_ENABLE;
   result->cleanup = NULL;
 
   /* No pending event.  */
diff --git a/nptl/cancellation.c b/nptl/cancellation.c
index 826071321e..7e8cbe9fe1 100644
--- a/nptl/cancellation.c
+++ b/nptl/cancellation.c
@@ -45,7 +45,8 @@ __pthread_enable_asynccancel (void)
 					      oldval);
       if (__glibc_likely (curval == oldval))
 	{
-	  if (CANCEL_ENABLED_AND_CANCELED_AND_ASYNCHRONOUS (newval))
+	  if (self->cancelstate == PTHREAD_CANCEL_ENABLE
+	      && CANCEL_CANCELED_AND_ASYNCHRONOUS (newval))
 	    {
 	      THREAD_SETMEM (self, result, PTHREAD_CANCELED);
 	      __do_cancel ();
diff --git a/nptl/cleanup_defer.c b/nptl/cleanup_defer.c
index 8ad9a90c50..33d4ea6eef 100644
--- a/nptl/cleanup_defer.c
+++ b/nptl/cleanup_defer.c
@@ -86,6 +86,6 @@ __pthread_unregister_cancel_restore (__pthread_unwind_buf_t *buf)
 	  cancelhandling = curval;
 	}
 
-      CANCELLATION_P (self);
+      __pthread_testcancel ();
     }
 }
diff --git a/nptl/cleanup_defer_compat.c b/nptl/cleanup_defer_compat.c
index 33e47888f2..a1ad291fcc 100644
--- a/nptl/cleanup_defer_compat.c
+++ b/nptl/cleanup_defer_compat.c
@@ -83,7 +83,7 @@ _pthread_cleanup_pop_restore (struct _pthread_cleanup_buffer *buffer,
 	  cancelhandling = curval;
 	}
 
-      CANCELLATION_P (self);
+      __pthread_testcancel ();
     }
 
   /* If necessary call the cleanup routine after we removed the
diff --git a/nptl/descr.h b/nptl/descr.h
index 9dcf480bdf..61665bf859 100644
--- a/nptl/descr.h
+++ b/nptl/descr.h
@@ -269,9 +269,6 @@ struct pthread
 
   /* Flags determining processing of cancellation.  */
   int cancelhandling;
-  /* Bit set if cancellation is disabled.  */
-#define CANCELSTATE_BIT		0
-#define CANCELSTATE_BITMASK	(0x01 << CANCELSTATE_BIT)
   /* Bit set if asynchronous cancellation mode is selected.  */
 #define CANCELTYPE_BIT		1
 #define CANCELTYPE_BITMASK	(0x01 << CANCELTYPE_BIT)
@@ -293,11 +290,8 @@ struct pthread
   /* Mask for the rest.  Helps the compiler to optimize.  */
 #define CANCEL_RESTMASK		0xffffff80
 
-#define CANCEL_ENABLED_AND_CANCELED(value) \
-  (((value) & (CANCELSTATE_BITMASK | CANCELED_BITMASK | EXITING_BITMASK	      \
-	       | CANCEL_RESTMASK | TERMINATED_BITMASK)) == CANCELED_BITMASK)
-#define CANCEL_ENABLED_AND_CANCELED_AND_ASYNCHRONOUS(value) \
-  (((value) & (CANCELSTATE_BITMASK | CANCELTYPE_BITMASK | CANCELED_BITMASK    \
+#define CANCEL_CANCELED_AND_ASYNCHRONOUS(value) \
+  (((value) & (CANCELTYPE_BITMASK | CANCELED_BITMASK    \
 	       | EXITING_BITMASK | CANCEL_RESTMASK | TERMINATED_BITMASK))     \
    == (CANCELTYPE_BITMASK | CANCELED_BITMASK))
 
@@ -394,6 +388,9 @@ struct pthread
   /* Indicates whether is a C11 thread created by thrd_creat.  */
   bool c11;
 
+  /* Thread cancel state (enable, disable).  */
+  unsigned char cancelstate;
+
   /* This member must be last.  */
   char end_padding[];
 
diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
index 06fb0d74c5..d55c3b26a4 100644
--- a/nptl/pthreadP.h
+++ b/nptl/pthreadP.h
@@ -258,18 +258,6 @@ extern int __pthread_debug attribute_hidden;
 #endif
 
 
-/* Cancellation test.  */
-#define CANCELLATION_P(self) \
-  do {									      \
-    int cancelhandling = THREAD_GETMEM (self, cancelhandling);		      \
-    if (CANCEL_ENABLED_AND_CANCELED (cancelhandling))			      \
-      {									      \
-	THREAD_SETMEM (self, result, PTHREAD_CANCELED);			      \
-	__do_cancel ();							      \
-      }									      \
-  } while (0)
-
-
 extern void __pthread_unwind (__pthread_unwind_buf_t *__buf)
      __cleanup_fct_attribute __attribute ((__noreturn__))
 #if !defined SHARED && !IS_IN (libpthread)
diff --git a/nptl/pthread_cancel.c b/nptl/pthread_cancel.c
index 88c1ab8f6a..5b2789d620 100644
--- a/nptl/pthread_cancel.c
+++ b/nptl/pthread_cancel.c
@@ -55,7 +55,8 @@ __pthread_cancel (pthread_t th)
       /* If the cancellation is handled asynchronously just send a
 	 signal.  We avoid this if possible since it's more
 	 expensive.  */
-      if (CANCEL_ENABLED_AND_CANCELED_AND_ASYNCHRONOUS (newval))
+      if (pd->cancelstate == PTHREAD_CANCEL_ENABLE
+	  && CANCEL_CANCELED_AND_ASYNCHRONOUS (newval))
 	{
 	  /* Mark the cancellation as "in progress".  */
 	  if (atomic_compare_and_exchange_bool_acq (&pd->cancelhandling,
diff --git a/nptl/pthread_join_common.c b/nptl/pthread_join_common.c
index a96ceafde4..03e202136f 100644
--- a/nptl/pthread_join_common.c
+++ b/nptl/pthread_join_common.c
@@ -105,7 +105,10 @@ __pthread_clockjoin_ex (pthread_t threadid, void **thread_return,
 	   && (pd->cancelhandling
 	       & (CANCELING_BITMASK | CANCELED_BITMASK | EXITING_BITMASK
 		  | TERMINATED_BITMASK)) == 0))
-      && !CANCEL_ENABLED_AND_CANCELED (self->cancelhandling))
+      && !(self->cancelstate == PTHREAD_CANCEL_ENABLE
+	   && (pd->cancelhandling & (CANCELED_BITMASK | EXITING_BITMASK
+				     | TERMINATED_BITMASK))
+	       == CANCELED_BITMASK))
     /* This is a deadlock situation.  The threads are waiting for each
        other to finish.  Note that this is a "may" error.  To be 100%
        sure we catch this error we would have to lock the data
diff --git a/nptl/pthread_setcancelstate.c b/nptl/pthread_setcancelstate.c
index 4d7f413e19..aa1c8073a8 100644
--- a/nptl/pthread_setcancelstate.c
+++ b/nptl/pthread_setcancelstate.c
@@ -31,39 +31,9 @@ __pthread_setcancelstate (int state, int *oldstate)
 
   self = THREAD_SELF;
 
-  int oldval = THREAD_GETMEM (self, cancelhandling);
-  while (1)
-    {
-      int newval = (state == PTHREAD_CANCEL_DISABLE
-		    ? oldval | CANCELSTATE_BITMASK
-		    : oldval & ~CANCELSTATE_BITMASK);
-
-      /* Store the old value.  */
-      if (oldstate != NULL)
-	*oldstate = ((oldval & CANCELSTATE_BITMASK)
-		     ? PTHREAD_CANCEL_DISABLE : PTHREAD_CANCEL_ENABLE);
-
-      /* Avoid doing unnecessary work.  The atomic operation can
-	 potentially be expensive if the memory has to be locked and
-	 remote cache lines have to be invalidated.  */
-      if (oldval == newval)
-	break;
-
-      /* Update the cancel handling word.  This has to be done
-	 atomically since other bits could be modified as well.  */
-      int curval = THREAD_ATOMIC_CMPXCHG_VAL (self, cancelhandling, newval,
-					      oldval);
-      if (__glibc_likely (curval == oldval))
-	{
-	  if (CANCEL_ENABLED_AND_CANCELED_AND_ASYNCHRONOUS (newval))
-	    __do_cancel ();
-
-	  break;
-	}
-
-      /* Prepare for the next round.  */
-      oldval = curval;
-    }
+  if (oldstate != NULL)
+    *oldstate = self->cancelstate;
+  self->cancelstate = state;
 
   return 0;
 }
diff --git a/nptl/pthread_setcanceltype.c b/nptl/pthread_setcanceltype.c
index fcaae8abc7..cc0507ae04 100644
--- a/nptl/pthread_setcanceltype.c
+++ b/nptl/pthread_setcanceltype.c
@@ -53,7 +53,8 @@ __pthread_setcanceltype (int type, int *oldtype)
 					      oldval);
       if (__glibc_likely (curval == oldval))
 	{
-	  if (CANCEL_ENABLED_AND_CANCELED_AND_ASYNCHRONOUS (newval))
+	  if (self->cancelstate == PTHREAD_CANCEL_ENABLE
+	      && CANCEL_CANCELED_AND_ASYNCHRONOUS (newval))
 	    {
 	      THREAD_SETMEM (self, result, PTHREAD_CANCELED);
 	      __do_cancel ();
diff --git a/nptl/pthread_testcancel.c b/nptl/pthread_testcancel.c
index 30408c2008..3ffff4ebef 100644
--- a/nptl/pthread_testcancel.c
+++ b/nptl/pthread_testcancel.c
@@ -23,7 +23,16 @@
 void
 __pthread_testcancel (void)
 {
-  CANCELLATION_P (THREAD_SELF);
+  struct pthread *self = THREAD_SELF;
+  int cancelhandling = THREAD_GETMEM (self, cancelhandling);
+  if (self->cancelstate == PTHREAD_CANCEL_ENABLE
+      && (cancelhandling & (CANCELED_BITMASK | EXITING_BITMASK
+			    | TERMINATED_BITMASK))
+	  == CANCELED_BITMASK)
+    {
+      THREAD_SETMEM (self, result, PTHREAD_CANCELED);
+      __do_cancel ();
+    }
 }
 strong_alias (__pthread_testcancel, pthread_testcancel)
 hidden_def (__pthread_testcancel)
-- 
2.17.1