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 EA35AA0C47; Thu, 14 Oct 2021 14:32:25 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id B8B8A40E50; Thu, 14 Oct 2021 14:32:24 +0200 (CEST) Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) by mails.dpdk.org (Postfix) with ESMTP id 1AED340041 for ; Thu, 14 Oct 2021 14:32:23 +0200 (CEST) Received: from dggemv711-chm.china.huawei.com (unknown [172.30.72.56]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4HVTJd5Sz5zWVRX; Thu, 14 Oct 2021 20:30:41 +0800 (CST) Received: from dggema767-chm.china.huawei.com (10.1.198.209) by dggemv711-chm.china.huawei.com (10.1.198.66) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.2308.8; Thu, 14 Oct 2021 20:32:20 +0800 Received: from [10.67.103.231] (10.67.103.231) 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.2308.8; Thu, 14 Oct 2021 20:32:20 +0800 Message-ID: <13a0f09e-fc93-3256-e58f-11cec1e02dc8@huawei.com> Date: Thu, 14 Oct 2021 20:32:20 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.2.0 To: Thomas Monjalon CC: , , , , References: <1627908397-51565-1-git-send-email-lihuisong@huawei.com> <20211012113934.23611-1-lihuisong@huawei.com> <2527815.Doo92MvZa3@thomas> From: "lihuisong (C)" In-Reply-To: <2527815.Doo92MvZa3@thomas> X-Originating-IP: [10.67.103.231] X-ClientProxiedBy: dggems701-chm.china.huawei.com (10.3.19.178) 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] [PATCH V2] ethdev: fix eth device released repeatedly 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, Thomas *The commit log：* In secondary process, rte_eth_dev_close() doesn't clear eth_dev->data. If calling rte_dev_remove() after rte_eth_dev_close(), in rte_eth_dev_pci_generic_remove() function, the released eth device still can be found by its name in shared memory. As a result, the eth device will be released repeatedly. The state of the eth device is modified to RTE_ETH_DEV_UNUSED after rte_eth_dev_close(). So this state can be used to avoid this problem. Is that will be more clear? /* * A released eth device can be found by its name in shared memory. * If the state of the eth device is RTE_ETH_DEV_UNUSED, which means * the eth device has been released. */ Is it ok to use the above description as a comment in the code? Hope for your reply.  Thanks. 在 2021/10/12 23:33, Thomas Monjalon 写道: > 12/10/2021 13:39, Huisong Li: >> The rte_eth_dev_pci_generic_remove() will be called to detach an Ethernet >> device when App calls rte_dev_remove() to detach a pci device. In addition, >> the rte_eth_dev_close() can also detach an Ethernet device. >> In secondary process, if App first calls rte_eth_dev_close() and then calls >> rte_dev_remove(), because rte_eth_dev_close() doesn't clear "eth_dev->data" > It would be clearer if you start this sentence with: > "In secondary process, rte_eth_dev_close() doesn't clear eth_dev->data." > Then you can explain that if calling rte_dev_remove() after rte_eth_dev_close(), > etc... > >> , the address of the released Ethernet device can still be found by device >> name. As a result, the Ethernet device will be released repeatedly in this >> case. The state of the Ethernet device is equal to RTE_ETH_DEV_UNUSED after >> calling rte_eth_dev_close(). Use this state to avoid this problem. >> >> Signed-off-by: Huisong Li >> --- >> + /* >> + * In secondary process, if applications first call rte_eth_dev_close() >> + * and then call this interface, because rte_eth_dev_close() doesn't >> + * clear eth_dev->data, the address of the released Ethernet device can >> + * still be found by device name. As a result, the Ethernet device will >> + * be released repeatedly in this case. >> + * The state of the Ethernet device is equal to RTE_ETH_DEV_UNUSED after >> + * calling rte_eth_dev_close(). Use this state to avoid this problem. > This is a comment for the commit log. > Inside the code, we should be more to the point. > I suggest this comment: > /* A released port can be found by its name in shared memory. */ > >> + */ >> + if (rte_eal_process_type() != RTE_PROC_PRIMARY && > Better to directly compare with RTE_PROC_SECONDARY > >> + eth_dev->state == RTE_ETH_DEV_UNUSED) { >> + RTE_ETHDEV_LOG(INFO, "The ethdev port has been released."); > Not sure we need any log here. > >> + return 0; >> + } > > > .