From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS17314 8.43.84.0/22 X-Spam-Status: No, score=-3.8 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI,RDNS_DYNAMIC,SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (ip-8-43-85-97.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPS id F40941F8C6 for ; Thu, 26 Aug 2021 14:35:21 +0000 (UTC) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 00DBA3858007 for ; Thu, 26 Aug 2021 14:35:21 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 00DBA3858007 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1629988521; bh=xdfb7u4tD2FCcYzhSdn9O70oWF/vyzESKRc4fIkYMIE=; h=To:Subject:References:Date:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=V6qRemsmjQTo1e140DLP1vj4tQ6/dFJXjo+VTr5gHGO5btwRZIbBSUFdjDk91W45B XmkqRaGu6xoyGyg73NtmaVR9qmbTOJgiN+mKJAkM+2r44t1R3EXiASs2az4ix3eW9d KTDZuBkg7CvxACpEBL6BJ+Yj756f6KRYfWz+YC+M= Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id 8D6323858406 for ; Thu, 26 Aug 2021 14:35:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 8D6323858406 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-384-9htzkUmOPsKZfsrmrGlfAg-1; Thu, 26 Aug 2021 10:34:57 -0400 X-MC-Unique: 9htzkUmOPsKZfsrmrGlfAg-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 7CC4D802E6E; Thu, 26 Aug 2021 14:34:49 +0000 (UTC) Received: from oldenburg.str.redhat.com (unknown [10.39.194.140]) by smtp.corp.redhat.com (Postfix) with ESMTPS id A6E0719C59; Thu, 26 Aug 2021 14:34:48 +0000 (UTC) To: Adhemerval Zanella Subject: Re: [PATCH v2 06/19] nptl: Replace struct thread cancelhandling field References: <20210823195047.543237-1-adhemerval.zanella@linaro.org> <20210823195047.543237-7-adhemerval.zanella@linaro.org> Date: Thu, 26 Aug 2021 16:34:46 +0200 In-Reply-To: <20210823195047.543237-7-adhemerval.zanella@linaro.org> (Adhemerval Zanella's message of "Mon, 23 Aug 2021 16:50:34 -0300") Message-ID: <87r1eg5l6h.fsf@oldenburg.str.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Florian Weimer via Libc-alpha Reply-To: Florian Weimer Cc: libc-alpha@sourceware.org Errors-To: libc-alpha-bounces+e=80x24.org@sourceware.org Sender: "Libc-alpha" * Adhemerval Zanella: > Now that both thread state and setxid is being tracked by two different > fields ('joinstate' and 'setxid_flag'), there is no need to keep track > of exiting (EXITING_BIT) and terminated (TERMINATED_BIT) state in > 'cancelhandling'. > > The 'cancelhandling' field is renamed to 'cancelstatus' and it only > signals whether the thread has been cancelled (set by pthread_cancel()). > It allows simplify the atomic operation required to avoid CAS > operations. =E2=80=9Ccancelstatus=E2=80=9D is a bad name because =E2=80=9Ccancel state= =E2=80=9D is POSIX terminology with different meaning. Please choose a different name. I think pending_cancel or cancel_requested would work. > There is no need to check the thread state on > __pthread_enable_asynccancel() nor on pthread_testcancel () anymore, > since cancellation is explicit disabled when thread start the exit code > on __do_cancel(). =E2=80=9Cexplict[ly]=E2=80=9D > > On __nptl_free_tcp(), the 'joinstate' now defines whether it is the =E2=80=9C__nptl_free_tcb=E2=80=9D. > creating thread or the created thread that calls it. So there is > no concurrent call within the function and thus no need to set the > TERMINATED_BIT. > > For SIGCANCEL handler, sigcancel_handler(), 'joinstate' is used instead > (pthread_cancel() might still be called concurrenty in assynchronous > mode). =E2=80=9Cas[]ynchronous=E2=80=9D > diff --git a/nptl/nptl_free_tcb.c b/nptl/nptl_free_tcb.c > index cbf3580f59..15e1a18562 100644 > --- a/nptl/nptl_free_tcb.c > +++ b/nptl/nptl_free_tcb.c > @@ -24,22 +24,12 @@ > void > __nptl_free_tcb (struct pthread *pd) > { > - /* The thread is exiting now. */ > - if (atomic_bit_test_set (&pd->cancelhandling, TERMINATED_BIT) =3D=3D 0= ) > - { > - /* Free TPP data. */ > - if (pd->tpp !=3D NULL) > - { > - struct priority_protection_data *tpp =3D pd->tpp; > + free (pd->tpp); > + pd->tpp =3D NULL; For detached threads, this is called on the thread after signals have been blocked. I don't think this is entirely correct. And things become rather interesting if malloc needs pd->tpp. It's not clear to me whether this could happen before. > + /* Queue the stack memory block for reuse and exit the process. The k= ernel > + will signal via writing to the address returned by QUEUE-STACK when= the > + stack is available. */ > + __nptl_deallocate_stack (pd); > } That old comment seems to be out-of-date. > libc_hidden_def (__nptl_free_tcb) > diff --git a/nptl/pthread_cancel.c b/nptl/pthread_cancel.c > index 67e00ef007..aed6c1ea47 100644 > --- a/nptl/pthread_cancel.c > +++ b/nptl/pthread_cancel.c > @@ -93,8 +90,9 @@ __pthread_cancel (pthread_t th) > } > #endif > =20 > - int oldch =3D atomic_fetch_or_acquire (&pd->cancelhandling, CANCELED_B= ITMASK); > - if ((oldch & CANCELED_BITMASK) !=3D 0) > + /* If already cancelled just return (cancellation will be acted upon i= n next > + cancellation entrypoint). */ > + if (atomic_exchange_acquire (&pd->cancelstatus, 1) =3D=3D 1) > return 0; This should probably use relaxed MO. The libthreaddb changes look okay to me. Thanks, Florian