| Message ID | 1308610037-6261-5-git-send-email-linuxram@us.ibm.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Mon, Jun 20, 2011 at 03:47:17PM -0700, Ram Pai wrote: > Allocate resources to cardbus bridge only after all other genuine > resources requests are satisfied. Dont retry if resource allocation > for cardbus-bridge fails. Well, for those who use cardbus cards, cardbus resources aren't "nice to have", they are absolutely required. Of course, not all cardbus cards need as many resources as are currently assigned, so I wouldn't oppose a patch which marks _some_ of the currently assigned resources as "nice to have". But this approach -- 0 required, all "nice to have" -- seems wrong to me. Best, Dominik -- 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
On Tue, Jun 21, 2011 at 09:57:00AM +0200, Dominik Brodowski wrote: > On Mon, Jun 20, 2011 at 03:47:17PM -0700, Ram Pai wrote: > > Allocate resources to cardbus bridge only after all other genuine > > resources requests are satisfied. Dont retry if resource allocation > > for cardbus-bridge fails. > > Well, for those who use cardbus cards, cardbus resources aren't "nice to > have", they are absolutely required. Of course, not all cardbus cards need > as many resources as are currently assigned, so I wouldn't oppose a patch > which marks _some_ of the currently assigned resources as "nice to have". > But this approach -- 0 required, all "nice to have" -- seems wrong to me. Do you know how much minimal resource is good enough? The value, before this patch, was 256 for IO ports and 64M for memory. BTW: If the BIOS has already assigned enough resources for all the devices on the system, no devices will be starved including the cardbus. The OS intervenes and is forced to make this hard choice only when it sees unassigned resources to some devices along with resource contention. RP -- 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
On Tue, 21 Jun 2011 09:23:21 -0700 Ram Pai <linuxram@us.ibm.com> wrote: > On Tue, Jun 21, 2011 at 09:57:00AM +0200, Dominik Brodowski wrote: > > On Mon, Jun 20, 2011 at 03:47:17PM -0700, Ram Pai wrote: > > > Allocate resources to cardbus bridge only after all other genuine > > > resources requests are satisfied. Dont retry if resource allocation > > > for cardbus-bridge fails. > > > > Well, for those who use cardbus cards, cardbus resources aren't "nice to > > have", they are absolutely required. Of course, not all cardbus cards need > > as many resources as are currently assigned, so I wouldn't oppose a patch > > which marks _some_ of the currently assigned resources as "nice to have". > > But this approach -- 0 required, all "nice to have" -- seems wrong to me. > > Do you know how much minimal resource is good enough? The value, before > this patch, was 256 for IO ports and 64M for memory. > > BTW: If the BIOS has already assigned enough resources for all the devices on > the system, no devices will be starved including the cardbus. The OS intervenes > and is forced to make this hard choice only when it sees unassigned resources to > some devices along with resource contention. I just know this is going to trigger regressions, so I think Dominik's concern is valid. We'll have some existing machine with a device whose resource was never assigned, but we didn't care because it was unused. Now this code will try to give it some address space at the expense of something that *is* being used. But OTOH this will at least try to allocate *some* space to cardbus, we just won't try as hard as with some other resources. I'd mainly like to avoid the situation Dominik pointed out, where we have perfectly good cardbus resources assigned (unlike in Oliver's case) but they're stolen for a bridge that may get a hotplugged device or some other device that didn't have a BIOS assignment.
Hey, On Tue, Jun 21, 2011 at 02:36:22PM -0700, Jesse Barnes wrote: > On Tue, 21 Jun 2011 09:23:21 -0700 > Ram Pai <linuxram@us.ibm.com> wrote: > > > On Tue, Jun 21, 2011 at 09:57:00AM +0200, Dominik Brodowski wrote: > > > On Mon, Jun 20, 2011 at 03:47:17PM -0700, Ram Pai wrote: > > > > Allocate resources to cardbus bridge only after all other genuine > > > > resources requests are satisfied. Dont retry if resource allocation > > > > for cardbus-bridge fails. > > > > > > Well, for those who use cardbus cards, cardbus resources aren't "nice to > > > have", they are absolutely required. Of course, not all cardbus cards need > > > as many resources as are currently assigned, so I wouldn't oppose a patch > > > which marks _some_ of the currently assigned resources as "nice to have". > > > But this approach -- 0 required, all "nice to have" -- seems wrong to me. > > > > Do you know how much minimal resource is good enough? The value, before > > this patch, was 256 for IO ports and 64M for memory. > > > > BTW: If the BIOS has already assigned enough resources for all the devices on > > the system, no devices will be starved including the cardbus. The OS intervenes > > and is forced to make this hard choice only when it sees unassigned resources to > > some devices along with resource contention. > > Dominik, presumably you have a few good cardbus test machines; can you > give Ram's patches a try? If we know they break existing > configurations, I'm afraid we'll just have to revert the whole > re-allocation patch yet again. If your stuff survives, I'll ping Linus > to see what he thinks, though he'll probably want to revert in any > case... Actually, I only have one cardbus-capable test machine, which does work in very most cases, and also I do care much more about the PCMCIA side of things than the PCI/CardBus side... Therefore, all I could do is some more or less informed guessing about how much minimal resource we should try to allocate... Best, Dominik -- 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
On Wed, Jun 22, 2011 at 12:13:01AM +0200, Dominik Brodowski wrote: > Hey, > > On Tue, Jun 21, 2011 at 02:36:22PM -0700, Jesse Barnes wrote: > > On Tue, 21 Jun 2011 09:23:21 -0700 > > Ram Pai <linuxram@us.ibm.com> wrote: > > > > > On Tue, Jun 21, 2011 at 09:57:00AM +0200, Dominik Brodowski wrote: > > > > On Mon, Jun 20, 2011 at 03:47:17PM -0700, Ram Pai wrote: > > > > > Allocate resources to cardbus bridge only after all other genuine > > > > > resources requests are satisfied. Dont retry if resource allocation > > > > > for cardbus-bridge fails. > > > > > > > > Well, for those who use cardbus cards, cardbus resources aren't "nice to > > > > have", they are absolutely required. Of course, not all cardbus cards need > > > > as many resources as are currently assigned, so I wouldn't oppose a patch > > > > which marks _some_ of the currently assigned resources as "nice to have". > > > > But this approach -- 0 required, all "nice to have" -- seems wrong to me. > > > > > > Do you know how much minimal resource is good enough? The value, before > > > this patch, was 256 for IO ports and 64M for memory. > > > > > > BTW: If the BIOS has already assigned enough resources for all the devices on > > > the system, no devices will be starved including the cardbus. The OS intervenes > > > and is forced to make this hard choice only when it sees unassigned resources to > > > some devices along with resource contention. > > > > Dominik, presumably you have a few good cardbus test machines; can you > > give Ram's patches a try? If we know they break existing > > configurations, I'm afraid we'll just have to revert the whole > > re-allocation patch yet again. If your stuff survives, I'll ping Linus > > to see what he thinks, though he'll probably want to revert in any > > case... > > Actually, I only have one cardbus-capable test machine, which does work in > very most cases, and also I do care much more about the PCMCIA side of > things than the PCI/CardBus side... Therefore, all I could do is some more > or less informed guessing about how much minimal resource we should try to > allocate... I assume majority of the platforms will have enough resources to satisfy all the resource requests, and their BIOS would have done a decent job. Even if the BIOS has not done a decent job, and there are enough resources available we should not see a regression. The only platforms that would expose a regression is when resources are under contention and the BIOS has assigned enough resource to the cardbus bridge but not to some other device. It will be hard to find such a platform, but I am sure there is one out somewhere there. I am sure we will see; some day, reports of regression because that platform would have the exact right characteristics to expose the issue. But then that platform is a highly constrained platform in the first place. Its debatable if that should be characterised as a regression, or a platform that was riding on good luck till now. Even Oliver's platform is a highly constrained platform, and we probably can treat his platform as 'riding on good luck till now'. We won't be able to satisfy all the platforms with resource constraints. I think our probable choice is to select a solution that breaks least number of platforms and special case those broken platforms through kernel command line parameters. RP -- 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
On Tue, 21 Jun 2011 17:48:16 -0700 Ram Pai <linuxram@us.ibm.com> wrote: > I assume majority of the platforms will have enough resources to satisfy all > the resource requests, and their BIOS would have done a decent job. > > Even if the BIOS has not done a decent job, and there are enough resources > available we should not see a regression. > > The only platforms that would expose a regression is when resources are under > contention and the BIOS has assigned enough resource to the cardbus bridge but > not to some other device. It will be hard to find such a platform, but I am > sure there is one out somewhere there. > > I am sure we will see; some day, reports of regression because that platform > would have the exact right characteristics to expose the issue. But then that > platform is a highly constrained platform in the first place. Its debatable if > that should be characterised as a regression, or a platform that was riding on > good luck till now. > > Even Oliver's platform is a highly constrained platform, and we probably can > treat his platform as 'riding on good luck till now'. > > We won't be able to satisfy all the platforms with resource constraints. I > think our probable choice is to select a solution that breaks least number of > platforms and special case those broken platforms through kernel command line > parameters. Another option is to hide the new allocation behavior behind a kernel parameter. I know Bjorn has opposed this in the past because really this sort of thing should "just work". But so far it hasn't, and we've had to revert both Bjorn's resource tracking changes as well as the re-allocation code. Hiding it behind a boot option would at least let us improve things over time and potentially switch over to new resource code in the future... Thoughts?
On Thursday, June 23, 2011, Jesse Barnes wrote: > On Tue, 21 Jun 2011 17:48:16 -0700 > Ram Pai <linuxram@us.ibm.com> wrote: > > I assume majority of the platforms will have enough resources to satisfy all > > the resource requests, and their BIOS would have done a decent job. > > > > Even if the BIOS has not done a decent job, and there are enough resources > > available we should not see a regression. > > > > The only platforms that would expose a regression is when resources are under > > contention and the BIOS has assigned enough resource to the cardbus bridge but > > not to some other device. It will be hard to find such a platform, but I am > > sure there is one out somewhere there. > > > > I am sure we will see; some day, reports of regression because that platform > > would have the exact right characteristics to expose the issue. But then that > > platform is a highly constrained platform in the first place. Its debatable if > > that should be characterised as a regression, or a platform that was riding on > > good luck till now. > > > > Even Oliver's platform is a highly constrained platform, and we probably can > > treat his platform as 'riding on good luck till now'. > > > > We won't be able to satisfy all the platforms with resource constraints. I > > think our probable choice is to select a solution that breaks least number of > > platforms and special case those broken platforms through kernel command line > > parameters. > > Another option is to hide the new allocation behavior behind a kernel > parameter. I know Bjorn has opposed this in the past because really > this sort of thing should "just work". But so far it hasn't, and we've > had to revert both Bjorn's resource tracking changes as well as the > re-allocation code. > > Hiding it behind a boot option would at least let us improve things > over time and potentially switch over to new resource code in the > future... > > Thoughts? Do I understand correctly that at the moment we have two set of systems, one of which works with the new code and doesn't work with the old code and the other one conversely? Rafael -- 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
On Thu, Jun 23, 2011 at 10:42:29PM +0200, Rafael J. Wysocki wrote: > On Thursday, June 23, 2011, Jesse Barnes wrote: > > On Tue, 21 Jun 2011 17:48:16 -0700 > > Ram Pai <linuxram@us.ibm.com> wrote: > > > I assume majority of the platforms will have enough resources to satisfy all > > > the resource requests, and their BIOS would have done a decent job. > > > > > > Even if the BIOS has not done a decent job, and there are enough resources > > > available we should not see a regression. > > > > > > The only platforms that would expose a regression is when resources are under > > > contention and the BIOS has assigned enough resource to the cardbus bridge but > > > not to some other device. It will be hard to find such a platform, but I am > > > sure there is one out somewhere there. > > > > > > I am sure we will see; some day, reports of regression because that platform > > > would have the exact right characteristics to expose the issue. But then that > > > platform is a highly constrained platform in the first place. Its debatable if > > > that should be characterised as a regression, or a platform that was riding on > > > good luck till now. > > > > > > Even Oliver's platform is a highly constrained platform, and we probably can > > > treat his platform as 'riding on good luck till now'. > > > > > > We won't be able to satisfy all the platforms with resource constraints. I > > > think our probable choice is to select a solution that breaks least number of > > > platforms and special case those broken platforms through kernel command line > > > parameters. > > > > Another option is to hide the new allocation behavior behind a kernel > > parameter. I know Bjorn has opposed this in the past because really > > this sort of thing should "just work". But so far it hasn't, and we've > > had to revert both Bjorn's resource tracking changes as well as the > > re-allocation code. > > > > Hiding it behind a boot option would at least let us improve things > > over time and potentially switch over to new resource code in the > > future... > > > > Thoughts? > > Do I understand correctly that at the moment we have two set of systems, > one of which works with the new code and doesn't work with the old code > and the other one conversely? Here is the current state: (a) As of 2.6.39, for platforms whose BIOS have not allocated enough resources to its devices, those devices will **continue to not work**. An example of such a platform is the one whose BIOS has not allocated enough resources to SRIOV BARs. (b) With Yinghai's patch the commit "PCI: update bridge resources to get more big ranges when allocating space (again)" http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=da7822e5ad71ec9b745b412639f1e5e0ba795a20 Most of the platforms that were not working in (a) will start working, but will break a few platforms, that have resource constraints and whose BIOS has not allocated enough resources to some of its devices. Oliver's and Ben Hutching's platform are two of the known platforms; as of now. (c) with my patch all the above platforms will start working. But the 4th patch in the series raises a genuine concern that it might break resource-constrained platforms with cardbus bridges. The question is which one of these is a lesser-evil :) Personally I think we should merge all the patches except the 4th patch, and support Oliver's platform through kernel command line parameter. And I think we should revert Yinghai's patch for now and merge it with all other patches in the 3.0.1 timeframe after thorough testing. RP -- 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
On Friday, June 24, 2011, Ram Pai wrote: > On Thu, Jun 23, 2011 at 10:42:29PM +0200, Rafael J. Wysocki wrote: > > On Thursday, June 23, 2011, Jesse Barnes wrote: > > > On Tue, 21 Jun 2011 17:48:16 -0700 > > > Ram Pai <linuxram@us.ibm.com> wrote: > > > > I assume majority of the platforms will have enough resources to satisfy all > > > > the resource requests, and their BIOS would have done a decent job. > > > > > > > > Even if the BIOS has not done a decent job, and there are enough resources > > > > available we should not see a regression. > > > > > > > > The only platforms that would expose a regression is when resources are under > > > > contention and the BIOS has assigned enough resource to the cardbus bridge but > > > > not to some other device. It will be hard to find such a platform, but I am > > > > sure there is one out somewhere there. > > > > > > > > I am sure we will see; some day, reports of regression because that platform > > > > would have the exact right characteristics to expose the issue. But then that > > > > platform is a highly constrained platform in the first place. Its debatable if > > > > that should be characterised as a regression, or a platform that was riding on > > > > good luck till now. > > > > > > > > Even Oliver's platform is a highly constrained platform, and we probably can > > > > treat his platform as 'riding on good luck till now'. > > > > > > > > We won't be able to satisfy all the platforms with resource constraints. I > > > > think our probable choice is to select a solution that breaks least number of > > > > platforms and special case those broken platforms through kernel command line > > > > parameters. > > > > > > Another option is to hide the new allocation behavior behind a kernel > > > parameter. I know Bjorn has opposed this in the past because really > > > this sort of thing should "just work". But so far it hasn't, and we've > > > had to revert both Bjorn's resource tracking changes as well as the > > > re-allocation code. > > > > > > Hiding it behind a boot option would at least let us improve things > > > over time and potentially switch over to new resource code in the > > > future... > > > > > > Thoughts? > > > > Do I understand correctly that at the moment we have two set of systems, > > one of which works with the new code and doesn't work with the old code > > and the other one conversely? > > Here is the current state: > > (a) As of 2.6.39, for platforms whose BIOS have not allocated enough resources to its > devices, those devices will **continue to not work**. An example of such a platform is > the one whose BIOS has not allocated enough resources to SRIOV BARs. > > (b) With Yinghai's patch > the commit "PCI: update bridge resources to get more big ranges when allocating space (again)" > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=da7822e5ad71ec9b745b412639f1e5e0ba795a20 > Most of the platforms that were not working in (a) will start working, but will break a few platforms, that > have resource constraints and whose BIOS has not allocated enough resources to some of its devices. > Oliver's and Ben Hutching's platform are two of the known platforms; as of now. > > (c) with my patch all the above platforms will start working. But the 4th patch in the series > raises a genuine concern that it might break resource-constrained platforms with cardbus bridges. > > The question is which one of these is a lesser-evil :) > > Personally I think we should merge all the patches except the 4th patch, and support > Oliver's platform through kernel command line parameter. And I think we should > revert Yinghai's patch for now and merge it with all other patches in the 3.0.1 > timeframe after thorough testing. Well, I think in that case the default behavior should be as for 2.6.39 and there may be a command line switch turning on some new behavior for whoever needs that. This way we'll avoid introducing regressions on systems that don't use the new command line switch and we'' allow systems that don't work without it to be handled. What the new behavior should be is to be determined I guess. Thanks, Rafael -- 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/setup-bus.c b/drivers/pci/setup-bus.c index 4f8873e..023fc9c 100644 --- a/drivers/pci/setup-bus.c +++ b/drivers/pci/setup-bus.c @@ -742,7 +742,8 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask, return 1; } -static void pci_bus_size_cardbus(struct pci_bus *bus) +static void pci_bus_size_cardbus(struct pci_bus *bus, + struct resource_list_x *add_head) { struct pci_dev *bridge = bus->self; struct resource *b_res = &bridge->resource[PCI_BRIDGE_RESOURCES]; @@ -753,12 +754,14 @@ static void pci_bus_size_cardbus(struct pci_bus *bus) * a fixed amount of bus space for CardBus bridges. */ b_res[0].start = 0; - b_res[0].end = pci_cardbus_io_size - 1; + b_res[0].end = 0; b_res[0].flags |= IORESOURCE_IO | IORESOURCE_SIZEALIGN; + add_to_list(add_head, bridge, b_res, pci_cardbus_io_size - 1, 1); b_res[1].start = 0; - b_res[1].end = pci_cardbus_io_size - 1; + b_res[1].end = 0; b_res[1].flags |= IORESOURCE_IO | IORESOURCE_SIZEALIGN; + add_to_list(add_head, bridge, b_res+1, pci_cardbus_io_size - 1, 1); /* * Check whether prefetchable memory is supported @@ -778,16 +781,19 @@ static void pci_bus_size_cardbus(struct pci_bus *bus) */ if (ctrl & PCI_CB_BRIDGE_CTL_PREFETCH_MEM0) { b_res[2].start = 0; - b_res[2].end = pci_cardbus_mem_size - 1; + b_res[2].end = 0; b_res[2].flags |= IORESOURCE_MEM | IORESOURCE_PREFETCH | IORESOURCE_SIZEALIGN; + add_to_list(add_head, bridge, b_res+2, pci_cardbus_mem_size - 1, 1); b_res[3].start = 0; - b_res[3].end = pci_cardbus_mem_size - 1; + b_res[3].end = 0; b_res[3].flags |= IORESOURCE_MEM | IORESOURCE_SIZEALIGN; + add_to_list(add_head, bridge, b_res+3, pci_cardbus_mem_size - 1, 1); } else { b_res[3].start = 0; - b_res[3].end = pci_cardbus_mem_size * 2 - 1; + b_res[3].end = 0; b_res[3].flags |= IORESOURCE_MEM | IORESOURCE_SIZEALIGN; + add_to_list(add_head, bridge, b_res+3, pci_cardbus_mem_size * 2 - 1, 1); } } @@ -805,7 +811,7 @@ void __ref __pci_bus_size_bridges(struct pci_bus *bus, switch (dev->class >> 8) { case PCI_CLASS_BRIDGE_CARDBUS: - pci_bus_size_cardbus(b); + pci_bus_size_cardbus(b, add_head); break; case PCI_CLASS_BRIDGE_PCI: