Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758684AbZKYLYy (ORCPT ); Wed, 25 Nov 2009 06:24:54 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1758490AbZKYLYx (ORCPT ); Wed, 25 Nov 2009 06:24:53 -0500 Received: from fgwmail7.fujitsu.co.jp ([192.51.44.37]:59001 "EHLO fgwmail7.fujitsu.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758487AbZKYLYv (ORCPT ); Wed, 25 Nov 2009 06:24:51 -0500 X-SecurityPolicyCheck-FJ: OK by FujitsuOutboundMailChecker v1.3.1 Message-ID: <4B0D13EB.9010403@jp.fujitsu.com> Date: Wed, 25 Nov 2009 20:24:27 +0900 From: Kenji Kaneshige User-Agent: Thunderbird 2.0.0.23 (Windows/20090812) MIME-Version: 1.0 To: Yinghai Lu CC: Jesse Barnes , "Eric W. Biederman" , Alex Chiang , Bjorn Helgaas , Ingo Molnar , "linux-kernel@vger.kernel.org" , "linux-pci@vger.kernel.org" , Ivan Kokshaysky Subject: Re: [PATCH 1/2] pci: release that leaf bridge' resource that is not big -v11 References: <4ADEB601.8020200@kernel.org> <4AE53883.3070709@kernel.org> <4AE5545E.1020900@jp.fujitsu.com> <4AE55D12.30403@kernel.org> <4AE57976.4060107@jp.fujitsu.com> <4AE5E37F.8070707@kernel.org> <4AE5EFDB.2060908@kernel.org> <4AE80170.6030402@jp.fujitsu.com> <4AE88305.8020207@kernel.org> <4AE899A0.3020006@kernel.org> <4AE95247.8080401@jp.fujitsu.com> <4AE952B9.1010603@kernel.org> <4AE9588E.90708@jp.fujitsu.com> <4AE9657F.7010302@kernel.org> <4AE965D9.9040702@kernel.org> <20091104093044.17ab628a@jbarnes-piketon> <4AF1CD79.4010602@kernel.org> <4AF22CF1.1020508@kernel.org> <4AF22D26.4070500@kernel.org> <4AF508F0.9060105@kernel.org> <4AF91F54.10507@jp.fujitsu.com> <4AF936DB.1030309@kernel.org> <4AFCF7D8.1090207@jp.fujitsu.com> <4AFCFC0D.4030002@kernel.org> <4AFD19DA.7010602@jp.fujitsu.com> <4AFE6F39.5080505@kernel.org> <4B0B321E.4010103@jp.fujitsu.com> <4B0B335E.1070809@kernel.org> <4B0B3C13.9030502@jp.fujit! su.com> <4B0C69AD.3030106@kernel. org> In-Reply-To: <4B0C69AD.3030106@kernel.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4959 Lines: 143 Hi Yinghai, I would like to reconfirm what is the problem you're trying to solve before reviewing and testing the additional patch. To be honest, your patch looks more and more complicated and it is becoming difficult for me to review and test it. By the way, if your problem is that BIOS doesn't assign the resource to the parent bridge (root port or switch downstream port) of PCIe hotplug slots, I guess we can improve it with simpler way. I made a sample patches (no enough testing). Please take a look. Patches are - [PATCH 1/2] pciehp: remove redundancy in bridge resource allocation - [PATCH 2/2] pciehp: add support for bridge resource reallocation Thanks, Kenji Kaneshige Yinghai Lu wrote: > Kenji Kaneshige wrote: >> Yinghai Lu wrote: >>> Kenji Kaneshige wrote: >>>> Hi, >>>> >>>> I tried v11 patches. This version seems to fix the problem I >>>> reported against previous version. >>>> >>>> I have no objection against the idea of resource allocation >>>> changes for PCI express hotplug slots. >>> thanks >>> >>>> But I still have concern about changing resource allocation for >>>> other than PCI express hotplug slots. For example, some hotplug >>>> controller other than PCI express can have multiple slots under >>>> the same bus. If some hotplug slots are occupied and the others >>>> are empty at the boot time, I think your code try to shrink the >>>> bus resources for hotplug slots allocated by BIOS. It would break >>>> the hot-add on the empty slots due to the resource allocation >>>> failure. >>> no, >>> it will not touch bridge resources that already assigned by BIOS except >>> some bridge resource is not big enough. and get big one for them. >>> >> Ok, I understood that if the BIOS assigns enough resources to the >> bridge, it has no impact. >> >> One question. I thought your patch shrinks the bridge resource to >> allocate enough resource for sibling bridge. But it actually doesn't. >> Right? > > please check if this one could fix the shrinking bridge resource problem... > > [PATCH] pci: don't shrink bridge resources > > when we are clearing leaf bridge resource and try to get big one, we could shrink the bridge if > there resource under it. > > let check with old resource size and make sure we are trying to get big one. > > Signed-off-by: Yinghai Lu > > --- > drivers/pci/setup-bus.c | 22 ++++++++++++---------- > 1 file changed, 12 insertions(+), 10 deletions(-) > > 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 > @@ -432,7 +432,7 @@ static void pbus_size_io(struct pci_bus > { > struct pci_dev *dev; > struct resource *b_res = find_free_bus_resource(bus, IORESOURCE_IO); > - unsigned long size = 0, size1 = 0; > + unsigned long size = 0, size1 = 0, old_size; > > if (!b_res) > return; > @@ -457,17 +457,18 @@ static void pbus_size_io(struct pci_bus > } > if (size < min_size) > size = min_size; > + old_size = resource_size(b_res); > + if (old_size == 1) > + old_size = 0; > /* To be fixed in 2.5: we should have sort of HAVE_ISA > flag in the struct pci_bus. */ > #if defined(CONFIG_ISA) || defined(CONFIG_EISA) > size = (size & 0xff) + ((size & ~0xffUL) << 2); > #endif > size = ALIGN(size + size1, 4096); > + if (size < old_size) > + size = old_size; > if (!size) { > - if (b_res->start || b_res->end) > - dev_info(&bus->self->dev, "disabling bridge window " > - "%pR to [bus %02x-%02x] (unused)\n", b_res, > - bus->secondary, bus->subordinate); > b_res->flags = 0; > return; > } > @@ -483,7 +484,7 @@ static int pbus_size_mem(struct pci_bus > unsigned long type, resource_size_t min_size) > { > struct pci_dev *dev; > - resource_size_t min_align, align, size; > + resource_size_t min_align, align, size, old_size; > resource_size_t aligns[12]; /* Alignments from 1Mb to 2Gb */ > int order, max_order; > struct resource *b_res = find_free_bus_resource(bus, type); > @@ -533,6 +534,11 @@ static int pbus_size_mem(struct pci_bus > } > if (size < min_size) > size = min_size; > + old_size = resource_size(b_res); > + if (old_size == 1) > + old_size = 0; > + if (size < old_size) > + size = old_size; > > align = 0; > min_align = 0; > @@ -549,10 +555,6 @@ static int pbus_size_mem(struct pci_bus > } > size = ALIGN(size, min_align); > if (!size) { > - if (b_res->start || b_res->end) > - dev_info(&bus->self->dev, "disabling bridge window " > - "%pR to [bus %02x-%02x] (unused)\n", b_res, > - bus->secondary, bus->subordinate); > b_res->flags = 0; > return 1; > } > > -- 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/