From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id CAF471B53A for ; Wed, 11 Jul 2018 12:01:48 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 11 Jul 2018 03:01:46 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,337,1526367600"; d="scan'208";a="65996130" Received: from jguo15x-mobl3.ccr.corp.intel.com (HELO [10.67.68.71]) ([10.67.68.71]) by fmsmga002.fm.intel.com with ESMTP; 11 Jul 2018 03:01:44 -0700 To: Stephen Hemminger References: <1498711073-42917-1-git-send-email-jia.guo@intel.com> <1531220607-2977-1-git-send-email-jia.guo@intel.com> <1531220607-2977-8-git-send-email-jia.guo@intel.com> <20180710145254.661da2cf@xeon-e3> <60ae9559-c658-5e2c-ddb3-09c6ababe2f5@intel.com> Cc: bruce.richardson@intel.com, ferruh.yigit@intel.com, konstantin.ananyev@intel.com, gaetan.rivet@6wind.com, jingjing.wu@intel.com, thomas@monjalon.net, motih@mellanox.com, matan@mellanox.com, harry.van.haaren@intel.com, qi.z.zhang@intel.com, shaopeng.he@intel.com, bernard.iremonger@intel.com, arybchenko@solarflare.com, wenzhuo.lu@intel.com, jblunck@infradead.org, shreyansh.jain@nxp.com, dev@dpdk.org, helin.zhang@intel.com From: Jeff Guo Message-ID: Date: Wed, 11 Jul 2018 18:01:37 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <60ae9559-c658-5e2c-ddb3-09c6ababe2f5@intel.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v8 7/7] igb_uio: fix uio release issue for hotplug 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, 11 Jul 2018 10:01:49 -0000 On 7/11/2018 10:46 AM, Jeff Guo wrote: > > > On 7/11/2018 5:52 AM, Stephen Hemminger wrote: >> On Tue, 10 Jul 2018 19:03:27 +0800 >> Jeff Guo wrote: >> >>> When hotplug out device, the device resource will be released in >>> kernel. >>> The fd sys file will disappear, and the irq will be released. At >>> this time, >>> if igb uio driver still try to release this resource, it will cause >>> kernel >>> crash. On the other hand, interrupt disabling do not automatically be >>> processed in kernel. If not handle it, this redundancy and dirty >>> thing will >>> affect the interrupt resource be used by other device. So the igb_uio >>> driver have to check the hotplug status, and the corresponding process >>> should be taken in igb uio driver. >>> >>> This patch propose to add enum rte_udev_state into struct >>> rte_uio_pci_dev >>> of igb uio driver, which will record the state of uio device, such as >>> probed/opened/released/removed. When detect the unexpected removal >>> which >>> cause of hotplug out behavior, it will corresponding disable interrupt >>> resource. For the part of releasement which kernel have already handle, >>> just skip it to avoid double free or null pointer crash issue. >>> >>> Signed-off-by: Jeff Guo >> The PCI hotplug management is an important and potentially error prone >> error of DPDK. >> >> I realize that English is not your native language, but the commit >> messages >> for this are hard to read. Perhaps you can get a volunteer or other >> person >> in the community to reword them. The commit logs and comments contain >> important information about the documentation of the code. > > yes, i think that it might not be the whole thing to let you confused > except something specific. But definitely it is my task to let > reviewer most easily know what i want to propose before they will ack it, > especial for some complex case. let's try my best for check my word. I > am also planning to go to more native English country and great > conference study, anyway to improve my word : ) > >> How does VFIO handle hotplug? We should direct all users to use VFIO >> since it is supported and secure. Igb uio has always been a slightly >> dangerous (as they say "running with scissors") way of accessing >> devices. > > You exposure VFIO here to replace igo uio, maybe we should check if it > is optional or not. > If we can fix all igb_uio issue it should be optional, if only vfio > show stable we should go to vfio only. > What other else guy comment here? Plus, the pci vfio hotplug enabling is still on the next plan, since the different framework between vifo and uio for uevent process. Per vfio, when user space control it will holding the resource so the uevent will be blocked to sent out from kernel. Anyway that should be another story. We hope this patch set just fix hotplug failure issue for igb uio case.