From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6461 invoked by alias); 12 Jan 2018 12:40:41 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 6448 invoked by uid 89); 12 Jan 2018 12:40:41 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.9 required=5.0 tests=BAYES_00,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=no version=3.3.2 spammy= X-HELO: mx1.redhat.com Subject: Re: [PATCH] Implement allocate_once To: Torvald Riegel Cc: Carlos O'Donell , GNU C Library , "Dmitry V. Levin" References: <6e4b6b22-3f38-f798-f2e0-eb22311ed6b5@redhat.com> <1515086144.9480.286.camel@redhat.com> <2d12ba41-e3ff-70db-484e-19d9805c5e53@redhat.com> <1515090825.9480.295.camel@redhat.com> <267148c7-61c9-b45b-7533-96284cae5e7d@redhat.com> <8a260297-4aaa-2a04-776a-cb5cbbb1fdf0@redhat.com> <1515757664.4439.113.camel@redhat.com> From: Florian Weimer Message-ID: <712e687f-f3a1-ee46-3a7d-4e1c71f9d433@redhat.com> Date: Fri, 12 Jan 2018 12:40:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: <1515757664.4439.113.camel@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2018-01/txt/msg00449.txt.bz2 On 01/12/2018 12:47 PM, Torvald Riegel wrote: > The most recent patch looks good to me. Thanks! > I have a minor comment for future patches: > > On Fri, 2018-01-12 at 10:46 +0100, Florian Weimer wrote: >> + while (true) >> + { >> + /* Synchronizes with the acquire MO load in allocate_once. */ >> + void *expected = NULL; >> + if (atomic_compare_exchange_weak_release (place, &expected, result)) >> + return result; >> + >> + /* The failed CAS has relaxed MO semantics, so perform another >> + acquire MO load. */ >> + void *other_result = atomic_load_acquire (place); > > You could just use expected here, because it has been updated, and ... > >> + if (other_result == NULL) >> + /* Spurious failure. Try again. */ >> + continue; >> + >> + /* We lost the race. Free what we allocated and return the >> + other result. */ > > ... add an atomic_thread_fence_acquire() here. Isn't a an acquire MO load potentially cheaper than an acquire fence? The fence needs to synchronize with all threads, while the load only needs to synchronize with threads which have performed release MO store (or stronger) on the variable? Florian