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 A9915A0548; Sun, 9 May 2021 16:40:42 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 301EB40140; Sun, 9 May 2021 16:40:42 +0200 (CEST) Received: from mail-il1-f172.google.com (mail-il1-f172.google.com [209.85.166.172]) by mails.dpdk.org (Postfix) with ESMTP id F1A204003E for ; Sun, 9 May 2021 16:40:40 +0200 (CEST) Received: by mail-il1-f172.google.com with SMTP id l19so11848031ilk.13 for ; Sun, 09 May 2021 07:40:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=semihalf-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=o0x2MF+ByTWMyVpLlwXKxtscGrpHdpjHgiT6r6ZG7Oc=; b=xg3Phb4vX9caggtV5lUX0Dj+V3tBunhcShSN01wJIms0ir3eqRZSVsET716j3LBIS2 SSS9KfyEpHTlLkBAzosP2tR4TP0ujcXqyqSBNYhvfeqrkQ81Kh/nP7SR/UiwcQpgUlcx pm6ySRgwtMDlO6xDNdgpu3demtcWn0wCcE1Tp00JuGroeundGufKQAyH88XouHjRXw05 E0CmD0iQxsS/1HMCTzVZglgYsoLjqRA2bkMgb5+ghevYvGhcjCJCN+BddgfKYfj25TrX bMreNcuLM84g+YZbnXV8Gm7SsoraYesW9gKbxP3uJqN1ZrKlulU8HpuOripH0zVtwrNl Aypw== 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:content-transfer-encoding; bh=o0x2MF+ByTWMyVpLlwXKxtscGrpHdpjHgiT6r6ZG7Oc=; b=Ak3EYPcsr7s4+q4Kr7b4YAmFQz2lDdsxJ4f46pm4agLDXkrRep5bYQT+542UBVYklu 87y2UnYALLZB/nTtp061UvcgKLAt3I7ESBnQEmCEf+JP466Z1iAolI4WveAvCbZZ2vU6 HY8u9NgQ6kTznk1KBBykU7gNz1gjo06appkLNrl6FP3lxJsdbbevhtIQJHRKx0rvSE4P PPx/Ekk4ZWwSj+RJDMWrB7KsHyie9YqWtrtWTaHU6lQUSrPw25VGEt3+2fN4NEwbaJho YgXvvEUw5Abwy/coCbPmYChUtRXowVfYBxUCK7qsWn8JUbw9tzzy3BV9S0bF0B3eqPG4 oiPg== X-Gm-Message-State: AOAM5302LKZw2d5msPEIqma3vOuokvzRoXTy9XBtQdNQ8DCEqP6d2KU0 LSP8b24EaCJUZHes7jWbilXqFNiDOPPZxkjglH9m/A== X-Google-Smtp-Source: ABdhPJyK7b3Xyp9tYXYaIEvmcH/85h1ic+5blZI6xRDdk6diSz9qVsw9YGm1k4pnKV2Ti/slBJeFTYS4es5jY5l8fYE= X-Received: by 2002:a05:6e02:d41:: with SMTP id h1mr17342872ilj.0.1620571240333; Sun, 09 May 2021 07:40:40 -0700 (PDT) MIME-Version: 1.0 References: <20210505073348.6394-1-mk@semihalf.com> <20210506142526.28245-1-mk@semihalf.com> <20210506142526.28245-20-mk@semihalf.com> <53b53264-f440-a377-6a78-d8a179754045@intel.com> In-Reply-To: <53b53264-f440-a377-6a78-d8a179754045@intel.com> From: =?UTF-8?Q?Micha=C5=82_Krawczyk?= Date: Sun, 9 May 2021 16:40:28 +0200 Message-ID: To: Ferruh Yigit Cc: dev , "Dagan, Noam" , "Tzalik, Guy" , "Chauskin, Igor" , upstream@semihalf.com, Stanislaw Kardach , Shay Agroskin Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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" pt., 7 maj 2021 o 18:49 Ferruh Yigit napisa=C5=82(= a): > > 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 st= ruct rte_timer *timer, > > void *arg) > > { > > struct ena_adapter *adapter =3D arg; > > - struct rte_eth_dev *dev =3D adapter->rte_dev; > > + struct rte_eth_dev *dev =3D &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_a= dapter"? Good suggestion, it will be changed in the v4. > > > 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 =3D &adapter->ena_dev; > > > > - adapter->rte_eth_dev_data =3D eth_dev->data; > > - adapter->rte_dev =3D eth_dev; > > + adapter->edev_data =3D eth_dev->data; > > + adapter->port_id =3D eth_dev->data->port_id; > > > > pci_dev =3D RTE_ETH_DEV_TO_PCI(eth_dev); > > - adapter->pdev =3D 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 *e= th_dev) > > } > > > > ena_dev->reg_bar =3D adapter->regs; > > - ena_dev->dmadev =3D adapter->pdev; > > + /* This is a dummy pointer for ena_com functions. */ > > + ena_dev->dmadev =3D adapter; > > > > adapter->id_number =3D adapters_found; > > > > @@ -1857,7 +1856,7 @@ static int eth_ena_dev_init(struct rte_eth_dev *e= th_dev) > > } > > > > /* device specific initialization routine */ > > - rc =3D ena_device_init(ena_dev, &get_feat_ctx, &wd_state); > > + rc =3D 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_de= v *dev, > > struct rte_eth_xstat_name *xstats_names, > > unsigned int n) > > { > > - unsigned int xstats_count =3D ena_xstats_calc_num(dev); > > + unsigned int xstats_count =3D ena_xstats_calc_num(dev->data); > > unsigned int stat, i, count =3D 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 =3D dev->data->dev_private; > > - unsigned int xstats_count =3D ena_xstats_calc_num(dev); > > + unsigned int xstats_count =3D ena_xstats_calc_num(dev->data); > > unsigned int stat, i, count =3D 0; > > int stat_offset; > > void *stats_begin; > > @@ -2997,7 +2996,7 @@ static void ena_update_on_link_change(void *adapt= er_data, > > > > adapter =3D adapter_data; > > aenq_link_desc =3D (struct ena_admin_aenq_link_change_desc *)aenq= _e; > > - eth_dev =3D adapter->rte_dev; > > + eth_dev =3D &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. > Yes, it's being executed directly in the callback associated with the admin interrupt, so as long as the interrupt callbacks are being executed in the same process which registered them (and from what I read in the documentation, it should be so, but please correct me if I'm wrong), the "struct rte_eth_dev" pointer should be valid. > > Meanhile, you can use "void *process_private;" field of "struct rte_eth_d= ev" to > keep process private eth_dev data, if you want. >