From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 6A47242B98;
	Thu, 25 May 2023 10:41:47 +0200 (CEST)
Received: from mails.dpdk.org (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id E6F0E40DDB;
	Thu, 25 May 2023 10:41:46 +0200 (CEST)
Received: from mga01.intel.com (mga01.intel.com [192.55.52.88])
 by mails.dpdk.org (Postfix) with ESMTP id 5D2AF40A82
 for <dev@dpdk.org>; Thu, 25 May 2023 10:41:45 +0200 (CEST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple;
 d=intel.com; i=@intel.com; q=dns/txt; s=Intel;
 t=1685004105; x=1716540105;
 h=date:from:to:cc:subject:message-id:references:
 content-transfer-encoding:in-reply-to:mime-version;
 bh=j5zXwzw5waksItqF9URoRcT9lsZs0SN9wIL2NnmvvUY=;
 b=OMXbZgHWCiNX0iBlENsqAV9lK2v73lMN3I3CMHUe1OVcnk3V4c6lEzkR
 zkNtZAjxPx538laNzo6xLJ/BmaoSgim9u+MIe7SZs4RGkNQsOR1SlSVb5
 h3ia+suRpPg0qhf5uNyjYP1GzXf9dbRUU3oDc2jtYHWnxHWLqCamh6u3A
 OGGv6PDDd75Qz8OPPo+3Dhk9Ba5zEweOgjmLAlsQhZ8B8W2MpZALO/Hv1
 AnT6D+baF7uJTrOsZcshEQL42Tca7jqU+N7O+2Z1+XzSIoRfeUS9O5nz+
 RWY9/4ObqWJUduSANkBmO/Umy+HFHR4NyQSxno2ZgIq8uk1CPtJAePZ8m Q==;
X-IronPort-AV: E=McAfee;i="6600,9927,10720"; a="382077977"
X-IronPort-AV: E=Sophos;i="6.00,190,1681196400"; d="scan'208";a="382077977"
Received: from orsmga001.jf.intel.com ([10.7.209.18])
 by fmsmga101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;
 25 May 2023 01:41:44 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=McAfee;i="6600,9927,10720"; a="737646889"
X-IronPort-AV: E=Sophos;i="6.00,190,1681196400"; d="scan'208";a="737646889"
Received: from fmsmsx601.amr.corp.intel.com ([10.18.126.81])
 by orsmga001.jf.intel.com with ESMTP; 25 May 2023 01:41:41 -0700
Received: from fmsmsx610.amr.corp.intel.com (10.18.126.90) by
 fmsmsx601.amr.corp.intel.com (10.18.126.81) with Microsoft SMTP Server
 (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id
 15.1.2507.23; Thu, 25 May 2023 01:41:41 -0700
Received: from fmsmsx610.amr.corp.intel.com (10.18.126.90) by
 fmsmsx610.amr.corp.intel.com (10.18.126.90) with Microsoft SMTP Server
 (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id
 15.1.2507.23; Thu, 25 May 2023 01:41:40 -0700
Received: from fmsedg601.ED.cps.intel.com (10.1.192.135) by
 fmsmsx610.amr.corp.intel.com (10.18.126.90) with Microsoft SMTP Server
 (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id
 15.1.2507.23 via Frontend Transport; Thu, 25 May 2023 01:41:40 -0700
Received: from NAM02-DM3-obe.outbound.protection.outlook.com (104.47.56.48) by
 edgegateway.intel.com (192.55.55.70) with Microsoft SMTP Server
 (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id
 15.1.2507.23; Thu, 25 May 2023 01:41:40 -0700
ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none;
 b=Sm4xjp8k9Ds5dD2RCOAIm9CTHfDh8lj24tOqF3JLvVR/1LDpB+NHotWIQ1MZN7Fw8kga1Ftnkz2wx/vIrIWtW2vcCA7/0uEH0DD1z3q7DQ9Qo93k0LxFJoaQ8rjjHu4+zv1DtDIG6uKG0fhvuG1ROCNqZiluIt4m0TStThy9OWSdY8oAJdtyIoZTVOIFXsaNX07F1LZA43X2q5wj0IluWiR9pU2YrQQsxHWZ0VDOkUyn3Fk4nxqQ23BfcHRoC2cnXgUykJfnBpKZ9qJN8GyKSdYvUbk1S2IiNcRIpcFNWUFOPpgn/XacfKKl6aXDLvMUr/D2Mr5S1wChJ9CAqBjEkA==
ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; 
 s=arcselector9901;
 h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1;
 bh=FE+WfjEOIgNLmm9NL1UFetudTRfTHu+deX2RSdKa0HI=;
 b=DWtQJxo4iahzjSN79CfNKfWFxxOFgrjapGGLcQGhGtCXIpZ7IiBFZf7yMvD/9O9KIkjjAM+7v9Vu3GQLAX9vrHK5O52aR8/6MTK9RX/c89jxjcO+5CnG/UJC3aLg5fc1OkiXUSHc5dnkDPxy8f7KOiD+ad4zOkyEhQ52APnRxdCX5TBjwi9Jg4gHR7EVnvRsmDrn5jXwgczN+P/LNdteIIPu5NyjVVczD92JlAgQWO89iJj+W9lswKiVaTtnYDvYGFgaJAch43yXehNoS7aaGFNyWgU/XXCcYApbDDv2mctD3v9djYlpulcnS2F0epn7mVC9g578Ypipi4e2Fx0YJg==
ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass
 smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com;
 dkim=pass header.d=intel.com; arc=none
Authentication-Results: dkim=none (message not signed)
 header.d=none;dmarc=none action=none header.from=intel.com;
Received: from DS0PR11MB7309.namprd11.prod.outlook.com (2603:10b6:8:13e::17)
 by SA2PR11MB4874.namprd11.prod.outlook.com (2603:10b6:806:f9::23) with
 Microsoft SMTP Server (version=TLS1_2,
 cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6433.15; Thu, 25 May
 2023 08:41:38 +0000
Received: from DS0PR11MB7309.namprd11.prod.outlook.com
 ([fe80::b8f3:958:d2c5:2232]) by DS0PR11MB7309.namprd11.prod.outlook.com
 ([fe80::b8f3:958:d2c5:2232%7]) with mapi id 15.20.6433.017; Thu, 25 May 2023
 08:41:38 +0000
Date: Thu, 25 May 2023 09:41:30 +0100
From: Bruce Richardson <bruce.richardson@intel.com>
To: David Marchand <david.marchand@redhat.com>
CC: Kevin Laatz <kevin.laatz@intel.com>, <dev@dpdk.org>,
 <Honnappa.Nagarahalli@arm.com>, <Ruifeng.Wang@arm.com>,
 <thomas@monjalon.net>, <stephen@networkplumber.org>,
 <mb@smartsharesystems.com>, Tyler Retzlaff <roretzla@linux.microsoft.com>
Subject: Re: [PATCH v3 3/7] dma/idxd: replace rte atomics with GCC builtin
 atomics
Message-ID: <ZG8fOmJTBFyEup1K@bricha3-MOBL.ger.corp.intel.com>
References: <1679084388-19267-1-git-send-email-roretzla@linux.microsoft.com>
 <1679612036-30773-1-git-send-email-roretzla@linux.microsoft.com>
 <1679612036-30773-4-git-send-email-roretzla@linux.microsoft.com>
 <CAJFAV8xDgwfFX0sVnPqrg=zVPQAVwNkfrOM=EEexg4rs6JWW1Q@mail.gmail.com>
Content-Type: text/plain; charset="utf-8"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAJFAV8xDgwfFX0sVnPqrg=zVPQAVwNkfrOM=EEexg4rs6JWW1Q@mail.gmail.com>
X-ClientProxiedBy: LO2P265CA0513.GBRP265.PROD.OUTLOOK.COM
 (2603:10a6:600:13b::20) To DS0PR11MB7309.namprd11.prod.outlook.com
 (2603:10b6:8:13e::17)
MIME-Version: 1.0
X-MS-PublicTrafficType: Email
X-MS-TrafficTypeDiagnostic: DS0PR11MB7309:EE_|SA2PR11MB4874:EE_
X-MS-Office365-Filtering-Correlation-Id: 95cf0749-6efd-433a-69b8-08db5cfbdacb
X-MS-Exchange-SenderADCheck: 1
X-MS-Exchange-AntiSpam-Relay: 0
X-Microsoft-Antispam: BCL:0;
X-Microsoft-Antispam-Message-Info: x+yryOtdNh5zx8HSyWoJlx79vFT4X9zmUxwMTN5T0bpverk/hHAn7qhMrKS+dQi/ZNLW5A64Jy7pbwToFDTJPIrF3TnA85cdgoJVGzrc9+azGQBPu61gem9qLaRohjgEQDSB0tiLDkW75uqV/DO3PGppXh+1BBxHYhOYWtQf7IT5pSITK/aO20gKzcSIWwCCs1U+FnrALGnSLRJOva6ra9aJ6eXtXBSdqpX219Hlh6BOMXtzU+80DLH0gFGrK6eLjTG25NemRL/jMlAFJXA0Mvi1YL20nw/Jk3gmtZLcZxJ1tITlIUxOdbRY4xoGqrc2NVzAJ5HCd/zJYMRfnYZJ4I2/AEfLO35Iil/2AonOJvH9kNHb4ysck/PZ/TFFWYxBHMBmHhN9M08Dn260OiTWMsWgBfQAYDKLmIdQp6NgLKpOemz3R/dbMlc/Zz16oGQrwWCcotepFBbFCjgyW/RMPJ/v6t7sDsMiHT3zyTLNeFd1EAqGCrNYGgm8guYe1dwgfRVz6taANJYlNH7aPM25yfaaI4KV1DhIM0KCMtMANW8zUH6MJZT3f/mKzR4HQcl5
X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:;
 IPV:NLI; SFV:NSPM; H:DS0PR11MB7309.namprd11.prod.outlook.com; PTR:; CAT:NONE;
 SFS:(13230028)(346002)(396003)(366004)(376002)(39860400002)(136003)(451199021)(478600001)(6916009)(54906003)(53546011)(6512007)(6506007)(26005)(4326008)(6666004)(316002)(41300700001)(6486002)(66476007)(66946007)(66556008)(8676002)(8936002)(5660300002)(83380400001)(38100700002)(44832011)(82960400001)(186003)(2906002)(86362001);
 DIR:OUT; SFP:1102; 
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?K3hyM2dzUytoVjh2MjhOVjVzV3pqUDJ2eVdLZm1NTGpiRVNEU1FMbUV0SHBr?=
 =?utf-8?B?WUFMUmFzK2FBZTV3T0NRaldQbys4ZEJvRS9GMlh6Uyt6WTlWT0RGdXpCaGtx?=
 =?utf-8?B?VUpwOFBuVHBqbVpHSmlHa1JRR2V4T1RJcWMzT0EwZWs0UTJJWXc5WnVvaXZh?=
 =?utf-8?B?YTlYeWtod1h6WDNSYmVFdFd5V01tVFFkSW54ZXg0eFNPNUx4bDVZWnlycUZU?=
 =?utf-8?B?a2d0ZEtDZk9MVGppWXhYNERHV0VOdE5TNmVrU0phUXozQUpqU2hDV3B6dUVn?=
 =?utf-8?B?cGtxSFh5RHZXZzVJWU5IRFhXVHpLVXJvMVdJR014Q0IvQzJDVnhpaEp6UFdy?=
 =?utf-8?B?WFg5UVBlbmJXOUFJYkhCNEg5WTNsbE5vVW0xeWdPUFovODIyVUhHMUJ3V2Zh?=
 =?utf-8?B?K24vbWowZDgwSGpwR29LNmc4VmhBSkg5OGxjSkUzMWRsbDV2QmxWc3g3cys2?=
 =?utf-8?B?SGlsRmhRYTkwN3pGeUtXaHpjWFdFVzFnMytHZTJxN240dzFBQ1FZZmV3dFI4?=
 =?utf-8?B?RlhGcnVyTzJja3RWRzJxa1lDenlBZXhDTEtwRnVHR3ZJV285bVI0KzBOZC9D?=
 =?utf-8?B?SFZ0WkJ2dVRyNm83MGg1MVl4enVyK1A4N3llVXZweGEyK25MZjJQTFk5S0Vv?=
 =?utf-8?B?YmJVcVUyamxlMUtCYVdHaVpkOW5vSFh0cnpXUlRTK05MdnkzWU1SU0dCSUYw?=
 =?utf-8?B?RktmQjZEd1ZvVlpjazRzVEcxNE44c25tbHJ3M0Rkci9KQzBVS0cvMGsyVS9K?=
 =?utf-8?B?UkVWZ1hwLzNwT25MQVdVek9pTnZlWFYvT3c0eHVKdEJSc0J1QzIvRWhIblhs?=
 =?utf-8?B?QzZ0b1VJMnN4VllUWExGb1o1QmNsTjkwUGV4cjNBSnlpLzYzVGM5QnJsd29j?=
 =?utf-8?B?T3dNRm9XVlBQZ0pwNGFsTXVXa3hxQWhQTC81NVF5SUtiN2JFT0pHNGE4b0hY?=
 =?utf-8?B?Z0xwYUFCM04wcmhTNU5lUXIwcEZKVWNBSXdWbUtVZWFLRzFXMnlnQkRGdHd1?=
 =?utf-8?B?S3pOdmJ4bC90Q0w2UUkveC9pTU1YVVVVVUFKcTRIMTdQTTNYNmVkZXdzYkU1?=
 =?utf-8?B?ZW80L25DcFpnZkF1RXFvSytVRThZY3lLejJzS1VJaW5iQzNwNEJydkpaSjlE?=
 =?utf-8?B?Y3hvNTk5K09zMVhNdDUrNFU5ZVQ1MEIxeDlWeDlKcXJ5N0JwSE9sY1cyWFht?=
 =?utf-8?B?ZTNNclMzdTRwcXd5SWRjYWNid2t2Tyt3SkJSM3E1WVpvVTNkUWVHd3dOZEdU?=
 =?utf-8?B?QXZJTGw4V2RpWWM1N3lPNi9FMGN4b3ZROHpOVlhmQnBDNDZKOG5Ga0Nwdmpp?=
 =?utf-8?B?YkRtUExDTU5RVEZCVEhyM2JBU0liRTNvVUlVV2ZDbzBnVjNiam42Q2pJc3hw?=
 =?utf-8?B?d3lib0VTM0dQeVhqYm5yeUdOSlQvNTlnT3l0ZVRWZ1ltWDJXUFk0OEI3N2RO?=
 =?utf-8?B?U0txek1zSmtlQnZuQnFFQnJ0VFpCTUQzYVEwMnovRDZaeFQ2Ukw1aG9ON2hD?=
 =?utf-8?B?eXZaUi9xdTFudDZoQUUvejRuVzJWV0FMY1pYRzNhRHViQzUvM0N1QjdMS3VL?=
 =?utf-8?B?QlNYRFpmaWY1WlBpcUdPVFJVYUpPVjB5K1hQbGVtaE5Iay9NZXNrSW01N0Z0?=
 =?utf-8?B?Z1ExR21FR3ZVNi9oNUVXb2pCbjJaY25EVE92clBFWGJpUUVTdldkckhVK3N6?=
 =?utf-8?B?dGw3LzhEeDljTmc2WWdIYnVYcVp6djdCUlJaSnNtU25WZkV3SkRCUkpZUENw?=
 =?utf-8?B?N0N1aWxnbjlOTDBLaXNYVkZTcG1WN0Q2ZDZicVRML1RyMXB2N2ltbUlyVEVk?=
 =?utf-8?B?TlQvSDR3QXhiamNQd1FUTXQ4R0NJVmFpZVIzTFlRRDJZNnFiRG9jYWpRaVds?=
 =?utf-8?B?R0l2NWYzUWNRb2o3OFdKYUV2NllYWmFVcnVXbHlodzFtYWUyeUR5SFcvV0JB?=
 =?utf-8?B?RWRrUDZTRnBLUzJ4ei9QWjJybldFUnNwQnorUEU2RjhzaE5SOHRzbm9zdTRQ?=
 =?utf-8?B?NEo0NDFqZ3R6N1Z2REdJd2IxbjN1WURaREhkVlluNTZBR3AwSzdXbW9NQWky?=
 =?utf-8?B?OEJCRFhBMld5OG9lcWlOSm1rVzhXZGloYW9ISy9Rbnc3VHpHMWhFK3RwVmlU?=
 =?utf-8?B?MGp6MEk4blowd2I2aTNrV0pFWmR6bzB1Mkl3Z1QyTWo5T0F4R1JCdHpwMlJr?=
 =?utf-8?B?ZVE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 95cf0749-6efd-433a-69b8-08db5cfbdacb
X-MS-Exchange-CrossTenant-AuthSource: DS0PR11MB7309.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 May 2023 08:41:38.2627 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: riPTfH+AI3s5OBt4VztmSOck3p2mak3rcK+p5/Mx7r9XKu3GPlZzLItGlfjWlWizC+o/GzBNyVOYNKm8fqgsLsKDVhA74P4qAv2K5NEcmJs=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA2PR11MB4874
X-OriginatorOrg: intel.com
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org

On Wed, May 24, 2023 at 10:09:04PM +0200, David Marchand wrote:
> Hello Bruce, Kevin,
> 
> Review please.
> 
> 
> On Thu, Mar 23, 2023 at 11:54 PM Tyler Retzlaff
> <roretzla@linux.microsoft.com> wrote:
> >
> > Replace the use of rte_atomic.h types and functions, instead use GCC
> > supplied C++11 memory model builtins.
> >
> > Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>

Two small comments inline below.

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

> > ---
> >  drivers/dma/idxd/idxd_internal.h | 3 +--
> >  drivers/dma/idxd/idxd_pci.c      | 8 +++++---
> >  2 files changed, 6 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/dma/idxd/idxd_internal.h b/drivers/dma/idxd/idxd_internal.h
> > index 180a858..cd41777 100644
> > --- a/drivers/dma/idxd/idxd_internal.h
> > +++ b/drivers/dma/idxd/idxd_internal.h
> > @@ -7,7 +7,6 @@
> >
> >  #include <rte_dmadev_pmd.h>
> >  #include <rte_spinlock.h>
> > -#include <rte_atomic.h>
> >
> >  #include "idxd_hw_defs.h"
> >
> > @@ -34,7 +33,7 @@ struct idxd_pci_common {
> >         rte_spinlock_t lk;
> >
> >         uint8_t wq_cfg_sz;
> > -       rte_atomic16_t ref_count;
> > +       uint16_t ref_count;
> >         volatile struct rte_idxd_bar0 *regs;
> >         volatile uint32_t *wq_regs_base;
> >         volatile struct rte_idxd_grpcfg *grp_regs;
> > diff --git a/drivers/dma/idxd/idxd_pci.c b/drivers/dma/idxd/idxd_pci.c
> > index 781fa02..2de5d15 100644
> > --- a/drivers/dma/idxd/idxd_pci.c
> > +++ b/drivers/dma/idxd/idxd_pci.c
> > @@ -6,7 +6,6 @@
> >  #include <rte_devargs.h>
> >  #include <rte_dmadev_pmd.h>
> >  #include <rte_malloc.h>
> > -#include <rte_atomic.h>
> >
> >  #include "idxd_internal.h"
> >
> > @@ -136,7 +135,9 @@
> >         /* if this is the last WQ on the device, disable the device and free
> >          * the PCI struct
> >          */
> > -       is_last_wq = rte_atomic16_dec_and_test(&idxd->u.pci->ref_count);
> > +       /* NOTE: review for potential ordering optimization */
> > +       is_last_wq = __atomic_fetch_sub(&idxd->u.pci->ref_count, 1,
> > +               __ATOMIC_SEQ_CST) - 1 == 0;

Rather than "__atomic_fetch_sub(...) - 1 == 0", I think just comparing
"== 1" is simpler and better. I would also bracket the comparison for
clarity.

> >         if (is_last_wq) {
> >                 /* disable the device */
> >                 err_code = idxd_pci_dev_command(idxd, idxd_disable_dev);
> > @@ -350,7 +351,8 @@
> >                                 free(idxd.u.pci);
> >                         return ret;
> >                 }
> > -               rte_atomic16_inc(&idxd.u.pci->ref_count);
> > +               /* NOTE: review for potential ordering optimization */

I think we can drop the note. Since this is not datapath code the perf is
not that important.

> > +               __atomic_fetch_add(&idxd.u.pci->ref_count, 1, __ATOMIC_SEQ_CST);
> >         }
> >
> >         return 0;
> > --
> > 1.8.3.1
> >
> 
> -- 
> David Marchand
>