From patchwork Wed Jul 11 10:41:57 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: "Guo, Jia" X-Patchwork-Id: 42826 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 6D8701B590; Wed, 11 Jul 2018 12:44:47 +0200 (CEST) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 2D6941B571 for ; Wed, 11 Jul 2018 12:44:45 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 11 Jul 2018 03:44:43 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,338,1526367600"; d="scan'208";a="54202548" Received: from jeffguo-z170x-ud5.sh.intel.com (HELO localhost.localdomain) ([10.67.104.10]) by fmsmga008.fm.intel.com with ESMTP; 11 Jul 2018 03:44:32 -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: Wed, 11 Jul 2018 18:41:57 +0800 Message-Id: <1531305717-15504-8-git-send-email-jia.guo@intel.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1531305717-15504-1-git-send-email-jia.guo@intel.com> References: <1498711073-42917-1-git-send-email-jia.guo@intel.com> <1531305717-15504-1-git-send-email-jia.guo@intel.com> MIME-Version: 1.0 Subject: [dpdk-dev] [PATCH v9 7/7] igb_uio: fix unexpected remove 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" When device be hotplug out, the pci resource will be released in kernel, the uio fd will disappear, and the irq will be released. At this time, if igb uio driver still try to access or release these resource, it will cause kernel crash. On the other hand, uio_remove will be called unexpectedly before uio_release. The uio_remove procedure will free resources which are needed by uio_release. So there is no chance to disable interrupt which is defined inside uio_release procedure. This will affect later usage of interrupt. So the case of unexpectedly removal by hot unplug should be identify and correspondingly processed. This patch propose to add enum rte_udev_state in struct rte_uio_pci_dev, that will keep the state of uio device as probed/opened/released/removed. This patch also checks kobject’s remove_uevent_sent state to detect the unexpectedly removal status which means hot unplug. Once hot unplug be detected, it will call uio_release as soon as possible and set the uio status to be “removed”. After that, uio will check this status in uio_release function, if uio have already been removed, it will only free the dirty uio resource. Signed-off-by: Jeff Guo Acked-by: Shaopeng He --- v9->v8: refine commit log to be more readable. --- 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);