From patchwork Thu Jun 10 19:36:38 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: 43827 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 5475F39A0468 for ; Thu, 10 Jun 2021 19:40:13 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 5475F39A0468 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1623354013; bh=6U6dosfU9/k2S2NPw6SEJ/Bv4ImeeLTNOS5G8I3JpNU=; 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=bJEFU53XpnCJVooLBK9s8Zvp6+ueJxQiZNIKOTa0M4xt+2RpxBhYMn0QU4Sl3fPAn jpZPGf0//AkvDayrNt6xEN3+XyK7IQkaDjeKBRlOapkBdwIp77WYfY86XbuyDuLHQt jyeTkZqCU9u9uDgkr41xvpVlJ2WTcfoIkCGioh7k= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-qk1-x731.google.com (mail-qk1-x731.google.com [IPv6:2607:f8b0:4864:20::731]) by sourceware.org (Postfix) with ESMTPS id 89223399E048 for ; Thu, 10 Jun 2021 19:36:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 89223399E048 Received: by mail-qk1-x731.google.com with SMTP id c138so15704742qkg.5 for ; Thu, 10 Jun 2021 12:36:51 -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=6U6dosfU9/k2S2NPw6SEJ/Bv4ImeeLTNOS5G8I3JpNU=; b=pyIPrMCg+0dENDb/pef7l4yrcaP+FHCsWiPQVPJrghhf23XtjLWWstWbYog8x6kylM BIpNqdwS7V+8MkZXwxPozhdL95CL1zDCORYbeGWDNXFO4LG5lzsR0KiJPy05W5p+j3bu HOSyh20IYoHTjsFmeNYi4rf0z6cS0OL+l79n/WXR9/WUzpKT9uSFHOYYU0Bo/22Pyl+1 rz+nIeaL3iQH4f2ochw2E3IGPqir9P7TT7avBeJ0Bp+Fk1FBZ23/M+C4+PCC+y8Igmwc S5nVCkKi/FV8Ulxu31Po+svq5wJF82TK+80THt/l4PLHBaRmiNVrNvCPWiDfexgJQ312 rO5g== X-Gm-Message-State: AOAM5332MaNYzmr6atHVyr4LsvEoHchINxblvBrOH27e+2BoIn0U8UAH gSwPtIRke+rDNuFz7949MTWT8OXzjUERkA== X-Google-Smtp-Source: ABdhPJx2vJMJNJFOKQLw7/jULg1C9Y9dsi6L3lBaF1MR5TfBoa4D4SwfvIq1TOxnk/Und/FYTN6qFw== X-Received: by 2002:a37:a417:: with SMTP id n23mr201985qke.265.1623353810886; Thu, 10 Jun 2021 12:36:50 -0700 (PDT) Received: from birita.. ([177.194.59.218]) by smtp.googlemail.com with ESMTPSA id i21sm3012583qkl.20.2021.06.10.12.36.49 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 10 Jun 2021 12:36:50 -0700 (PDT) To: libc-alpha@sourceware.org Subject: [PATCH 5/6] nptl: Move setxid flag out of cancelhandling Date: Thu, 10 Jun 2021 16:36:38 -0300 Message-Id: <20210610193639.3650754-6-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210610193639.3650754-1-adhemerval.zanella@linaro.org> References: <20210610193639.3650754-1-adhemerval.zanella@linaro.org> MIME-Version: 1.0 X-Spam-Status: No, score=-12.4 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+patchwork=sourceware.org@sourceware.org Sender: "Libc-alpha" Now that the thread state is tracked by PD->joinstate, there is no need to keep the setxid flag within the PD->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 9be6c42894..aae2281517 100644 --- a/nptl/allocatestack.c +++ b/nptl/allocatestack.c @@ -140,6 +140,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. */ @@ -342,6 +343,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. */ @@ -463,6 +465,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 0c0f2c03e5..0b1c67f6d2 100644 --- a/nptl/pthread_create.c +++ b/nptl/pthread_create.c @@ -531,7 +531,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. */ @@ -541,7 +541,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;