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 AC27DA0524; Fri, 7 May 2021 18:49:30 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 9811740686; Fri, 7 May 2021 18:49:30 +0200 (CEST) Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by mails.dpdk.org (Postfix) with ESMTP id A77AA40040 for ; Fri, 7 May 2021 18:49:28 +0200 (CEST) IronPort-SDR: ZGJuzuagdVVCCwY+C1SAcxo4JcomGezyPAxzhG9hQt04Sn61dTXDZydwu3aWgXv8SJgsX5pcp8 YjYcRObOe6vw== X-IronPort-AV: E=McAfee;i="6200,9189,9977"; a="178330819" X-IronPort-AV: E=Sophos;i="5.82,281,1613462400"; d="scan'208";a="178330819" Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 07 May 2021 09:49:27 -0700 IronPort-SDR: QTRMyO+6w4rNxlHggiuafoac2QF9lID421OWOrO08Gm1vriGTpkuS3qvPLEDc8v6bDSPmr0f10 F6xahuD5Uq5Q== X-IronPort-AV: E=Sophos;i="5.82,281,1613462400"; d="scan'208";a="431443062" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.213.210.230]) ([10.213.210.230]) by fmsmga008-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 07 May 2021 09:49:25 -0700 To: Michal Krawczyk , dev@dpdk.org Cc: ndagan@amazon.com, gtzalik@amazon.com, igorch@amazon.com, upstream@semihalf.com, Stanislaw Kardach , Shay Agroskin References: <20210505073348.6394-1-mk@semihalf.com> <20210506142526.28245-1-mk@semihalf.com> <20210506142526.28245-20-mk@semihalf.com> From: Ferruh Yigit X-User: ferruhy Message-ID: <53b53264-f440-a377-6a78-d8a179754045@intel.com> Date: Fri, 7 May 2021 17:49:23 +0100 MIME-Version: 1.0 In-Reply-To: <20210506142526.28245-20-mk@semihalf.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v3 19/22] net/ena: make ethdev references smp safe 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 Sender: "dev" On 5/6/2021 3:25 PM, Michal Krawczyk wrote: > From: Stanislaw Kardach > > rte_pci_device and rte_eth_dev are process-local structures. Therefore > ena_adapter::pdev and ena_adapter::rte_dev cannot be used universally. > Switch this to extracting those structures via rte_eth_devices indexing > and remove pdev since it's not used outside of init. > > Signed-off-by: Stanislaw Kardach > Reviewed-by: Michal Krawczyk > Reviewed-by: Igor Chauskin > Reviewed-by: Shay Agroskin <...> > @@ -1634,7 +1633,7 @@ static void ena_timer_wd_callback(__rte_unused struct rte_timer *timer, > void *arg) > { > struct ena_adapter *adapter = arg; > - struct rte_eth_dev *dev = adapter->rte_dev; > + struct rte_eth_dev *dev = &rte_eth_devices[adapter->port_id]; > I think better to try to avoid using 'rte_eth_devices' global variable as much as possible, although it may not be possible always, but for this case it seems it can be done. Why not just pass "struct rte_eth_dev" as 'arg', instead of "struct ena_adapter"? > check_for_missing_keep_alive(adapter); > check_for_admin_com_state(adapter); > @@ -1819,11 +1818,10 @@ static int eth_ena_dev_init(struct rte_eth_dev *eth_dev) > memset(adapter, 0, sizeof(struct ena_adapter)); > ena_dev = &adapter->ena_dev; > > - adapter->rte_eth_dev_data = eth_dev->data; > - adapter->rte_dev = eth_dev; > + adapter->edev_data = eth_dev->data; > + adapter->port_id = eth_dev->data->port_id; > > pci_dev = RTE_ETH_DEV_TO_PCI(eth_dev); > - adapter->pdev = pci_dev; > > PMD_INIT_LOG(INFO, "Initializing %x:%x:%x.%d", > pci_dev->addr.domain, > @@ -1843,7 +1841,8 @@ static int eth_ena_dev_init(struct rte_eth_dev *eth_dev) > } > > ena_dev->reg_bar = adapter->regs; > - ena_dev->dmadev = adapter->pdev; > + /* This is a dummy pointer for ena_com functions. */ > + ena_dev->dmadev = adapter; > > adapter->id_number = adapters_found; > > @@ -1857,7 +1856,7 @@ static int eth_ena_dev_init(struct rte_eth_dev *eth_dev) > } > > /* device specific initialization routine */ > - rc = ena_device_init(ena_dev, &get_feat_ctx, &wd_state); > + rc = ena_device_init(ena_dev, pci_dev, &get_feat_ctx, &wd_state); > if (rc) { > PMD_INIT_LOG(CRIT, "Failed to init ENA device"); > goto err; > @@ -2716,7 +2715,7 @@ static int ena_xstats_get_names(struct rte_eth_dev *dev, > struct rte_eth_xstat_name *xstats_names, > unsigned int n) > { > - unsigned int xstats_count = ena_xstats_calc_num(dev); > + unsigned int xstats_count = ena_xstats_calc_num(dev->data); > unsigned int stat, i, count = 0; > > if (n < xstats_count || !xstats_names) > @@ -2765,7 +2764,7 @@ static int ena_xstats_get(struct rte_eth_dev *dev, > unsigned int n) > { > struct ena_adapter *adapter = dev->data->dev_private; > - unsigned int xstats_count = ena_xstats_calc_num(dev); > + unsigned int xstats_count = ena_xstats_calc_num(dev->data); > unsigned int stat, i, count = 0; > int stat_offset; > void *stats_begin; > @@ -2997,7 +2996,7 @@ static void ena_update_on_link_change(void *adapter_data, > > adapter = adapter_data; > aenq_link_desc = (struct ena_admin_aenq_link_change_desc *)aenq_e; > - eth_dev = adapter->rte_dev; > + eth_dev = &rte_eth_devices[adapter->port_id]; > Similarly, instead of passing "struct ena_adapter" as "void *adapter_data", it can be possible to pass "struct rte_eth_dev", as far as I can that chain has more layers but still possible. Meanhile, you can use "void *process_private;" field of "struct rte_eth_dev" to keep process private eth_dev data, if you want.