From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Zhang, Qi Z" Subject: Re: [PATCH] bus/vdev: fix probe same device twice Date: Wed, 7 Nov 2018 16:53:50 +0000 Message-ID: <039ED4275CED7440929022BC67E70611532E0AC3@SHSMSX103.ccr.corp.intel.com> References: <20181106003150.10560-1-qi.z.zhang@intel.com> <11443385.dze8hbQCXQ@xps> <039ED4275CED7440929022BC67E70611532E0279@SHSMSX103.ccr.corp.intel.com> <2180900.HKzicAuZ6Y@xps> <20181106233352.qhe7kuhgiexpvpih@bidouze.vm.6wind.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Cc: "dev@dpdk.org" , "Yigit, Ferruh" To: =?iso-8859-1?Q?Ga=EBtan_Rivet?= , Thomas Monjalon Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 766942B94 for ; Wed, 7 Nov 2018 17:53:55 +0100 (CET) In-Reply-To: <20181106233352.qhe7kuhgiexpvpih@bidouze.vm.6wind.com> Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" > -----Original Message----- > From: Ga=EBtan Rivet [mailto:gaetan.rivet@6wind.com] > Sent: Tuesday, November 6, 2018 4:34 PM > To: Thomas Monjalon > Cc: Zhang, Qi Z ; dev@dpdk.org; Yigit, Ferruh > > Subject: Re: [dpdk-dev] [PATCH] bus/vdev: fix probe same device twice >=20 > On Tue, Nov 06, 2018 at 09:36:22PM +0100, Thomas Monjalon wrote: > > 06/11/2018 16:46, Zhang, Qi Z: > > > From: Thomas Monjalon [mailto:thomas@monjalon.net] > > > > > > > > Hi, > > > > > > > > 06/11/2018 01:31, Qi Zhang: > > > > > When probe the same device at second time > > > > > > > > Sorry I stop on this first sentence. > > > > How and why do you probe a vdev twice? > > > > > > if we do rte_dev_hotplug_add or rte_dev_proble on a probed device. > > > (yes, this is not usually what an application want, but it can > > > happen by miss-operation, and this is covered by our test case, it > > > make sense to me that hotplug API should be robust enough to handle > > > that situation.) > > > > Yes I agree we must handle this situation. > > > > > we will failed at the second time as expected, but will not able to > > > detach the device any more, since during the second scan, original > vdev->device.devargs is corrupted. > > > > The root cause is we remove a devargs which was referenced. > > Could we overwrite the first devargs instead of removing it? > > > > >=20 > It's also possible to add a back-reference to an rte_device in [1], but t= hat can > only work if only one rte_device references a devargs. > It seems to be the case now, but it might be good to enforce explicitly t= hat > when a bus scans its devices, it should do a 1-to-1 map to devargs. >=20 > If mapping rte_device to rte_devargs needs to respect rules, it could hel= p bus > developpers to have a function that will do the job: verify that the deva= rgs is > not currently used, add the back-reference to the rte_device. >=20 > With the proper back-reference, it is possible to clean-up the device whe= n > removing the devargs=20 This may still not work for vdev, since the old reference is used in vdev_f= ind to find a exist device by name during scan. (For PCI device, we have pci_addr, but vdev we use devargs->name to identif= y device, anyway this can be fixed in vdev, but that required a clone on th= e device name also break the coupling somehow.) I just don't understand "why we must tight the tighten the device -> devarg= s coupling, not loosen it" (and also to add the rte_devargs_extract() function > that would allow keeping the original devargs and insert it back if the h= otplug > fails, then the mapping must be restored). >=20 > [1]: https://mails.dpdk.org/archives/dev/2018-November/118274.html >=20 > -- > Ga=EBtan Rivet > 6WIND