From patchwork Mon Feb 11 21:11:27 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Carlos O'Donell X-Patchwork-Id: 1040156 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=sourceware.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=libc-alpha-return-99941-incoming=patchwork.ozlabs.org@sourceware.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.b="Xkg8VJFb"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 43yz4C6tKRz9s6w for ; Tue, 12 Feb 2019 08:11:39 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:to:from:subject:message-id:date:mime-version :content-type:content-transfer-encoding; q=dns; s=default; b=Oq0 7BJhPSDJHSN/DaCTR54uv5c9T+0Y9vLZIJ3Qk32ZdUyqc/Iv46oSafl9D5eZjSzx IXwdZ72NvOqadr43v0kjJXNjGlzHh6vJkVrio+p/rZpliAWEB53LHI7l5KNKW/su 7NG0FM+HJSO+YX2EFFHkarvSQAkrL3wd5X0iDoHg= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:to:from:subject:message-id:date:mime-version :content-type:content-transfer-encoding; s=default; bh=PVU8/SD3P 7A05/d2P8f0MFZAbAA=; b=Xkg8VJFbK4BmH3HCYyM2xr51w7wGEPS8QhUNnar6Q VX3hZcx1V6zDjL/icCXgDbQT/NX6I7QmjSfNNZGNAmoK7pBkrCHCk26AQxEGRl+1 JX1kSBgPJGeWpCLi1byIIk5RStxnb4E6xYtdUdjFqkQClEK2Z2Nqd5sB7n8bXU2R Co= Received: (qmail 122688 invoked by alias); 11 Feb 2019 21:11:34 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 122679 invoked by uid 89); 11 Feb 2019 21:11:33 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 spammy=secondly, cancelled, mistakes, Hx-languages-length:3982 X-HELO: mail-qk1-f194.google.com To: GNU C Library , Florian Weimer From: Carlos O'Donell Subject: [PATCH] nptl: Fix join/tryjoin comments. Openpgp: preference=signencrypt Message-ID: <448cd43d-8f24-7d02-e6dc-b6fa8e0dc44e@redhat.com> Date: Mon, 11 Feb 2019 16:11:27 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 MIME-Version: 1.0 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 --- 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 + + * 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 * 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; \