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 29684A0C46; Tue, 24 Aug 2021 04:10:45 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id B5EC8406A3; Tue, 24 Aug 2021 04:10:44 +0200 (CEST) Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) by mails.dpdk.org (Postfix) with ESMTP id 8B77F4014D for ; Tue, 24 Aug 2021 04:10:43 +0200 (CEST) Received: from dggemv704-chm.china.huawei.com (unknown [172.30.72.56]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4Gtsy90xdjzYt01; Tue, 24 Aug 2021 10:10:09 +0800 (CST) Received: from dggema767-chm.china.huawei.com (10.1.198.209) by dggemv704-chm.china.huawei.com (10.3.19.47) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.2176.2; Tue, 24 Aug 2021 10:10:40 +0800 Received: from [10.66.74.184] (10.66.74.184) by dggema767-chm.china.huawei.com (10.1.198.209) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2176.2; Tue, 24 Aug 2021 10:10:40 +0800 To: "Singh, Aman Deep" , CC: , References: <1627908397-51565-1-git-send-email-lihuisong@huawei.com> <44a1b455-31e8-7909-3011-1643293f3210@intel.com> From: Huisong Li Message-ID: <91cf7c95-038f-bba2-4e05-b921c3e0427e@huawei.com> Date: Tue, 24 Aug 2021 10:10:40 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.8.0 MIME-Version: 1.0 In-Reply-To: <44a1b455-31e8-7909-3011-1643293f3210@intel.com> X-Originating-IP: [10.66.74.184] X-ClientProxiedBy: dggems705-chm.china.huawei.com (10.3.19.182) To dggema767-chm.china.huawei.com (10.1.198.209) X-CFilter-Loop: Reflected Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit X-Content-Filtered-By: Mailman/MimeDel 2.1.29 Subject: Re: [dpdk-dev] [RFC V1] ethdev: fix the issue that dev uninit may be called twice 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" Hi, Singh, Aman Deep Sorry, I missed your review. Thank you for your review.😁 在 2021/8/18 17:47, Singh, Aman Deep 写道: > > Hi Huison, > > On 8/2/2021 6:16 PM, Huisong Li wrote: >> Ethernet devices in DPDK can be released by rte_eth_dev_close() and >> rte_dev_remove(). However, these two APIs do not have explicit invocation >> restrictions. In other words, at the ethdev layer, calling >> rte_eth_dev_close() and then rte_dev_remove() or rte_eal_hotplug_remove() >> is allowed. In such a bad scenario, the primary process may be fine, but it >> may cause that dev_unint() in the secondary process will be called twice, > Shouldn't dev_unint() for Secondary process, simply return with no-action. The prerequisite is that the secondary process does not have any resources that need to be released. However, some resources from secondary process may also need to be released. For example, mp action registered by rte_mp_action_register() is used to for multi-process communication. It should be unloaded when all eth devices driven by one PMD in a process are removed. In order to achieve the above purpose, secondary process may have data recording the number of device to decide when to deregister the action. Of course, this is just the case. In short, secondary process may have its own private data or resources to be released. It is mentioned in RFC v2. Please go to discussion line of RFC v2. >> and even other serious problems. So this patch fixes it. >> >> Fixes: 99a2dd955fba ("lib: remove librte_ prefix from directory names") >> >> Signed-off-by: Huisong Li >> --- >> lib/ethdev/ethdev_pci.h | 13 +++++++++++++ >> 1 file changed, 13 insertions(+) >> >> diff --git a/lib/ethdev/ethdev_pci.h b/lib/ethdev/ethdev_pci.h >> index 8edca82..14a0e01 100644 >> --- a/lib/ethdev/ethdev_pci.h >> +++ b/lib/ethdev/ethdev_pci.h >> @@ -151,6 +151,19 @@ rte_eth_dev_pci_generic_remove(struct rte_pci_device *pci_dev, >> if (!eth_dev) >> return 0; >> >> + /* >> + * The eth_dev->data->name doesn't be cleared by the secondary precess, > Can we reprase above sentence "doesn't be cleared " ok >> + * so above "eth_dev" isn't NULL after rte_eth_dev_close() called. >> + * Namely, whether "eth_dev" is NULL cannot be used to determine whether >> + * an ethdev port has been released. >> + * For both primary precess and secondary precess, eth_dev->state is > s/ precess / process Thanks. RFC v2 has corrected it. >> + * RTE_ETH_DEV_UNUSED, which means the ethdev port has been released. >> + */ >> + if (eth_dev->state == RTE_ETH_DEV_UNUSED) { >> + RTE_ETHDEV_LOG(INFO, "The ethdev port has been released."); >> + return 0; >> + } >> + >> if (dev_uninit) { >> ret = dev_uninit(eth_dev); >> if (ret)