From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qv1-xf31.google.com (mail-qv1-xf31.google.com [IPv6:2607:f8b0:4864:20::f31]) by sourceware.org (Postfix) with ESMTPS id E9309384783E for ; Tue, 1 Jun 2021 16:36:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E9309384783E Received: by mail-qv1-xf31.google.com with SMTP id a7so7496433qvf.11 for ; Tue, 01 Jun 2021 09:36:10 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:from:to:references:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=2ZLsOOUMXAyYzWWSw8vd1VFGnhF51eHhn3gsNCnwXe8=; b=mQH87eV1AzBtaYt7w6DQlQnUuds8Z2Ro3HB3uNjU4slEcHdq7zVgmCHIpKfkGiQRsl /2mFTU6THDS6TxALPQ21EfJ0pEj7QZTYxMfCv9CODLt7csRHI2z+CuvXurhhpsw1QlWd UuFcuCmUzrnufp5P5BV1yCL1hjR98Otx6xFbV0FV6r49YiMBUaB06UWid2Lifaku302s rumSSaaflC0XJ6LVLGwv8NLZ5v5NJMMTuac4N/X0G+38qF830rXm8zHOCVSn7HPIhigc kAyo5VMfUvVuwxsiw0fuVne6NsXrLDlr28jizy2X1G9OuksKuWU8MaV3SXUbxW4QSPn4 Gzqw== X-Gm-Message-State: AOAM531qx2J6qNc3SJMZMgn6GX5h4IO3NY23cHJzs/PJHH+fD0KJiKol NKeMPGiWwSbca9Iy5zuirQd/EJpsSIfLqg== X-Google-Smtp-Source: ABdhPJyTCCOyN3udwtjGH9RTIJUwE6f5M9tv15fHkOnRBnURSPU1jp8AkVYWRggt2qZ0QyDBsZ2NuA== X-Received: by 2002:a0c:c30e:: with SMTP id f14mr4005015qvi.19.1622565370370; Tue, 01 Jun 2021 09:36:10 -0700 (PDT) Received: from [192.168.1.4] ([177.194.59.218]) by smtp.gmail.com with ESMTPSA id e2sm11534508qkm.18.2021.06.01.09.36.09 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 01 Jun 2021 09:36:10 -0700 (PDT) Subject: Re: [PATCH v2 4/9] nptl: Remove CANCELING_BITMASK From: Adhemerval Zanella To: Florian Weimer , Adhemerval Zanella via Libc-alpha References: <20210527172823.3461314-1-adhemerval.zanella@linaro.org> <20210527172823.3461314-5-adhemerval.zanella@linaro.org> <87bl8q6jok.fsf@oldenburg.str.redhat.com> <10f40817-50da-5997-c2e8-3aa4edfab4a4@linaro.org> Message-ID: <837f443f-d47f-5b06-6170-8cec8219a55f@linaro.org> Date: Tue, 1 Jun 2021 13:36:07 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 MIME-Version: 1.0 In-Reply-To: <10f40817-50da-5997-c2e8-3aa4edfab4a4@linaro.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org 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: , X-List-Received-Date: Tue, 01 Jun 2021 16:36:12 -0000 On 01/06/2021 10:50, Adhemerval Zanella wrote: >>> diff --git a/nptl/pthread_cancel.c b/nptl/pthread_cancel.c >>> index deb404600c..8dfbcff8c3 100644 >>> --- a/nptl/pthread_cancel.c >>> +++ b/nptl/pthread_cancel.c >> >>> @@ -104,72 +87,33 @@ __pthread_cancel (pthread_t th) >>> " must be installed for pthread_cancel to work\n"); >>> } >>> + >>> + int oldch = atomic_fetch_or_acquire (&pd->cancelhandling, CANCELED_BITMASK); >>> + if ((oldch & CANCELED_BITMASK) != 0) >>> + return 0; >>> + >>> + if (pd == THREAD_SELF) >>> { >>> + /* A single-threaded process should be able to kill itself, since there >>> + is nothing in the POSIX specification that says that it cannot. So >>> + we set multiple_threads to true so that cancellation points get >>> + executed. */ >>> + THREAD_SETMEM (THREAD_SELF, header.multiple_threads, 1); >>> #ifndef TLS_MULTIPLE_THREADS_IN_TCB >>> - __libc_multiple_threads = 1; >>> + __libc_multiple_threads = 1; >>> #endif >>> + >>> + THREAD_SETMEM (pd, result, PTHREAD_CANCELED); >>> + if ((oldch & CANCELTYPE_BITMASK) != 0) >>> + __do_cancel (); >>> + return 0; >>> } >> >> The last part is for asynchronous self-cancel, right? Don't we have to >> check that cancellation is enabled, too? Ok, so I recall why I did this originally and checking if cancel is enable is not strictly necessary since SIGCANCEL signal handler will bail early if cancellation is not enabled. But it does seems the correct way to do it. > > Indeed it should be: > > if ((oldch & CANCELSTATE_BIT) This should be '(oldch & CANCELSTATE_BIT) == 0'. > && (oldch & CANCELTYPE_BITMASK) != 0) > > I ended up fixing it later in the serie with the cancel type and state > refactor. I have fixed it locally. >