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 2AB8342820; Thu, 23 Mar 2023 09:52:15 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id B3DE740E09; Thu, 23 Mar 2023 09:52:14 +0100 (CET) Received: from out4-smtp.messagingengine.com (out4-smtp.messagingengine.com [66.111.4.28]) by mails.dpdk.org (Postfix) with ESMTP id 3E62E4021E; Thu, 23 Mar 2023 09:52:13 +0100 (CET) Received: from compute2.internal (compute2.nyi.internal [10.202.2.46]) by mailout.nyi.internal (Postfix) with ESMTP id 7804B5C0120; Thu, 23 Mar 2023 04:52:11 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute2.internal (MEProxy); Thu, 23 Mar 2023 04:52:11 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= cc:cc:content-transfer-encoding:content-type:content-type:date :date:from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:sender:subject:subject:to:to; s=fm2; t= 1679561531; x=1679647931; bh=Nsfrs+Hw0oKLPGjKuGLGkrK5DnagzKm8QML iieFfp/Q=; b=Dt4FrqmFcFXqyJAKuP5Gzk8kCY78ys5VisraonbeLQwWs9VtGbs 4dpMUDAEUBeqfNfbPk6xWL/DAPEEFSSptA9HTnaEWGYAKxDVjLnftY8//O6mpfo7 dnkVYP6FMs/DgsYZiHc3UleYPeB/e7l6u+OgiT8xfpE0Ew2O15iwfttfHKrDubww 8vnl32V/l0lwCe/demlBJ8IKHftd441xcLs13a+tH3R/0m//+6rmnyIDYZSe/iaQ U5W0EUrsmIhYp8UPc9kF4IzaRfWXCkPJuZzq9fWqh8auLk8C+btNIoFJoII+aIA1 ME44YrFcKA/18XHJQ5yRFL9BSBMFmNT+ssA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:sender:subject:subject:to:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm2; t= 1679561531; x=1679647931; bh=Nsfrs+Hw0oKLPGjKuGLGkrK5DnagzKm8QML iieFfp/Q=; b=PzvBQxaSGCsb72Uxz13f3hyEFt0FEz1RmPsN5twY+6a/vwupx6h nsvcxUfhsq1nnPh75kV2fzgKbz6VuwwJJQaHxMNzh8h3T5csKSAFHe02yD5wQUPm Z/fA1ptLapbyXUqH4v2qddzM2eu0q4GgzvpgqF49iRm1bii5LxdEFwaChycrVvDO xX/8h+acyfrJFQ5kvCScadDD0wYOsmsj4T1y2AoiWFxgaBMoWS64K2jXMVwyRFOD JEvRIbphq6F2cC0IqqU1MMG5ENGWEScjRfWF2P2wKVOE0khFOcuBt3LUflEeorg6 HYGeyv93Jc0AAF5hpwmoGTkU3Sdnavqju8A== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvhedrvdegfedguddvgecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpefhvfevufffkfgjfhgggfgtsehtufertddttddvnecuhfhrohhmpefvhhho mhgrshcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqne cuggftrfgrthhtvghrnheptdejieeifeehtdffgfdvleetueeffeehueejgfeuteeftddt ieekgfekudehtdfgnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilh hfrhhomhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvth X-ME-Proxy: Feedback-ID: i47234305:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 23 Mar 2023 04:52:09 -0400 (EDT) From: Thomas Monjalon To: "Huang, Wei" Cc: "dev@dpdk.org" , "david.marchand@redhat.com" , "stable@dpdk.org" , "Xu, Rosen" , "Zhang, Tianfei" , "Zhang, Qi Z" Subject: Re: [PATCH v1] raw/ifpga: remove virtual device unplug operation Date: Thu, 23 Mar 2023 09:52:07 +0100 Message-ID: <1923267.fIoEIV5pvu@thomas> In-Reply-To: References: <20230316204445.360330-1-wei.huang@intel.com> <5367342.29KlJPOoH8@thomas> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" 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 23/03/2023 04:26, Huang, Wei: > From: Thomas Monjalon > > 22/03/2023 02:26, Huang, Wei: > > > From: Thomas Monjalon > > > > 21/03/2023 09:41, Huang, Wei: > > > > > From: Thomas Monjalon > > > > > > 21/03/2023 01:11, Huang, Wei: > > > > > > > From: Thomas Monjalon > > > > > > > > 16/03/2023 21:44, Wei Huang: > > > > > > > > > VDEV bus has implemented cleanup() function to perform > > > > > > > > > cleanup for devices on the bus during eal_cleanup(), so > > > > > > > > > there is no need for ifpga driver to record virtual devices and > > unplug them. > > > > > > > > > > > > > > > > Why no need? > > > > > > > > If the application wants to explicitly remove a device, what > > happens? > > > > > > > > > > > > > > > > > > > > > > > EAL will output an error information "Cannot find plugged device > > (%s)". > > > > > > > > > > > > It does not look what we expect. > > > > > > > > > > > Let me clear it. > > > > > With this patch, no error information will be outputted. > > > > > Without this patch, error information will be outputted. > > > > > Because bus cleanup action will unplug virtual device, then ifpga > > > > > PMD unplug the virtual device which is already be cleanup, > > > > > > > > Why ipfga unplug the device after the bus cleanup? > > > > I'm not following. > > > > > > > The virtual device is created upon ifpga, if VDEV bus doesn't perform > > > cleanup, ifpga has the responsibility to unplug these virtual devices. > > > > Really I don't understand the flow. > > Are you talking about EAL cleanup case? > Yes, it's about EAL cleanup. > > What happens first? Do you need ifpga to be called first? > The cleanup flow is rte_eal_cleanup() -> eal_bus_cleanup() > eal_bus_cleanup() will call each bus's cleanup method to complete cleanup work. > There are three types of device related to ifpga PMD: PCI device, VDEV device and AFU device. > VDEV device is created on PCI device, it's a mediate device which purpose is to plug a AFU device on IFPGA bus. > Before eal_bus_cleanup() is implemented, application will call rte_pmd_ifpga_cleanup() to remove PCI device, VDEV device will be removed when PCI device is removed, AFU device will be removed when VDEV device is removed, it works fine. > Now eal_bus_cleanup() takes the job, application has no need to call rte_pmd_ifpga_cleanup(), ifpga PCI device has no need to remove ifpga VDEV device and ifpga VDEV device has no need to remove ifpga AFU device. That's the problem in your approach. You need to keep the code removing children devices. And if all children are removed, then the parent should remove itself. If you implement these 2 conditions, the cleanup can happen in any order. > > I think you need the correct checks to allow any order of cleanup. > When this patch is committed, no order dependent on cleanup. > > > > > > bus->find_device() returns NULL, > > > > > EAL output "Cannot find plugged device (%s)\n" at line 302 in > > > > > eal_common_dev.c > > > > > > > > Anyway, the good answer is not to completely remove the "remove" > > > > operation. > > > > > > > If not to completely remove the "remove", the same virtual device will be > > unplug twice, is it reasonable? > > > > You need to add a check to not unplug something already unplugged. > > But you must allow the user calling "remove" directly. > > > rte_pmd_ifpga_cleanup() is the only one interface for user to calling "remove" directly, No, there are functions in EAL to "remove" devices, like rte_dev_remove(), and we must make sure it works effectively. > when this patch is committed, VDEV and AFU device will not be unplugged twice. > For PCI device, the implementation of rte_pmd_ifpga_cleanup() is like below > for (i = 0; i < IFPGA_RAWDEV_NUM; i++) { > dev = &ifpga_rawdevices[i]; > if (dev->rawdev) { > rte_rawdev_pmd_release(dev->rawdev); > dev->rawdev = NULL; > } > } > If ifpga PCI device is already removed, dev->rawdev is NULL, it will not be unplugged again.