From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oo1-xc29.google.com (mail-oo1-xc29.google.com [IPv6:2607:f8b0:4864:20::c29]) by sourceware.org (Postfix) with ESMTPS id AAEB63858C2B for ; Tue, 21 Feb 2023 19:22:59 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org AAEB63858C2B 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-oo1-xc29.google.com with SMTP id h8-20020a4a6f08000000b0051762fdf955so556772ooc.3 for ; Tue, 21 Feb 2023 11:22:59 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1677007379; 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=aMuuRzeKfqLa9tVG0RwDbPinKoHYnhfALOUXgzx7oRk=; b=JBcvRhMyymENAkoxuSugeycHx+FHvqhaUXmHG2Tk2P2mPRk0S3OftObRMZsPsMXGi+ N3eH+M/jdEBLh/f7wLLrpoLJ5n9yw9Ct9AUUDu2b26TpytrKCE6HZ50usRLvR3EMIcmg xIQ5sLHK9wmWu0KSPk49fvA9WWNRMJeAfcQA/O7grsG36GpW2pC6t7BrVr9WeImvV4X0 Bhp+K0tsWL+92YjKrMtjuLg95OoLHh+hT8x0bNi5UEbvR6I5q04mea97wpElgY7gxqUV jKEQnRE/aF8oXYBm6R9orhvqLys7aj9cj7+GmtRjI0Ta6U/hLNPJIv70eczh/qEGARgP CQ3Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1677007379; 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=aMuuRzeKfqLa9tVG0RwDbPinKoHYnhfALOUXgzx7oRk=; b=SRNObeMEMulDwrtbdxkKCjQJgE93QwWMRkm9DgMbGLHNV0yylBHhCvROSqKr+tJCyD lwPoFMwMqjxKP/RZwrBUkFYHjRE8iDEQ0iCPAaKNzrQ1V+r66TU3ZPWkTmlmP5jDX6au hh84+9m9gkRq4bUzrEI6NhXx2bLBi7UgFAMh7dTLsXyuc2R3umMoWYvn17G15F0kuqDz XXEoI8wE+IpF64XXpWNvSsTylN8NWPmOBmjNrYsr5C7IaO3+JE9ZRZigjhs/m67/jUBg 0ye0m4DJCn5NSNiVu/xdvrem7qhsdPnVfDsZWxJDhTxdkrqPOVnxIc7NMpS6GHVA1Agg 0ryA== X-Gm-Message-State: AO0yUKWKQYGhmQI8KDKajeXhYiYM4KKYgHuf1L1CHL6KMuaWma8Sqko3 uE/qQgGSanY9BPQEZIj9g1P5Nw== X-Google-Smtp-Source: AK7set9hrK23bEGqm8RhipoTGcpfwuIeNwH3hJ/vNG4/y8X/AIHUs1zuHZf28SRX8XenMEYZIrogWA== X-Received: by 2002:a4a:d638:0:b0:51a:6ea9:5053 with SMTP id n24-20020a4ad638000000b0051a6ea95053mr2457834oon.9.1677007378923; Tue, 21 Feb 2023 11:22:58 -0800 (PST) Received: from ?IPV6:2804:1b3:a7c3:3a5:a0:9596:f951:7d30? ([2804:1b3:a7c3:3a5:a0:9596:f951:7d30]) by smtp.gmail.com with ESMTPSA id b8-20020a4ae808000000b005228aa095d3sm1417133oob.39.2023.02.21.11.22.57 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 21 Feb 2023 11:22:58 -0800 (PST) Message-ID: <121176d3-b725-3ec4-373a-ff077cc63c90@linaro.org> Date: Tue, 21 Feb 2023 16:22:55 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.7.2 Subject: Re: [PATCH] i386: Use pthread_barrier for synchronization on tst-bz21269 Content-Language: en-US To: DJ Delorie Cc: libc-alpha@sourceware.org, fweimer@redhat.com 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=-5.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,KAM_NUMSUBJECT,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=no 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 20/02/23 19:55, DJ Delorie wrote: > > The resulting test has two calls to pthread_barrier_wait() in the > subthread but three calls in main(). These will quickly get out of > sync. > > > Adhemerval Zanella via Libc-alpha writes: >> - - C11 atomics instead of plain access. >> + - Use pthread_barrier instead of atomic and futexes. > > Is this true relative to the original testcase? Still, merely a > comment, so OK. The original refers to the kernel tests in here. > >> -#include >> - >> #include >> -#include > > Ok. > >> +#include > > Ok. > >> -static int >> -futex (int *uaddr, int futex_op, int val, void *timeout, int *uaddr2, >> - int val3) >> -{ >> - return syscall (SYS_futex, uaddr, futex_op, val, timeout, uaddr2, val3); >> -} > > We remove all calls to futex, so no longer need this. Ok. > >> - TEST_VERIFY_EXIT (sigaction (sig, &sa, 0) == 0); >> + xsigaction (sig, &sa, 0); > > Ok. > >> -/* Possible values of futex: >> - 0: thread is idle. >> - 1: thread armed. >> - 2: thread should clear LDT entry 0. >> - 3: thread should exit. */ >> -static atomic_uint ftx; > > Ok. > >> +static pthread_barrier_t barrier; > > New, ok. > >> static void * >> threadproc (void *ctx) >> { >> - while (1) >> + for (int i = 0; i < 5; i++) > > This matches the loop in main. Ok. In the future, a #define loop limit > would be appropriate, to prevent these getting out of sync. Or a > comment that it has to match main(). Ack. > >> { >> - futex ((int *) &ftx, FUTEX_WAIT, 1, NULL, NULL, 0); >> - while (atomic_load (&ftx) != 2) >> - { >> - if (atomic_load (&ftx) >= 3) >> - return NULL; >> - } >> + xpthread_barrier_wait (&barrier); > > First barrier, ok. > >> /* clear LDT entry 0. */ >> const struct user_desc desc = { 0 }; >> xmodify_ldt (1, &desc, sizeof (desc)); > > Leave the stuff the thread is actually testing ;-) > >> - /* If ftx == 2, set it to zero, If ftx == 100, quit. */ >> - if (atomic_fetch_add (&ftx, -2) != 2) >> - return NULL; >> + /* Wait for 'ss' set in main thread. */ >> + xpthread_barrier_wait (&barrier); > > Second barrier, ok. > >> } >> + >> + return NULL; > > Ok, moved from above. > >> } >> >> >> @@ -180,6 +162,8 @@ do_test (void) >> /* Some kernels send SIGBUS instead. */ >> xsethandler (SIGBUS, sigsegv_handler, 0); >> >> + xpthread_barrier_init (&barrier, NULL, 2); > > Initialize; must have two callers. Ok - main and thread both call. > >> thread = xpthread_create (0, threadproc, 0); >> >> asm volatile ("mov %%ss, %0" : "=rm" (orig_ss)); >> @@ -190,8 +174,7 @@ do_test (void) >> continue; >> >> /* Make sure the thread is ready after the last test. */ >> - while (atomic_load (&ftx) != 0) >> - ; >> + xpthread_barrier_wait (&barrier); > > First barrier, ok. > >> struct user_desc desc = { >> .entry_number = 0, >> @@ -207,28 +190,21 @@ do_test (void) >> >> xmodify_ldt (0x11, &desc, sizeof (desc)); >> >> - /* Arm the thread. */ >> - ftx = 1; >> - futex ((int*) &ftx, FUTEX_WAKE, 0, NULL, NULL, 0); >> + xpthread_barrier_wait (&barrier); > > Second barrier, ok. > >> asm volatile ("mov %0, %%ss" : : "r" (0x7)); >> >> - /* Fire up thread modify_ldt call. */ >> - atomic_store (&ftx, 2); >> - >> - while (atomic_load (&ftx) != 0) >> - ; >> + xpthread_barrier_wait (&barrier); > > Third barrier? This puts main and the thread out of sync. Indeed, this is wrong. I will remove it. > >> /* On success, modify_ldt will segfault us synchronously and we will >> escape via siglongjmp. */ >> support_record_failure (); >> } >> >> - atomic_store (&ftx, 100); >> - futex ((int*) &ftx, FUTEX_WAKE, 0, NULL, NULL, 0); >> - > > Ok. > >> xpthread_join (thread); >> >> + xpthread_barrier_destroy (&barrier); >> + > > Ok. >