From patchwork Wed May 26 16:57:18 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella Netto X-Patchwork-Id: 43589 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 1C6EA384783F; Wed, 26 May 2021 16:57:39 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 1C6EA384783F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1622048259; bh=TG3FGP1e32SP9ysExoG4wxFDWI22sTIBCtfnJc0vpk0=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=fzlASUAguQ9Ters+38CodCIdHtHlDO1IlLya2wchajQRdZvpvzlOqyGvqTc/4CLq0 qs/OzoFOl2CzlVk/WF8zcXBZCDYNnzIxqmMW+iNTSCPSwJrT3c6hVbWUMqYmx+z7Sp QFcf9CbTY2FuLNuOMifkPWL3kcYJQbmAOYZKsIPI= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-qv1-xf32.google.com (mail-qv1-xf32.google.com [IPv6:2607:f8b0:4864:20::f32]) by sourceware.org (Postfix) with ESMTPS id AD2E4385703A for ; Wed, 26 May 2021 16:57:35 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org AD2E4385703A Received: by mail-qv1-xf32.google.com with SMTP id e8so1076572qvp.7 for ; Wed, 26 May 2021 09:57:35 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=TG3FGP1e32SP9ysExoG4wxFDWI22sTIBCtfnJc0vpk0=; b=PKtbSeYhB90pgUoS7UQ12OqBwYZBjwbDbgKJ/mv6Q3+bJXTIfUxwe3c8WrqHx6dYRu AsioWu4AVRDFD7uFGBy1tVJgKCCSWt9DbzNS8kPU3NloEgcrbLPq3pERCvx9C0MrE3/f GYSwICMAdzhRQWk8quTIGdXmUp3EC8IGcd+VWHVcNtJukJ9/US1CR/I79QYhD0LnZl5l G2gnu2pwxruVj4Oa67ajCO3uU3plv/UaMRhUyAm85W9tVuoeQdYHsFMrI7EKT4+spkzZ rzDT94Ve6RXLM+AIgnuhtBlGQ93jaI/t/FTo3lNFNMnMsvf48fe9deq7pQGnBMEp/4ly aZ3w== X-Gm-Message-State: AOAM530KMdXS322b4BqrAOV20tPX2atjhxn54sGp4rt3XOIkjMCMU0Bp WNq4QZbACMTHzHQixTgu+KnsFaOhNKIwhg== X-Google-Smtp-Source: ABdhPJwQKHX88EZQSSulG8bLEeNHg2p4wlOrtrDjsVWSg+jvyb6Wi4/CXr7rcdiQOpey0n79K4iW4Q== X-Received: by 2002:ad4:4dc1:: with SMTP id cw1mr43146331qvb.15.1622048254932; Wed, 26 May 2021 09:57:34 -0700 (PDT) Received: from birita.. ([177.194.37.86]) by smtp.googlemail.com with ESMTPSA id k125sm1859761qkf.53.2021.05.26.09.57.33 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 May 2021 09:57:34 -0700 (PDT) To: libc-alpha@sourceware.org Subject: [PATCH 01/11] nptl: Move Linux createthread to nptl Date: Wed, 26 May 2021 13:57:18 -0300 Message-Id: <20210526165728.1772546-2-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210526165728.1772546-1-adhemerval.zanella@linaro.org> References: <20210526165728.1772546-1-adhemerval.zanella@linaro.org> MIME-Version: 1.0 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, KAM_SHORT, 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-Patchwork-Original-From: Adhemerval Zanella via Libc-alpha From: Adhemerval Zanella Netto Reply-To: Adhemerval Zanella Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" git mv -f sysdeps/unix/sysv/linux/createthread.c nptl/createthread.c No functional change. --- nptl/createthread.c | 124 ++++++++++++++++++-- sysdeps/unix/sysv/linux/createthread.c | 153 ------------------------- 2 files changed, 116 insertions(+), 161 deletions(-) delete mode 100644 sysdeps/unix/sysv/linux/createthread.c diff --git a/nptl/createthread.c b/nptl/createthread.c index 46943b33fe..bc3409b326 100644 --- a/nptl/createthread.c +++ b/nptl/createthread.c @@ -1,6 +1,7 @@ -/* Low-level thread creation for NPTL. Stub version. - Copyright (C) 2014-2021 Free Software Foundation, Inc. +/* Low-level thread creation for NPTL. Linux version. + Copyright (C) 2002-2021 Free Software Foundation, Inc. This file is part of the GNU C Library. + Contributed by Ulrich Drepper , 2002. The GNU C Library is free software; you can redistribute it and/or modify it under the terms of the GNU Lesser General Public @@ -16,30 +17,137 @@ License along with the GNU C Library; if not, see . */ +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#ifdef __NR_clone2 +# define ARCH_CLONE __clone2 +#else +# define ARCH_CLONE __clone +#endif + /* See the comments in pthread_create.c for the requirements for these two macros and the create_thread function. */ #define START_THREAD_DEFN \ - static void __attribute__ ((noreturn)) start_thread (void) -#define START_THREAD_SELF THREAD_SELF + static int __attribute__ ((noreturn)) start_thread (void *arg) +#define START_THREAD_SELF arg + +/* pthread_create.c defines this using START_THREAD_DEFN + We need a forward declaration here so we can take its address. */ +static int start_thread (void *arg) __attribute__ ((noreturn)); static int create_thread (struct pthread *pd, const struct pthread_attr *attr, bool *stopped_start, STACK_VARIABLES_PARMS, bool *thread_ran) { - /* If the implementation needs to do some tweaks to the thread after - it has been created at the OS level, it can set STOPPED_START here. */ + /* Determine whether the newly created threads has to be started + stopped since we have to set the scheduling parameters or set the + affinity. */ + bool need_setaffinity = (attr != NULL && attr->extension != NULL + && attr->extension->cpuset != 0); + if (attr != NULL + && (__glibc_unlikely (need_setaffinity) + || __glibc_unlikely ((attr->flags & ATTR_FLAG_NOTINHERITSCHED) != 0))) + *stopped_start = true; pd->stopped_start = *stopped_start; if (__glibc_unlikely (*stopped_start)) - /* See CONCURRENCY NOTES in nptl/pthread_create.c. */ + /* See CONCURRENCY NOTES in nptl/pthread_creat.c. */ lll_lock (pd->lock, LLL_PRIVATE); - return ENOSYS; + /* We rely heavily on various flags the CLONE function understands: + + CLONE_VM, CLONE_FS, CLONE_FILES + These flags select semantics with shared address space and + file descriptors according to what POSIX requires. + + CLONE_SIGHAND, CLONE_THREAD + This flag selects the POSIX signal semantics and various + other kinds of sharing (itimers, POSIX timers, etc.). + + CLONE_SETTLS + The sixth parameter to CLONE determines the TLS area for the + new thread. + + CLONE_PARENT_SETTID + The kernels writes the thread ID of the newly created thread + into the location pointed to by the fifth parameters to CLONE. + + Note that it would be semantically equivalent to use + CLONE_CHILD_SETTID but it is be more expensive in the kernel. + + CLONE_CHILD_CLEARTID + The kernels clears the thread ID of a thread that has called + sys_exit() in the location pointed to by the seventh parameter + to CLONE. + + The termination signal is chosen to be zero which means no signal + is sent. */ + const int clone_flags = (CLONE_VM | CLONE_FS | CLONE_FILES | CLONE_SYSVSEM + | CLONE_SIGHAND | CLONE_THREAD + | CLONE_SETTLS | CLONE_PARENT_SETTID + | CLONE_CHILD_CLEARTID + | 0); + + TLS_DEFINE_INIT_TP (tp, pd); + + if (__glibc_unlikely (ARCH_CLONE (&start_thread, STACK_VARIABLES_ARGS, + clone_flags, pd, &pd->tid, tp, &pd->tid) + == -1)) + return errno; /* It's started now, so if we fail below, we'll have to cancel it and let it clean itself up. */ *thread_ran = true; + /* Now we have the possibility to set scheduling parameters etc. */ + if (attr != NULL) + { + int res; + + /* Set the affinity mask if necessary. */ + if (need_setaffinity) + { + assert (*stopped_start); + + res = INTERNAL_SYSCALL_CALL (sched_setaffinity, pd->tid, + attr->extension->cpusetsize, + attr->extension->cpuset); + + if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (res))) + err_out: + { + /* The operation failed. We have to kill the thread. + We let the normal cancellation mechanism do the work. */ + + pid_t pid = __getpid (); + INTERNAL_SYSCALL_CALL (tgkill, pid, pd->tid, SIGCANCEL); + + return INTERNAL_SYSCALL_ERRNO (res); + } + } + + /* Set the scheduling parameters. */ + if ((attr->flags & ATTR_FLAG_NOTINHERITSCHED) != 0) + { + assert (*stopped_start); + + res = INTERNAL_SYSCALL_CALL (sched_setscheduler, pd->tid, + pd->schedpolicy, &pd->schedparam); + + if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (res))) + goto err_out; + } + } + return 0; } diff --git a/sysdeps/unix/sysv/linux/createthread.c b/sysdeps/unix/sysv/linux/createthread.c deleted file mode 100644 index bc3409b326..0000000000 --- a/sysdeps/unix/sysv/linux/createthread.c +++ /dev/null @@ -1,153 +0,0 @@ -/* Low-level thread creation for NPTL. Linux version. - Copyright (C) 2002-2021 Free Software Foundation, Inc. - This file is part of the GNU C Library. - Contributed by Ulrich Drepper , 2002. - - The GNU C Library is free software; you can redistribute it and/or - modify it under the terms of the GNU Lesser General Public - License as published by the Free Software Foundation; either - version 2.1 of the License, or (at your option) any later version. - - The GNU C Library is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - Lesser General Public License for more details. - - You should have received a copy of the GNU Lesser General Public - License along with the GNU C Library; if not, see - . */ - -#include -#include -#include -#include -#include -#include -#include -#include - -#include - -#ifdef __NR_clone2 -# define ARCH_CLONE __clone2 -#else -# define ARCH_CLONE __clone -#endif - -/* See the comments in pthread_create.c for the requirements for these - two macros and the create_thread function. */ - -#define START_THREAD_DEFN \ - static int __attribute__ ((noreturn)) start_thread (void *arg) -#define START_THREAD_SELF arg - -/* pthread_create.c defines this using START_THREAD_DEFN - We need a forward declaration here so we can take its address. */ -static int start_thread (void *arg) __attribute__ ((noreturn)); - -static int -create_thread (struct pthread *pd, const struct pthread_attr *attr, - bool *stopped_start, STACK_VARIABLES_PARMS, bool *thread_ran) -{ - /* Determine whether the newly created threads has to be started - stopped since we have to set the scheduling parameters or set the - affinity. */ - bool need_setaffinity = (attr != NULL && attr->extension != NULL - && attr->extension->cpuset != 0); - if (attr != NULL - && (__glibc_unlikely (need_setaffinity) - || __glibc_unlikely ((attr->flags & ATTR_FLAG_NOTINHERITSCHED) != 0))) - *stopped_start = true; - - pd->stopped_start = *stopped_start; - if (__glibc_unlikely (*stopped_start)) - /* See CONCURRENCY NOTES in nptl/pthread_creat.c. */ - lll_lock (pd->lock, LLL_PRIVATE); - - /* We rely heavily on various flags the CLONE function understands: - - CLONE_VM, CLONE_FS, CLONE_FILES - These flags select semantics with shared address space and - file descriptors according to what POSIX requires. - - CLONE_SIGHAND, CLONE_THREAD - This flag selects the POSIX signal semantics and various - other kinds of sharing (itimers, POSIX timers, etc.). - - CLONE_SETTLS - The sixth parameter to CLONE determines the TLS area for the - new thread. - - CLONE_PARENT_SETTID - The kernels writes the thread ID of the newly created thread - into the location pointed to by the fifth parameters to CLONE. - - Note that it would be semantically equivalent to use - CLONE_CHILD_SETTID but it is be more expensive in the kernel. - - CLONE_CHILD_CLEARTID - The kernels clears the thread ID of a thread that has called - sys_exit() in the location pointed to by the seventh parameter - to CLONE. - - The termination signal is chosen to be zero which means no signal - is sent. */ - const int clone_flags = (CLONE_VM | CLONE_FS | CLONE_FILES | CLONE_SYSVSEM - | CLONE_SIGHAND | CLONE_THREAD - | CLONE_SETTLS | CLONE_PARENT_SETTID - | CLONE_CHILD_CLEARTID - | 0); - - TLS_DEFINE_INIT_TP (tp, pd); - - if (__glibc_unlikely (ARCH_CLONE (&start_thread, STACK_VARIABLES_ARGS, - clone_flags, pd, &pd->tid, tp, &pd->tid) - == -1)) - return errno; - - /* It's started now, so if we fail below, we'll have to cancel it - and let it clean itself up. */ - *thread_ran = true; - - /* Now we have the possibility to set scheduling parameters etc. */ - if (attr != NULL) - { - int res; - - /* Set the affinity mask if necessary. */ - if (need_setaffinity) - { - assert (*stopped_start); - - res = INTERNAL_SYSCALL_CALL (sched_setaffinity, pd->tid, - attr->extension->cpusetsize, - attr->extension->cpuset); - - if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (res))) - err_out: - { - /* The operation failed. We have to kill the thread. - We let the normal cancellation mechanism do the work. */ - - pid_t pid = __getpid (); - INTERNAL_SYSCALL_CALL (tgkill, pid, pd->tid, SIGCANCEL); - - return INTERNAL_SYSCALL_ERRNO (res); - } - } - - /* Set the scheduling parameters. */ - if ((attr->flags & ATTR_FLAG_NOTINHERITSCHED) != 0) - { - assert (*stopped_start); - - res = INTERNAL_SYSCALL_CALL (sched_setscheduler, pd->tid, - pd->schedpolicy, &pd->schedparam); - - if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (res))) - goto err_out; - } - } - - return 0; -} From patchwork Wed May 26 16:57:19 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella Netto X-Patchwork-Id: 43590 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id A68F5386FC1A; Wed, 26 May 2021 16:57:40 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org A68F5386FC1A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1622048260; bh=CK4ZE/JVYBB+YzM2dBTahfYJlk6/4A/mILjpOPvOVu0=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=YAMeLTYNedRpNcMdBFsmZCnNaBs0fBMdD8a45IZlmMV6W2WQTMpU5vtInF3xTJm+b tStt0itpUwBMsbSCOZf1benu/JJ8P5ibhaYZTkREpDiUfIxutI92TXE38W0ulosJGG MvDP+MRSBTcgoW2STsQ5O+TDpRNHr8VBipwNsOw0= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-qv1-xf32.google.com (mail-qv1-xf32.google.com [IPv6:2607:f8b0:4864:20::f32]) by sourceware.org (Postfix) with ESMTPS id D4189385782E for ; Wed, 26 May 2021 16:57:36 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org D4189385782E Received: by mail-qv1-xf32.google.com with SMTP id ee9so1075931qvb.8 for ; Wed, 26 May 2021 09:57:36 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=CK4ZE/JVYBB+YzM2dBTahfYJlk6/4A/mILjpOPvOVu0=; b=dUoEux8KosztWm/3FH4h/+BSIZs8UBeDT0EdTa9RckfJU4tcKJnOsmDy9Og+ixNZWI bT9pNcNCgB47PvVHDOKiU4eIv3MFi+NEvzktcAKF423GX1F85Dot3sRHhsUDZV+5mNEQ 1ptDUGQ8FrVqHqJWZiz5Qk9eVG7MuxY0/Lxm/y2zZYOT5NvP/HI7k7IcE9PeWNubnKRZ rfelV8h6/KpP00by0iLwc/+ZViSDbtDMIPkOE94cdFS+ywuvifpf9UIJBACEgdfqehVn YKrCEbx5/CgtYMniryzVFmyynrUSntKLc7G86SyJd5oacZnNNz23GNkf+HVqb3vbhjvx JYhA== X-Gm-Message-State: AOAM533OyozgQ+RrXHesdU/HxzPnwF9Fi5b+G4fyMtBqtUz3VgyGBxSc pdStvh2Sth2jfUoIvdfKaKXExLvsRSPucw== X-Google-Smtp-Source: ABdhPJw4afTCWcLzmADIVf1RxlguD6zREmnPHORL39YFOoWUGH6Z04rpb9JnOB1N6Ilq2TPqLOThag== X-Received: by 2002:ad4:4f21:: with SMTP id fc1mr42868627qvb.16.1622048256151; Wed, 26 May 2021 09:57:36 -0700 (PDT) Received: from birita.. ([177.194.37.86]) by smtp.googlemail.com with ESMTPSA id k125sm1859761qkf.53.2021.05.26.09.57.35 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 May 2021 09:57:35 -0700 (PDT) To: libc-alpha@sourceware.org Subject: [PATCH 02/11] nptl: Move createthread to pthread_create Date: Wed, 26 May 2021 13:57:19 -0300 Message-Id: <20210526165728.1772546-3-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210526165728.1772546-1-adhemerval.zanella@linaro.org> References: <20210526165728.1772546-1-adhemerval.zanella@linaro.org> MIME-Version: 1.0 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, KAM_SHORT, 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-Patchwork-Original-From: Adhemerval Zanella via Libc-alpha From: Adhemerval Zanella Netto Reply-To: Adhemerval Zanella Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" The 'create_thread' function is moved to pthread_create.c. It removes the START_THREAD_DEFN and START_THREAD_SELF macros and make the lock usage more clear (no need to cross-reference multiple files). No functional change. --- nptl/createthread.c | 153 ------------------------------------------ nptl/pthread_create.c | 120 +++++++++++++++++++++++++++++++-- 2 files changed, 113 insertions(+), 160 deletions(-) delete mode 100644 nptl/createthread.c diff --git a/nptl/createthread.c b/nptl/createthread.c deleted file mode 100644 index bc3409b326..0000000000 --- a/nptl/createthread.c +++ /dev/null @@ -1,153 +0,0 @@ -/* Low-level thread creation for NPTL. Linux version. - Copyright (C) 2002-2021 Free Software Foundation, Inc. - This file is part of the GNU C Library. - Contributed by Ulrich Drepper , 2002. - - The GNU C Library is free software; you can redistribute it and/or - modify it under the terms of the GNU Lesser General Public - License as published by the Free Software Foundation; either - version 2.1 of the License, or (at your option) any later version. - - The GNU C Library is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - Lesser General Public License for more details. - - You should have received a copy of the GNU Lesser General Public - License along with the GNU C Library; if not, see - . */ - -#include -#include -#include -#include -#include -#include -#include -#include - -#include - -#ifdef __NR_clone2 -# define ARCH_CLONE __clone2 -#else -# define ARCH_CLONE __clone -#endif - -/* See the comments in pthread_create.c for the requirements for these - two macros and the create_thread function. */ - -#define START_THREAD_DEFN \ - static int __attribute__ ((noreturn)) start_thread (void *arg) -#define START_THREAD_SELF arg - -/* pthread_create.c defines this using START_THREAD_DEFN - We need a forward declaration here so we can take its address. */ -static int start_thread (void *arg) __attribute__ ((noreturn)); - -static int -create_thread (struct pthread *pd, const struct pthread_attr *attr, - bool *stopped_start, STACK_VARIABLES_PARMS, bool *thread_ran) -{ - /* Determine whether the newly created threads has to be started - stopped since we have to set the scheduling parameters or set the - affinity. */ - bool need_setaffinity = (attr != NULL && attr->extension != NULL - && attr->extension->cpuset != 0); - if (attr != NULL - && (__glibc_unlikely (need_setaffinity) - || __glibc_unlikely ((attr->flags & ATTR_FLAG_NOTINHERITSCHED) != 0))) - *stopped_start = true; - - pd->stopped_start = *stopped_start; - if (__glibc_unlikely (*stopped_start)) - /* See CONCURRENCY NOTES in nptl/pthread_creat.c. */ - lll_lock (pd->lock, LLL_PRIVATE); - - /* We rely heavily on various flags the CLONE function understands: - - CLONE_VM, CLONE_FS, CLONE_FILES - These flags select semantics with shared address space and - file descriptors according to what POSIX requires. - - CLONE_SIGHAND, CLONE_THREAD - This flag selects the POSIX signal semantics and various - other kinds of sharing (itimers, POSIX timers, etc.). - - CLONE_SETTLS - The sixth parameter to CLONE determines the TLS area for the - new thread. - - CLONE_PARENT_SETTID - The kernels writes the thread ID of the newly created thread - into the location pointed to by the fifth parameters to CLONE. - - Note that it would be semantically equivalent to use - CLONE_CHILD_SETTID but it is be more expensive in the kernel. - - CLONE_CHILD_CLEARTID - The kernels clears the thread ID of a thread that has called - sys_exit() in the location pointed to by the seventh parameter - to CLONE. - - The termination signal is chosen to be zero which means no signal - is sent. */ - const int clone_flags = (CLONE_VM | CLONE_FS | CLONE_FILES | CLONE_SYSVSEM - | CLONE_SIGHAND | CLONE_THREAD - | CLONE_SETTLS | CLONE_PARENT_SETTID - | CLONE_CHILD_CLEARTID - | 0); - - TLS_DEFINE_INIT_TP (tp, pd); - - if (__glibc_unlikely (ARCH_CLONE (&start_thread, STACK_VARIABLES_ARGS, - clone_flags, pd, &pd->tid, tp, &pd->tid) - == -1)) - return errno; - - /* It's started now, so if we fail below, we'll have to cancel it - and let it clean itself up. */ - *thread_ran = true; - - /* Now we have the possibility to set scheduling parameters etc. */ - if (attr != NULL) - { - int res; - - /* Set the affinity mask if necessary. */ - if (need_setaffinity) - { - assert (*stopped_start); - - res = INTERNAL_SYSCALL_CALL (sched_setaffinity, pd->tid, - attr->extension->cpusetsize, - attr->extension->cpuset); - - if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (res))) - err_out: - { - /* The operation failed. We have to kill the thread. - We let the normal cancellation mechanism do the work. */ - - pid_t pid = __getpid (); - INTERNAL_SYSCALL_CALL (tgkill, pid, pd->tid, SIGCANCEL); - - return INTERNAL_SYSCALL_ERRNO (res); - } - } - - /* Set the scheduling parameters. */ - if ((attr->flags & ATTR_FLAG_NOTINHERITSCHED) != 0) - { - assert (*stopped_start); - - res = INTERNAL_SYSCALL_CALL (sched_setscheduler, pd->tid, - pd->schedpolicy, &pd->schedparam); - - if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (res))) - goto err_out; - } - } - - return 0; -} diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c index 5680687efe..b53e3f30a0 100644 --- a/nptl/pthread_create.c +++ b/nptl/pthread_create.c @@ -215,9 +215,6 @@ late_init (void) /* CREATE THREAD NOTES: - createthread.c defines the create_thread function, and two macros: - START_THREAD_DEFN and START_THREAD_SELF (see below). - create_thread must initialize PD->stopped_start. It should be true if the STOPPED_START parameter is true, or if create_thread needs the new thread to synchronize at startup for some other implementation @@ -242,20 +239,129 @@ late_init (void) so create_thread need not do that. On failure, *THREAD_RAN should be set to true iff the thread actually started up and then got canceled before calling user code (*PD->start_routine). */ + +static int _Noreturn start_thread (void *arg); + static int create_thread (struct pthread *pd, const struct pthread_attr *attr, bool *stopped_start, STACK_VARIABLES_PARMS, - bool *thread_ran); + bool *thread_ran) +{ + /* Determine whether the newly created threads has to be started + stopped since we have to set the scheduling parameters or set the + affinity. */ + bool need_setaffinity = (attr != NULL && attr->extension != NULL + && attr->extension->cpuset != 0); + if (attr != NULL + && (__glibc_unlikely (need_setaffinity) + || __glibc_unlikely ((attr->flags & ATTR_FLAG_NOTINHERITSCHED) != 0))) + *stopped_start = true; + + pd->stopped_start = *stopped_start; + if (__glibc_unlikely (*stopped_start)) + lll_lock (pd->lock, LLL_PRIVATE); + + /* We rely heavily on various flags the CLONE function understands: + + CLONE_VM, CLONE_FS, CLONE_FILES + These flags select semantics with shared address space and + file descriptors according to what POSIX requires. + + CLONE_SIGHAND, CLONE_THREAD + This flag selects the POSIX signal semantics and various + other kinds of sharing (itimers, POSIX timers, etc.). + + CLONE_SETTLS + The sixth parameter to CLONE determines the TLS area for the + new thread. + + CLONE_PARENT_SETTID + The kernels writes the thread ID of the newly created thread + into the location pointed to by the fifth parameters to CLONE. + + Note that it would be semantically equivalent to use + CLONE_CHILD_SETTID but it is be more expensive in the kernel. + + CLONE_CHILD_CLEARTID + The kernels clears the thread ID of a thread that has called + sys_exit() in the location pointed to by the seventh parameter + to CLONE. + + The termination signal is chosen to be zero which means no signal + is sent. */ + const int clone_flags = (CLONE_VM | CLONE_FS | CLONE_FILES | CLONE_SYSVSEM + | CLONE_SIGHAND | CLONE_THREAD + | CLONE_SETTLS | CLONE_PARENT_SETTID + | CLONE_CHILD_CLEARTID + | 0); + + TLS_DEFINE_INIT_TP (tp, pd); + +#ifdef __NR_clone2 +# define ARCH_CLONE __clone2 +#else +# define ARCH_CLONE __clone +#endif + if (__glibc_unlikely (ARCH_CLONE (&start_thread, STACK_VARIABLES_ARGS, + clone_flags, pd, &pd->tid, tp, &pd->tid) + == -1)) + return errno; + + /* It's started now, so if we fail below, we'll have to cancel it + and let it clean itself up. */ + *thread_ran = true; -#include + /* Now we have the possibility to set scheduling parameters etc. */ + if (attr != NULL) + { + int res; + + /* Set the affinity mask if necessary. */ + if (need_setaffinity) + { + assert (*stopped_start); + + res = INTERNAL_SYSCALL_CALL (sched_setaffinity, pd->tid, + attr->extension->cpusetsize, + attr->extension->cpuset); + + if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (res))) + err_out: + { + /* The operation failed. We have to kill the thread. + We let the normal cancellation mechanism do the work. */ + + pid_t pid = __getpid (); + INTERNAL_SYSCALL_CALL (tgkill, pid, pd->tid, SIGCANCEL); + + return INTERNAL_SYSCALL_ERRNO (res); + } + } + + /* Set the scheduling parameters. */ + if ((attr->flags & ATTR_FLAG_NOTINHERITSCHED) != 0) + { + assert (*stopped_start); + + res = INTERNAL_SYSCALL_CALL (sched_setscheduler, pd->tid, + pd->schedpolicy, &pd->schedparam); + + if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (res))) + goto err_out; + } + } + + return 0; +} /* Local function to start thread and handle cleanup. createthread.c defines the macro START_THREAD_DEFN to the declaration that its create_thread function will refer to, and START_THREAD_SELF to the expression to optimally deliver the new thread's THREAD_SELF value. */ -START_THREAD_DEFN +static int _Noreturn +start_thread (void *arg) { - struct pthread *pd = START_THREAD_SELF; + struct pthread *pd = arg; /* Initialize resolver state pointer. */ __resp = &pd->res; From patchwork Wed May 26 16:57:20 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella Netto X-Patchwork-Id: 43591 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 4CB943945C15; Wed, 26 May 2021 16:57:41 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 4CB943945C15 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1622048261; bh=L5keZK/39HtxFWJrH344+W6ipg1rRcsJSxlDHiGvT5I=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=oSjrwRQdqrjlWTdj7AVCaIGl948jLSLMw6NhrWHyCJMY2kfkIIdwg+dwnBTxYrjRk cKjR8HZpt3eMFh2LljXw1+MICyyY4miexe3CXCLTz9Ocx0sjyCGR+S0OMJxLubavIJ GpnoDh0phTVU7D2WqJdHeF5BeTVyBuAVYZwfZx0A= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-qv1-xf29.google.com (mail-qv1-xf29.google.com [IPv6:2607:f8b0:4864:20::f29]) by sourceware.org (Postfix) with ESMTPS id E4F41384782E for ; Wed, 26 May 2021 16:57:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org E4F41384782E Received: by mail-qv1-xf29.google.com with SMTP id e8so1076623qvp.7 for ; Wed, 26 May 2021 09:57:37 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=L5keZK/39HtxFWJrH344+W6ipg1rRcsJSxlDHiGvT5I=; b=pbk08jmbzGva51XkWqWdgrKh32QhQ/klhctHy+OJCB+jrTkQHl9bMs693BMHXrmRaG qxV4/PB3arnndlv2KyaEC+SSSIpF747RJj0MjqkygdEf1T4y2K9HpyuLN3nWud+cMcsy wPCs5WOqBKxDGewV4qEdCdawL6nW/oIunu29gNkfFzsJxKMI/TPISisk1Tso4l3HEtG4 v1+pZkpj6yhk0j7oYvl3rCqD8bG2KIwFVjg53HN9rLno0mxjtm50nMYrNhndFFKY+yvB LLYYHmvqAT6kBl207mhjB+eqq9mXoCGVnebsYKm+qpMq21ZNAUqCol0fPcaxX/O0cdDb rDxQ== X-Gm-Message-State: AOAM533xYhlLSP7ScQG46qtj/sTXQxw+i3QS0fiZra/zMc4dejGT8Wwt 3iMcgoi8HHH0MhEaXMQMoaD5bJkbeyB9xQ== X-Google-Smtp-Source: ABdhPJxQ6JGQSfeQtXkClRkT7iVbXR7fCnGUomHiaJXDmfzoZQepYbx4lB3NL0PjSD3k78TFuSbJaQ== X-Received: by 2002:a05:6214:207:: with SMTP id i7mr10661057qvt.10.1622048257357; Wed, 26 May 2021 09:57:37 -0700 (PDT) Received: from birita.. ([177.194.37.86]) by smtp.googlemail.com with ESMTPSA id k125sm1859761qkf.53.2021.05.26.09.57.36 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 May 2021 09:57:37 -0700 (PDT) To: libc-alpha@sourceware.org Subject: [PATCH 03/11] support: Add xpthread_attr_setaffinity_np wrapper Date: Wed, 26 May 2021 13:57:20 -0300 Message-Id: <20210526165728.1772546-4-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210526165728.1772546-1-adhemerval.zanella@linaro.org> References: <20210526165728.1772546-1-adhemerval.zanella@linaro.org> MIME-Version: 1.0 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, KAM_SHORT, 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-Patchwork-Original-From: Adhemerval Zanella via Libc-alpha From: Adhemerval Zanella Netto Reply-To: Adhemerval Zanella Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" --- support/Makefile | 1 + support/xpthread_attr_setaffinity_np.c | 28 ++++++++++++++++++++++++++ support/xthread.h | 3 +++ 3 files changed, 32 insertions(+) create mode 100644 support/xpthread_attr_setaffinity_np.c diff --git a/support/Makefile b/support/Makefile index da6dc58d37..0a4b057db5 100644 --- a/support/Makefile +++ b/support/Makefile @@ -128,6 +128,7 @@ libsupport-routines = \ xpthread_attr_init \ xpthread_attr_setdetachstate \ xpthread_attr_setguardsize \ + xpthread_attr_setaffinity_np \ xpthread_attr_setstack \ xpthread_attr_setstacksize \ xpthread_barrier_destroy \ diff --git a/support/xpthread_attr_setaffinity_np.c b/support/xpthread_attr_setaffinity_np.c new file mode 100644 index 0000000000..cddcd5fdb4 --- /dev/null +++ b/support/xpthread_attr_setaffinity_np.c @@ -0,0 +1,28 @@ +/* pthread_attr_setaffinity_np with error checking. + Copyright (C) 2021 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#include + +void +xpthread_attr_setaffinity_np (pthread_attr_t *attr, + size_t cpusetsize, const cpu_set_t *cpuset) +{ + xpthread_check_return ("pthread_attr_setaffinity_np", + pthread_attr_setaffinity_np (attr, cpusetsize, + cpuset)); +} diff --git a/support/xthread.h b/support/xthread.h index 1ba3f133ad..cffd9c1694 100644 --- a/support/xthread.h +++ b/support/xthread.h @@ -66,6 +66,9 @@ void *xpthread_join (pthread_t thr); void xpthread_once (pthread_once_t *guard, void (*func) (void)); void xpthread_attr_destroy (pthread_attr_t *attr); void xpthread_attr_init (pthread_attr_t *attr); +void xpthread_attr_setaffinity_np (pthread_attr_t *attr, + size_t cpusetsize, + const cpu_set_t *cpuset); void xpthread_attr_setdetachstate (pthread_attr_t *attr, int detachstate); void xpthread_attr_setstack (pthread_attr_t *attr, void *stackaddr, From patchwork Wed May 26 16:57:21 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella Netto X-Patchwork-Id: 43592 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 624F73947406; Wed, 26 May 2021 16:57:43 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 624F73947406 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1622048263; bh=pXd8FhvikOXsIQebBWH/AUdyZ+Toak4nFDEyrEZcZqY=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=rb/zCzZEk18vsnjw8+qlnrAkpuPG4QiQ2peBDx9vZykqXLGTUj4oxwyVZX+H+dNfe CeXsEvemZuz2IGLKcAnBKVnU0PYDzVLeE3XxB9VfTUIsllakhKxAbellYowm+pOqo6 dnRMMYojNKc/Ujwg9LvflT2qDhWbB2vW2dKiS5pA= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-qv1-xf2a.google.com (mail-qv1-xf2a.google.com [IPv6:2607:f8b0:4864:20::f2a]) by sourceware.org (Postfix) with ESMTPS id 284073846400 for ; Wed, 26 May 2021 16:57:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 284073846400 Received: by mail-qv1-xf2a.google.com with SMTP id ee9so1075992qvb.8 for ; Wed, 26 May 2021 09:57:39 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=pXd8FhvikOXsIQebBWH/AUdyZ+Toak4nFDEyrEZcZqY=; b=SjBmrDSmS/bU0/5Ji8cmdTdJCmVn/Y/UPqBU4nc05EerjALmmZAiqVgN1MKvSmvuZ+ WXY95v+Q/t6csN48al+qsAR/GpSEcR1ODqQ+zF2fu00P/fOzoPHlCIW8HtweOkG2ZMgd aHc3xCVU8I1fbVZlH1xvosInQbNmGqx+tNv2az1n9Ao2xRYyaBTIr+ZGi6d0FeMD+wrP Lc7+JU5ez53CV/i6d9Nj4JYUHJdQ539hSQPs6PpcT8SF/Mm72DoIjHZp/OSASH/cwK4M Bj2VzScDMoeYN1xtTvjyNkhwZ8SmOnI2264W3DdETvDvUUHkm9ofzSZqPPFUFVoCQhq0 4oKQ== X-Gm-Message-State: AOAM5329SbL6DNhGLaTzi/OyM219GtLB+vZc8F+goe/Qrr7nwhvSJY5p UZ0QskoUnAUxw42TqveCvAMuiVccI55r2Q== X-Google-Smtp-Source: ABdhPJyl2TdgQj9h1ueOs4LlDKQraPycAXXfivO2qaPF+GXRuzF3rvNM6tpgYb0Ur9infD4J7lmJCg== X-Received: by 2002:a0c:e847:: with SMTP id l7mr32706843qvo.44.1622048258576; Wed, 26 May 2021 09:57:38 -0700 (PDT) Received: from birita.. ([177.194.37.86]) by smtp.googlemail.com with ESMTPSA id k125sm1859761qkf.53.2021.05.26.09.57.37 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 May 2021 09:57:38 -0700 (PDT) To: libc-alpha@sourceware.org Subject: [PATCH 04/11] nptl: Add pthread_attr_setaffinity_np failure test Date: Wed, 26 May 2021 13:57:21 -0300 Message-Id: <20210526165728.1772546-5-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210526165728.1772546-1-adhemerval.zanella@linaro.org> References: <20210526165728.1772546-1-adhemerval.zanella@linaro.org> MIME-Version: 1.0 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, KAM_SHORT, 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-Patchwork-Original-From: Adhemerval Zanella via Libc-alpha From: Adhemerval Zanella Netto Reply-To: Adhemerval Zanella Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" It checks whether an invalid affinity mask does return an error, similar to what sysdeps/pthread/tst-bad-schedattr.c does for pthread_attr_setschedparam. Checked on x86_64-linux-gnu. --- nptl/Makefile | 1 + nptl/tst-pthread-attr-affinity-fail.c | 54 +++++++++++++++++++++++++++ 2 files changed, 55 insertions(+) create mode 100644 nptl/tst-pthread-attr-affinity-fail.c diff --git a/nptl/Makefile b/nptl/Makefile index 16eaf58948..9a5628b751 100644 --- a/nptl/Makefile +++ b/nptl/Makefile @@ -286,6 +286,7 @@ tests = tst-attr2 tst-attr3 tst-default-attr \ tst-exec4 tst-exec5 \ tst-stack2 tst-stack3 tst-stack4 \ tst-pthread-attr-affinity \ + tst-pthread-attr-affinity-fail \ tst-dlsym1 \ tst-context1 \ tst-sched1 \ diff --git a/nptl/tst-pthread-attr-affinity-fail.c b/nptl/tst-pthread-attr-affinity-fail.c new file mode 100644 index 0000000000..41a87ea8cb --- /dev/null +++ b/nptl/tst-pthread-attr-affinity-fail.c @@ -0,0 +1,54 @@ +/* Check if invalid pthread_attr_getaffinity_np does not run any code + in the thread function. + Copyright (C) 2021 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#include +#include +#include +#include +#include + +static void * +thr_func (void *arg) +{ + abort (); + return NULL; +} + +static int +do_test (void) +{ + int max_cpu = xsysconf (_SC_NPROCESSORS_CONF) + 1; + /* Set a affinaty mask with an invalid CPU. */ + cpu_set_t *cpuset = CPU_ALLOC (max_cpu); + size_t cpusetsize = CPU_ALLOC_SIZE (max_cpu); + CPU_ZERO_S (cpusetsize, cpuset); + CPU_SET_S (max_cpu, cpusetsize, cpuset); + + pthread_attr_t attr; + xpthread_attr_init (&attr); + xpthread_attr_setaffinity_np (&attr, cpusetsize, cpuset); + + pthread_t thr; + TEST_COMPARE (pthread_create (&thr, &attr, thr_func, NULL), EINVAL); + xpthread_attr_destroy (&attr); + + return 0; +} + +#include From patchwork Wed May 26 16:57:22 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella Netto X-Patchwork-Id: 43593 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id E96933947413; Wed, 26 May 2021 16:57:43 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org E96933947413 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1622048264; bh=HDLnmApGXWm8JEKgdi/vqXf6THwgxEL72F8RErSqxeo=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=S2mHUgFEW5tWnYRfudMgZpmI0tBfNHDca0WOmHvBx7K8E/GvPtpZw9nlfLtsXXCRN kwoPyg5FOyKIMHnA7h3+4ZGS7EXrHucf/u9CLes5kPQ0ueqZVrC/dsHFaXCeXL4PeA Xl4jaj2ER8EYH6SMe7SWKR5z4wxlJM7iPhqMGV5g= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-qv1-xf36.google.com (mail-qv1-xf36.google.com [IPv6:2607:f8b0:4864:20::f36]) by sourceware.org (Postfix) with ESMTPS id 7EEF93846416 for ; Wed, 26 May 2021 16:57:40 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 7EEF93846416 Received: by mail-qv1-xf36.google.com with SMTP id ez19so1089192qvb.3 for ; Wed, 26 May 2021 09:57:40 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=HDLnmApGXWm8JEKgdi/vqXf6THwgxEL72F8RErSqxeo=; b=ZJZmpUplLcFQhkfZUk8qR3RZHEpfyyTUtmf4MtTd8GJrCfg1qw6+CQD4qn6iuUQibi PExS65qLyLkxyYpbuMqb2MymXj9xhfDBRQvPxXR4Ud0yfQWNh7H9shoOtyLBepFGVzoC vZCzKtczQDgQJ+x5Aqw4Rj8v2PN6hyxNs3ItCPp/Cgy0xdIPwykWckSTjy8NsvS4nd2E 84VEJf0F2CsPaWZi30lTRisfFyt1BFagXpOOBUpryFWI24dhX5SLDiPrkrkAG7Xc3U1O gdp3c9unUhE3EmdLg9bvGf7Lik+u/NasB7XUvALjcNAZbuHoeigG1b3GD6luxamMSgiA PT2w== X-Gm-Message-State: AOAM532gC3LNIdnn5wS5NW3QxF5owWQlFmeXOys7p7VBpMyG/0+LiVFo iZ3m7pOk78NzzpBHauUQV8Gj8za6jRD3KQ== X-Google-Smtp-Source: ABdhPJwV7+BuN1yDwtQf03UTzPSjqI6BAU2DCKNjqW0ae8620yfZS4rAjsW2cMwhuqVrqgR9WqGk5Q== X-Received: by 2002:a0c:c391:: with SMTP id o17mr16470638qvi.3.1622048259804; Wed, 26 May 2021 09:57:39 -0700 (PDT) Received: from birita.. ([177.194.37.86]) by smtp.googlemail.com with ESMTPSA id k125sm1859761qkf.53.2021.05.26.09.57.38 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 May 2021 09:57:39 -0700 (PDT) To: libc-alpha@sourceware.org Subject: [PATCH 05/11] nptl: Deallocate the thread stack on setup failure (BZ #19511) Date: Wed, 26 May 2021 13:57:22 -0300 Message-Id: <20210526165728.1772546-6-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210526165728.1772546-1-adhemerval.zanella@linaro.org> References: <20210526165728.1772546-1-adhemerval.zanella@linaro.org> MIME-Version: 1.0 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, 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-Patchwork-Original-From: Adhemerval Zanella via Libc-alpha From: Adhemerval Zanella Netto Reply-To: Adhemerval Zanella Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" To setup either the thread scheduling parameters or affinity, pthread_create enforce synchronization on created thread wait until its parent either unlock the 'struct pthread' or send a cancellation signal if a failure occurs. However, cancelling the thread does not deallocate the newly created stack since cancellation expects that a pthread_join to deallocate any allocated thread resouces (threads stack or TLS). I used a different strategy than the one proposed on BZ#19511 comment #4. Making the parent responsible for the cleanup requires additional synchronization similar to what pthread_join does. Instead, this patch reassigns an unused 'struct pthread' member, parent_cancelhandling_unsed, to indicate whether the setup has failed and set the thread itself to deallocate the allocated resouces (similar to what detached mode does). This strategy also simplifies by not using thread cancellation and thus not running libgcc_so load in the signal handler (which is avoided in thread cancellation since 'pthread_cancel' is the one responsible to dlopen libgcc_s). Checked on x86_64-linux-gnu and aarch64-linux-gnu. --- nptl/allocatestack.c | 1 + nptl/descr.h | 5 ++-- nptl/pthread_create.c | 56 +++++++++++++++++++------------------------ 3 files changed, 29 insertions(+), 33 deletions(-) diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c index dc81a2ca73..2114bd2e27 100644 --- a/nptl/allocatestack.c +++ b/nptl/allocatestack.c @@ -161,6 +161,7 @@ get_cached_stack (size_t *sizep, void **memp) /* Cancellation handling is back to the default. */ result->cancelhandling = 0; result->cleanup = NULL; + result->setup_failed = 0; /* No pending event. */ result->nextevent = NULL; diff --git a/nptl/descr.h b/nptl/descr.h index 3de9535449..9d8297b45f 100644 --- a/nptl/descr.h +++ b/nptl/descr.h @@ -340,8 +340,9 @@ struct pthread /* True if thread must stop at startup time. */ bool stopped_start; - /* Formerly used for dealing with cancellation. */ - int parent_cancelhandling_unsed; + /* Indicate that a thread creation setup has failed (for instance the + scheduler or affinity). */ + int setup_failed; /* Lock to synchronize access to the descriptor. */ int lock; diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c index b53e3f30a0..d6b907827a 100644 --- a/nptl/pthread_create.c +++ b/nptl/pthread_create.c @@ -176,8 +176,6 @@ late_init (void) state. Ownership cannot be released to the process via the return of pthread_create since a non-zero result entails PD is undefined and therefore cannot be joined to free the resources. - We privately call pthread_join on the thread to finish handling - the resource shutdown (Or at least we should, see bug 19511). (e) If the thread creation failed and THREAD_RAN is false, then the creating thread retains ownership of PD and must cleanup state. @@ -313,28 +311,19 @@ static int create_thread (struct pthread *pd, const struct pthread_attr *attr, /* Now we have the possibility to set scheduling parameters etc. */ if (attr != NULL) { - int res; - /* Set the affinity mask if necessary. */ if (need_setaffinity) { assert (*stopped_start); - res = INTERNAL_SYSCALL_CALL (sched_setaffinity, pd->tid, - attr->extension->cpusetsize, - attr->extension->cpuset); - + int res = INTERNAL_SYSCALL_CALL (sched_setaffinity, pd->tid, + attr->extension->cpusetsize, + attr->extension->cpuset); if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (res))) - err_out: - { - /* The operation failed. We have to kill the thread. - We let the normal cancellation mechanism do the work. */ - - pid_t pid = __getpid (); - INTERNAL_SYSCALL_CALL (tgkill, pid, pd->tid, SIGCANCEL); - - return INTERNAL_SYSCALL_ERRNO (res); - } + { + pd->setup_failed = 1; + return INTERNAL_SYSCALL_ERRNO (res); + } } /* Set the scheduling parameters. */ @@ -342,11 +331,13 @@ static int create_thread (struct pthread *pd, const struct pthread_attr *attr, { assert (*stopped_start); - res = INTERNAL_SYSCALL_CALL (sched_setscheduler, pd->tid, - pd->schedpolicy, &pd->schedparam); - + int res = INTERNAL_SYSCALL_CALL (sched_setscheduler, pd->tid, + pd->schedpolicy, &pd->schedparam); if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (res))) - goto err_out; + { + pd->setup_failed = 1; + return INTERNAL_SYSCALL_ERRNO (res); + } } } @@ -426,8 +417,6 @@ start_thread (void *arg) have ownership (see CONCURRENCY NOTES above). */ if (__glibc_unlikely (pd->stopped_start)) { - int oldtype = LIBC_CANCEL_ASYNC (); - /* Get the lock the parent locked to force synchronization. */ lll_lock (pd->lock, LLL_PRIVATE); @@ -435,10 +424,11 @@ start_thread (void *arg) /* And give it up right away. */ lll_unlock (pd->lock, LLL_PRIVATE); - - LIBC_CANCEL_RESET (oldtype); } + if (pd->setup_failed == 1) + __do_cancel (); + LIBC_PROBE (pthread_start, 3, (pthread_t) pd, pd->start_routine, pd->arg); /* Run the code the user provided. */ @@ -563,8 +553,8 @@ start_thread (void *arg) pd->setxid_futex = 0; } - /* If the thread is detached free the TCB. */ - if (IS_DETACHED (pd)) + /* If the thread is detached or an setup error occurred, free the TCB. */ + if (IS_DETACHED (pd) || pd->setup_failed == 1) /* Free the TCB. */ __nptl_free_tcb (pd); @@ -819,9 +809,13 @@ __pthread_create_2_1 (pthread_t *newthread, const pthread_attr_t *attr, CONCURRENCY NOTES above). We can assert that STOPPED_START must have been true because thread creation didn't fail, but thread attribute setting did. */ - /* See bug 19511 which explains why doing nothing here is a - resource leak for a joinable thread. */ - assert (stopped_start); + { + assert (stopped_start); + if (stopped_start) + /* State (a), we own PD. The thread blocked on this lock will + finish due setup_failed being set. */ + lll_unlock (pd->lock, LLL_PRIVATE); + } else { /* State (e) and we have ownership of PD (see CONCURRENCY From patchwork Wed May 26 16:57:23 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella Netto X-Patchwork-Id: 43594 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 7B4F2394741E; Wed, 26 May 2021 16:57:44 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 7B4F2394741E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1622048264; bh=vg2J2VhPqeuYYoYX8FiumS0FdI/jZmcamVLTAqP/Tvc=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=Qd5J9rte0s9dlIYf5OsqxLwYa4LnjQ+Xv0rs1b2FIBC+42q1RxZ0kkCwmzTzZgRgm m5002JlM9gtgaie4DWeujxCnW1DkXfHBA0Vphq76x8yPyWqGaInyBqt8d4hYqyrjST 3nYsF9DHKJzQxzm7hcONRDvIRV3kqTN3CpyzFNrc= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-qv1-xf2d.google.com (mail-qv1-xf2d.google.com [IPv6:2607:f8b0:4864:20::f2d]) by sourceware.org (Postfix) with ESMTPS id B556D3846410 for ; Wed, 26 May 2021 16:57:41 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org B556D3846410 Received: by mail-qv1-xf2d.google.com with SMTP id 5so1105337qvk.0 for ; Wed, 26 May 2021 09:57:41 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=vg2J2VhPqeuYYoYX8FiumS0FdI/jZmcamVLTAqP/Tvc=; b=KS2qYgUbumlaaPLYtN3whDb9QGaHWL2XBxn097xqR04//T36dmwNBQ+LCbMH/YBkmJ ot7F7Ri8CK7CReSpqyOrVOYV+hTNuZURdn6RzJfhahklJVRthn8QlxToPDTUWgEjSaof lP8r12eKPJFNlo9jPXDeLAjwCMk48aj1jdIB6Z0ve65qs7RY8uX1GFqHrbO/hVv0VuD/ KCqy7z04d7JWqGUt5bNglIkxWH/ZL3UHFCpMPs0EF2ErRA19UTY6rFX3YS3z0+s6Pnne 1ReOMDDv+9mnm6yyli9jgwDapGOlLK7py8zm0+EHrHYz/iDaXo/6PcIZ+MOtQok0WxjS XF1Q== X-Gm-Message-State: AOAM532qLae/W+OOxocc4C/p69h7hPA3fmjmliLG8euEfD+c9c9F8E2q DELqCeZkBEY8sZcH/dWmakeyuc3E2GTyWQ== X-Google-Smtp-Source: ABdhPJwupXzVxFzqPt1Poqf2Nj/yqGzvS5yttfzuYuMBrbgpUT0yXT0BXub2q54Q/kbBv5AR/9BOtw== X-Received: by 2002:a0c:fa4e:: with SMTP id k14mr44685419qvo.51.1622048261094; Wed, 26 May 2021 09:57:41 -0700 (PDT) Received: from birita.. ([177.194.37.86]) by smtp.googlemail.com with ESMTPSA id k125sm1859761qkf.53.2021.05.26.09.57.40 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 May 2021 09:57:40 -0700 (PDT) To: libc-alpha@sourceware.org Subject: [PATCH 06/11] nptl: Install cancellation handler on pthread_cancel Date: Wed, 26 May 2021 13:57:23 -0300 Message-Id: <20210526165728.1772546-7-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210526165728.1772546-1-adhemerval.zanella@linaro.org> References: <20210526165728.1772546-1-adhemerval.zanella@linaro.org> MIME-Version: 1.0 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, 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-Patchwork-Original-From: Adhemerval Zanella via Libc-alpha From: Adhemerval Zanella Netto Reply-To: Adhemerval Zanella Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" Now that cancellation is not used anymore to handle thread setup creation failure, the sighandle can be installed only when pthread_cancel is actually used. Checked on x86_64-linux-gnu and aarch64-linux-gnu. --- nptl/Versions | 3 +-- nptl/pthreadP.h | 6 ------ nptl/pthread_cancel.c | 49 ++++++++++++++++++++++++------------------- nptl/pthread_create.c | 15 ------------- 4 files changed, 28 insertions(+), 45 deletions(-) diff --git a/nptl/Versions b/nptl/Versions index af62a47cca..590761e730 100644 --- a/nptl/Versions +++ b/nptl/Versions @@ -395,7 +395,6 @@ libc { __nptl_free_tcb; __nptl_nthreads; __nptl_setxid_sighandler; - __nptl_sigcancel_handler; __nptl_stack_list_add; __nptl_stack_list_del; __pthread_attr_copy; @@ -514,4 +513,4 @@ ld { __nptl_initial_report_events; __nptl_set_robust_list_avail; } -} \ No newline at end of file +} diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h index 05f2bae521..48d48e7008 100644 --- a/nptl/pthreadP.h +++ b/nptl/pthreadP.h @@ -569,12 +569,6 @@ libc_hidden_proto (__pthread_attr_setsigmask_internal) extern __typeof (pthread_attr_getsigmask_np) __pthread_attr_getsigmask_np; libc_hidden_proto (__pthread_attr_getsigmask_np) -/* The cancellation signal handler defined alongside with - pthread_cancel. This is included in statically linked binaries - only if pthread_cancel is linked in. */ -void __nptl_sigcancel_handler (int sig, siginfo_t *si, void *ctx); -libc_hidden_proto (__nptl_sigcancel_handler) - /* Special versions which use non-exported functions. */ extern void __pthread_cleanup_push (struct _pthread_cleanup_buffer *buffer, void (*routine) (void *), void *arg); diff --git a/nptl/pthread_cancel.c b/nptl/pthread_cancel.c index 802c691874..f264a4096a 100644 --- a/nptl/pthread_cancel.c +++ b/nptl/pthread_cancel.c @@ -28,11 +28,19 @@ #include #include -/* For asynchronous cancellation we use a signal. This is the core - logic of the signal handler. */ +/* For asynchronous cancellation we use a signal. */ static void -sigcancel_handler (void) +sigcancel_handler (int sig, siginfo_t *si, void *ctx) { + /* Safety check. It would be possible to call this function for + other signals and send a signal from another process. This is not + correct and might even be a security problem. Try to catch as + many incorrect invocations as possible. */ + if (sig != SIGCANCEL + || si->si_pid != __getpid() + || si->si_code != SI_TKILL) + return; + struct pthread *self = THREAD_SELF; int oldval = THREAD_GETMEM (self, cancelhandling); @@ -66,24 +74,6 @@ sigcancel_handler (void) } } -/* This is the actually installed SIGCANCEL handler. It adds some - safety checks before performing the cancellation. */ -void -__nptl_sigcancel_handler (int sig, siginfo_t *si, void *ctx) -{ - /* Safety check. It would be possible to call this function for - other signals and send a signal from another process. This is not - correct and might even be a security problem. Try to catch as - many incorrect invocations as possible. */ - if (sig != SIGCANCEL - || si->si_pid != __getpid() - || si->si_code != SI_TKILL) - return; - - sigcancel_handler (); -} -libc_hidden_def (__nptl_sigcancel_handler) - int __pthread_cancel (pthread_t th) { @@ -94,6 +84,17 @@ __pthread_cancel (pthread_t th) /* Not a valid thread handle. */ return ESRCH; + static int init_sigcancel = 0; + if (atomic_load_relaxed (&init_sigcancel) == 0) + { + struct sigaction sa; + sa.sa_sigaction = sigcancel_handler; + sa.sa_flags = SA_SIGINFO; + __sigemptyset (&sa.sa_mask); + __libc_sigaction (SIGCANCEL, &sa, NULL); + atomic_store_relaxed (&init_sigcancel, 1); + } + #ifdef SHARED /* Trigger an error if libgcc_s cannot be loaded. */ { @@ -134,7 +135,11 @@ __pthread_cancel (pthread_t th) call pthread_cancel (pthread_self ()) without calling pthread_create, so the signal handler may not have been set up for a self-cancel. */ - sigcancel_handler (); + { + THREAD_SETMEM (pd, result, PTHREAD_CANCELED); + if ((newval & CANCELTYPE_BITMASK) != 0) + __do_cancel (); + } else { /* The cancellation handler will take care of marking the diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c index d6b907827a..44d135212d 100644 --- a/nptl/pthread_create.c +++ b/nptl/pthread_create.c @@ -68,21 +68,6 @@ late_init (void) struct sigaction sa; __sigemptyset (&sa.sa_mask); - /* Install the cancellation signal handler (in static builds only if - pthread_cancel has been linked in). If for some reason we cannot - install the handler we do not abort. Maybe we should, but it is - only asynchronous cancellation which is affected. */ -#ifndef SHARED - extern __typeof (__nptl_sigcancel_handler) __nptl_sigcancel_handler - __attribute__ ((weak)); - if (__nptl_sigcancel_handler != NULL) -#endif - { - sa.sa_sigaction = __nptl_sigcancel_handler; - sa.sa_flags = SA_SIGINFO; - (void) __libc_sigaction (SIGCANCEL, &sa, NULL); - } - /* Install the handle to change the threads' uid/gid. */ sa.sa_sigaction = __nptl_setxid_sighandler; sa.sa_flags = SA_SIGINFO | SA_RESTART; From patchwork Wed May 26 16:57:24 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella Netto X-Patchwork-Id: 43595 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 1A2A93945C30; Wed, 26 May 2021 16:57:46 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 1A2A93945C30 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1622048266; bh=u98OM6DsDp3rqugcyn91ND8u7aMGLkiuWD0MQ9n6/1I=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=vkgOWJihR6y7FS0AU+rwQRLkAeCgCEorifNYENwlpJyxXCZdOUF7H1L7NpxQSbk93 5+Rwr7rsu3h78OKtRTLWpEvRx0Iq3S0Zia2isjaIPTYrPaf2LexCJkbMimkU96qPUO xNkNCrfRCxhNMCVwmEAN9my0uBNgBkx1JKC2yIo0= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-qk1-x72b.google.com (mail-qk1-x72b.google.com [IPv6:2607:f8b0:4864:20::72b]) by sourceware.org (Postfix) with ESMTPS id 310533945C31 for ; Wed, 26 May 2021 16:57:43 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 310533945C31 Received: by mail-qk1-x72b.google.com with SMTP id i5so1641536qkf.12 for ; Wed, 26 May 2021 09:57:43 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=u98OM6DsDp3rqugcyn91ND8u7aMGLkiuWD0MQ9n6/1I=; b=UYZZQEsFTN4S+00gYwBlXWuqCmVwC6oXo8/dG9RJgUBJqEPvueCDduAxYkj7JR1lFm /AjWCkkiPXGTIxRVDWoBgPabZqflO7Ku+BCPU+lVFZYyx833GUO4PG9yfgJqJ+171Am+ 5uPXqeQ/4lvr2C/CSPdkk/BNhgZvAU0PKIKF5pi/M6D0lTxWf8ofAmfUp4OT+8MzakJG 0vshtzRM6/QKxVUVoF7G2mvL2jDx0b6KbveT+/s6wz8Y9kQHgu+qbC2Z33rVrv1qNCSJ acFdzdy+We8NlaitmKGwB/mThIzDBhbqDLuCxBUPqFphAHn+5VLyHACQza2xawRWttrG DSqg== X-Gm-Message-State: AOAM533Lggmns2kwdE4FE0zCSdQGFneLMdTe8x8HDFbEFdEm2kwLPfEe 6YLe+WVaBUNMUebTDa5Z2BlTQKNU2vvI9A== X-Google-Smtp-Source: ABdhPJyvOS+9BGhZZdKGN4cHePbU6CaNQOmjLdIE0nCisNcSvbV3HaCNUffOczsOXuHu/HpBAgLzsA== X-Received: by 2002:a05:620a:15e3:: with SMTP id p3mr41933272qkm.115.1622048262304; Wed, 26 May 2021 09:57:42 -0700 (PDT) Received: from birita.. ([177.194.37.86]) by smtp.googlemail.com with ESMTPSA id k125sm1859761qkf.53.2021.05.26.09.57.41 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 May 2021 09:57:42 -0700 (PDT) To: libc-alpha@sourceware.org Subject: [PATCH 07/11] nptl: Remove CANCELING_BITMASK Date: Wed, 26 May 2021 13:57:24 -0300 Message-Id: <20210526165728.1772546-8-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210526165728.1772546-1-adhemerval.zanella@linaro.org> References: <20210526165728.1772546-1-adhemerval.zanella@linaro.org> MIME-Version: 1.0 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, 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-Patchwork-Original-From: Adhemerval Zanella via Libc-alpha From: Adhemerval Zanella Netto Reply-To: Adhemerval Zanella Errors-To: libc-alpha-bounces@sourceware.org Sender: "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). Checked on x86_64-linux-gnu and aarch64-linux-gnu. --- nptl/cancellation.c | 12 ---- nptl/descr.h | 3 - nptl/pthread_cancel.c | 124 ++++++++++--------------------------- nptl/pthread_join_common.c | 2 +- 4 files changed, 35 insertions(+), 106 deletions(-) diff --git a/nptl/cancellation.c b/nptl/cancellation.c index c20845adc0..b15f25d8f6 100644 --- a/nptl/cancellation.c +++ b/nptl/cancellation.c @@ -88,17 +88,5 @@ __pthread_disable_asynccancel (int oldtype) /* Prepare the next round. */ oldval = curval; } - - /* We cannot return when we are being canceled. Upon return the - thread might be things which would have to be undone. The - following loop should loop until the cancellation signal is - delivered. */ - while (__builtin_expect ((newval & (CANCELING_BITMASK | CANCELED_BITMASK)) - == CANCELING_BITMASK, 0)) - { - futex_wait_simple ((unsigned int *) &self->cancelhandling, newval, - FUTEX_PRIVATE); - newval = THREAD_GETMEM (self, cancelhandling); - } } libc_hidden_def (__pthread_disable_asynccancel) diff --git a/nptl/descr.h b/nptl/descr.h index 9d8297b45f..a120365f88 100644 --- a/nptl/descr.h +++ b/nptl/descr.h @@ -283,9 +283,6 @@ struct pthread /* Bit set if asynchronous cancellation mode is selected. */ #define CANCELTYPE_BIT 1 #define CANCELTYPE_BITMASK (0x01 << CANCELTYPE_BIT) - /* Bit set if canceling has been initiated. */ -#define CANCELING_BIT 2 -#define CANCELING_BITMASK (0x01 << CANCELING_BIT) /* Bit set if canceled. */ #define CANCELED_BIT 3 #define CANCELED_BITMASK (0x01 << CANCELED_BIT) diff --git a/nptl/pthread_cancel.c b/nptl/pthread_cancel.c index f264a4096a..c11a2b4551 100644 --- a/nptl/pthread_cancel.c +++ b/nptl/pthread_cancel.c @@ -43,35 +43,18 @@ sigcancel_handler (int sig, siginfo_t *si, void *ctx) struct pthread *self = THREAD_SELF; - int oldval = THREAD_GETMEM (self, cancelhandling); - while (1) - { - /* We are canceled now. When canceled by another thread this flag - is already set but if the signal is directly send (internally or - from another process) is has to be done here. */ - int newval = oldval | CANCELING_BITMASK | CANCELED_BITMASK; - - if (oldval == newval || (oldval & EXITING_BITMASK) != 0) - /* Already canceled or exiting. */ - break; - - int curval = THREAD_ATOMIC_CMPXCHG_VAL (self, cancelhandling, newval, - oldval); - if (curval == oldval) - { - /* Set the return value. */ - THREAD_SETMEM (self, result, PTHREAD_CANCELED); - - /* Make sure asynchronous cancellation is still enabled. */ - if ((newval & CANCELTYPE_BITMASK) != 0) - /* Run the registered destructors and terminate the thread. */ - __do_cancel (); - - break; - } - - oldval = curval; - } + int ch = atomic_load_relaxed (&self->cancelhandling); + /* Cancelation not enabled, not cancelled, or already exitting. */ + if ((ch & CANCELSTATE_BITMASK) != 0 + || (ch & CANCELED_BITMASK) == 0 + || (ch & EXITING_BITMASK) != 0) + return; + + /* Set the return value. */ + THREAD_SETMEM (self, result, PTHREAD_CANCELED); + /* Make sure asynchronous cancellation is still enabled. */ + if ((ch & CANCELTYPE_BITMASK) != 0) + __do_cancel (); } int @@ -104,72 +87,33 @@ __pthread_cancel (pthread_t th) " must be installed for pthread_cancel to work\n"); } #endif - int result = 0; - int oldval; - int newval; - do + + int oldch = atomic_fetch_or_acquire (&pd->cancelhandling, CANCELED_BITMASK); + if ((oldch & CANCELED_BITMASK) != 0) + return 0; + + if (pd == THREAD_SELF) { - again: - oldval = pd->cancelhandling; - newval = oldval | CANCELING_BITMASK | CANCELED_BITMASK; - - /* Avoid doing unnecessary work. The atomic operation can - potentially be expensive if the bug has to be locked and - remote cache lines have to be invalidated. */ - if (oldval == newval) - break; - - /* If the cancellation is handled asynchronously just send a - signal. We avoid this if possible since it's more - expensive. */ - if (CANCEL_ENABLED_AND_CANCELED_AND_ASYNCHRONOUS (newval)) - { - /* Mark the cancellation as "in progress". */ - if (atomic_compare_and_exchange_bool_acq (&pd->cancelhandling, - oldval | CANCELING_BITMASK, - oldval)) - goto again; - - if (pd == THREAD_SELF) - /* This is not merely an optimization: An application may - call pthread_cancel (pthread_self ()) without calling - pthread_create, so the signal handler may not have been - set up for a self-cancel. */ - { - THREAD_SETMEM (pd, result, PTHREAD_CANCELED); - if ((newval & CANCELTYPE_BITMASK) != 0) - __do_cancel (); - } - else - { - /* The cancellation handler will take care of marking the - thread as canceled. */ - pid_t pid = __getpid (); - - int val = INTERNAL_SYSCALL_CALL (tgkill, pid, pd->tid, - SIGCANCEL); - if (INTERNAL_SYSCALL_ERROR_P (val)) - result = INTERNAL_SYSCALL_ERRNO (val); - } - - break; - } - - /* 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); + /* 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; } - /* Mark the thread as canceled. This has to be done - atomically since other bits could be modified as well. */ - while (atomic_compare_and_exchange_bool_acq (&pd->cancelhandling, newval, - oldval)); - return result; + pid_t pid = __getpid (); + int val = INTERNAL_SYSCALL_CALL (tgkill, pid, pd->tid, SIGCANCEL); + return INTERNAL_SYSCALL_ERROR_P (val) + ? INTERNAL_SYSCALL_ERRNO (val) + : 0; } versioned_symbol (libc, __pthread_cancel, pthread_cancel, GLIBC_2_34); diff --git a/nptl/pthread_join_common.c b/nptl/pthread_join_common.c index e87801b5a3..f842c91a08 100644 --- a/nptl/pthread_join_common.c +++ b/nptl/pthread_join_common.c @@ -57,7 +57,7 @@ __pthread_clockjoin_ex (pthread_t threadid, void **thread_return, if ((pd == self || (self->joinid == pd && (pd->cancelhandling - & (CANCELING_BITMASK | CANCELED_BITMASK | EXITING_BITMASK + & (CANCELED_BITMASK | EXITING_BITMASK | TERMINATED_BITMASK)) == 0)) && !CANCEL_ENABLED_AND_CANCELED (self->cancelhandling)) /* This is a deadlock situation. The threads are waiting for each From patchwork Wed May 26 16:57:25 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella Netto X-Patchwork-Id: 43596 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 6985D394742F; Wed, 26 May 2021 16:57:47 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 6985D394742F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1622048267; bh=GDCVr1e5CWrN8/8b9pgzMFxSoVxKMtk+qlgRLUujbzA=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=qsXZFHeoDNpnTPafA8P26zqOAAbLYUTyD16lS1v/pahSLbqxsTdTzHCchFtTwqC45 JiCct5a2Pm58Wx8dmPhpkHQuROyEaJdSjNkQg5/+pdEMdizs2bP9AnQk10feKs9Qxu P4GYIBL5c0XYaO/BwPiAMlDMwO6aZH2HIPiKFAjk= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-qt1-x834.google.com (mail-qt1-x834.google.com [IPv6:2607:f8b0:4864:20::834]) by sourceware.org (Postfix) with ESMTPS id 60AB1394741A for ; Wed, 26 May 2021 16:57:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 60AB1394741A Received: by mail-qt1-x834.google.com with SMTP id h24so1374163qtm.12 for ; Wed, 26 May 2021 09:57:44 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=GDCVr1e5CWrN8/8b9pgzMFxSoVxKMtk+qlgRLUujbzA=; b=eRqJwlLBipi6YJXN4tq4F8hsT3SJaTaKsDP/IoIGn4SThpyk8MEQKU0MXKCi9TX75u wB/buBvObAWTd/UXDbQXQVr/QKJrOs7cb4nR8yCtn0ByC9CYiiVL73W5Z0RTgTnj76Z4 jdDw0qZ4yq2g1dUfQyHVPMBgtX43mD1Ek2I4DXvwPY2IeMgBri0xLS7rzpspU8icc2ND 4zSuhbHdSZt3f3LYKbl9wY9qJiMTXIsBM6xNZZBMmfaaJpxjhYt5l5zFnTHG7lsA0Ib2 Z6LVaHQcUive223RxoiHqNNoJAQwaYC6rLmueoCTeuVC219z+jS7SbkpnGVMWPNe2P51 k3SA== X-Gm-Message-State: AOAM531Ot+RysG35xW70ETjqPIldkgFsKeyrbMoi2x4Q7PshxP26B1vd bKaOZmyEPwCwYBG/XBhKP5RMaf0xoPmU2g== X-Google-Smtp-Source: ABdhPJyjeSKq3t65ZzidmzAPNRpv/W06unhrI9/LypWp7cMEwcb0uxVt8ZTS1qkQRCZjXTBVVButog== X-Received: by 2002:ac8:5d8f:: with SMTP id d15mr39428514qtx.350.1622048263583; Wed, 26 May 2021 09:57:43 -0700 (PDT) Received: from birita.. ([177.194.37.86]) by smtp.googlemail.com with ESMTPSA id k125sm1859761qkf.53.2021.05.26.09.57.42 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 May 2021 09:57:43 -0700 (PDT) To: libc-alpha@sourceware.org Subject: [PATCH 08/11] nptl: Move cancel state out of cancelhandling Date: Wed, 26 May 2021 13:57:25 -0300 Message-Id: <20210526165728.1772546-9-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210526165728.1772546-1-adhemerval.zanella@linaro.org> References: <20210526165728.1772546-1-adhemerval.zanella@linaro.org> MIME-Version: 1.0 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, 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-Patchwork-Original-From: Adhemerval Zanella via Libc-alpha From: Adhemerval Zanella Netto Reply-To: Adhemerval Zanella Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" Now that thread cancellation state is not accessed concurrently anymore, it is possible to move it out the 'cancelhandling'. The code is also simplified: the CANCELLATION_P is replaced with a internal pthread_testcancel call and the CANCELSTATE_BIT{MASK} is removed. The second part of this patchset also keeps the pthread_setcanceltype as cancellation entrypoint by calling pthread_testcancel if the new type is PTHREAD_CANCEL_ASYNCHRONOUS. With this behavior pthread_setcancelstate does not require to act on cancellation if cancel type is asynchronous (is already handled either by pthread_setcanceltype or by the signal handler). Checked on x86_64-linux-gnu and aarch64-linux-gnu. --- manual/pattern.texi | 1 - manual/process.texi | 3 +-- nptl/allocatestack.c | 1 + nptl/cancellation.c | 3 ++- nptl/cleanup_defer.c | 2 +- nptl/descr.h | 14 ++++++-------- nptl/libc-cleanup.c | 2 +- nptl/pthreadP.h | 12 ------------ nptl/pthread_cancel.c | 2 +- nptl/pthread_join_common.c | 5 ++++- nptl/pthread_setcancelstate.c | 36 +++-------------------------------- nptl/pthread_setcanceltype.c | 3 ++- nptl/pthread_testcancel.c | 11 ++++++++++- 13 files changed, 32 insertions(+), 63 deletions(-) diff --git a/manual/pattern.texi b/manual/pattern.texi index 39ae97a3c4..4fa4c25525 100644 --- a/manual/pattern.texi +++ b/manual/pattern.texi @@ -1820,7 +1820,6 @@ the beginning of the vector. @c (disable cancellation around exec_comm; it may do_cancel the @c second time, if async cancel is enabled) @c THREAD_ATOMIC_CMPXCHG_VAL dup ok -@c CANCEL_ENABLED_AND_CANCELED_AND_ASYNCHRONOUS dup ok @c do_cancel @ascuplugin @ascuheap @acsmem @c THREAD_ATOMIC_BIT_SET dup ok @c pthread_unwind @ascuplugin @ascuheap @acsmem diff --git a/manual/process.texi b/manual/process.texi index 54e65f76c6..134d5c6143 100644 --- a/manual/process.texi +++ b/manual/process.texi @@ -68,7 +68,7 @@ until the subprogram terminates before you can do anything else. @c CLEANUP_HANDLER @ascuplugin @ascuheap @acsmem @c libc_cleanup_region_start @ascuplugin @ascuheap @acsmem @c pthread_cleanup_push_defer @ascuplugin @ascuheap @acsmem -@c CANCELLATION_P @ascuplugin @ascuheap @acsmem +@c __pthread_testcancel @ascuplugin @ascuheap @acsmem @c CANCEL_ENABLED_AND_CANCELED ok @c do_cancel @ascuplugin @ascuheap @acsmem @c cancel_handler ok @@ -86,7 +86,6 @@ until the subprogram terminates before you can do anything else. @c SINGLE_THREAD_P ok @c LIBC_CANCEL_ASYNC @ascuplugin @ascuheap @acsmem @c libc_enable_asynccancel @ascuplugin @ascuheap @acsmem -@c CANCEL_ENABLED_AND_CANCELED_AND_ASYNCHRONOUS dup ok @c do_cancel dup @ascuplugin @ascuheap @acsmem @c LIBC_CANCEL_RESET ok @c libc_disable_asynccancel ok diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c index 2114bd2e27..54e95baad7 100644 --- a/nptl/allocatestack.c +++ b/nptl/allocatestack.c @@ -160,6 +160,7 @@ get_cached_stack (size_t *sizep, void **memp) /* Cancellation handling is back to the default. */ result->cancelhandling = 0; + result->cancelstate = PTHREAD_CANCEL_ENABLE; result->cleanup = NULL; result->setup_failed = 0; diff --git a/nptl/cancellation.c b/nptl/cancellation.c index b15f25d8f6..ce00603109 100644 --- a/nptl/cancellation.c +++ b/nptl/cancellation.c @@ -44,7 +44,8 @@ __pthread_enable_asynccancel (void) oldval); if (__glibc_likely (curval == oldval)) { - if (CANCEL_ENABLED_AND_CANCELED_AND_ASYNCHRONOUS (newval)) + if (self->cancelstate == PTHREAD_CANCEL_ENABLE + && CANCEL_CANCELED_AND_ASYNCHRONOUS (newval)) { THREAD_SETMEM (self, result, PTHREAD_CANCELED); __do_cancel (); diff --git a/nptl/cleanup_defer.c b/nptl/cleanup_defer.c index 6d85359118..873ab007fd 100644 --- a/nptl/cleanup_defer.c +++ b/nptl/cleanup_defer.c @@ -92,7 +92,7 @@ ___pthread_unregister_cancel_restore (__pthread_unwind_buf_t *buf) cancelhandling = curval; } - CANCELLATION_P (self); + __pthread_testcancel (); } } versioned_symbol (libc, ___pthread_unregister_cancel_restore, diff --git a/nptl/descr.h b/nptl/descr.h index a120365f88..a3084fdf60 100644 --- a/nptl/descr.h +++ b/nptl/descr.h @@ -277,9 +277,6 @@ struct pthread /* Flags determining processing of cancellation. */ int cancelhandling; - /* Bit set if cancellation is disabled. */ -#define CANCELSTATE_BIT 0 -#define CANCELSTATE_BITMASK (0x01 << CANCELSTATE_BIT) /* Bit set if asynchronous cancellation mode is selected. */ #define CANCELTYPE_BIT 1 #define CANCELTYPE_BITMASK (0x01 << CANCELTYPE_BIT) @@ -298,11 +295,8 @@ struct pthread /* Mask for the rest. Helps the compiler to optimize. */ #define CANCEL_RESTMASK 0xffffff80 -#define CANCEL_ENABLED_AND_CANCELED(value) \ - (((value) & (CANCELSTATE_BITMASK | CANCELED_BITMASK | EXITING_BITMASK \ - | CANCEL_RESTMASK | TERMINATED_BITMASK)) == CANCELED_BITMASK) -#define CANCEL_ENABLED_AND_CANCELED_AND_ASYNCHRONOUS(value) \ - (((value) & (CANCELSTATE_BITMASK | CANCELTYPE_BITMASK | CANCELED_BITMASK \ +#define CANCEL_CANCELED_AND_ASYNCHRONOUS(value) \ + (((value) & (CANCELTYPE_BITMASK | CANCELED_BITMASK \ | EXITING_BITMASK | CANCEL_RESTMASK | TERMINATED_BITMASK)) \ == (CANCELTYPE_BITMASK | CANCELED_BITMASK)) @@ -407,6 +401,10 @@ struct pthread /* Used on strsignal. */ struct tls_internal_t tls_state; + /* Thread cancel state (PTHREAD_CANCEL_ENABLE or + PTHREAD_CANCEL_DISABLE). */ + unsigned char cancelstate; + /* This member must be last. */ char end_padding[]; diff --git a/nptl/libc-cleanup.c b/nptl/libc-cleanup.c index 14ccfe9285..6286b8b525 100644 --- a/nptl/libc-cleanup.c +++ b/nptl/libc-cleanup.c @@ -79,7 +79,7 @@ __libc_cleanup_pop_restore (struct _pthread_cleanup_buffer *buffer) cancelhandling = curval; } - CANCELLATION_P (self); + __pthread_testcancel (); } } libc_hidden_def (__libc_cleanup_pop_restore) diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h index 48d48e7008..3e7a4f52ab 100644 --- a/nptl/pthreadP.h +++ b/nptl/pthreadP.h @@ -245,18 +245,6 @@ libc_hidden_proto (__pthread_current_priority) #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) \ - do { \ - int cancelhandling = THREAD_GETMEM (self, cancelhandling); \ - if (CANCEL_ENABLED_AND_CANCELED (cancelhandling)) \ - { \ - THREAD_SETMEM (self, result, PTHREAD_CANCELED); \ - __do_cancel (); \ - } \ - } while (0) - - extern void __pthread_unwind (__pthread_unwind_buf_t *__buf) __cleanup_fct_attribute __attribute ((__noreturn__)) #if !defined SHARED && !IS_IN (libpthread) diff --git a/nptl/pthread_cancel.c b/nptl/pthread_cancel.c index c11a2b4551..0588f086a8 100644 --- a/nptl/pthread_cancel.c +++ b/nptl/pthread_cancel.c @@ -45,7 +45,7 @@ sigcancel_handler (int sig, siginfo_t *si, void *ctx) int ch = atomic_load_relaxed (&self->cancelhandling); /* Cancelation not enabled, not cancelled, or already exitting. */ - if ((ch & CANCELSTATE_BITMASK) != 0 + if (self->cancelstate == PTHREAD_CANCEL_DISABLE || (ch & CANCELED_BITMASK) == 0 || (ch & EXITING_BITMASK) != 0) return; diff --git a/nptl/pthread_join_common.c b/nptl/pthread_join_common.c index f842c91a08..7303069316 100644 --- a/nptl/pthread_join_common.c +++ b/nptl/pthread_join_common.c @@ -59,7 +59,10 @@ __pthread_clockjoin_ex (pthread_t threadid, void **thread_return, && (pd->cancelhandling & (CANCELED_BITMASK | EXITING_BITMASK | TERMINATED_BITMASK)) == 0)) - && !CANCEL_ENABLED_AND_CANCELED (self->cancelhandling)) + && !(self->cancelstate == PTHREAD_CANCEL_ENABLE + && (pd->cancelhandling & (CANCELED_BITMASK | EXITING_BITMASK + | TERMINATED_BITMASK)) + == CANCELED_BITMASK)) /* This is a deadlock situation. The threads are waiting for each other to finish. Note that this is a "may" error. To be 100% sure we catch this error we would have to lock the data diff --git a/nptl/pthread_setcancelstate.c b/nptl/pthread_setcancelstate.c index e3696ca348..7e2b6e4974 100644 --- a/nptl/pthread_setcancelstate.c +++ b/nptl/pthread_setcancelstate.c @@ -31,39 +31,9 @@ __pthread_setcancelstate (int state, int *oldstate) self = THREAD_SELF; - int oldval = THREAD_GETMEM (self, cancelhandling); - while (1) - { - int newval = (state == PTHREAD_CANCEL_DISABLE - ? oldval | CANCELSTATE_BITMASK - : oldval & ~CANCELSTATE_BITMASK); - - /* Store the old value. */ - if (oldstate != NULL) - *oldstate = ((oldval & CANCELSTATE_BITMASK) - ? PTHREAD_CANCEL_DISABLE : PTHREAD_CANCEL_ENABLE); - - /* Avoid doing unnecessary work. The atomic operation can - potentially be expensive if the memory has to be locked and - remote cache lines have to be invalidated. */ - if (oldval == newval) - break; - - /* Update the cancel handling word. This has to be done - atomically since other bits could be modified as well. */ - int curval = THREAD_ATOMIC_CMPXCHG_VAL (self, cancelhandling, newval, - oldval); - if (__glibc_likely (curval == oldval)) - { - if (CANCEL_ENABLED_AND_CANCELED_AND_ASYNCHRONOUS (newval)) - __do_cancel (); - - break; - } - - /* Prepare for the next round. */ - oldval = curval; - } + if (oldstate != NULL) + *oldstate = self->cancelstate; + self->cancelstate = state; return 0; } diff --git a/nptl/pthread_setcanceltype.c b/nptl/pthread_setcanceltype.c index 5f061d512b..ae5df1d591 100644 --- a/nptl/pthread_setcanceltype.c +++ b/nptl/pthread_setcanceltype.c @@ -53,7 +53,8 @@ __pthread_setcanceltype (int type, int *oldtype) oldval); if (__glibc_likely (curval == oldval)) { - if (CANCEL_ENABLED_AND_CANCELED_AND_ASYNCHRONOUS (newval)) + if (self->cancelstate == PTHREAD_CANCEL_ENABLE + && CANCEL_CANCELED_AND_ASYNCHRONOUS (newval)) { THREAD_SETMEM (self, result, PTHREAD_CANCELED); __do_cancel (); diff --git a/nptl/pthread_testcancel.c b/nptl/pthread_testcancel.c index a9e941ddb7..920374643a 100644 --- a/nptl/pthread_testcancel.c +++ b/nptl/pthread_testcancel.c @@ -23,7 +23,16 @@ void ___pthread_testcancel (void) { - CANCELLATION_P (THREAD_SELF); + struct pthread *self = THREAD_SELF; + int cancelhandling = THREAD_GETMEM (self, cancelhandling); + if (self->cancelstate == PTHREAD_CANCEL_ENABLE + && (cancelhandling & CANCELED_BITMASK) + && !(cancelhandling & EXITING_BITMASK) + && !(cancelhandling & TERMINATED_BITMASK)) + { + THREAD_SETMEM (self, result, PTHREAD_CANCELED); + __do_cancel (); + } } versioned_symbol (libc, ___pthread_testcancel, pthread_testcancel, GLIBC_2_34); versioned_symbol (libc, ___pthread_testcancel, __pthread_testcancel, From patchwork Wed May 26 16:57:26 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella Netto X-Patchwork-Id: 43597 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 170933947403; Wed, 26 May 2021 16:57:50 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 170933947403 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1622048270; bh=T21vdovwjxtlzwTo7Ep4F7q+YV0pVAVeGc1yHGpSMoY=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=IDG8nmoCzJFYaW9fv+Oga3TyDZI5IZJkxbY9fkSlqNcASlPC+7zaaDlj0YPW156+a tN2lkwYnpeqZ2iRVu25Dq2mU/MeyeFCpC1hjVmj3dZCTjVI3rRshhyarP5ECTNjtbe 7gD/vjLk3yyxK076GXe53/Gasugk+qgH/FJrPnLg= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-qt1-x829.google.com (mail-qt1-x829.google.com [IPv6:2607:f8b0:4864:20::829]) by sourceware.org (Postfix) with ESMTPS id A66C43846416 for ; Wed, 26 May 2021 16:57:45 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org A66C43846416 Received: by mail-qt1-x829.google.com with SMTP id s12so1392077qta.3 for ; Wed, 26 May 2021 09:57:45 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=T21vdovwjxtlzwTo7Ep4F7q+YV0pVAVeGc1yHGpSMoY=; b=E9jSy/m3FYvPH0LiI4icrOareD0igzAguApumHveQe0l97Ju4vt66uFORUyr2HWbOa 9+7SFWNxO4HWdgHgQATFjjBrADyhMn5zwTjo/oHYQcc5SuBhSxNELQ+tNMDoXl4et5Ro seG7R1CgkVvJu4cnTKXueuXQZufdSxw7eXERxvlroYwO+ipOHJoTpxm7tk7niUN4IWYA 7iRQiknsmkfRiydHqd4PiiJHzChe/+vFLt8tDpggI6iL2LZewIVbWn+L9pBIrvHCQtRy yMveWdVMfwouzs6ia1kj/7NCf6oLm2bROq3Vm9zl8ylkv1ioExDrae/wfxIZSBoj8cMQ fk7Q== X-Gm-Message-State: AOAM533x4AXkuC5gaZcRdFyxlE5SZmD1bScRYlwcqG5k4q0YE4d12Tho WOJ895VdOwn8JnHRbk4E8kbGMi9f4NArbw== X-Google-Smtp-Source: ABdhPJzvGL45CkrZ2x5E0mbjjj5DnsyKbG+esTgUeM9iC1BMkr16hXUhnCPhkk/5m/2PLpT5IygEmw== X-Received: by 2002:a05:622a:1044:: with SMTP id f4mr39156545qte.181.1622048264849; Wed, 26 May 2021 09:57:44 -0700 (PDT) Received: from birita.. ([177.194.37.86]) by smtp.googlemail.com with ESMTPSA id k125sm1859761qkf.53.2021.05.26.09.57.43 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 May 2021 09:57:44 -0700 (PDT) To: libc-alpha@sourceware.org Subject: [PATCH 09/11] nptl: Move cancel type out of cancelhandling Date: Wed, 26 May 2021 13:57:26 -0300 Message-Id: <20210526165728.1772546-10-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210526165728.1772546-1-adhemerval.zanella@linaro.org> References: <20210526165728.1772546-1-adhemerval.zanella@linaro.org> MIME-Version: 1.0 X-Spam-Status: No, score=-12.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_ASCII_DIVIDERS, 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-Patchwork-Original-From: Adhemerval Zanella via Libc-alpha From: Adhemerval Zanella Netto Reply-To: Adhemerval Zanella Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" Now that the thread cancellation type is not accessed concurrently anymore, it is possible to move it out the cancelhandling. By removing the cancel state out of the internal thread cancel handling state there is no need to check if cancelled bit was set in CAS operation. It allows simplifing the cancellation wrappers and the CANCEL_CANCELED_AND_ASYNCHRONOUS is removed. Checked on x86_64-linux-gnu and aarch64-linux-gnu. --- nptl/allocatestack.c | 1 + nptl/cancellation.c | 51 +++++++++--------------------------- nptl/cleanup_defer.c | 46 ++++---------------------------- nptl/descr.h | 14 +++------- nptl/libc-cleanup.c | 44 +++---------------------------- nptl/pthread_cancel.c | 4 +-- nptl/pthread_setcanceltype.c | 42 ++++------------------------- 7 files changed, 33 insertions(+), 169 deletions(-) diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c index 54e95baad7..9be6c42894 100644 --- a/nptl/allocatestack.c +++ b/nptl/allocatestack.c @@ -161,6 +161,7 @@ get_cached_stack (size_t *sizep, void **memp) /* Cancellation handling is back to the default. */ result->cancelhandling = 0; result->cancelstate = PTHREAD_CANCEL_ENABLE; + result->canceltype = PTHREAD_CANCEL_DEFERRED; result->cleanup = NULL; result->setup_failed = 0; diff --git a/nptl/cancellation.c b/nptl/cancellation.c index ce00603109..05962784d5 100644 --- a/nptl/cancellation.c +++ b/nptl/cancellation.c @@ -31,31 +31,19 @@ int __pthread_enable_asynccancel (void) { struct pthread *self = THREAD_SELF; - int oldval = THREAD_GETMEM (self, cancelhandling); - while (1) - { - int newval = oldval | CANCELTYPE_BITMASK; - - if (newval == oldval) - break; - - int curval = THREAD_ATOMIC_CMPXCHG_VAL (self, cancelhandling, newval, - oldval); - if (__glibc_likely (curval == oldval)) - { - if (self->cancelstate == PTHREAD_CANCEL_ENABLE - && CANCEL_CANCELED_AND_ASYNCHRONOUS (newval)) - { - THREAD_SETMEM (self, result, PTHREAD_CANCELED); - __do_cancel (); - } + int oldval = THREAD_GETMEM (self, canceltype); + THREAD_SETMEM (self, canceltype, PTHREAD_CANCEL_ASYNCHRONOUS); - break; - } + int ch = THREAD_GETMEM (self, cancelhandling); - /* Prepare the next round. */ - oldval = curval; + if (self->cancelstate == PTHREAD_CANCEL_ENABLE + && (ch & CANCELED_BITMASK) + && !(ch & EXITING_BITMASK) + && !(ch & TERMINATED_BITMASK)) + { + THREAD_SETMEM (self, result, PTHREAD_CANCELED); + __do_cancel (); } return oldval; @@ -69,25 +57,10 @@ __pthread_disable_asynccancel (int oldtype) { /* If asynchronous cancellation was enabled before we do not have anything to do. */ - if (oldtype & CANCELTYPE_BITMASK) + if (oldtype == PTHREAD_CANCEL_ASYNCHRONOUS) return; struct pthread *self = THREAD_SELF; - int newval; - - int oldval = THREAD_GETMEM (self, cancelhandling); - - while (1) - { - newval = oldval & ~CANCELTYPE_BITMASK; - - int curval = THREAD_ATOMIC_CMPXCHG_VAL (self, cancelhandling, newval, - oldval); - if (__glibc_likely (curval == oldval)) - break; - - /* Prepare the next round. */ - oldval = curval; - } + self->canceltype = PTHREAD_CANCEL_DEFERRED; } libc_hidden_def (__pthread_disable_asynccancel) diff --git a/nptl/cleanup_defer.c b/nptl/cleanup_defer.c index 873ab007fd..7e858d0df0 100644 --- a/nptl/cleanup_defer.c +++ b/nptl/cleanup_defer.c @@ -31,27 +31,9 @@ ___pthread_register_cancel_defer (__pthread_unwind_buf_t *buf) ibuf->priv.data.prev = THREAD_GETMEM (self, cleanup_jmp_buf); ibuf->priv.data.cleanup = THREAD_GETMEM (self, cleanup); - int cancelhandling = THREAD_GETMEM (self, cancelhandling); - /* Disable asynchronous cancellation for now. */ - if (__glibc_unlikely (cancelhandling & CANCELTYPE_BITMASK)) - while (1) - { - int curval = THREAD_ATOMIC_CMPXCHG_VAL (self, cancelhandling, - cancelhandling - & ~CANCELTYPE_BITMASK, - cancelhandling); - if (__glibc_likely (curval == cancelhandling)) - /* Successfully replaced the value. */ - break; - - /* Prepare for the next round. */ - cancelhandling = curval; - } - - ibuf->priv.data.canceltype = (cancelhandling & CANCELTYPE_BITMASK - ? PTHREAD_CANCEL_ASYNCHRONOUS - : PTHREAD_CANCEL_DEFERRED); + ibuf->priv.data.canceltype = THREAD_GETMEM (self, canceltype); + THREAD_SETMEM (self, canceltype, PTHREAD_CANCEL_DEFERRED); /* Store the new cleanup handler info. */ THREAD_SETMEM (self, cleanup_jmp_buf, (struct pthread_unwind_buf *) buf); @@ -73,27 +55,9 @@ ___pthread_unregister_cancel_restore (__pthread_unwind_buf_t *buf) THREAD_SETMEM (self, cleanup_jmp_buf, ibuf->priv.data.prev); - int cancelhandling; - if (ibuf->priv.data.canceltype != PTHREAD_CANCEL_DEFERRED - && ((cancelhandling = THREAD_GETMEM (self, cancelhandling)) - & CANCELTYPE_BITMASK) == 0) - { - while (1) - { - int curval = THREAD_ATOMIC_CMPXCHG_VAL (self, cancelhandling, - cancelhandling - | CANCELTYPE_BITMASK, - cancelhandling); - if (__glibc_likely (curval == cancelhandling)) - /* Successfully replaced the value. */ - break; - - /* Prepare for the next round. */ - cancelhandling = curval; - } - - __pthread_testcancel (); - } + THREAD_SETMEM (self, canceltype, ibuf->priv.data.canceltype); + if (ibuf->priv.data.canceltype == PTHREAD_CANCEL_ASYNCHRONOUS) + __pthread_testcancel (); } versioned_symbol (libc, ___pthread_unregister_cancel_restore, __pthread_unregister_cancel_restore, GLIBC_2_34); diff --git a/nptl/descr.h b/nptl/descr.h index a3084fdf60..04476b3f4e 100644 --- a/nptl/descr.h +++ b/nptl/descr.h @@ -277,9 +277,6 @@ struct pthread /* Flags determining processing of cancellation. */ int cancelhandling; - /* Bit set if asynchronous cancellation mode is selected. */ -#define CANCELTYPE_BIT 1 -#define CANCELTYPE_BITMASK (0x01 << CANCELTYPE_BIT) /* Bit set if canceled. */ #define CANCELED_BIT 3 #define CANCELED_BITMASK (0x01 << CANCELED_BIT) @@ -292,13 +289,6 @@ struct pthread /* Bit set if thread is supposed to change XID. */ #define SETXID_BIT 6 #define SETXID_BITMASK (0x01 << SETXID_BIT) - /* Mask for the rest. Helps the compiler to optimize. */ -#define CANCEL_RESTMASK 0xffffff80 - -#define CANCEL_CANCELED_AND_ASYNCHRONOUS(value) \ - (((value) & (CANCELTYPE_BITMASK | CANCELED_BITMASK \ - | EXITING_BITMASK | CANCEL_RESTMASK | TERMINATED_BITMASK)) \ - == (CANCELTYPE_BITMASK | CANCELED_BITMASK)) /* Flags. Including those copied from the thread attribute. */ int flags; @@ -405,6 +395,10 @@ struct pthread PTHREAD_CANCEL_DISABLE). */ unsigned char cancelstate; + /* Thread cancel type (PTHREAD_CANCEL_DEFERRED or + PTHREAD_CANCEL_ASYNCHRONOUS). */ + unsigned char canceltype; + /* This member must be last. */ char end_padding[]; diff --git a/nptl/libc-cleanup.c b/nptl/libc-cleanup.c index 6286b8b525..180d15bc9e 100644 --- a/nptl/libc-cleanup.c +++ b/nptl/libc-cleanup.c @@ -27,27 +27,9 @@ __libc_cleanup_push_defer (struct _pthread_cleanup_buffer *buffer) buffer->__prev = THREAD_GETMEM (self, cleanup); - int cancelhandling = THREAD_GETMEM (self, cancelhandling); - /* Disable asynchronous cancellation for now. */ - if (__glibc_unlikely (cancelhandling & CANCELTYPE_BITMASK)) - while (1) - { - int curval = THREAD_ATOMIC_CMPXCHG_VAL (self, cancelhandling, - cancelhandling - & ~CANCELTYPE_BITMASK, - cancelhandling); - if (__glibc_likely (curval == cancelhandling)) - /* Successfully replaced the value. */ - break; - - /* Prepare for the next round. */ - cancelhandling = curval; - } - - buffer->__canceltype = (cancelhandling & CANCELTYPE_BITMASK - ? PTHREAD_CANCEL_ASYNCHRONOUS - : PTHREAD_CANCEL_DEFERRED); + buffer->__canceltype = THREAD_GETMEM (self, canceltype); + THREAD_SETMEM (self, canceltype, PTHREAD_CANCEL_DEFERRED); THREAD_SETMEM (self, cleanup, buffer); } @@ -60,26 +42,8 @@ __libc_cleanup_pop_restore (struct _pthread_cleanup_buffer *buffer) THREAD_SETMEM (self, cleanup, buffer->__prev); - int cancelhandling; - if (__builtin_expect (buffer->__canceltype != PTHREAD_CANCEL_DEFERRED, 0) - && ((cancelhandling = THREAD_GETMEM (self, cancelhandling)) - & CANCELTYPE_BITMASK) == 0) - { - while (1) - { - int curval = THREAD_ATOMIC_CMPXCHG_VAL (self, cancelhandling, - cancelhandling - | CANCELTYPE_BITMASK, - cancelhandling); - if (__glibc_likely (curval == cancelhandling)) - /* Successfully replaced the value. */ - break; - - /* Prepare for the next round. */ - cancelhandling = curval; - } - + THREAD_SETMEM (self, canceltype, buffer->__canceltype); + if (buffer->__canceltype == PTHREAD_CANCEL_ASYNCHRONOUS) __pthread_testcancel (); - } } libc_hidden_def (__libc_cleanup_pop_restore) diff --git a/nptl/pthread_cancel.c b/nptl/pthread_cancel.c index 0588f086a8..3be3d0fce7 100644 --- a/nptl/pthread_cancel.c +++ b/nptl/pthread_cancel.c @@ -53,7 +53,7 @@ sigcancel_handler (int sig, siginfo_t *si, void *ctx) /* Set the return value. */ THREAD_SETMEM (self, result, PTHREAD_CANCELED); /* Make sure asynchronous cancellation is still enabled. */ - if ((ch & CANCELTYPE_BITMASK) != 0) + if (self->canceltype == PTHREAD_CANCEL_ASYNCHRONOUS) __do_cancel (); } @@ -104,7 +104,7 @@ __pthread_cancel (pthread_t th) #endif THREAD_SETMEM (pd, result, PTHREAD_CANCELED); - if ((oldch & CANCELTYPE_BITMASK) != 0) + if (pd->canceltype == PTHREAD_CANCEL_ASYNCHRONOUS) __do_cancel (); return 0; } diff --git a/nptl/pthread_setcanceltype.c b/nptl/pthread_setcanceltype.c index ae5df1d591..e7b24ae733 100644 --- a/nptl/pthread_setcanceltype.c +++ b/nptl/pthread_setcanceltype.c @@ -29,43 +29,11 @@ __pthread_setcanceltype (int type, int *oldtype) volatile struct pthread *self = THREAD_SELF; - int oldval = THREAD_GETMEM (self, cancelhandling); - while (1) - { - int newval = (type == PTHREAD_CANCEL_ASYNCHRONOUS - ? oldval | CANCELTYPE_BITMASK - : oldval & ~CANCELTYPE_BITMASK); - - /* Store the old value. */ - if (oldtype != NULL) - *oldtype = ((oldval & CANCELTYPE_BITMASK) - ? PTHREAD_CANCEL_ASYNCHRONOUS : PTHREAD_CANCEL_DEFERRED); - - /* Avoid doing unnecessary work. The atomic operation can - potentially be expensive if the memory has to be locked and - remote cache lines have to be invalidated. */ - if (oldval == newval) - break; - - /* Update the cancel handling word. This has to be done - atomically since other bits could be modified as well. */ - int curval = THREAD_ATOMIC_CMPXCHG_VAL (self, cancelhandling, newval, - oldval); - if (__glibc_likely (curval == oldval)) - { - if (self->cancelstate == PTHREAD_CANCEL_ENABLE - && CANCEL_CANCELED_AND_ASYNCHRONOUS (newval)) - { - THREAD_SETMEM (self, result, PTHREAD_CANCELED); - __do_cancel (); - } - - break; - } - - /* Prepare for the next round. */ - oldval = curval; - } + if (oldtype != NULL) + *oldtype = self->canceltype; + self->canceltype = type; + if (type == PTHREAD_CANCEL_ASYNCHRONOUS) + __pthread_testcancel (); return 0; } From patchwork Wed May 26 16:57:27 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella Netto X-Patchwork-Id: 43598 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 9B0263947436; Wed, 26 May 2021 16:57:50 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 9B0263947436 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1622048270; bh=AXDBXj2vGE9aiPEfxAByhjhcE+bWnsVcNW9mDvSxc50=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=A9z2pWc3IETYVCUTboo5iaQpc/H9s2V8q6txZasboGVIGbwbpGNkj6IY1wq+o08Lw Y8gzAkxCrK4yL1GSaTalbcTOL9znzql1c4CCUJIcTZzjm/oECseC0MYUgqPO0hmGRo Oc1Fy4OaHW7ykmyPi8sqwEQWxDqDm9RTmqAdCzgc= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-qt1-x82c.google.com (mail-qt1-x82c.google.com [IPv6:2607:f8b0:4864:20::82c]) by sourceware.org (Postfix) with ESMTPS id CF5CE3947427 for ; Wed, 26 May 2021 16:57:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org CF5CE3947427 Received: by mail-qt1-x82c.google.com with SMTP id m13so1372953qtk.13 for ; Wed, 26 May 2021 09:57:46 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=AXDBXj2vGE9aiPEfxAByhjhcE+bWnsVcNW9mDvSxc50=; b=tXR7aiSTeJYlSleJY2F79EwLfz/YXfoAKVYKPSRZLI9M61yX7FjTklgGGw/zZwVuzV ViDAe/1cDDMDv93CYc/B/JNUsGzmcYLSvGKdKi9IVCHrVbISDYXCOdI3fp+Re81H15Rs NdwvC6zNlqGa0Hp0/uYY80FDAaOsmCR4zvotuS9wbUNVDVSNH8z65NL2Fk11ptW9a87u i5+NMgs3DP+aqXXoZo7WpAjPgiv7z29ciw201GYSCGoBB8fteJ0PYZCbd0lnrlDFg/lK lg9s5mL8UrPKJiqCsVcYrGRDpvN/72XEU3sNflufwj4yu1UCgvzokKN8ng8+i2lDG//v LsJg== X-Gm-Message-State: AOAM530Da4M9/iGHReh4UlmfUZ+dAw6aIEWmWvfbaXcd3DLEtytAk/P5 N/+hUIK3OZQke0n8uAv5CK3YfNNzvEzdfw== X-Google-Smtp-Source: ABdhPJxYVV1egoyPq9F/vdwJMBI89UTV0hRa0lr9Y0QLYWO6ZdXv1wDNIyaODukG1X7rG4a/PMUU6g== X-Received: by 2002:ac8:740a:: with SMTP id p10mr11901798qtq.241.1622048266098; Wed, 26 May 2021 09:57:46 -0700 (PDT) Received: from birita.. ([177.194.37.86]) by smtp.googlemail.com with ESMTPSA id k125sm1859761qkf.53.2021.05.26.09.57.45 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 May 2021 09:57:45 -0700 (PDT) To: libc-alpha@sourceware.org Subject: [PATCH 10/11] nptl: Implement raise in terms of pthread_kill Date: Wed, 26 May 2021 13:57:27 -0300 Message-Id: <20210526165728.1772546-11-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210526165728.1772546-1-adhemerval.zanella@linaro.org> References: <20210526165728.1772546-1-adhemerval.zanella@linaro.org> MIME-Version: 1.0 X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, URIBL_BLACK 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-Patchwork-Original-From: Adhemerval Zanella via Libc-alpha From: Adhemerval Zanella Netto Reply-To: Adhemerval Zanella Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" Now that pthread_kill is provided by libc.so it is possible to implement the generic POSIX implementation as 'pthread_kill(pthread_self(), sig)'. For Linux implementation, pthread_kill read the targeting TID from the TCB. For raise, this it not possible because it would make raise fail when issue after vfork (where creates the resulting process has a different TID from the parent, but its TCB is not updated as for pthread_create). To make raise use pthread_kill, it is make usable from vfork by getting the target thread id through gettid syscall. Checked on x86_64-linux-gnu and aarch64-linux-gnu. --- include/pthread.h | 5 ++++ nptl/pthreadP.h | 4 ++- nptl/pthread_kill.c | 40 ++++++++++++++++--------- nptl/pthread_self.c | 4 ++- sysdeps/htl/pthreadP.h | 2 -- sysdeps/posix/raise.c | 11 +++++-- sysdeps/unix/sysv/linux/raise.c | 52 --------------------------------- 7 files changed, 47 insertions(+), 71 deletions(-) delete mode 100644 sysdeps/unix/sysv/linux/raise.c diff --git a/include/pthread.h b/include/pthread.h index a3e1cf51b0..1158919247 100644 --- a/include/pthread.h +++ b/include/pthread.h @@ -16,4 +16,9 @@ extern int __pthread_barrier_wait (pthread_barrier_t *__barrier) /* This function is called to initialize the pthread library. */ extern void __pthread_initialize (void) __attribute__ ((weak)); + +extern int __pthread_kill (pthread_t threadid, int signo); + +extern pthread_t __pthread_self (void); + #endif diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h index 3e7a4f52ab..618922f47a 100644 --- a/nptl/pthreadP.h +++ b/nptl/pthreadP.h @@ -508,11 +508,13 @@ extern int __pthread_once (pthread_once_t *once_control, libc_hidden_proto (__pthread_once) extern int __pthread_atfork (void (*prepare) (void), void (*parent) (void), void (*child) (void)); -extern pthread_t __pthread_self (void); +libc_hidden_proto (__pthread_self) extern int __pthread_equal (pthread_t thread1, pthread_t thread2); extern int __pthread_detach (pthread_t th); libc_hidden_proto (__pthread_detach) extern int __pthread_kill (pthread_t threadid, int signo); +libc_hidden_proto (__pthread_kill) +extern int __pthread_cancel (pthread_t th); extern void __pthread_exit (void *value) __attribute__ ((__noreturn__)); libc_hidden_proto (__pthread_exit) extern int __pthread_join (pthread_t threadid, void **thread_return); diff --git a/nptl/pthread_kill.c b/nptl/pthread_kill.c index ad7e011779..0392c3f9e5 100644 --- a/nptl/pthread_kill.c +++ b/nptl/pthread_kill.c @@ -28,24 +28,38 @@ __pthread_kill (pthread_t threadid, int signo) if (__is_internal_signal (signo)) return EINVAL; - /* 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. */ + pid_t tid; struct pthread *pd = (struct pthread *) threadid; - pid_t tid = atomic_forced_read (pd->tid); - if (__glibc_unlikely (tid <= 0)) - /* Not a valid thread handle. */ - return ESRCH; - /* We have a special syscall to do the work. */ - pid_t pid = __getpid (); + if (pd == THREAD_SELF) + tid = INLINE_SYSCALL_CALL (gettid); + else + /* 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. */ + tid = atomic_forced_read (pd->tid); - int val = INTERNAL_SYSCALL_CALL (tgkill, pid, tid, signo); - return (INTERNAL_SYSCALL_ERROR_P (val) - ? INTERNAL_SYSCALL_ERRNO (val) : 0); + int val; + if (__glibc_likely (tid > 0)) + { + pid_t pid = __getpid (); + + val = INTERNAL_SYSCALL_CALL (tgkill, pid, tid, signo); + val = (INTERNAL_SYSCALL_ERROR_P (val) + ? INTERNAL_SYSCALL_ERRNO (val) : 0); + } + else + val = ESRCH; + + return val; } +/* Some architectures (for instance arm) might pull raise through libgcc, so + avoid the symbol version if it ends up being used on ld.so. */ +#if !IS_IN(rtld) +libc_hidden_def (__pthread_kill) versioned_symbol (libc, __pthread_kill, pthread_kill, GLIBC_2_34); -#if OTHER_SHLIB_COMPAT (libpthread, GLIBC_2_0, GLIBC_2_34) +# if OTHER_SHLIB_COMPAT (libpthread, GLIBC_2_0, GLIBC_2_34) compat_symbol (libc, __pthread_kill, pthread_kill, GLIBC_2_0); +# endif #endif diff --git a/nptl/pthread_self.c b/nptl/pthread_self.c index f877a2e6bd..196d93fb8e 100644 --- a/nptl/pthread_self.c +++ b/nptl/pthread_self.c @@ -20,7 +20,9 @@ #include pthread_t -pthread_self (void) +__pthread_self (void) { return (pthread_t) THREAD_SELF; } +libc_hidden_def (__pthread_self) +weak_alias (__pthread_self, pthread_self) diff --git a/sysdeps/htl/pthreadP.h b/sysdeps/htl/pthreadP.h index 8e2cf2ce65..3b357b7bdc 100644 --- a/sysdeps/htl/pthreadP.h +++ b/sysdeps/htl/pthreadP.h @@ -31,8 +31,6 @@ extern void __pthread_init_static_tls (struct link_map *) attribute_hidden; /* These represent the interface used by glibc itself. */ -extern pthread_t __pthread_self (void); -extern int __pthread_kill (pthread_t threadid, int signo); extern struct __pthread **__pthread_threads; extern int __pthread_mutex_init (pthread_mutex_t *__mutex, const pthread_mutexattr_t *__attr); diff --git a/sysdeps/posix/raise.c b/sysdeps/posix/raise.c index 9fdb4d538b..4806c0cc63 100644 --- a/sysdeps/posix/raise.c +++ b/sysdeps/posix/raise.c @@ -16,13 +16,20 @@ . */ #include -#include +#include +#include /* Raise the signal SIG. */ int raise (int sig) { - return __kill (__getpid (), sig); + int ret = __pthread_kill (__pthread_self (), sig); + if (ret != 0) + { + __set_errno (ret); + ret = -1; + } + return ret; } libc_hidden_def (raise) weak_alias (raise, gsignal) diff --git a/sysdeps/unix/sysv/linux/raise.c b/sysdeps/unix/sysv/linux/raise.c deleted file mode 100644 index 9be3b37f53..0000000000 --- a/sysdeps/unix/sysv/linux/raise.c +++ /dev/null @@ -1,52 +0,0 @@ -/* Copyright (C) 2002-2021 Free Software Foundation, Inc. - This file is part of the GNU C Library. - Contributed by Ulrich Drepper , 2002. - - The GNU C Library is free software; you can redistribute it and/or - modify it under the terms of the GNU Lesser General Public - License as published by the Free Software Foundation; either - version 2.1 of the License, or (at your option) any later version. - - The GNU C Library is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - Lesser General Public License for more details. - - You should have received a copy of the GNU Lesser General Public - License along with the GNU C Library; if not, see - . */ - -#include -#include -#include -#include -#include -#include - -int -raise (int sig) -{ - /* rt_sigprocmask may fail if: - - 1. sigsetsize != sizeof (sigset_t) (EINVAL) - 2. a failure in copy from/to user space (EFAULT) - 3. an invalid 'how' operation (EINVAL) - - The first case is already handle in glibc syscall call by using the arch - defined _NSIG. Second case is handled by using a stack allocated mask. - The last one should be handled by the block/unblock functions. */ - - sigset_t set; - __libc_signal_block_app (&set); - - pid_t pid = INTERNAL_SYSCALL_CALL (getpid); - pid_t tid = INTERNAL_SYSCALL_CALL (gettid); - - int ret = INLINE_SYSCALL_CALL (tgkill, pid, tid, sig); - - __libc_signal_restore_set (&set); - - return ret; -} -libc_hidden_def (raise) -weak_alias (raise, gsignal) From patchwork Wed May 26 16:57:28 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella Netto X-Patchwork-Id: 43599 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 2ACA6394740E; Wed, 26 May 2021 16:57:51 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 2ACA6394740E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1622048271; bh=XKzL+yIVHNKhmPjjXm5ei1c+LgrwxqUSmVEWOme9dss=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=uS7wkrJu/Ixb5LxJprT7YlBSepNiU1eKwVtH6CnU4D1FhrE4EETKFsimDyraEUEUz oDAY/0mPnnpxZIvEI6AzpIDFaKGbXNWeGq4oWdg2H4IAg48XIBpiO5KLgAaC8Ait43 BFkthnVBovO4VcN4o3iPPW9zmoNxuehnCAKg0i0A= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-qt1-x830.google.com (mail-qt1-x830.google.com [IPv6:2607:f8b0:4864:20::830]) by sourceware.org (Postfix) with ESMTPS id EF7633947433 for ; Wed, 26 May 2021 16:57:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org EF7633947433 Received: by mail-qt1-x830.google.com with SMTP id a15so1303045qta.0 for ; Wed, 26 May 2021 09:57:47 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=XKzL+yIVHNKhmPjjXm5ei1c+LgrwxqUSmVEWOme9dss=; b=s+sPlM/7FkaL7qX4qc0STtXCsUE2NFPitzJ7X2XCap6fer4LMR5xW2U/K+MJVD+FZa siTYKX7Yixu9j6BdMsFkHd1QD/HxqHKxP/lNxP4ii1uXvKZEPyy1szs1MdKmTAYrNOwR 71ASaiJ1YSjbEwZYMy+ObD6Tf49LlNuOSnsiobeYwPuWuL9jUGXIBcrt0qz1iYzCVYji jUvcFJ0pKbww5mU+WxScL0t+DrGHYsgmBTTQWFm2Opi9MFq8hc4KgTsqLKtV6wimYDpn JKcbK4osfyPoTftq1ffMaf2lVc0fMoM7J5492Yb8ByFQ7nHnbRTqzvRYApt30q4EjNKZ o1uA== X-Gm-Message-State: AOAM531IhLIgLUPz8JTk9H0eW0UXHf9Kv0t433t6/GO+W7KKS6X1+SmW T1HzqN11t8jCM9YOCnuM44NVUmSawKsPMA== X-Google-Smtp-Source: ABdhPJxxN1knVpvSZF5c7xYVk7oYhl70OQ9LWuIGuAZKVsVd2mQDhPpQ9/NRupe2FBNOCCUrfWzOLQ== X-Received: by 2002:a05:622a:210:: with SMTP id b16mr38168916qtx.51.1622048267325; Wed, 26 May 2021 09:57:47 -0700 (PDT) Received: from birita.. ([177.194.37.86]) by smtp.googlemail.com with ESMTPSA id k125sm1859761qkf.53.2021.05.26.09.57.46 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 May 2021 09:57:47 -0700 (PDT) To: libc-alpha@sourceware.org Subject: [PATCH 11/11] nptl: Use pthread_kill on pthread_cancel Date: Wed, 26 May 2021 13:57:28 -0300 Message-Id: <20210526165728.1772546-12-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210526165728.1772546-1-adhemerval.zanella@linaro.org> References: <20210526165728.1772546-1-adhemerval.zanella@linaro.org> MIME-Version: 1.0 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, 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-Patchwork-Original-From: Adhemerval Zanella via Libc-alpha From: Adhemerval Zanella Netto Reply-To: Adhemerval Zanella Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" It consolidates the tgkill call and it is the first step of making pthread_cancel async-signal-safe. It also fix a possible issue where the 'struct pthread' tid is not read atomically, which might send an invalid cancellation signal (similar to what db988e50a87f613cb6b9e98a2fc66a4848bc3546 fixed for pthread_join). Checked on x86_64-linux-gnu and aarch64-linux-gnu. --- nptl/pthreadP.h | 2 ++ nptl/pthread_cancel.c | 6 +----- nptl/pthread_kill.c | 18 ++++++++++++------ 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h index 618922f47a..675d1de753 100644 --- a/nptl/pthreadP.h +++ b/nptl/pthreadP.h @@ -515,6 +515,8 @@ libc_hidden_proto (__pthread_detach) extern int __pthread_kill (pthread_t threadid, int signo); libc_hidden_proto (__pthread_kill) extern int __pthread_cancel (pthread_t th); +extern int __pthread_kill_internal (pthread_t threadid, int signo) + attribute_hidden; extern void __pthread_exit (void *value) __attribute__ ((__noreturn__)); libc_hidden_proto (__pthread_exit) extern int __pthread_join (pthread_t threadid, void **thread_return); diff --git a/nptl/pthread_cancel.c b/nptl/pthread_cancel.c index 3be3d0fce7..e6dc135e9b 100644 --- a/nptl/pthread_cancel.c +++ b/nptl/pthread_cancel.c @@ -109,11 +109,7 @@ __pthread_cancel (pthread_t th) return 0; } - pid_t pid = __getpid (); - int val = INTERNAL_SYSCALL_CALL (tgkill, pid, pd->tid, SIGCANCEL); - return INTERNAL_SYSCALL_ERROR_P (val) - ? INTERNAL_SYSCALL_ERRNO (val) - : 0; + return __pthread_kill_internal (th, SIGCANCEL); } versioned_symbol (libc, __pthread_cancel, pthread_cancel, GLIBC_2_34); diff --git a/nptl/pthread_kill.c b/nptl/pthread_kill.c index 0392c3f9e5..93387dd60a 100644 --- a/nptl/pthread_kill.c +++ b/nptl/pthread_kill.c @@ -21,13 +21,8 @@ #include int -__pthread_kill (pthread_t threadid, int signo) +__pthread_kill_internal (pthread_t threadid, int signo) { - /* Disallow sending the signal we use for cancellation, timers, - for the setxid implementation. */ - if (__is_internal_signal (signo)) - return EINVAL; - pid_t tid; struct pthread *pd = (struct pthread *) threadid; @@ -53,6 +48,17 @@ __pthread_kill (pthread_t threadid, int signo) return val; } + +int +__pthread_kill (pthread_t threadid, int signo) +{ + /* Disallow sending the signal we use for cancellation, timers, + for the setxid implementation. */ + if (__is_internal_signal (signo)) + return EINVAL; + + return __pthread_kill_internal (threadid, signo); +} /* Some architectures (for instance arm) might pull raise through libgcc, so avoid the symbol version if it ends up being used on ld.so. */ #if !IS_IN(rtld)