Re: [PATCH v2 0/4] nptl: Add pthread_clockjoin_np

[Mike Crowe <mac at mcrowe dot com>]

On Thursday 31 October 2019 at 11:31:30 -0300, Adhemerval Zanella wrote:
> On 18/09/2019 09:30, Mike Crowe wrote:
> > The aim of this series is to add pthread_clockjoin_np, which works
> > like pthread_timedjoin_np except it accepts an additional parameter to
> > indicate which clock the timeout should be measured against.
> > 
> > The naming matches the pthread_cond_clockwait,
> > pthread_mutex_clocklock, sem_clockwait, etc. functions added in glibc
> > v2.30 and documented at http://austingroupbugs.net/view.php?id=1216.
> > 
> > The series also includes some minimal documentation for
> > pthread_tryjoin_np and pthread_timedjoin_np, along with some new
> > tests.
> > 
> > Thanks to everyone who provided feedback for the first version[1] of
> > this series.
> > 
> > [1] https://sourceware.org/ml/libc-alpha/2019-06/msg00911.html
> 
> Hi Mike,
> 
> I think your patchset it ready to push upstream.  I have fixed the small
> nits I brought on the review along with some more specific changes below:
> 
>   - Move the NEWS entry for pthread_clockjoin_np to its own bullet on
>     2.31.
> 
>   - Moved __pthread_clockjoin_ex to a hidden definition and removed the
>     hidden_{proto,def}.
> 
>   - Added some missing one line file description.
> 
> I also pushed the set on a personal branch [1]. If you are ok with this 
> set, I would like to push it upstream.
> 
> [1] https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/azanella/pthread_clockjoin_np

Hi Adhemerval,

Thanks for doing all that. I thought that I'd addressed the nits, but I
must have missed or lost some. :(

Your branch looks great, except it appears to be missing this hunk from the
"nptl: Add pthread_clockjoin_np" change:

diff --git a/nptl/tst-join3.c b/nptl/tst-join3.c
index c06e65a247..faf7683eaa 100644
--- a/nptl/tst-join3.c
+++ b/nptl/tst-join3.c
@@ -56,7 +56,12 @@ do_test_clock (clockid_t clockid)
   struct timespec timeout = timespec_add (xclock_now (clockid_for_get),
                                           make_timespec (0, 200000000));

-  int val = pthread_timedjoin_np (th, &status, &timeout);
+  int val;
+  if (clockid == CLOCK_USE_TIMEDJOIN)
+    val = pthread_timedjoin_np (th, &status, &timeout);
+  else
+    val = pthread_clockjoin_np (th, &status, clockid, &timeout);
+
   TEST_COMPARE (val, ETIMEDOUT);

   xpthread_mutex_unlock (&lock);


I suspect that it's not causing the test to fail because the existing call
to pthread_timedjoin_np will just time out immediately if passed the
relatively small absolute CLOCK_MONOTONIC time. I shall see if I can
improve this test to catch that in the future.

Thanks again.

Mike.