From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS31976 209.132.180.0/23 X-Spam-Status: No, score=-3.2 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED, SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.1 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 dcvr.yhbt.net (Postfix) with ESMTPS id 283A11F597 for ; Sat, 21 Jul 2018 13:41:00 +0000 (UTC) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:subject:to:cc:references:from:message-id:date :mime-version:in-reply-to:content-type :content-transfer-encoding; q=dns; s=default; b=mO5IMktyXbw3Z/aA 46MZt/9ooOa8dQxu7J7mqZ448PytzUPhBre9bLA7+6qa2UQIplpWdqQ8qf9FYMtI Zr7QLHXeLlkRdgWdIyZXAiO4VL33M9My8nn7Ty1P29fMPk+IVwEippvNrUS1Dp2s Uak8JJfUZeM9R76Rnyt5PHwRpbo= 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:subject:to:cc:references:from:message-id:date :mime-version:in-reply-to:content-type :content-transfer-encoding; s=default; bh=Ua+GoPmoPGf/3U1z6jmXfK pkal4=; b=Kg/HXGrrMLE5nDZj7HETBVXxw+kHnLKtnMyEhXG6HxmctVOQSrr3kJ Z9Heoh4vSZpqE6qbvXrVRKz7RnqjcCmerZ8pdnY2B+UnP9ldpGtOogKTydt8p+pE hKyY15LhyJv6Sdmzn98GobS5bnA0uq2i8GDiioRJeniqiWFp75bxY= Received: (qmail 60511 invoked by alias); 21 Jul 2018 13:40:57 -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 Received: (qmail 60490 invoked by uid 89); 21 Jul 2018 13:40:57 -0000 Authentication-Results: sourceware.org; auth=none X-HELO: mail-qk0-f173.google.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=subject:to:cc:references:from:openpgp:autocrypt:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=DFOq31ZzuMzmUe/IljQZtlKwuZpoGpn/Yt1XtU+mzj4=; b=VEPEIDSU8Htn+hg+1YCHnnsQ7UAak5CdX5pg9p+KqAYHCMfvelf5ZgOeI99URJcQ4t 3PJgp3F0Hd31l8PqplANv9k00m2MdpWMn5uy1DKFEs3jjCfTWdjcF+xxxv8OHUoNx4/w fbmn/U1kcbE3SVsvyaz8EK7VGdtMqCNjRIcoE= Subject: Re: [PATCH v8 0/8] Add support for ISO C threads.h To: Rical Jasan , Carlos O'Donell Cc: libc-alpha@sourceware.org, Florian Weimer References: <1517591084-11347-1-git-send-email-adhemerval.zanella@linaro.org> <203a3def-74d0-fa66-9792-a587a089a359@linaro.org> <46c7b370-f98d-30a3-9aac-c5e2c5d5e1d0@linaro.org> <00cbaea3-d979-d766-58b1-a4b0822ca678@redhat.com> <614bd248-b727-d517-6cba-a7968dc888c1@2c3t.io> <2630c1fd-d1b5-7bca-d853-838bbc15ac07@linaro.org> <31423f60-2d82-64c2-ff10-d545b414346d@2c3t.io> <031e5e99-2bef-7fc2-d87a-fb8386f3f6e8@redhat.com> <5550e6dd-9241-a898-3993-8562d0d9a327@2c3t.io> From: Adhemerval Zanella Openpgp: preference=signencrypt Message-ID: <57e257ac-07f1-90ad-ee3b-6bc3b08fa1c3@linaro.org> Date: Sat, 21 Jul 2018 10:40:48 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <5550e6dd-9241-a898-3993-8562d0d9a327@2c3t.io> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 21/07/2018 03:22, Rical Jasan wrote: > On 07/20/2018 07:11 PM, Carlos O'Donell wrote: >> On 07/20/2018 08:26 PM, Rical Jasan wrote: > ... >>> +This section describes the @glibcadj{} ISO C threads implementation. >>> +To have a deeper understanding of this API, it is strongly recomended >> >> s/recomended/recommended/g > > Fixed. > >>> +@deftp {Data Type} thrd_t >>> +@standards{C11, threads.h} >>> +A unique object that identifies a thread unequivocally. >> >> Suggest: >> A unique object that identifies one thread. >> >> Do more with less. > > I call, and raise you to "a thread". > >>> +@item mtx_recursive >>> +@standards{C11, threads.h} >>> +A mutex that supports recursive locking, which means that the owner >>> +thread can lock it more than once without causing deadlock. >> >> Traditional technical language is "owning thread" >> >> s/owner thread/owning thread/g > > Fixed. > >>> +@deftypefun int mtx_init (mtx_t *@var{mutex}, int @var{type}) >>> +@standards{C11, threads.h} >>> +@safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}} >>> +@code{mtx_init} creates a new mutex object with type @var{type}. The >>> +object pointed to by @var{mutex} is set to the identifier of the newly >>> +created mutex. >>> + >>> +The @code{type} argument may be either @code{mtx_plain} or >>> +@code{mtx_timed}, optionally combined with @code{mtx_recursive}. >> >> This should be expanded to ensure clarity. >> >> Suggest: >> ~~~ >> The @code{type} argument may be either @code{mtx_plain}, >> @code{mtx_timed}, @code{mtx_plain | mtx_recursive}, or >> @code{mtx_timed | mtx_recursive}. >> ~~~ > > I turned this into a table. > >>> +@deftypefun int mtx_unlock (mtx_t *@var{mutex}) >>> +@standards{C11, threads.h} >>> +@safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}} >>> +@code{mtx_unlock} unlocks the mutex pointed to by @var{mutex}. The >>> +behavior is undefined if the mutex is not locked by the calling >>> +thread. >> >> OK. What about unlocking a lock you already unlocked? > > I don't know. The specification is quiet on that point, as well as the > text I was working with. nptl/mtx_unlock.c in Adhemerval's branch calls > __pthread_mutex_unlock. In nptl/pthread_mutex_unlock.c, going through > the various tests for the mutex type (most of which result in a call to > lll_unlock or some variant), there are three tests that are conditional > upon "! lll_islocked", and they all return EPERM. I believe, however, > that is an implementation detail we wouldn't necessarily include in the > manual, so the best I could say here is that it's UB because the spec > doesn't say anything about it. (I didn't in the new patch, but I can > add that in if you like.) > >>> +@deftypefun int cnd_wait (cnd_t *@var{cond}, mtx_t *@var{mutex}) >>> +@standards{C11, threads.h} >>> +@safety{@prelim{}@mtsafe{}@asunsafe{@asulock{}}@acunsafe{@aculock{}}} >>> +@code{cnd_wait} atomically unlocks the mutex pointed to by @var{mutex} >>> +and blocks on the condition variable pointed to by @var{cond} until >>> +the thread is signalled by @code{cnd_signal} or @code{cnd_broadcast}. >> >> s/signalled/signaled/g > > Fixed. > >>> +@deftypefun int cnd_timedwait (cnd_t *restrict @var{cond}, mtx_t *restrict @var{mutex}, const struct timespec *restrict @var{time_point}) >>> +@standards{C11, threads.h} >>> +@safety{@prelim{}@mtsafe{}@asunsafe{@asulock{}}@acunsafe{@aculock{}}} >>> +@code{cnd_timedwait} atomically unlocks the mutex pointed to by >>> +@var{mutex} and blocks on the condition variable pointed to by >>> +@var{cond} until the thread is signalled by @code{cnd_signal} or >> >> s/signalled/signaled/g > > Fixed. > >>> +@Theglibc{} implements functions to provide @dfn{thread-local >>> +storage}, which means each thread can have their own variables that >>> +are not visible by other threads. >> >> I would tighten this up a bit like this, since it's less about >> visibility and more about storage: >> ~~~ >> @Theglibc{} implements functions to provide @dfn{thread-local >> storage}. Variables can be defined to have unique storage per-thread, >> lifetimes that match the thread lifetime, and destructors that cleanup >> the unique per-thread storage. >> ~~~ > > Fixed. I joined it into one sentence to bind the explanation a little > more tightly to the @dfn. > >>> +@emph{Note:} For C++, C++11 or later is required to get the >> >> s/get/use/g >> >> You "use" a keyword. > > Fixed. > >>> +@defvr Macro TSS_DTOR_ITERATIONS >>> +@standards{C11, threads.h} >>> +@code{TSS_DTOR_ITERATIONS} is an integer constant expression >>> +representing the maximum number of times that destructors will be >>> +called when a thread terminates. >> >> Suggest a broader explanation: >> >> @code{TSS_DTOR_ITERATIONS} is an integer constant expression >> representing the maximum number of iterations over all thread-local >> destructors at the time of thread termination. This value provides >> a bounded limit to the destruction of thread-local storage e.g. >> consider a destructor that creates more thread-local storage. > > Thanks. I had largely left that one alone. > > Updated patch attached. > > Rical > Thanks, I pushed it on my personal branch [1] with you as author. [1] https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/azanella/c11-threads