From patchwork Mon Aug 24 22:14:26 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yinghai Lu X-Patchwork-Id: 7066981 Return-Path: X-Original-To: patchwork-linux-acpi@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id C7E52C05AC for ; Mon, 24 Aug 2015 22:14:31 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id B1269207AA for ; Mon, 24 Aug 2015 22:14:30 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id A3323207A8 for ; Mon, 24 Aug 2015 22:14:29 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751989AbbHXWO2 (ORCPT ); Mon, 24 Aug 2015 18:14:28 -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 Received: by iods203 with SMTP id s203so165948530iod.0; Mon, 24 Aug 2015 15:14:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=yQwaPQrXCnuUJC30/mcl4y7EiuIpEtksiDc2y9k2teA=; b=t9LXgjgpCEVjxlzS1bgBxtdSWI745pb7YdOzUEQECIR6Iauv98J8HXaEabBuNTi2Hg ex2k126jwIqmi/vzdQek2l5oSCcd2OOukXmBOc3eyaXx+ixPdBvRnGjTR7zKQXfMJyPt 2m+giDYdhqK9GboR221MX9zE2XYZBeR+wITEhyqoI4NoT+iLtZjgUr86RTSx6q/LQpoh WqmeJ+vM5gsRnZxYIjvacanc/sBW6e7kO+x4tJRTi5CiaHvCn5yjG19bmK4OvWppds5Y KWYu4Vw5TEhR8PL2mBaO0h2xwFcm2q9e21jxKVWPIpLXNuFDiybDEfdUMqndqa5cfA91 8BWQ== MIME-Version: 1.0 X-Received: by 10.107.19.163 with SMTP id 35mr24757570iot.170.1440454466758; Mon, 24 Aug 2015 15:14:26 -0700 (PDT) Received: by 10.64.120.34 with HTTP; Mon, 24 Aug 2015 15:14:26 -0700 (PDT) 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 Sender: linux-acpi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org X-Spam-Status: No, score=-8.2 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,T_DKIM_INVALID,UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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(-) -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html 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);