From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, MENTIONS_GIT_HOSTING,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 55D45ECE58E for ; Thu, 17 Oct 2019 12:45:51 +0000 (UTC) Received: from dpdk.org (dpdk.org [92.243.14.124]) by mail.kernel.org (Postfix) with ESMTP id D596E20854 for ; Thu, 17 Oct 2019 12:45:50 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="alhKkJZi" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D596E20854 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=dev-bounces@dpdk.org Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id AAAD21E954; Thu, 17 Oct 2019 14:45:49 +0200 (CEST) Received: from us-smtp-delivery-1.mimecast.com (us-smtp-2.mimecast.com [207.211.31.81]) by dpdk.org (Postfix) with ESMTP id 146B21E94A for ; Thu, 17 Oct 2019 14:45:47 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1571316347; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=I8a7a+hk1VmEFH7wBlarYPq8jTqyq6huUUlmIxydF7E=; b=alhKkJZi+sZHtkPE2MXpWoHBiZNaF5/Y3cmQaF6o7iXncvcE1xydPQHZBUJFanOE2Z9mxw NR6H8YgqKcfHRCWxB34qWwpVNe2ZCyN16tG3aH15XkM1dSKpikBGXMgvjUowHofr8ELoyt MU+Xo7BawY+XJLBl1gfDJOpDKWLvRPA= Received: from mail-io1-f72.google.com (mail-io1-f72.google.com [209.85.166.72]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-229-sHyksjVpP861YSdHsyELsg-1; Thu, 17 Oct 2019 08:45:46 -0400 Received: by mail-io1-f72.google.com with SMTP id w8so2944382iod.21 for ; Thu, 17 Oct 2019 05:45:45 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=OBqXSQ5g3JZEgKjjkQFUIn6xRES09oH+ZYMOR/mTqjg=; b=YZHMd2NkVNCqr4/brGrJKJLVY4al3Vh4VNf1yBWev72qJQB41y/Waf4ZIRullI8VeK cw6B++wPShW/SJOzhj9o77pOCuNBgZYhPg/ULO9dJQoCjwuKbBnJa9o1pVb7zVszVyM8 dkFRF+qGzjfzhhUDRnPKmnjAJSazeETt80l4yC58pBAvzj9EPsGMJ8uhuqLRGfZNOaH1 bMWQ12ajZmv68MqMpAqbkB0ELJuw4arflTnEemFvDGzmvEXwnxkiSZ5ScAkKKewhbaCD VJKEPojpt/3OX8pLydJBK+Z8BglTFc9BFPUFBUqqfYwsNNu8zo7aFdlpkrkRTnzFRCPg 0R2w== X-Gm-Message-State: APjAAAW0s/RCcdD5uoqo/5YxWwIZODYVMxSSDtdLhTJNJax8cdqhv/HV PNOkUEJM+kmLjXQC5cF8oMjpKW0l4jal5gqA3vGrwxAsTOJr3Z6IwDowr70y8GacXOaCLrp64FH 2FvPLDyvOCKw/O50SlgY= X-Received: by 2002:a6b:7d4b:: with SMTP id d11mr2782073ioq.40.1571316345376; Thu, 17 Oct 2019 05:45:45 -0700 (PDT) X-Google-Smtp-Source: APXvYqxHQw7grVm9EDfD0o98JFdcYqwMFgHthNGAznzbmGSlYQeJyPGmfhYoqK8AMPvuYDG09hZb3MU8yIzIs1zGTgs= X-Received: by 2002:a6b:7d4b:: with SMTP id d11mr2782054ioq.40.1571316345059; Thu, 17 Oct 2019 05:45:45 -0700 (PDT) MIME-Version: 1.0 References: <20190723070536.30342-1-jerinj@marvell.com> <1565771263-27353-1-git-send-email-phil.yang@arm.com> In-Reply-To: From: David Marchand Date: Thu, 17 Oct 2019 14:45:33 +0200 Message-ID: To: "Phil Yang (Arm Technology China)" Cc: "thomas@monjalon.net" , "jerinj@marvell.com" , Gage Eads , dev , "hemant.agrawal@nxp.com" , Honnappa Nagarahalli , "Gavin Hu (Arm Technology China)" , nd X-MC-Unique: sHyksjVpP861YSdHsyELsg-1 X-Mimecast-Spam-Score: 0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [dpdk-dev] [PATCH v9 1/3] eal/arm64: add 128-bit atomic compare exchange X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Wed, Oct 16, 2019 at 11:04 AM Phil Yang (Arm Technology China) wrote: > > > -----Original Message----- > > From: David Marchand > > Sent: Tuesday, October 15, 2019 8:16 PM > > To: Phil Yang (Arm Technology China) > > Cc: thomas@monjalon.net; jerinj@marvell.com; Gage Eads > > ; dev ; hemant.agrawal@nxp.com; > > Honnappa Nagarahalli ; Gavin Hu (Arm > > Technology China) ; nd > > Subject: Re: [dpdk-dev] [PATCH v9 1/3] eal/arm64: add 128-bit atomic > > compare exchange > > > > On Tue, Oct 15, 2019 at 1:32 PM Phil Yang (Arm Technology China) > > wrote: > > > > -----Original Message----- > > > > From: David Marchand > > > > If LSE is available, we expose __rte_cas_XX (explicitely) *non* > > > > inlined functions, while without LSE, we expose inlined __rte_ldr_X= X > > > > and __rte_stx_XX functions. > > > > So we have a first disparity with non-inlined vs inlined functions > > > > depending on a #ifdef. > > > > You did not comment on the inline / no inline part and I still see > > this in the v10. > > Is this __rte_noinline on the CAS function intentional? > > Apologize for missing this item. Yes, it is to avoid ABI break. > Please check > 5b40ec6b966260e0ff66a8a2c689664f75d6a0e6 ("mempool/octeontx2: fix possibl= e arm64 ABI break") Looked at the kernel parts on LSE CAS (thanks for the pointer) but I see inlines are used: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arc= h/arm64/include/asm/atomic_lse.h#n365?h=3Dv5.4-rc3 What is special in the kernel or in dpdk that makes this different? > > > > > > > > > Then, we have a second disparity with two sets of "apis" depending = on > > > > this #ifdef. > > > > > > > > And we expose those sets with a rte_ prefix, meaning people will tr= y > > > > to use them, but those are not part of a public api. > > > > > > > > Can't we do without them ? (see below [2] for a proposal with ldr/s= tx, > > > > cas should be the same) > > > > > > No, it doesn't work. > > > Because we need to verify the return value at the end of the loop for= these > > macros. > > > > Do you mean the return value for the stores? > > It is my bad. I missed the ret option in the macro. This approach works. Ok, thanks for confirming. > > However, I suggest to keep them as static inline functions rather than a = piece of macro in the rte_atomic128_cmp_exchange API. > One reason is APIs name can indicate the memory ordering of these operati= ons. API? Those inlines are not part of a public API and we agree this patch is not about adding 128 bits load/store apis. My proposal gives us small code that looks like: if (ldx_mo =3D=3D __ATOMIC_RELAXED) __READ_128("ldxp", dst, old); else __READ_128("ldaxp", dst, old); I am not a memory order guru, but with this, I can figure the asm instruction depends on it. And, since we are looking at internals of an implementation, this is mainly for people looking at/maintaining these low level details. > Moreover, it uses the register type to pass the value in the inline funct= ion, so it should not have too much cost comparing with the macro. This is not a problem of cost, this is about hiding architecture details from the final user. If you expose something, you can expect someone will start using it and will complain later if you break it. > I also think these 128bit load and store functions can be used in other p= laces, once it has been proved valuable in rte_atomic128_cmp_exchange API. = But let's keep them private for the current stage. Yes I agree this could be introduced in the future. > BTW, Linux kernel implemented in the same way. https://github.com/torvald= s/linux/blob/master/arch/arm64/include/asm/atomic_lse.h#L19 Ok kernel exposes its internals, but I think kernel developpers are more vigilant than dpdk developpers on what is part of the public API and what is internal. -- David Marchand