From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 ; 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 To: David Marchand CC: Kevin Laatz , , , , , , , Tyler Retzlaff Subject: Re: [PATCH v3 3/7] dma/idxd: replace rte atomics with GCC builtin atomics Message-ID: 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> Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-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 > 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 Two small comments inline below. Acked-by: Bruce Richardson > > --- > > 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 > > #include > > -#include > > > > #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 > > #include > > #include > > -#include > > > > #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 >