From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1758921AbcIHWYW (ORCPT <rfc822;w@1wt.eu>);
        Thu, 8 Sep 2016 18:24:22 -0400
Received: from smtprelay.synopsys.com ([198.182.60.111]:37563 "EHLO
        smtprelay.synopsys.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1758750AbcIHWYV (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Thu, 8 Sep 2016 18:24:21 -0400
Subject: Re: [PATCH] ARCv2: Implement atomic64 based on LLOCKD/SCONDD
 instructions
To: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "Peter Zijlstra" <peterz@infradead.org>
References: <1473362967-23750-1-git-send-email-vgupta@synopsys.com>
CC: Alexey Brodkin <Alexey.Brodkin@synopsys.com>,
        Will Deacon <will.deacon@arm.com>,
        "linux-snps-arc@lists.infradead.org" 
        <linux-snps-arc@lists.infradead.org>
From: Vineet Gupta <Vineet.Gupta1@synopsys.com>
Message-ID: <7012c761-a355-d830-22bf-48db96a12452@synopsys.com>
Date: Thu, 8 Sep 2016 15:24:10 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101
 Thunderbird/45.2.0
MIME-Version: 1.0
In-Reply-To: <1473362967-23750-1-git-send-email-vgupta@synopsys.com>
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
X-Originating-IP: [10.9.130.78]
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 09/08/2016 12:29 PM, Vineet Gupta wrote:
> One thing I'm not sure of is the lack of explicit memory clobber in
> barrier-less ops e.g. atomic64_add() (BTW same is true for 32-bit
> atomic_add() as well). Per commit 398aa66827 ("ARM: 6212/1: atomic ops:
> add memory constraints to inline asm ") Will fixed ARM code by adding
> appropriate constraint to atomic64_add(). For ARC instead adding memory
> clobber to atomic64_set() does the trick (otherwise self-test is broked)
> This is on ARC we can't possibly use "m" in atomic64_add() since that make gcc
> emit register relative effective addresses which LLOCKD/SCONDD are not
> allowed by ISA

So interestingly my self-test run fine, but I had this oldish version stashed
somewhere which did something liek below and that clearly generates wrong code.

int my_test_atomic(void)
{
    long v0 = 0x33333333;
    long onestwos = 0x11112222;

    atomic_t v = ATOMIC_INIT(v0);
    long long r = v0;
    int ret = 0;

    atomic_set(&v, v0); r = v0;
    atomic_add(onestwos, &v);
    r += onestwos;
    if (v.counter != r) {      /* <------ */
                ret = 3;   /* error */
    }

    return ret;
}

key here is the check - if we access the atomic directly, I get error. If I use
atomic_read() which forces a reload due to volatile, things are hunky dory. So it
seems to me we don't need memory clobber or equivalent in barrier less atomics
except the set. Seems too fragile ?

-Vineet