From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 47806 invoked by alias); 1 Nov 2016 16:00:01 -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 47691 invoked by uid 89); 1 Nov 2016 16:00:01 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.4 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=casa, route, exchanges, H*Ad:U*davem X-HELO: mx1.redhat.com Message-ID: <1478015984.7146.645.camel@localhost.localdomain> Subject: Re: [RFC][PATCH 0/2] Make sparcv8 work again on cas enabled hardware From: Torvald Riegel To: Andreas Larsson Cc: GNU C Library , Adhemerval Zanella , "Carlos O'Donell" , David Miller , software@gaisler.com Date: Tue, 01 Nov 2016 16:00:00 -0000 In-Reply-To: <1478012867-6031-1-git-send-email-andreas@gaisler.com> References: <1478012867-6031-1-git-send-email-andreas@gaisler.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-SW-Source: 2016-11/txt/msg00032.txt.bz2 On Tue, 2016-11-01 at 16:07 +0100, Andreas Larsson wrote: > This patch series: > > 1) Fixes a sparcv8 bug introduced since the #error was added to > sysdeps/sparc/sparc32/pthread_barrier_wait.c in 2.23. This fix stops > incorrect usage of sendmsg and recvmsg Linux system calls for sparcv8. I don't know whether the changes this patch applies make sense, but otherwise the patch looks okay to me. This could also be a separate patch I think. Do you have a copyright assignment in place for glibc? > 2) Makes use of the CASA compare and swap instruction for atomic_* > functions sparcv8, that is available for most LEON3 and LEON4 designs > and implied by -mcpu=leon3, but not part of the sparcv8 standard. To > allow for easy kernel emulation on systems that lack the instruction, > the CASA instruction is used for all writing atomic_* functions. This > approach is discussed in thread [1]. Before I can review the patch in detail, I think there are a few high-level things that need to be taken care of. We need to document what sparc32 systems we actually support. Your patch looks like for now, CAS is required. If this is true, this should be documented (e.g., I guess this would need a NEWS item too). Is there a way to test this new requirement at build time, or is this just a runtime requirement? If this is a build-time requirement, is the assembler actually already aware that it can use a CAS (which would remove the need to hand-code the instruction). If we require CAS, the 24b exchanges should just be removed altogether; it seems the only remaining user is the low-level lock. I don't think you need to make all modifying atomic accesses use a CAS underneath, at least if we require CAS. If we will also allow for kernel emulation in the future, it would also be possible to check whether emulation is required and only then route all modifying accesses through the kernel. In the future, we will most likely require 8b atomic loads and stores (perhaps we can do without an 8b CAS, though doing that with a 32b cas is possible). I suggest to also look at whether you can use the new __atomic builtins (ie, as noted by the USE_ATOMIC_COMPILER_BUILTINS define). On the sparc systems that we want to support, will GCC do the right thing for CAS etc. (eg, if the requirement is to build with -mcpu=leon3)? In particular, will it either always emit a CAS instruction or will libatomic use a CAS instruction? If so, you could also just rely on the new atomic builtins and define the legacy atomic operations (ie, those not in C11 style) on top of those. > Any comments are most welcome. The spin lock based sparcv8 semaphore > implementation is currently unchanged by this patchset, but I would say > that that should go as well. Agreed.