From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qk1-x735.google.com (mail-qk1-x735.google.com [IPv6:2607:f8b0:4864:20::735]) by sourceware.org (Postfix) with ESMTPS id C720B383A803 for ; Wed, 16 Jun 2021 12:46:54 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C720B383A803 Received: by mail-qk1-x735.google.com with SMTP id k11so2426042qkk.1 for ; Wed, 16 Jun 2021 05:46:54 -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=VFKMepb5GfrpOVCS//xLyWNhQ5c1GNT5jHttSmtO5VE=; b=GhIAK9Sa4AkYa/iIL05FnVSK12naBF8lXVDzTycSu+69pd1obkQ5Wb0Kd8XPCJ0fXu 02pR3fqbJkJgpRolZ385xbyAGo93Gme6lolF+s6ZSAbOe9fo0xldXrx18CwiT55NeYIQ 55e88Ff0gdwlwXXzF9/yakL0/O98MSJfdZ8CH7cENAE1w+nI+cVHeUjuFxmba5TNiEHF RzHoYf08OL8FeOj8iyYJU5sbnaOte528qItaMbKfHaQvc5q2+cY2UmVB/qIiTaIlbCH2 ksERXdyn/BhaTupORO/fyhGMnF5OwHR9o/0hoUaHlaggGR/O3Ge9afPSFEcFNSpxPsuC O4IQ== X-Gm-Message-State: AOAM530p9NKiQ6l/CsKELBQ08TGr2OI5WC+ji0d1TeeLyhUZF+DK8v3X t6F/nwchbdj9e6ifFPLnPJNhhyRyMp80iw== X-Google-Smtp-Source: ABdhPJwVnzYkNkVWz9gkpADtRnGymnE+0YGPGRpY2ahfXWOjUFyR89EKr7XPIJ5fQwANXwClT2Y2Dg== X-Received: by 2002:a37:664b:: with SMTP id a72mr5074703qkc.389.1623847614157; Wed, 16 Jun 2021 05:46:54 -0700 (PDT) Received: from [192.168.1.4] ([177.194.59.218]) by smtp.gmail.com with ESMTPSA id o2sm1123794qtv.47.2021.06.16.05.46.52 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 16 Jun 2021 05:46:53 -0700 (PDT) Subject: Re: [PATCH 07/11] nptl: Remove CANCELING_BITMASK From: Adhemerval Zanella To: Florian Weimer , Adhemerval Zanella via Libc-alpha References: <20210526165728.1772546-1-adhemerval.zanella@linaro.org> <20210526165728.1772546-8-adhemerval.zanella@linaro.org> <878s3ag4tc.fsf@oldenburg.str.redhat.com> <198b0728-f255-0fc6-6eed-64802e8e5157@linaro.org> Message-ID: <41114f1a-6566-9afd-4bf6-e3a71e241a86@linaro.org> Date: Wed, 16 Jun 2021 09:46:51 -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: <198b0728-f255-0fc6-6eed-64802e8e5157@linaro.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-6.4 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, 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: Wed, 16 Jun 2021 12:46:56 -0000 On 15/06/2021 20:33, Adhemerval Zanella wrote: > > > On 15/06/2021 19:07, Florian Weimer wrote: >> * Adhemerval Zanella via Libc-alpha: >> >>> The CANCELING_BITMASK is used as an optimization to avoid sending >>> the signal when pthread_cancel is called in a concurrent manner. >>> >>> This requires then to put both the cancellation state and type on >>> a shared state (cancelhandling), since 'pthread_cancel' checks >>> whether cancellation is enabled and asynchrnous to either cancel >>> itself of sending the signal. >>> >>> It also requires handle the CANCELING_BITMASK on >>> __pthread_disable_asynccancel, however this is incurs in the same >>> issues described on BZ#12683: the cancellation is acting even *after* >>> the syscalls returns with user visible side-effects. >>> >>> This patch removes this optimization and simplifies the pthread >>> cancellation implementation: pthread_cancel now first check if >>> cancellation is already pending and if not always send a signal >>> if the target is not itself. The SIGCANCEL handler is also simpified >>> since there is not need to setup a CAS loop. >>> >>> It also alows to move both the cancellation state and mode out of >>> 'cancelhadling' (it is done in subsequent patches). >> >> It looks like this causes sporadic failures in nptl/tst-sem16 because >> sem_wait fails with EINTR, revealing that cancellation is implemented >> using a signal (something that must not be visible according to POSIX). >> >> This was not a problem before because pthread_cancel sent the signal >> only when asynchronous cancellation was enabled. >> >> We either have to revert this, or push forward with the transition so >> that we can install the SIGCANCEL handler with SA_RESTART. > > Indeed now that SIGCANCEL is always sent by pthread_cancel we must > install the signal handle with SA_RESTART, the signal handle will > either act on cancellation or it should be not visible by the > application. > > I don't think there is a need to revert this change, running nptl/tst-sem16 > with SA_RESTART I don't see the regression anymore with multiple thousands > runs. > I will send a patch to use SA_RESTART on cancellation.