From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-f65.google.com (mail-wm1-f65.google.com [209.85.128.65]) by dpdk.org (Postfix) with ESMTP id 3C0742B94 for ; Wed, 7 Nov 2018 18:15:46 +0100 (CET) Received: by mail-wm1-f65.google.com with SMTP id f10-v6so2671470wme.3 for ; Wed, 07 Nov 2018 09:15:46 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=Aqm4VP1Ylv457GkfsgiSPB4loODpx+0+4HAMtm1d0LI=; b=IbdjPL0JjpHWxHqPxyEj1uiXEtWSVIFf8PuGRWDTOByRtC8BBDyVXELTPCV2Lu7EJe hzOgSdcn9cjcDZbf9sR7t+ZFAvPatig+MNpLZ3A/2tYKcFoaSRY7OfIAg4jn4BNifCkF Oa+oj6ft0ffdOjAOPu3XbcDUKV+Ni2yiBRXgY+KAJerGc5DapmINbl3i+ZRpcDCy816/ +4FDkKNrLFcD/hiP4McTbtPhdaWz6RVQXdWCq+yyBA8SpvbG0Gxgq0bCiWNa14UBToco 9DB/8a9t5NAIEuq38TbdzTUzxeRxXmCEHdgD16pFkV7rX9XgOmijH8DLnrpjE380BtGm v9aQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=Aqm4VP1Ylv457GkfsgiSPB4loODpx+0+4HAMtm1d0LI=; b=DfqNsAaidvQgiV83NT7bWB0DPH5fngXMgrLSZ7pvFFmnSvHxkpsAdovaIOjyvSitBZ 6sejzFuZwRM8y/hFzU++NGHUpyjP3XJRxZBDe2U4PHzjmMpqx01cMG50vunUVDY9MJJH 2yAwaWHYTJXfLxkTNAqLrmnTsZSTfLOgZnCFbp5gh1O17CsQdaM+hKNMbeamWvVmWPBG wQg5Y6FOxgRPLne3yTXVLV3xs7hIu4e+vBrd6T2SEf7usgae07jSIeaOK0l0nikAM2mL jqt4KgwkKuYtADQCMNRnGAB3dVkj4tbN1CX7dZZzYCagEf+JTVh+/CIZpmkpjw54jJNI pYcA== X-Gm-Message-State: AGRZ1gJM80hyt9eR17FiY1bF3HqU02l4Yv3yvb5Wqrv9cH2o2uq0na4g D5d69U9H75JnRA5eEAL9dmA7KA== X-Google-Smtp-Source: AJdET5dbuKaQu9tB7X2NYIdn7yQvVNS6cVxwvLa3W3ZGoHdcV0VNCM3MN0ykvYmZXga4NGF9trkgSA== X-Received: by 2002:a1c:6a01:: with SMTP id f1-v6mr872658wmc.85.1541610945281; Wed, 07 Nov 2018 09:15:45 -0800 (PST) Received: from bidouze.vm.6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id j4-v6sm1478996wrp.68.2018.11.07.09.15.44 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 07 Nov 2018 09:15:44 -0800 (PST) Date: Wed, 7 Nov 2018 18:15:25 +0100 From: =?iso-8859-1?Q?Ga=EBtan?= Rivet To: "Zhang, Qi Z" Cc: Thomas Monjalon , "dev@dpdk.org" , "Yigit, Ferruh" Message-ID: <20181107171525.urcwrvaqgh7e7amq@bidouze.vm.6wind.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> <039ED4275CED7440929022BC67E70611532E0AC3@SHSMSX103.ccr.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <039ED4275CED7440929022BC67E70611532E0AC3@SHSMSX103.ccr.corp.intel.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] [PATCH] bus/vdev: fix probe same device twice X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 07 Nov 2018 17:15:46 -0000 On Wed, Nov 07, 2018 at 04:53:50PM +0000, Zhang, Qi Z wrote: > > > > -----Original Message----- > > From: Gaëtan 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 > > > > 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? > > > > > > > > > > It's also possible to add a back-reference to an rte_device in [1], but that 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 that > > when a bus scans its devices, it should do a 1-to-1 map to devargs. > > > > If mapping rte_device to rte_devargs needs to respect rules, it could help bus > > developpers to have a function that will do the job: verify that the devargs is > > not currently used, add the back-reference to the rte_device. > > > > With the proper back-reference, it is possible to clean-up the device when > > removing the devargs > > This may still not work for vdev, since the old reference is used in vdev_find to find a exist device by name during scan. > (For PCI device, we have pci_addr, but vdev we use devargs->name to identify device, anyway this can be fixed in vdev, but that required a clone on the device name also break the coupling somehow.) A bus should keep device identifiers within a device, without relying on objects belonging to the EAL. > I just don't understand "why we must tight the tighten the device -> devargs coupling, not loosen it" > My point is that we are seemingly having problems with loose pointers, broken mappings, memory leaks. So managing seems already too complicated. Adding clones and copies will only make it more difficult to get right. It seems we have identified in this thread problematic behaviors from developpers, instead of giving them more tools to shoot feet we can instead give helpers to do what they are trying to do, but properly. The end-goal is not to have several devargs lying around, copies of each other, it is to avoid breaking devargs references. > (and also to add the rte_devargs_extract() function > > that would allow keeping the original devargs and insert it back if the hotplug > > fails, then the mapping must be restored). > > > > > [1]: https://mails.dpdk.org/archives/dev/2018-November/118274.html > > > > -- > > Gaëtan Rivet > > 6WIND -- Gaëtan Rivet 6WIND