From patchwork Thu Mar 26 21:38:39 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alexander Chiang X-Patchwork-Id: 14614 Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n2QLcggh030710 for ; Thu, 26 Mar 2009 21:38:42 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756009AbZCZVim (ORCPT ); Thu, 26 Mar 2009 17:38:42 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1755016AbZCZVim (ORCPT ); Thu, 26 Mar 2009 17:38:42 -0400 Received: from g1t0029.austin.hp.com ([15.216.28.36]:25738 "EHLO g1t0029.austin.hp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754190AbZCZVil (ORCPT ); Thu, 26 Mar 2009 17:38:41 -0400 Received: from g1t0039.austin.hp.com (g1t0039.austin.hp.com [16.236.32.45]) by g1t0029.austin.hp.com (Postfix) with ESMTP id 415C3380F3; Thu, 26 Mar 2009 21:38:40 +0000 (UTC) Received: from ldl.fc.hp.com (ldl.fc.hp.com [15.11.146.30]) by g1t0039.austin.hp.com (Postfix) with ESMTP id 0B04034025; Thu, 26 Mar 2009 21:38:39 +0000 (UTC) Received: by ldl.fc.hp.com (Postfix, from userid 17609) id B80F439C009; Thu, 26 Mar 2009 15:38:39 -0600 (MDT) Date: Thu, 26 Mar 2009 15:38:39 -0600 From: Alex Chiang To: Kenji Kaneshige Cc: "linux-pci@vger.kernel.org" , Jesse Barnes , "Eric W. Biederman" Subject: Re: [PATCH] PCI: fix kernel oops on bridge rmoval Message-ID: <20090326213839.GA1112@ldl.fc.hp.com> References: <49CB33A0.80401@jp.fujitsu.com> <20090326125622.GA7766@ldl.fc.hp.com> <49CB930A.4080103@jp.fujitsu.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <49CB930A.4080103@jp.fujitsu.com> User-Agent: Mutt/1.5.17+20080114 (2008-01-14) Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org * Kenji Kaneshige : > > Thank you very much for testing. > > We still have similar kernel oops (see below) with ACPI pci slot > detection driver. I guess the same problem would also occur with > acpiphp though I've not tried yet. I don't look at Trent's bus > notifier approach yet, but I think we need something like this to > fix this problem. > > Here are steps to reproduce and kernel oops message. > > * Steps to reproduce > > (1) Load ACPI pci slot detection driver > (2) Remove the parent bridge of the slot > (3) Unload ACPI pci slot detection driver Thanks for the report. I believe this patch will fix the case for bridges. I haven't tested what happens if we remove an endpoint yet though. Can you try this? Thanks. /ac --- commit 557ce38e78cf06ce16aefcf273051ea0bac3d35c Author: Alex Chiang Date: Thu Mar 26 15:36:34 2009 -0600 PCI: pci_create_slot / pci_destroy_slot need to grab reference to parent bus If a logical hotunplug (remove) is performed on a PCI bridge claimed by a hotplug or slot detection driver, and then the hotplug/detection module is unloaded, we will encounter an oops: Call Trace: [] die+0x1c0/0x2c0 sp=e0000005062ff9e0 bsp=e0000005062f13b0 [] die_if_kernel+0x40/0x60 sp=e0000005062ff9e0 bsp=e0000005062f1380 [] ia64_fault+0x1230/0x1280 sp=e0000005062ff9e0 bsp=e0000005062f1300 [] ia64_native_leave_kernel+0x0/0x270 sp=e0000005062ffbf0 bsp=e0000005062f1300 [] pci_slot_release+0x70/0x1c0 sp=e0000005062ffdc0 bsp=e0000005062f12b0 [] kobject_release+0x4f0/0x5e0 sp=e0000005062ffdc0 bsp=e0000005062f1270 [] kref_put+0xd0/0x100 sp=e0000005062ffdc0 bsp=e0000005062f1248 [] kobject_put+0x90/0xc0 sp=e0000005062ffdc0 bsp=e0000005062f1220 [] pci_destroy_slot+0xa0/0xe0 sp=e0000005062ffdc0 bsp=e0000005062f11f0 [] pci_hp_deregister+0x510/0x560 [pci_hotplug] sp=e0000005062ffdc0 bsp=e0000005062f11a8 [] acpiphp_unregister_hotplug_slot+0x80/0x100 [acpiphp] sp=e0000005062ffdc0 bsp=e0000005062f1180 [] cleanup_bridge+0x3a0/0x4c0 [acpiphp] sp=e0000005062ffdc0 bsp=e0000005062f1128 [] cleanup_p2p_bridge+0x80/0xc0 [acpiphp] sp=e0000005062ffdc0 bsp=e0000005062f1108 [] acpi_ns_walk_namespace+0x160/0x2e0 sp=e0000005062ffdc0 bsp=e0000005062f1098 [] acpi_walk_namespace+0x90/0xe0 sp=e0000005062ffdc0 bsp=e0000005062f1048 [] remove_bridge+0x50/0xe0 [acpiphp] sp=e0000005062ffdc0 bsp=e0000005062f1028 [] acpi_pci_unregister_driver+0x1f0/0x2a0 sp=e0000005062ffdc0 bsp=e0000005062f0fe8 [] acpiphp_glue_exit+0x30/0x60 [acpiphp] sp=e0000005062ffdc0 bsp=e0000005062f0fd0 [] acpiphp_exit+0x20/0x40 [acpiphp] sp=e0000005062ffdc0 bsp=e0000005062f0fb8 [] sys_delete_module+0x410/0x520 sp=e0000005062ffdc0 bsp=e0000005062f0f38 This is because pci_slot_release will access the parent PCI bus, which has already been released by the user's prior hot unplug. The solution is for pci_create_slot to grab a reference on the parent PCI bus (and pci_destroy_slot to put the reference). This will prevent the parent from release while the hotplug or slot detection driver is loaded. Reported-by: Kenji Kaneshige Signed-off-by: Alex Chiang -- 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/slot.c b/drivers/pci/slot.c index 2118944..459d6a2 100644 --- a/drivers/pci/slot.c +++ b/drivers/pci/slot.c @@ -248,6 +248,7 @@ placeholder: if (PCI_SLOT(dev->devfn) == slot_nr) dev->slot = slot; + get_device(&parent->dev); dev_dbg(&parent->dev, "dev %02x, created physical slot %s\n", slot_nr, pci_slot_name(slot)); @@ -302,6 +303,7 @@ void pci_destroy_slot(struct pci_slot *slot) slot->number, atomic_read(&slot->kobj.kref.refcount) - 1); down_write(&pci_bus_sem); + put_device(&slot->bus->dev); kobject_put(&slot->kobj); up_write(&pci_bus_sem); }