From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oa1-x30.google.com (mail-oa1-x30.google.com [IPv6:2001:4860:4864:20::30]) by sourceware.org (Postfix) with ESMTPS id 210383858CDB for ; Wed, 21 Sep 2022 17:17:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 210383858CDB 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-oa1-x30.google.com with SMTP id 586e51a60fabf-127ba06d03fso10059160fac.3 for ; Wed, 21 Sep 2022 10:17:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=content-transfer-encoding:in-reply-to:organization:from:references :to:content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date; bh=2h99Vk+6wIHm4jQxDLvVNrNGhLoJPFr5GMBWbIQ6YaE=; b=UNKsNvE5yhbyf4hjiQ/xh+H+l9vLhGrQGvyl9depQikhkH671C0y06AscXqp7312nY rtmSmWoTp+KGInd2f0r+C2fz1oCOoBKCCf1z6xR6oDN65jGMb6p+7Q42YaGd4bI9TqXe WL2j+WB/Gco8F2fwtUszg/0vBDddFgdncfZsXGV1udDJYc0ZycW2VNw13QG/0bXF7tcv a/Myf53M4pTJe20sb3CCaWzackW1QK5qU5yCM02phgNEW1laYfJYILTWZQbQO79MIYGP sgDbpGmewc/yaN+66vYTnGd8jacQZtEpMW9kQMpC5q5DIklHM0IRsDC39mj6wBa8qjAd 9Xbw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:organization:from:references :to:content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date; bh=2h99Vk+6wIHm4jQxDLvVNrNGhLoJPFr5GMBWbIQ6YaE=; b=ecvYmVOwOYLMIiO0dL+gN8p5bcMVTRprb2bnA3XEqxFitrzB7L2rtah06JIAeZpfzk n1eOnxTdf8juMbzX+RLIiS0v/VxlGwrJEQdsclUfbRlJfFxHpbqp5tnRJMO5o76yxsXl nOAg5gxZkkp+4i2A2Yr4enx+MhZI9GnwYw6vIpI6AD4kkHAh8S8ZE5p0I9UM3r7OgxzT PE+lW3H96ojfPVlXMgt9cQZDQq9YwfnhltPSmK5e8O9+1onsNShyxB1Dtle2JDq1SPCs dHfOCjN/YCeoOX6CiLWj+nnaYKsepnz6KzOYWURjLjmIQYapdomCM2mZohH6VGax1rrw 7fJw== X-Gm-Message-State: ACrzQf1H6Qx/5Ctl8swGTouOcumqZhQztcIPtlweQra9T7vJlyooIkyV BN4yyybhSP4yHQv4dXkMlmdELuSIiA6zuSf2 X-Google-Smtp-Source: AMsMyM5p9V+tQr0UkVQ5mcCfof4k4tHHhNp7sFE3d5DCUjK5s8PEA6bnmfPTzDCDu7txSXF6eis0RA== X-Received: by 2002:a05:6870:4193:b0:127:3eb7:b99b with SMTP id y19-20020a056870419300b001273eb7b99bmr5654621oac.157.1663780668092; Wed, 21 Sep 2022 10:17:48 -0700 (PDT) Received: from ?IPV6:2804:1b3:a7c1:c266:cc93:9f8:1eba:d2e8? ([2804:1b3:a7c1:c266:cc93:9f8:1eba:d2e8]) by smtp.gmail.com with ESMTPSA id y12-20020a0568301d8c00b00655bb503564sm1534652oti.50.2022.09.21.10.17.46 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 21 Sep 2022 10:17:47 -0700 (PDT) Message-ID: Date: Wed, 21 Sep 2022 14:17:45 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.2.2 Subject: Re: [PATCH] Use C11 atomics instead of atomic_increment(_val) Content-Language: en-US To: Wilco Dijkstra , 'GNU C Library' 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=-14.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_LOTSOFHASH,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 08/09/22 11:01, Wilco Dijkstra wrote: > Replace atomic_increment and atomic_increment_val with atomic_fetch_add_relaxed. > One case in sem_post.c uses release semantics (see comment above it). > > Passes regress on AArch64. Looks good, I am just not sure about two relaxed MO below. > > --- > > diff --git a/htl/pt-create.c b/htl/pt-create.c > index ce52ed9f52210a4e4c7a049ebee817ec9ccfeeb1..14f02cd2b8a19e8581a170dfba2b948ef8304203 100644 > --- a/htl/pt-create.c > +++ b/htl/pt-create.c > @@ -228,7 +228,7 @@ __pthread_create_internal (struct __pthread **thread, > the number of threads from within the new thread isn't an option > since this thread might return and call `pthread_exit' before the > new thread runs. */ > - atomic_increment (&__pthread_total); > + atomic_fetch_add_relaxed (&__pthread_total, 1); > > /* Store a pointer to this thread in the thread ID lookup table. We > could use __thread_setid, however, we only lock for reading as no Not sure if this is correct for hurd, __pthread_total is used on __pthread_exit to call exit although for i686 atomic_decrement_and_test will be used and it has strong MO. > diff --git a/manual/ipc.texi b/manual/ipc.texi > index 081b98fe29e0b3b5b7f4f916ad2085d170bd3825..f7cbdc3e09b0b4aea9a96ddcaf571c474024cc32 100644 > --- a/manual/ipc.texi > +++ b/manual/ipc.texi > @@ -85,7 +85,7 @@ by @theglibc{}. > > @deftypefun int sem_wait (sem_t *@var{sem}); > @safety{@prelim{}@mtsafe{}@assafe{}@acunsafe{@acucorrupt{}}} > -@c atomic_increment (nwaiters) acucorrupt > +@c atomic_fetch_add_relaxed (nwaiters) acucorrupt > @c > @c Given the use atomic operations this function seems > @c to be AS-safe. It is AC-unsafe because there is still > diff --git a/manual/llio.texi b/manual/llio.texi > index 92bfd93e067ce2a782369084ed7dae099300f418..4e6e3fb672bb8ecbb1bb52faf3d70d9b7b33973f 100644 > --- a/manual/llio.texi > +++ b/manual/llio.texi > @@ -2528,7 +2528,7 @@ aiocb64}, since the LFS transparently replaces the old interface. > @c _dl_allocate_tls_init ok > @c GET_DTV ok > @c mmap ok > -@c atomic_increment_val ok > +@c atomic_fetch_add_relaxed ok > @c munmap ok > @c change_stack_perm ok > @c mprotect ok > @@ -2567,7 +2567,7 @@ aiocb64}, since the LFS transparently replaces the old interface. > @c do_clone @asulock @ascuheap @aculock @acsmem > @c PREPARE_CREATE ok > @c lll_lock (pd->lock) @asulock @aculock > -@c atomic_increment ok > +@c atomic_fetch_add_relaxed ok > @c clone ok > @c atomic_decrement ok > @c atomic_exchange_acquire ok Ok. > diff --git a/nptl/nptl_setxid.c b/nptl/nptl_setxid.c > index aa863c7ea8122ea01d1aa4cffe101bbb7c11270c..3b7e2d434abe8a15145349d1a08a4e706061c74d 100644 > --- a/nptl/nptl_setxid.c > +++ b/nptl/nptl_setxid.c > @@ -163,7 +163,7 @@ setxid_signal_thread (struct xid_command *cmdp, struct pthread *t) > /* If this failed, it must have had not started yet or else exited. */ > if (!INTERNAL_SYSCALL_ERROR_P (val)) > { > - atomic_increment (&cmdp->cntr); > + atomic_fetch_add_relaxed (&cmdp->cntr, 1); > return 1; > } > else I am not sure if relaxed MO is correct here, shouldn't it synchronize with the __nptl_setxid_sighandler ? > diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c > index 8533d609684bc6681fbdccd9f2635d03ef3aa937..2602dba54e872f36bd54955af7587378e5dc3812 100644 > --- a/nptl/pthread_create.c > +++ b/nptl/pthread_create.c > @@ -759,7 +759,7 @@ __pthread_create_2_1 (pthread_t *newthread, const pthread_attr_t *attr, > we momentarily store a false value; this doesn't matter because there > is no kosher thing a signal handler interrupting us right here can do > that cares whether the thread count is correct. */ > - atomic_increment (&__nptl_nthreads); > + atomic_fetch_add_relaxed (&__nptl_nthreads, 1); > > /* Our local value of stopped_start and thread_ran can be accessed at > any time. The PD->stopped_start may only be accessed if we have Ok. > diff --git a/nptl/sem_post.c b/nptl/sem_post.c > index 9e5741753a741034db96cfff8b8978908fafc1f6..7ec21e92eb4c71d7f17764e96bc7603837f7522d 100644 > --- a/nptl/sem_post.c > +++ b/nptl/sem_post.c > @@ -91,7 +91,7 @@ __old_sem_post (sem_t *sem) > /* We must need to synchronize with consumers of this token, so the atomic > increment must have release MO semantics. */ > atomic_write_barrier (); > - (void) atomic_increment_val (futex); > + atomic_fetch_add_release (futex, 1); > /* We always have to assume it is a shared semaphore. */ > futex_wake (futex, 1, LLL_SHARED); > return 0; Ok. > diff --git a/nscd/cache.c b/nscd/cache.c > index b66c35334a79a315ac9fd533df09ec097221d8e0..21af9a0f9550dfc7c1121fb5014cd80acdfcf696 100644 > --- a/nscd/cache.c > +++ b/nscd/cache.c > @@ -192,7 +192,7 @@ cache_add (int type, const void *key, size_t len, struct datahead *packet, > > /* We depend on this value being correct and at least as high as the > real number of entries. */ > - atomic_increment (&table->head->nentries); > + atomic_fetch_add_relaxed (&table->head->nentries, 1); > > /* It does not matter that we are not loading the just increment > value, this is just for statistics. */ Ok. > diff --git a/nscd/nscd_helper.c b/nscd/nscd_helper.c > index 8e66fa2548e87c9b24cade41339c83f4cef7f2cf..d3e05e272a854e13c0e4d18594ea57336b8db2bf 100644 > --- a/nscd/nscd_helper.c > +++ b/nscd/nscd_helper.c > @@ -425,7 +425,7 @@ __nscd_get_map_ref (request_type type, const char *name, > 0)) > cur = NO_MAPPING; > else > - atomic_increment (&cur->counter); > + atomic_fetch_add_relaxed (&cur->counter, 1); > } > } > Ok. > diff --git a/sysdeps/unix/sysv/linux/check_pf.c b/sysdeps/unix/sysv/linux/check_pf.c > index 4d486ca9b5026c7a7950bd7c7155212966df3c44..0b77a2d897027edc36e956301f018c2ab0121444 100644 > --- a/sysdeps/unix/sysv/linux/check_pf.c > +++ b/sysdeps/unix/sysv/linux/check_pf.c > @@ -72,8 +72,8 @@ static uint32_t nl_timestamp; > uint32_t > __bump_nl_timestamp (void) > { > - if (atomic_increment_val (&nl_timestamp) == 0) > - atomic_increment (&nl_timestamp); > + if (atomic_fetch_add_relaxed (&nl_timestamp, 1) + 1 == 0) > + atomic_fetch_add_relaxed (&nl_timestamp, 1); > > return nl_timestamp; > } > @@ -309,7 +309,7 @@ __check_pf (bool *seen_ipv4, bool *seen_ipv6, > if (cache_valid_p ()) > { > data = cache; > - atomic_increment (&cache->usecnt); > + atomic_fetch_add_relaxed (&cache->usecnt, 1); > } > else > { > Ok.