From patchwork Thu Apr 5 17:55:30 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marcelo Henrique Cerri X-Patchwork-Id: 895478 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=lists.ubuntu.com (client-ip=91.189.94.19; helo=huckleberry.canonical.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=canonical.com Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) by ozlabs.org (Postfix) with ESMTP id 40H9VK4Rrqz9s2B; Fri, 6 Apr 2018 03:55:53 +1000 (AEST) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.86_2) (envelope-from ) id 1f496y-000700-S6; Thu, 05 Apr 2018 17:55:48 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by huckleberry.canonical.com with esmtps (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:128) (Exim 4.86_2) (envelope-from ) id 1f496w-0006zp-QP for kernel-team@lists.ubuntu.com; Thu, 05 Apr 2018 17:55:46 +0000 Received: from mail-qt0-f200.google.com ([209.85.216.200]) by youngberry.canonical.com with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.76) (envelope-from ) id 1f496w-0000TP-Ds for kernel-team@lists.ubuntu.com; Thu, 05 Apr 2018 17:55:46 +0000 Received: by mail-qt0-f200.google.com with SMTP id q19so18225610qta.17 for ; Thu, 05 Apr 2018 10:55:46 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references; bh=+G+RhdNH4Su4gXmI1dYvNSdsGEn1ROYiLcvFGpdQVkM=; b=SsNl4ozhYIbGQCLq3PgaQOt7D1geJ5Z6zTS7p2YvndlFdgESWe/KzcOCpE413k10Ks MLx7gK1ohmi+Gw3L42z9Ha5ukO0+7qoBYtjy/BSE+YzJWxBKtuCzgxABgg73njEBdcy5 6xcv6ePHXgMum9bDREBz/mF+8UIHH6pQrKuYVNIRxNv+SmgmDq7DIRhnzqBbWDe1Yoia qgriAe4NlljCSQViw67wDAbyKQaMyJjWtN2YzRhvu+D4glIeyNQSDNsx8On5IcymqUSn Owj3IDtz/WAgP6fa3wlvimp7OLDWV0sx70c1CwX1sCJ+4Xcc/F+/E5BR0oEHmTQC/8mq gMeg== X-Gm-Message-State: ALQs6tCik0jDdgV4y+cOWnoNLd3k0dHpSsoNymKcXUuhSM67SD7ynDxx ZOvmzDa4U/ssEOdaveU6xF7tPfMWmcU5G8dKgsoYgA8wEzGV/+K08XK6Lk+M6LlmFGidJpqhE+L lHX1YmUvpxPHy/OdIBGStCssJmxPZN27ZT69SXEKI X-Received: by 10.200.42.146 with SMTP id b18mr33422177qta.262.1522950945171; Thu, 05 Apr 2018 10:55:45 -0700 (PDT) X-Google-Smtp-Source: AIpwx4/ixWi17ZVcqA0hZR4MzWHX4Y3VVntWqyhkuYmiTLX7kDJp6x+O/jBxdggV4kk1pIWsJjgDPQ== X-Received: by 10.200.42.146 with SMTP id b18mr33422154qta.262.1522950944885; Thu, 05 Apr 2018 10:55:44 -0700 (PDT) Received: from localhost.localdomain ([191.8.81.129]) by smtp.gmail.com with ESMTPSA id m53sm6854230qtf.67.2018.04.05.10.55.42 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 05 Apr 2018 10:55:43 -0700 (PDT) From: Marcelo Henrique Cerri To: kernel-team@lists.ubuntu.com Subject: [azure:x][PATCH 1/5] UBUNTU: SAUCE: PCI: hv: Serialize the present and eject work items Date: Thu, 5 Apr 2018 14:55:30 -0300 Message-Id: <1522950934-8127-2-git-send-email-marcelo.cerri@canonical.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1522950934-8127-1-git-send-email-marcelo.cerri@canonical.com> References: <1522950934-8127-1-git-send-email-marcelo.cerri@canonical.com> X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" From: Dexuan Cui BugLink: http://bugs.launchpad.net/bugs/1758378 When we hot-remove the device, we first receive a PCI_EJECT message and then receive a PCI_BUS_RELATIONS message with bus_rel->device_count == 0. The first message is offloaded to hv_eject_device_work(), and the second is offloaded to pci_devices_present_work(). Both the paths can be running list_del(&hpdev->list_entry), causing general protection fault, because system_wq can run them concurrently. The patch eliminates the race condition. Since access to present/eject work items is serialized, we do not need the hbus->enum_sem anymore, so remove it. Fixes: 4daace0d8ce8 ("PCI: hv: Add paravirtual PCI front-end for Microsoft Hyper-V VMs") Link: https://lkml.kernel.org/r/KL1P15301MB00064DA6B4D221123B5241CFBFD70@KL1P15301MB0006.APCP153.PROD.OUTLOOK.COM Tested-by: Adrian Suhov Tested-by: Chris Valean Signed-off-by: Dexuan Cui [lorenzo.pieralisi@arm.com: squashed semaphore removal patch] Signed-off-by: Lorenzo Pieralisi Reviewed-by: Michael Kelley Acked-by: Haiyang Zhang Cc: # v4.6+ Cc: Vitaly Kuznetsov Cc: Jack Morgenstein Cc: Stephen Hemminger Cc: K. Y. Srinivasan Signed-off-by: Marcelo Henrique Cerri --- drivers/pci/host/pci-hyperv.c | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c index 8b5f66d3c4f2..f699af20c012 100644 --- a/drivers/pci/host/pci-hyperv.c +++ b/drivers/pci/host/pci-hyperv.c @@ -457,7 +457,6 @@ struct hv_pcibus_device { spinlock_t device_list_lock; /* Protect lists below */ void __iomem *cfg_addr; - struct semaphore enum_sem; struct list_head resources_for_children; struct list_head children; @@ -471,6 +470,8 @@ struct hv_pcibus_device { struct retarget_msi_interrupt retarget_msi_interrupt_params; spinlock_t retarget_msi_interrupt_lock; + + struct workqueue_struct *wq; }; /* @@ -1606,12 +1607,8 @@ static struct hv_pci_dev *get_pcichild_wslot(struct hv_pcibus_device *hbus, * It must also treat the omission of a previously observed device as * notification that the device no longer exists. * - * Note that this function is a work item, and it may not be - * invoked in the order that it was queued. Back to back - * updates of the list of present devices may involve queuing - * multiple work items, and this one may run before ones that - * were sent later. As such, this function only does something - * if is the last one in the queue. + * Note that this function is serialized with hv_eject_device_work(), + * because both are pushed to the ordered workqueue hbus->wq. */ static void pci_devices_present_work(struct work_struct *work) { @@ -1632,11 +1629,6 @@ static void pci_devices_present_work(struct work_struct *work) INIT_LIST_HEAD(&removed); - if (down_interruptible(&hbus->enum_sem)) { - put_hvpcibus(hbus); - return; - } - /* Pull this off the queue and process it if it was the last one. */ spin_lock_irqsave(&hbus->device_list_lock, flags); while (!list_empty(&hbus->dr_list)) { @@ -1653,7 +1645,6 @@ static void pci_devices_present_work(struct work_struct *work) spin_unlock_irqrestore(&hbus->device_list_lock, flags); if (!dr) { - up(&hbus->enum_sem); put_hvpcibus(hbus); return; } @@ -1740,7 +1731,6 @@ static void pci_devices_present_work(struct work_struct *work) break; } - up(&hbus->enum_sem); put_hvpcibus(hbus); kfree(dr); } @@ -1786,7 +1776,7 @@ static void hv_pci_devices_present(struct hv_pcibus_device *hbus, spin_unlock_irqrestore(&hbus->device_list_lock, flags); get_hvpcibus(hbus); - schedule_work(&dr_wrk->wrk); + queue_work(hbus->wq, &dr_wrk->wrk); } /** @@ -1864,7 +1854,7 @@ static void hv_pci_eject_device(struct hv_pci_dev *hpdev) get_pcichild(hpdev, hv_pcidev_ref_pnp); INIT_WORK(&hpdev->wrk, hv_eject_device_work); get_hvpcibus(hpdev->hbus); - schedule_work(&hpdev->wrk); + queue_work(hpdev->hbus->wq, &hpdev->wrk); } /** @@ -2477,13 +2467,18 @@ static int hv_pci_probe(struct hv_device *hdev, spin_lock_init(&hbus->config_lock); spin_lock_init(&hbus->device_list_lock); spin_lock_init(&hbus->retarget_msi_interrupt_lock); - sema_init(&hbus->enum_sem, 1); init_completion(&hbus->remove_event); + hbus->wq = alloc_ordered_workqueue("hv_pci_%x", 0, + hbus->sysdata.domain); + if (!hbus->wq) { + ret = -ENOMEM; + goto free_bus; + } ret = vmbus_open(hdev->channel, pci_ring_size, pci_ring_size, NULL, 0, hv_pci_onchannelcallback, hbus); if (ret) - goto free_bus; + goto destroy_wq; hv_set_drvdata(hdev, hbus); @@ -2552,6 +2547,8 @@ static int hv_pci_probe(struct hv_device *hdev, hv_free_config_window(hbus); close: vmbus_close(hdev->channel); +destroy_wq: + destroy_workqueue(hbus->wq); free_bus: free_page((unsigned long)hbus); return ret; @@ -2631,6 +2628,7 @@ static int hv_pci_remove(struct hv_device *hdev) irq_domain_free_fwnode(hbus->sysdata.fwnode); put_hvpcibus(hbus); wait_for_completion(&hbus->remove_event); + destroy_workqueue(hbus->wq); free_page((unsigned long)hbus); return 0; }