From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 112542 invoked by alias); 18 Oct 2019 12:20:25 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 112532 invoked by uid 89); 18 Oct 2019 12:20:24 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-9.0 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy= X-HELO: mail-qt1-f195.google.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=to:cc:references:from:openpgp:autocrypt:subject:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=e3Q0ePLWu2Jx+4sqhdJ6XQHt6bA+LHeWhEAOXky7/Mw=; b=GdDbzmSr4gTn2b84ODTBvcmpHZxazuWJUJVyV3w/SX2pgmkyhcbvHesLk5YxS9z+pZ 9D1/rKZ6yR9GL/bSlxc+yZgxVJJy43MsideUyrQu6suySeECS+g1ov0jMlcLAvmnVz/P Np3DYM1xtQhDuXSr+CcY4izYKA35yyWyi4GIao88yTFaBzfMtUSU+1/PrVmlpf3hiP96 IZEqp6pvExRGqFse19QXGSUSeLabMMo25SiPXXY+JYEqGfrnFGkphpOIVkcekteijJCP /Lrn2s+f3lJdOjeifzDLhtNtdnZBQtonDvO0dDzzyDLdeBtxrHgX0h4Z3xsCjcF5yNFQ 2OUQ== Return-Path: To: Florian Weimer Cc: libc-alpha@sourceware.org References: <87k197ik0o.fsf@oldenburg2.str.redhat.com> <37512974-b590-967b-25aa-6bf0082adeb7@linaro.org> <87eezbvy81.fsf@oldenburg2.str.redhat.com> From: Adhemerval Zanella Openpgp: preference=signencrypt Subject: Re: [PATCH] nptl: Start new threads with all signals blocked [BZ #25098] Message-ID: Date: Fri, 18 Oct 2019 12:20:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 In-Reply-To: <87eezbvy81.fsf@oldenburg2.str.redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-SW-Source: 2019-10/txt/msg00550.txt.bz2 On 17/10/2019 18:49, Florian Weimer wrote: > * Adhemerval Zanella: > >>> The cost of doing this is two rt_sigprocmask system calls on the old >>> thread, and one rt_sigprocmask system call on the new thread. (If >>> there was a way to clone a new thread with a signals disabled, this >>> could be brought down to one system call each.) The thread descriptor >>> increases in size, too, and sigset_t is fairly large. This increase >>> could be brought down by reusing space the in the descriptor which is >>> not needed before running user code, or by switching to an internal >>> sigset_t definition which only covers the signals supported by the >>> kernel definition. (Part of the thread descriptor size increase is >>> already offset by reduced stack usage in the thread start wrapper >>> routine after this commit.) >> >> I think this change worth parametrizing it on Linux to save some space >> on the pthread_t structure, since it save about 120 bytes per thread >> and it is unlikely Linux will eventually increase the signal size. > > Do you see this as a precondition for this change? I think we can add it as follow-up optimization, since it is orthogonal to the change. > >> I think the idea of keeping the fields was that tools that abuse >> the ABI and access such metadata directly could work across glibc >> versions. However, the C11 thread state already changed the internal >> layout so I don't see much gain on keep this idea. I would say to >> just remove the field altogether. > > I can certainly do that. > >>> + /* Block alll signals, so that the new thread starts out with >>> + signals disabled. This avoids race conditions in the thread >>> + startup. */ >> >> s/alll/all > > Fixed locally. > >>> + sigset_t original_sigmask; >>> + __libc_signal_block_all (&original_sigmask); >>> + >>> + /* Conceptually, the new thread needs to inherit the signal mask of >>> + this thread. Therefore, it needs to restore the saved signal >>> + mask of this thread, so save it in the startup information. */ >>> + pd->sigmask = original_sigmask; >>> +#ifdef SIGCANCEL >>> + /* Reset the cancellation signal mask in case this thread is running >>> + cancellation. */ >>> + __sigdelset (&pd->sigmask, SIGCANCEL); >>> +#endif >>> + >> >> Do we still have a nptl target that does not have SIGCANCEL support? > > I don't think so. I can remove all the #ifdef's and run a build with > build-many-glibcs.py tomorrow. > > Thanks, > Florian >