From patchwork Mon Aug 23 19:50:33 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: 44750 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 DFDD33857C77 for ; Mon, 23 Aug 2021 19:54:22 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org DFDD33857C77 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1629748462; bh=hnLlLHJYVEGYYVQQfyPrR3WOqa5ucVWUXTDEhhEZ+eY=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=TbIpohGybpHj8ms/IBYV8hxchcv4I1o8pGvtOf7Nw/SOMu4EPKQtYFoI93o39x+Lk SMpPfB1qlESUV7UxzNSFD59gFCM74yWmDLjuvYPF5RvjHSoRgnPOcW58oX0TX+peCU 730hNExmd2vMApkR/iaujSBsnJdUY0B4h/XpS33U= 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 A568C3858022 for ; Mon, 23 Aug 2021 19:50:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A568C3858022 Received: by mail-qv1-xf2a.google.com with SMTP id c14so10378187qvs.9 for ; Mon, 23 Aug 2021 12:50:57 -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:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=hnLlLHJYVEGYYVQQfyPrR3WOqa5ucVWUXTDEhhEZ+eY=; b=Y2cOKOvReAy32OLwF+1L/TW40dln0m5jrL466JbwS+7kYAb/9BmnVONwfDaf5Ih7DT U6fc6i4IYL+nybLJxM+uJ7a4CIh/k0RoyXo7Lhy8vo+XFz6cHjyJuY8E9RGYjMPF6EzY DiJak/kz06O3ZIDzmUyrVVv25zza4C1VOLwSXRNI1U4bYt5kT4aXOo9UHm7ZFYWN/U1b qH8EYuVpNutc6u62lx9F7SCPsqNSjs7ui6aAxxFVRdTlYP+dfc5oxZbyKKmM7haM/JLE egICYdqFFNZcanZxgNwjsJxh3t3vFUOHlkBGbmcLh4d04ryteXd8SHTAuGd/HHvf3tEF ZytA== X-Gm-Message-State: AOAM533U/myTVAQxQFtshdZnEEtT4gzNDUepDTU+I1H5+M1XY2FWc59G yyZv/UOuwSx1f9mYmgbryvNsexJD2B+4/g== X-Google-Smtp-Source: ABdhPJyl9QTdN/scvO6A5rzLkgPywM825c+Db5OwrfU5ahIb38311Ko5NSkU9yp0FDsP0E8ZNbI/hg== X-Received: by 2002:a05:6214:a02:: with SMTP id dw2mr34862780qvb.61.1629748257056; Mon, 23 Aug 2021 12:50:57 -0700 (PDT) Received: from birita.. ([2804:431:c7ca:cd83:c38b:b50d:5d9a:43d4]) by smtp.gmail.com with ESMTPSA id s10sm9210935qko.134.2021.08.23.12.50.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 23 Aug 2021 12:50:56 -0700 (PDT) To: libc-alpha@sourceware.org Subject: [PATCH v2 05/19] nptl: Move setxid flag out of cancelhandling Date: Mon, 23 Aug 2021 16:50:33 -0300 Message-Id: <20210823195047.543237-6-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210823195047.543237-1-adhemerval.zanella@linaro.org> References: <20210823195047.543237-1-adhemerval.zanella@linaro.org> MIME-Version: 1.0 X-Spam-Status: No, score=-12.3 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.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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 Cc: Florian Weimer Errors-To: libc-alpha-bounces+patchwork=sourceware.org@sourceware.org Sender: "Libc-alpha" Now that the thread state is tracked by 'joinstate' field, there is no need to keep the setxid flag within the 'cancelhandling'. It simplifies the atomic code to set and reset it (since there is no need to handle the thread state concurrent update). Checked on x86_64-linux-gnu and aarch64-linux-gnu. --- nptl/allocatestack.c | 3 +++ nptl/descr.h | 5 ++--- nptl/nptl_setxid.c | 49 ++++++++++--------------------------------- nptl/pthread_create.c | 4 ++-- 4 files changed, 18 insertions(+), 43 deletions(-) diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c index cfe37a3443..ccb1aa21c1 100644 --- a/nptl/allocatestack.c +++ b/nptl/allocatestack.c @@ -99,6 +99,7 @@ get_cached_stack (size_t *sizep, void **memp) } /* Don't allow setxid until cloned. */ + result->setxid_flag = 0; result->setxid_futex = -1; /* Dequeue the entry. */ @@ -301,6 +302,7 @@ allocate_stack (const struct pthread_attr *attr, struct pthread **pdp, #endif /* Don't allow setxid until cloned. */ + pd->setxid_flag = 0; pd->setxid_futex = -1; /* Allocate the DTV for this thread. */ @@ -422,6 +424,7 @@ allocate_stack (const struct pthread_attr *attr, struct pthread **pdp, #endif /* Don't allow setxid until cloned. */ + pd->setxid_flag = 0; pd->setxid_futex = -1; /* Allocate the DTV for this thread. */ diff --git a/nptl/descr.h b/nptl/descr.h index 4b2db6edab..563b152bff 100644 --- a/nptl/descr.h +++ b/nptl/descr.h @@ -297,9 +297,6 @@ struct pthread /* Bit set if thread terminated and TCB is freed. */ #define TERMINATED_BIT 5 #define TERMINATED_BITMASK (0x01 << TERMINATED_BIT) - /* Bit set if thread is supposed to change XID. */ -#define SETXID_BIT 6 -#define SETXID_BITMASK (0x01 << SETXID_BIT) /* Flags. Including those copied from the thread attribute. */ int flags; @@ -339,6 +336,8 @@ struct pthread /* Lock to synchronize access to the descriptor. */ int lock; + /* Indicate whether thread is supposed to change XID. */ + int setxid_flag; /* Lock for synchronizing setxid calls. */ unsigned int setxid_futex; diff --git a/nptl/nptl_setxid.c b/nptl/nptl_setxid.c index 2f35772411..c337fa8577 100644 --- a/nptl/nptl_setxid.c +++ b/nptl/nptl_setxid.c @@ -76,14 +76,7 @@ __nptl_setxid_sighandler (int sig, siginfo_t *si, void *ctx) /* Reset the SETXID flag. */ struct pthread *self = THREAD_SELF; - int flags, newval; - do - { - flags = THREAD_GETMEM (self, cancelhandling); - newval = THREAD_ATOMIC_CMPXCHG_VAL (self, cancelhandling, - flags & ~SETXID_BITMASK, flags); - } - while (flags != newval); + atomic_store_release (&self->setxid_flag, 0); /* And release the futex. */ self->setxid_futex = 1; @@ -97,8 +90,6 @@ libc_hidden_def (__nptl_setxid_sighandler) static void setxid_mark_thread (struct xid_command *cmdp, struct pthread *t) { - int ch; - /* Wait until this thread is cloned. */ if (t->setxid_futex == -1 && ! atomic_compare_and_exchange_bool_acq (&t->setxid_futex, -2, -1)) @@ -109,41 +100,23 @@ setxid_mark_thread (struct xid_command *cmdp, struct pthread *t) /* Don't let the thread exit before the setxid handler runs. */ t->setxid_futex = 0; - do - { - ch = t->cancelhandling; + /* If thread is exiting right now, ignore it. */ + if (atomic_load_acquire (&t->joinstate) == THREAD_STATE_EXITING) + return; - /* If the thread is exiting right now, ignore it. */ - if ((ch & EXITING_BITMASK) != 0) - { - /* Release the futex if there is no other setxid in - progress. */ - if ((ch & SETXID_BITMASK) == 0) - { - t->setxid_futex = 1; - futex_wake (&t->setxid_futex, 1, FUTEX_PRIVATE); - } - return; - } + /* Release the futex if there is no other setxid in progress. */ + if (atomic_exchange_acquire (&t->setxid_flag, 1) == 0) + { + t->setxid_futex = 1; + futex_wake (&t->setxid_futex, 1, FUTEX_PRIVATE); } - while (atomic_compare_and_exchange_bool_acq (&t->cancelhandling, - ch | SETXID_BITMASK, ch)); } static void setxid_unmark_thread (struct xid_command *cmdp, struct pthread *t) { - int ch; - - do - { - ch = t->cancelhandling; - if ((ch & SETXID_BITMASK) == 0) - return; - } - while (atomic_compare_and_exchange_bool_acq (&t->cancelhandling, - ch & ~SETXID_BITMASK, ch)); + atomic_exchange_release (&t->setxid_flag, 0); /* Release the futex just in case. */ t->setxid_futex = 1; @@ -154,7 +127,7 @@ setxid_unmark_thread (struct xid_command *cmdp, struct pthread *t) static int setxid_signal_thread (struct xid_command *cmdp, struct pthread *t) { - if ((t->cancelhandling & SETXID_BITMASK) == 0) + if (atomic_load_relaxed (&t->setxid_flag) == 0) return 0; int val; diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c index 763e32bc3e..c00d6ae00c 100644 --- a/nptl/pthread_create.c +++ b/nptl/pthread_create.c @@ -541,7 +541,7 @@ start_thread (void *arg) advise_stack_range (pd->stackblock, pd->stackblock_size, (uintptr_t) pd, pd->guardsize); - if (__glibc_unlikely (pd->cancelhandling & SETXID_BITMASK)) + if (__glibc_unlikely (atomic_load_relaxed (&pd->setxid_flag) == 1)) { /* Some other thread might call any of the setXid functions and expect us to reply. In this case wait until we did that. */ @@ -551,7 +551,7 @@ start_thread (void *arg) condition used in the surrounding loop (cancelhandling). We need to check and document why this is correct. */ futex_wait_simple (&pd->setxid_futex, 0, FUTEX_PRIVATE); - while (pd->cancelhandling & SETXID_BITMASK); + while (atomic_load_relaxed (&pd->setxid_flag) == 1); /* Reset the value so that the stack can be reused. */ pd->setxid_futex = 0;