This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] nptl: Fix join/tryjoin comments.


Florian,

I noticed these comment mistakes while reviewing your patch for BZ #24211.

OK for master?

~~~
This commit fixes several inaccurate comments in the implementation of
__pthread_timedjoin_ex(), pthread_tryjoin_np, and lll_wait_tid, all of
which are used to implement POSIX thread joining.

Firstly, in pthread_tryjoin_np() we only attempt the join if a read of
pd->tid == 0, because that means the thread has already been reaped by
the kernel and we can safely join it without blocking.

Secondly, all join implementations call the common
__pthread_timedjoin_ex and only if abstime is NULL (block) do we
actually need to use cancellation (to cancel the potentially infinite
wait).

Lastly, in lll_wait_tid we only call the canellation futex operations if
abstime is non-NULL.

With the comments corrected the implementation notes are consistent and
logical.

Signed-off-by: Carlos O'Donell <carlos@redhat.com>
---
 ChangeLog                   | 7 +++++++
 nptl/lll_timedwait_tid.c    | 3 +--
 nptl/pthread_join_common.c  | 4 ++--
 nptl/pthread_tryjoin.c      | 2 +-
 sysdeps/nptl/lowlevellock.h | 2 +-
 5 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 6f1d967f62..37e7babf75 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2019-02-11  Carlos O'Donell  <carlos@redhat.com>
+
+	* nptl/lll_timedwait_tid.c: Use GNU-style comment.
+	* nptl/pthread_join_common.c: Fix comment.
+	* nptl/pthread_tryjoin.c: Likewise.
+	* sysdeps/nptl/lowlevellock.h: Likewise.
+
 2019-02-11  Paul A. Clarke  <pc@us.ibm.com>
 
 	* sysdeps/powerpc/fpu/e_sqrt.c (__slow_ieee754_sqrtf):
diff --git a/nptl/lll_timedwait_tid.c b/nptl/lll_timedwait_tid.c
index 59ecbb4446..8f22da9b05 100644
--- a/nptl/lll_timedwait_tid.c
+++ b/nptl/lll_timedwait_tid.c
@@ -60,8 +60,7 @@ __lll_timedwait_tid (int *tidp, const struct timespec *abstime)
 
       /* If *tidp == tid, wait until thread terminates or the wait times out.
          The kernel up to version 3.16.3 does not use the private futex
-         operations for futex wake-up when the clone terminates.
-      */
+         operations for futex wake-up when the clone terminates.  */
       if (lll_futex_timed_wait (tidp, tid, &rt, LLL_SHARED) == -ETIMEDOUT)
         return ETIMEDOUT;
     }
diff --git a/nptl/pthread_join_common.c b/nptl/pthread_join_common.c
index ecb78ffba5..e99854ac39 100644
--- a/nptl/pthread_join_common.c
+++ b/nptl/pthread_join_common.c
@@ -76,8 +76,8 @@ __pthread_timedjoin_ex (pthread_t threadid, void **thread_return,
 
   if (block)
     {
-      /* During the wait we change to asynchronous cancellation.  If we
-	 are cancelled the thread we are waiting for must be marked as
+      /* If abstime is NULL we switch to asynchronous cancellation.  If we
+	 are cancelled then the thread we are waiting for must be marked as
 	 un-wait-ed for again.  */
       pthread_cleanup_push (cleanup, &pd->joinid);
 
diff --git a/nptl/pthread_tryjoin.c b/nptl/pthread_tryjoin.c
index aa4fe07af5..7dbf22868d 100644
--- a/nptl/pthread_tryjoin.c
+++ b/nptl/pthread_tryjoin.c
@@ -26,7 +26,7 @@ pthread_tryjoin_np (pthread_t threadid, void **thread_return)
   if (pd->tid != 0)
     return EBUSY;
 
-  /* If pd->tid != 0 then lll_wait_tid will not block on futex
+  /* If pd->tid == 0 then lll_wait_tid will not block on futex
      operation.  */
   return __pthread_timedjoin_ex (threadid, thread_return, NULL, false);
 }
diff --git a/sysdeps/nptl/lowlevellock.h b/sysdeps/nptl/lowlevellock.h
index 36bbc472e7..2581ed5ca9 100644
--- a/sysdeps/nptl/lowlevellock.h
+++ b/sysdeps/nptl/lowlevellock.h
@@ -185,7 +185,7 @@ extern int __lll_timedwait_tid (int *, const struct timespec *)
    operations for futex wake-up when the clone terminates.
    If ABSTIME is not NULL, is used a timeout for futex call.  If the timeout
    occurs then return ETIMEOUT, if ABSTIME is invalid, return EINVAL.
-   The futex operation are issues with cancellable versions.  */
+   If abstime is NULL we use a cancellable futex wait operation.  */
 #define lll_wait_tid(tid, abstime)					\
   ({									\
     int __res = 0;							\
-- 
2.20.1



-- 
Cheers,
Carlos.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]