From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qk1-x72f.google.com (mail-qk1-x72f.google.com [IPv6:2607:f8b0:4864:20::72f]) by sourceware.org (Postfix) with ESMTPS id BBCF43857C6D for ; Mon, 10 May 2021 13:47:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org BBCF43857C6D Received: by mail-qk1-x72f.google.com with SMTP id f29so3063234qka.0 for ; Mon, 10 May 2021 06:47:27 -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:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=1+pKd1U21IrYcSDMw1W3bofxX/L8+RioCQOfKrBReN8=; b=TfTsEutFj6HZ7yL1CvekHzLaobAKx6+mobbt6GZrFXFQdKPErrNNVRE+ZwTleDal6/ MFyJu9IREIRbAWwTD9y820NSEn8t/Rh/vFU8BcmtKqrlSL/4NkqyaEF6MTuypv3EGLDk 9AuUxutYPN+QLv37/+2lsB+5j8/yo9ohmIGcexkc8W90E6jACRME2+W1oqrbZSFpJj+D Sxtd1JvUpNSG3BYUcMwO14O+0eVQgjhx++zfu1gs45SaJai4e+BCTPT+ZXCygzcwKOAz mrGXy/bA3hbp4yAdlPvPUu/lInAo024OqPaS7AOrzAmUW6FWgDyQMKKpkCxaGJ6r03pR 2GBg== X-Gm-Message-State: AOAM532i3LPAoxtWIKaSEGmbRiZePKLCbHM1Rmyk1quiQVZvh1bWCDBH XVdJ4KSSQowbCrJOm0K40NzqzV3StIbb0Q== X-Google-Smtp-Source: ABdhPJySLQpOkttgXMIvYN5q5WEGHnZ52K09YimeB4It5+PKi2/xlRK8vc/L1+Mol9dxjlGyoEGHLg== X-Received: by 2002:a05:620a:481:: with SMTP id 1mr22487018qkr.46.1620654447024; Mon, 10 May 2021 06:47:27 -0700 (PDT) Received: from [192.168.1.4] ([177.194.37.86]) by smtp.gmail.com with ESMTPSA id 66sm11729314qkh.54.2021.05.10.06.47.25 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 10 May 2021 06:47:26 -0700 (PDT) Subject: Re: [PATCH 3/8] nptl: Remove always-disabled debugging support To: Florian Weimer , libc-alpha@sourceware.org References: <313425893d8419592a17bdad60ce2bf37d485b3f.1620650045.git.fweimer@redhat.com> From: Adhemerval Zanella Message-ID: Date: Mon, 10 May 2021 10:47:24 -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: <313425893d8419592a17bdad60ce2bf37d485b3f.1620650045.git.fweimer@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.7 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: Mon, 10 May 2021 13:47:29 -0000 On 10/05/2021 09:37, Florian Weimer via Libc-alpha wrote: > This removes the DEBUGGING_P macro and the __pthread_debug variable. > The __find_in_stack_list function is now unused and deleted as well. LGTM, thanks. As a side note we might revise the INVALID_TD_P and INVALID_NOT_TERMINATED_TD_P in the future, I don't their usage are entirely safe within glibc. Reviewed-by: Adhemerval Zanella > --- > nptl/pthreadP.h | 26 +++++----------------- > nptl/pthread_create.c | 49 ----------------------------------------- > nptl/pthread_sigqueue.c | 5 ----- > 3 files changed, 5 insertions(+), 75 deletions(-) > > diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h > index d9b97c814a..d9a6137bd3 100644 > --- a/nptl/pthreadP.h > +++ b/nptl/pthreadP.h > @@ -243,23 +243,11 @@ libc_hidden_proto (__pthread_tpp_change_priority) > extern int __pthread_current_priority (void); > libc_hidden_proto (__pthread_current_priority) > > -/* The library can run in debugging mode where it performs a lot more > - tests. */ > -extern int __pthread_debug attribute_hidden; > -/** For now disable debugging support. */ > -#if 0 > -# define DEBUGGING_P __builtin_expect (__pthread_debug, 0) > -# define INVALID_TD_P(pd) (DEBUGGING_P && __find_in_stack_list (pd) == NULL) > -# define INVALID_NOT_TERMINATED_TD_P(pd) INVALID_TD_P (pd) > -#else > -# define DEBUGGING_P 0 > -/* Simplified test. This will not catch all invalid descriptors but > - is better than nothing. And if the test triggers the thread > - descriptor is guaranteed to be invalid. */ > -# define INVALID_TD_P(pd) __builtin_expect ((pd)->tid <= 0, 0) > -# define INVALID_NOT_TERMINATED_TD_P(pd) __builtin_expect ((pd)->tid < 0, 0) > -#endif > - > +/* This will not catch all invalid descriptors but is better than > + nothing. And if the test triggers the thread descriptor is > + guaranteed to be invalid. */ > +#define INVALID_TD_P(pd) __builtin_expect ((pd)->tid <= 0, 0) > +#define INVALID_NOT_TERMINATED_TD_P(pd) __builtin_expect ((pd)->tid < 0, 0) > > /* Cancellation test. */ > #define CANCELLATION_P(self) \ > @@ -322,10 +310,6 @@ __do_cancel (void) > > /* Internal prototypes. */ > > -/* Thread list handling. */ > -extern struct pthread *__find_in_stack_list (struct pthread *pd) > - attribute_hidden; > - > /* Deallocate a thread's stack after optionally making sure the thread > descriptor is still valid. */ > extern void __free_tcb (struct pthread *pd) attribute_hidden; Ok. > diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c > index 775287d0e4..d19456d48b 100644 > --- a/nptl/pthread_create.c > +++ b/nptl/pthread_create.c > @@ -42,9 +42,6 @@ > #include > > > -/* Nozero if debugging mode is enabled. */ > -int __pthread_debug; > - > /* Globally enabled events. */ > static td_thr_events_t __nptl_threads_events __attribute_used__; > > @@ -210,46 +207,6 @@ static int create_thread (struct pthread *pd, const struct pthread_attr *attr, > > #include > > - > -struct pthread * > -__find_in_stack_list (struct pthread *pd) > -{ > - list_t *entry; > - struct pthread *result = NULL; > - > - lll_lock (GL (dl_stack_cache_lock), LLL_PRIVATE); > - > - list_for_each (entry, &GL (dl_stack_used)) > - { > - struct pthread *curp; > - > - curp = list_entry (entry, struct pthread, list); > - if (curp == pd) > - { > - result = curp; > - break; > - } > - } > - > - if (result == NULL) > - list_for_each (entry, &GL (dl_stack_user)) > - { > - struct pthread *curp; > - > - curp = list_entry (entry, struct pthread, list); > - if (curp == pd) > - { > - result = curp; > - break; > - } > - } > - > - lll_unlock (GL (dl_stack_cache_lock), LLL_PRIVATE); > - > - return result; > -} > - > - > /* Deallocate a thread's stack after optionally making sure the thread > descriptor is still valid. */ > void Ok. > @@ -259,12 +216,6 @@ __free_tcb (struct pthread *pd) > if (__builtin_expect (atomic_bit_test_set (&pd->cancelhandling, > TERMINATED_BIT) == 0, 1)) > { > - /* Remove the descriptor from the list. */ > - if (DEBUGGING_P && __find_in_stack_list (pd) == NULL) > - /* Something is really wrong. The descriptor for a still > - running thread is gone. */ > - abort (); > - > /* Free TPP data. */ > if (__glibc_unlikely (pd->tpp != NULL)) > { Ok. > diff --git a/nptl/pthread_sigqueue.c b/nptl/pthread_sigqueue.c > index 64bacfe41b..3ffb595489 100644 > --- a/nptl/pthread_sigqueue.c > +++ b/nptl/pthread_sigqueue.c > @@ -31,11 +31,6 @@ pthread_sigqueue (pthread_t threadid, int signo, const union sigval value) > #ifdef __NR_rt_tgsigqueueinfo > struct pthread *pd = (struct pthread *) threadid; > > - /* Make sure the descriptor is valid. */ > - if (DEBUGGING_P && INVALID_TD_P (pd)) > - /* Not a valid thread handle. */ > - return ESRCH; > - > /* Force load of pd->tid into local variable or register. Otherwise > if a thread exits between ESRCH test and tgkill, we might return > EINVAL, because pd->tid would be cleared by the kernel. */ > Ok.