From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 88882 invoked by alias); 6 Oct 2017 06:38:32 -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 87771 invoked by uid 89); 6 Oct 2017 06:38:31 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=employ, H*M:5876 X-HELO: mx1.redhat.com DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 148A3820F7 Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=fweimer@redhat.com Subject: Re: [PATCH v2] Fix double-checked locking in __gconv_get_path and __gconv_read_conf [BZ #22062] To: Carlos O'Donell , Arjun Shankar , libc-alpha@sourceware.org References: <20171002143148.GA41930@aloka.lostca.se> <940c9be0-37ae-ebf7-1013-e6e6a4991028@redhat.com> <08eed690-d5eb-ce13-0c61-15a93e4bc8a7@redhat.com> From: Florian Weimer Message-ID: <1b058316-b54d-165d-5876-af1327d0cfde@redhat.com> Date: Fri, 06 Oct 2017 06:38:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <08eed690-d5eb-ce13-0c61-15a93e4bc8a7@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2017-10/txt/msg00309.txt.bz2 On 10/06/2017 01:05 AM, Carlos O'Donell wrote: > Is there any reason we need this atomic load? > >> + gconv_path_elem_local = atomic_load_relaxed (&__gconv_path_elem); > ^^^^^^^^^^^^^^^^^^^ Oh, this follows just the general rule that if atomics are used, all accesses should employ atomics. This follows from the general principle of C11 compatibility, where an atomic type would default to seq-cst access (which we don't want here). I see this was not mentioned in the Concurrency wiki page. I've fixed that. I did not notice that there was a __libc_once guard. So it would be better to remove the locking from __gconv_get_path and document that (including that this serializes initialization of the gconv cache via __gconv_load_cache). Thanks, Florian