Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754049AbbHXWO3 (ORCPT ); Mon, 24 Aug 2015 18:14:29 -0400 Received: from mail-io0-f175.google.com ([209.85.223.175]:33008 "EHLO mail-io0-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751561AbbHXWO1 (ORCPT ); Mon, 24 Aug 2015 18:14:27 -0400 MIME-Version: 1.0 In-Reply-To: <205780224.0d92UNafyF@vostro.rjw.lan> References: <1440138067-4314-1-git-send-email-yinghai@kernel.org> <1440138067-4314-14-git-send-email-yinghai@kernel.org> <205780224.0d92UNafyF@vostro.rjw.lan> Date: Mon, 24 Aug 2015 15:14:26 -0700 X-Google-Sender-Auth: DeaEHNMONMq2LDQVuusUOmOJYjU Message-ID: Subject: Re: [PATCH v4 13/52] PCI, acpiphp: Add missing realloc list checking after resource allocation From: Yinghai Lu To: "Rafael J. Wysocki" Cc: Bjorn Helgaas , David Miller , Benjamin Herrenschmidt , Wei Yang , TJ , Yijing Wang , Andrew Morton , "linux-pci@vger.kernel.org" , Linux Kernel Mailing List , Len Brown , ACPI Devel Maling List Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4700 Lines: 134 On Mon, Aug 24, 2015 at 3:09 PM, Rafael J. Wysocki wrote: > On Thursday, August 20, 2015 11:20:28 PM Yinghai Lu wrote: >> We check the realloc list, as list must be empty after allocation. >> >> Add missing one acpiphp driver. >> >> Signed-off-by: Yinghai Lu >> Cc: "Rafael J. Wysocki" >> Cc: Len Brown >> Cc: linux-acpi@vger.kernel.org >> --- >> drivers/pci/hotplug/acpiphp_glue.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c >> index ff53856..1c7c1d7 100644 >> --- a/drivers/pci/hotplug/acpiphp_glue.c >> +++ b/drivers/pci/hotplug/acpiphp_glue.c >> @@ -507,6 +507,7 @@ static void enable_slot(struct acpiphp_slot *slot) >> } >> } >> __pci_bus_assign_resources(bus, &add_list, NULL); >> + BUG_ON(!list_empty(&add_list)); > > Is crashing the kernel the best we can do here? > > What about bailing out with a WARN_ON() instead? Surely, the kernel can work > without the new device? That should not happen. If that list is not empty, we could have broken the assign logic for optional or additional resource allocation. We have other two BUG_ON checking in drivers/pci/setup-bus.c. Do we need to change them all to WARN_ON()? or you prefer this version instead: --- Subject: [PATCH] PCI: Separate realloc list checking after allocation We check the realloc list, as list must be empty after allocation. Separate the realloc list checking to another function. Add checking that is missed in acpiphp driver. -v2: change to WARN_ON addording to Rafael. Signed-off-by: Yinghai Lu Cc: "Rafael J. Wysocki" Cc: Len Brown Cc: linux-acpi@vger.kernel.org --- drivers/pci/hotplug/acpiphp_glue.c | 1 + drivers/pci/pci.h | 1 + drivers/pci/setup-bus.c | 12 +++++++++--- 3 files changed, 11 insertions(+), 3 deletions(-) Index: linux-2.6/drivers/pci/hotplug/acpiphp_glue.c =================================================================== --- linux-2.6.orig/drivers/pci/hotplug/acpiphp_glue.c +++ linux-2.6/drivers/pci/hotplug/acpiphp_glue.c @@ -507,6 +507,7 @@ static void enable_slot(struct acpiphp_s } } __pci_bus_assign_resources(bus, &add_list, NULL); + pci_bus_check_realloc(&add_list); acpiphp_sanitize_bus(bus); pcie_bus_configure_settings(bus); Index: linux-2.6/drivers/pci/pci.h =================================================================== --- linux-2.6.orig/drivers/pci/pci.h +++ linux-2.6/drivers/pci/pci.h @@ -237,6 +237,7 @@ void __pci_bus_size_bridges(struct pci_b void __pci_bus_assign_resources(const struct pci_bus *bus, struct list_head *realloc_head, struct list_head *fail_head); +void pci_bus_check_realloc(struct list_head *realloc_head); bool pci_bus_clip_resource(struct pci_dev *dev, int idx); void pci_reassigndev_resource_alignment(struct pci_dev *dev); Index: linux-2.6/drivers/pci/setup-bus.c =================================================================== --- linux-2.6.orig/drivers/pci/setup-bus.c +++ linux-2.6/drivers/pci/setup-bus.c @@ -349,6 +349,12 @@ out: } } +void pci_bus_check_realloc(struct list_head *realloc_head) +{ + if (WARN_ON(!list_empty(realloc_head))) + free_list(realloc_head); +} + /** * assign_requested_resources_sorted() - satisfy resource requests * @@ -1860,7 +1866,7 @@ again: /* Depth last, allocate resources and update the hardware. */ __pci_bus_assign_resources(bus, add_list, &fail_head); if (add_list) - BUG_ON(!list_empty(add_list)); + pci_bus_check_realloc(add_list); tried_times++; /* any device complain? */ @@ -1935,7 +1941,7 @@ void pci_assign_unassigned_bridge_resour again: __pci_bus_size_bridges(parent, &add_list); __pci_bridge_assign_resources(bridge, &add_list, &fail_head); - BUG_ON(!list_empty(&add_list)); + pci_bus_check_realloc(&add_list); tried_times++; if (list_empty(&fail_head)) @@ -1994,6 +2000,6 @@ void pci_assign_unassigned_bus_resources &add_list); up_read(&pci_bus_sem); __pci_bus_assign_resources(bus, &add_list, NULL); - BUG_ON(!list_empty(&add_list)); + pci_bus_check_realloc(&add_list); } EXPORT_SYMBOL_GPL(pci_assign_unassigned_bus_resources); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/