From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 3C6A61B485 for ; Tue, 10 Jul 2018 13:06:03 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 10 Jul 2018 04:06:02 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,334,1526367600"; d="scan'208";a="71076453" Received: from jeffguo-z170x-ud5.sh.intel.com (HELO localhost.localdomain) ([10.67.104.10]) by fmsmga001.fm.intel.com with ESMTP; 10 Jul 2018 04:06:00 -0700 From: Jeff Guo To: stephen@networkplumber.org, 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 Cc: jblunck@infradead.org, shreyansh.jain@nxp.com, dev@dpdk.org, jia.guo@intel.com, helin.zhang@intel.com Date: Tue, 10 Jul 2018 19:03:27 +0800 Message-Id: <1531220607-2977-8-git-send-email-jia.guo@intel.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1531220607-2977-1-git-send-email-jia.guo@intel.com> References: <1498711073-42917-1-git-send-email-jia.guo@intel.com> <1531220607-2977-1-git-send-email-jia.guo@intel.com> Subject: [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: Tue, 10 Jul 2018 11:06:03 -0000 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 --- v8->v7: change enum of udev state, refine code to release udev resource --- kernel/linux/igb_uio/igb_uio.c | 69 +++++++++++++++++++++++++++++++++--------- 1 file changed, 55 insertions(+), 14 deletions(-) diff --git a/kernel/linux/igb_uio/igb_uio.c b/kernel/linux/igb_uio/igb_uio.c index 3398eac..d126371 100644 --- a/kernel/linux/igb_uio/igb_uio.c +++ b/kernel/linux/igb_uio/igb_uio.c @@ -19,6 +19,14 @@ #include "compat.h" +/* uio pci device state */ +enum rte_udev_state { + RTE_UDEV_PROBED, + RTE_UDEV_OPENNED, + RTE_UDEV_RELEASED, + RTE_UDEV_REMOVED, +}; + /** * A structure describing the private information for a uio device. */ @@ -28,6 +36,7 @@ struct rte_uio_pci_dev { enum rte_intr_mode mode; struct mutex lock; int refcnt; + enum rte_udev_state state; }; static int wc_activate; @@ -309,6 +318,17 @@ igbuio_pci_disable_interrupts(struct rte_uio_pci_dev *udev) #endif } +/* Unmap previously ioremap'd resources */ +static void +igbuio_pci_release_iomem(struct uio_info *info) +{ + int i; + + for (i = 0; i < MAX_UIO_MAPS; i++) { + if (info->mem[i].internal_addr) + iounmap(info->mem[i].internal_addr); + } +} /** * This gets called while opening uio device file. @@ -331,20 +351,35 @@ igbuio_pci_open(struct uio_info *info, struct inode *inode) /* enable interrupts */ err = igbuio_pci_enable_interrupts(udev); - mutex_unlock(&udev->lock); if (err) { dev_err(&dev->dev, "Enable interrupt fails\n"); + pci_clear_master(dev); + mutex_unlock(&udev->lock); return err; } + udev->state = RTE_UDEV_OPENNED; + mutex_unlock(&udev->lock); return 0; } +/** + * This gets called while closing uio device file. + */ static int igbuio_pci_release(struct uio_info *info, struct inode *inode) { struct rte_uio_pci_dev *udev = info->priv; struct pci_dev *dev = udev->pdev; + if (udev->state == RTE_UDEV_REMOVED) { + mutex_destroy(&udev->lock); + igbuio_pci_release_iomem(&udev->info); + pci_disable_device(dev); + pci_set_drvdata(dev, NULL); + kfree(udev); + return 0; + } + mutex_lock(&udev->lock); if (--udev->refcnt > 0) { mutex_unlock(&udev->lock); @@ -356,7 +391,7 @@ igbuio_pci_release(struct uio_info *info, struct inode *inode) /* stop the device from further DMA */ pci_clear_master(dev); - + udev->state = RTE_UDEV_RELEASED; mutex_unlock(&udev->lock); return 0; } @@ -414,18 +449,6 @@ igbuio_pci_setup_ioport(struct pci_dev *dev, struct uio_info *info, return 0; } -/* Unmap previously ioremap'd resources */ -static void -igbuio_pci_release_iomem(struct uio_info *info) -{ - int i; - - for (i = 0; i < MAX_UIO_MAPS; i++) { - if (info->mem[i].internal_addr) - iounmap(info->mem[i].internal_addr); - } -} - static int igbuio_setup_bars(struct pci_dev *dev, struct uio_info *info) { @@ -562,6 +585,9 @@ igbuio_pci_probe(struct pci_dev *dev, const struct pci_device_id *id) (unsigned long long)map_dma_addr, map_addr); } + mutex_lock(&udev->lock); + udev->state = RTE_UDEV_PROBED; + mutex_unlock(&udev->lock); return 0; fail_remove_group: @@ -579,6 +605,21 @@ static void igbuio_pci_remove(struct pci_dev *dev) { struct rte_uio_pci_dev *udev = pci_get_drvdata(dev); + struct pci_dev *pdev = udev->pdev; + int ret; + + /* handle unexpected removal */ + if (udev->state == RTE_UDEV_OPENNED || + (&pdev->dev.kobj)->state_remove_uevent_sent == 1) { + dev_notice(&dev->dev, "Unexpected removal!\n"); + ret = igbuio_pci_release(&udev->info, NULL); + if (ret) + return; + mutex_lock(&udev->lock); + udev->state = RTE_UDEV_REMOVED; + mutex_unlock(&udev->lock); + return; + } mutex_destroy(&udev->lock); sysfs_remove_group(&dev->dev.kobj, &dev_attr_grp); -- 2.7.4