From patchwork Thu Jan 31 23:29:25 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Myron Stowe X-Patchwork-Id: 2076121 X-Patchwork-Delegate: bhelgaas@google.com Return-Path: X-Original-To: patchwork-linux-pci@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork1.kernel.org (Postfix) with ESMTP id 59BCC3FD1A for ; Thu, 31 Jan 2013 23:29:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752923Ab3AaX3i (ORCPT ); Thu, 31 Jan 2013 18:29:38 -0500 Received: from mx1.redhat.com ([209.132.183.28]:65436 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751523Ab3AaX3h (ORCPT ); Thu, 31 Jan 2013 18:29:37 -0500 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r0VNTRXc013709 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 31 Jan 2013 18:29:27 -0500 Received: from [10.3.113.26] (ovpn-113-26.phx2.redhat.com [10.3.113.26]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r0VNTQ7Y026941; Thu, 31 Jan 2013 18:29:26 -0500 Subject: Re: [PATCH 1/2] PCI: ASPM exit link state code could skip devices From: Myron Stowe To: Joe Lawrence Cc: linux-pci@vger.kernel.org, Matthew Garrett , David Bulkow , shli@kernel.org, shli@fusionio.com, kaneshige.kenji@jp.fujitsu.com, linux-kernel In-Reply-To: References: Date: Thu, 31 Jan 2013 16:29:25 -0700 Message-ID: <1359674965.2426.13.camel@zim.stowe> Mime-Version: 1.0 X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org On Fri, 2013-01-18 at 13:23 -0500, Joe Lawrence wrote: > From 3a51bbad5555ba6c6e144aa5176c8112eb449325 Mon Sep 17 00:00:00 2001 > From: Joe Lawrence > Date: Tue, 15 Jan 2013 14:51:57 -0500 > Subject: [PATCH 1/2] PCI: ASPM exit link state code could skip devices > > On PCI bus hotplug removal, pcie_aspm_exit_link_state could potentially skip > parent devices that have link_state allocated. Instead of exiting early if > a given device is not PCIe, check that the device's parent is PCIe. This > enables pcie_aspm_exit_link_state to properly clean up parent link_state when > the last function in a slot might not be PCIe. > > Reviewed-by: David Bulkow > Signed-off-by: Joe Lawrence > --- > drivers/pci/pcie/aspm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c > index b52630b..6122447 100644 > --- a/drivers/pci/pcie/aspm.c > +++ b/drivers/pci/pcie/aspm.c > @@ -634,7 +634,7 @@ void pcie_aspm_exit_link_state(struct pci_dev *pdev) > struct pci_dev *parent = pdev->bus->self; > struct pcie_link_state *link, *root, *parent_link; > > - if (!pci_is_pcie(pdev) || !parent || !parent->link_state) > + if (!parent || !pci_is_pcie(parent) || !parent->link_state) > return; > if ((pci_pcie_type(parent) != PCI_EXP_TYPE_ROOT_PORT) && > (pci_pcie_type(parent) != PCI_EXP_TYPE_DOWNSTREAM)) Joe: Bjorn and I looked at this again today and think the checks that occur early in 'pcie_aspm_exit_link_state()' could be simplified (as shown below) to avoid issues such as you are encountering. I think part of the confusion concerning the asymmetry between 'pcie_aspm_init_link_state()' and 'pcie_aspm_exit_link_state()' is that the former is passed in a pointer to a bridge device whereas the latter is passed in a pointer to a end-point device. Frankly, I find the entire driver to be confusing and very hard to understand. Could you please test with the following patch and let us know the results? PCI: ASPM exit link state code is skipping devices From: Myron Stowe On PCI bus hotplug removal, 'pcie_aspm_exit_link_state' can potentially skip parent devices that have link_state allocated. Instead of exiting early if a given device in not PCIe, check whether or not the device's parent has link_state allocated. This enables 'pcie_aspm_exit_link_state' to properly clean up parent link_state when the last function in a slot might not be PCIe. Reported-by: Joe Lawrence Signed-off-by: Myron Stowe --- drivers/pci/pcie/aspm.c | 5 +---- 1 files changed, 1 insertions(+), 4 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c index 8474b6a..aa52727 100644 --- a/drivers/pci/pcie/aspm.c +++ b/drivers/pci/pcie/aspm.c @@ -634,10 +634,7 @@ void pcie_aspm_exit_link_state(struct pci_dev *pdev) struct pci_dev *parent = pdev->bus->self; struct pcie_link_state *link, *root, *parent_link; - if (!pci_is_pcie(pdev) || !parent || !parent->link_state) - return; - if ((pci_pcie_type(parent) != PCI_EXP_TYPE_ROOT_PORT) && - (pci_pcie_type(parent) != PCI_EXP_TYPE_DOWNSTREAM)) + if (!parent || !parent->link_state) return; down_read(&pci_bus_sem);