From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oi1-x234.google.com (mail-oi1-x234.google.com [IPv6:2607:f8b0:4864:20::234]) by sourceware.org (Postfix) with ESMTPS id 6320F3858D37 for ; Thu, 2 Mar 2023 17:00:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 6320F3858D37 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linaro.org Received: by mail-oi1-x234.google.com with SMTP id r40so13032094oiw.0 for ; Thu, 02 Mar 2023 09:00:14 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1677776413; h=content-transfer-encoding:in-reply-to:organization:from:references :cc:to:content-language:subject:user-agent:mime-version:date :message-id:from:to:cc:subject:date:message-id:reply-to; bh=N+5grcPIZfdwxZyyDSKTLImyihjcr2n/KQ9ljD/xqBQ=; b=hUJeNs2DZ+FOF8MFMHIc98hb580LNmaHj6u13mOgnimKDM2TFKx5UgGyDH0tytEstw aaM+dRdJfYsn44VjsCopKTS2iIuJ9fue+XxZc4/Vs1KhAo0clNykicg/Zs4WdS5RHvUS oyGe8ZcCT0eJ7j1mV5ITwNO55cmkD2oiJltNqJRuhnN9MHRNJKJ9RKOVeGwMtE11IVp6 SG3/067r2zxQ+AWV0eOuvHBa9m3O8GBVnSElSvbmvFDPRI3EHAHsAymf1LweL8S6OslO CvwjW3rA9ft6380T9pngx/THPbmAXu+bl2s8zhlxSgBVmfnBFW1TVwXmlLA3d0inGclu iCWw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1677776413; h=content-transfer-encoding:in-reply-to:organization:from:references :cc:to:content-language:subject:user-agent:mime-version:date :message-id:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=N+5grcPIZfdwxZyyDSKTLImyihjcr2n/KQ9ljD/xqBQ=; b=OPYT4ofKWg6sjZDrNfRuBaSpdcMnZ0GdxL4O6K99op75ahyQftzFmWRTsbwj8/J0Q3 Bd4xucAZnPeyvo/EPHaU59LBc04RZlP9s/ghbGXDkzaKSsHG53n5iGhOp6XPtGMojyQy YeFgeIpU4ygR1NaMeRqnFmTM80ubItOaMh70sKDx8BvCzOQ92nHrmHPaREEkIldm/0hp eZ9oRxBqJILc1PLtvHlkyfQbgrpFlAkpeVPVoUpHm/QauYus5/65U5ouNDw1qlPG+LTu Fok7BpVWS7LHS057CKT5I91iIeO1MIyEqAsE8fwDPfKXjcCYRbePtNqcsGHLX3YgnAiy sj/g== X-Gm-Message-State: AO0yUKWIigs+rz5rvzX8NJAfXU/IGOkMxnoxJU/PwrSdsb0sFUQygUit tFqdq6ubZ6teWKz6bvCY8erHP6whQqMgXNFPuok= X-Google-Smtp-Source: AK7set+ovDfU4LR5aYEJc8bw/WtfMtiHZkK6/LkkPgThhuhY+k7Sa/465vB8tP6Mnx+Mud82shlqag== X-Received: by 2002:a05:6808:208:b0:383:e256:f1b9 with SMTP id l8-20020a056808020800b00383e256f1b9mr5360605oie.1.1677776412209; Thu, 02 Mar 2023 09:00:12 -0800 (PST) Received: from ?IPV6:2804:1b3:a7c3:d849:65ac:94e7:b706:d532? ([2804:1b3:a7c3:d849:65ac:94e7:b706:d532]) by smtp.gmail.com with ESMTPSA id f9-20020a4a6709000000b0051ffe0fe11bsm6239718ooc.6.2023.03.02.09.00.10 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 02 Mar 2023 09:00:11 -0800 (PST) Message-ID: <9375da3a-1faf-245e-4d39-2ec4d2638563@linaro.org> Date: Thu, 2 Mar 2023 14:00:09 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.8.0 Subject: Re: [PATCH v3] i386: Use pthread_barrier for synchronization on tst-bz21269 Content-Language: en-US To: DJ Delorie Cc: libc-alpha@sourceware.org References: From: Adhemerval Zanella Netto Organization: Linaro In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-11.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_NUMSUBJECT,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 02/03/23 02:10, DJ Delorie wrote: > > So I was able to reproduce the hangs in the original source, and debug > it, and fix it. In doing so, I realized that we can't use anything > complex to trigger the thread because that "anything" might also cause > the expected segfault and force everything out of sync again. > > Here's what I ended up with, and it doesn't seem to hang where the > original one hung quite often (in a tight while..end loop). The key > changes are: > > 1. Calls to futex are error checked, with retries, to ensure that the > futexes are actually doing what they're supposed to be doing. In the > original code, nearly every futex call returned an error. > > 2. The main loop has checks for whether the thread ran or not, and > "unlocks" the thread if it didn't (this is how the original source > hangs). This makes sense but... > > diff --git a/sysdeps/unix/sysv/linux/i386/tst-bz21269.c b/sysdeps/unix/sysv/linux/i386/tst-bz21269.c > index c95bfcb917..51d4a1b082 100644 > --- a/sysdeps/unix/sysv/linux/i386/tst-bz21269.c > +++ b/sysdeps/unix/sysv/linux/i386/tst-bz21269.c > @@ -1,5 +1,5 @@ > /* Test for i386 sigaction sa_restorer handling (BZ#21269) > - Copyright (C) 2017-2022 Free Software Foundation, Inc. > + Copyright (C) 2017-2023 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 > @@ -135,7 +135,14 @@ threadproc (void *ctx) > { > while (1) > { > - futex ((int *) &ftx, FUTEX_WAIT, 1, NULL, NULL, 0); > + /* Continue to wait here until we've successfully waited, unless > + we're supposed to be clearing the LDT already. */ > + while (futex ((int *) &ftx, FUTEX_WAIT, 1, NULL, NULL, 0) < 0) > + if (atomic_load (&ftx) >= 2) > + break; > + > + /* Normally there's time to hit this busy loop and wait for ftx > + to be set to 2. */ > while (atomic_load (&ftx) != 2) > { > if (atomic_load (&ftx) >= 3) > @@ -189,7 +196,14 @@ do_test (void) > if (sigsetjmp (jmpbuf, 1) != 0) > continue; > > - /* Make sure the thread is ready after the last test. */ > + /* We may have longjmp'd before triggering the thread. If so, > + trigger the thread now and wait for it. */ > + if (atomic_load (&ftx) == 1) > + atomic_store (&ftx, 2); > + > + /* Make sure the thread is ready after the last test. FTX is > + initially zero for the first loop, and set to zero each time > + the thread clears the LDT. */ > while (atomic_load (&ftx) != 0) > ; > > @@ -207,15 +221,24 @@ do_test (void) > > xmodify_ldt (0x11, &desc, sizeof (desc)); > > - /* Arm the thread. */ > - ftx = 1; > - futex ((int*) &ftx, FUTEX_WAKE, 0, NULL, NULL, 0); > + /* Arm the thread. We loop here until we've woken up one thread. */ > + atomic_store (&ftx, 1); > + while (futex ((int*) &ftx, FUTEX_WAKE, 1, NULL, NULL, 0) < 1) > + ; > + > + /* Give the thread a chance to get into it's busy loop. */ > + usleep (5); ... I shivers every time I see sleep used as synchronization mechanism, since most likely in some environment the sleep won't work as expected due scheduling pressuer and we will end up with a false positive. I am wondering if it would be better to just remove this test saying we can't really make it work reliable. > > + /* At *ANY* point after this instruction, we may segfault and > + longjump back to the top of the loop. The intention is to > + have this happen when the thread clears the LDT, but it could > + happen elsewhen. */ > asm volatile ("mov %0, %%ss" : : "r" (0x7)); > > /* Fire up thread modify_ldt call. */ > atomic_store (&ftx, 2); > > + /* And wait for it. */ > while (atomic_load (&ftx) != 0) > ; > >