Re: [PATCH 2/3] pci: Clamp pcie_set_readrq() when using "performance" settings

Bjorn Helgaas
2011-10-04T15:57:06Z

On Tue, Oct 4, 2011 at 9:48 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Tue, Oct 4, 2011 at 8:40 AM, Benjamin Herrenschmidt
> <benh@kernel.crashing.org> wrote:
>>
>> Hopefully most of these patches only affect the "performance" setting
>> which is no longer the default.
>>
>> But yes, more reviews are always welcome.
>
> Well, bcrl argues that patches 1-2 of 3 are actively wrong.
>
> Quite frankly, my gut feel is that this late in the game, the only
> patch I should apply is 3/3, and even that I wonder about. But that
> seems to *really* disable all the games we do, and that all seem to be
> questionable.

The current state (with none of these three applied) is that we do
change MPS configuration in some cases and there is an unsolved
regression (Avi's e1000e problem) and bcrl's concern.

In my opinion, we need to turn off all MPS fiddling by default for rc9
with something like the following. This is part of 3/3, which I think
should have been split into two pieces -- this one and the "also add"
part.

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index d369316..5db5986 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -77,7 +77,7 @@ unsigned long pci_cardbus_mem_size = DEFAULT_CARDBUS_MEM_SIZE;
unsigned long pci_hotplug_io_size = DEFAULT_HOTPLUG_IO_SIZE;
unsigned long pci_hotplug_mem_size = DEFAULT_HOTPLUG_MEM_SIZE;

-enum pcie_bus_config_types pcie_bus_config = PCIE_BUS_SAFE;
+enum pcie_bus_config_types pcie_bus_config = PCIE_BUS_TUNE_OFF;

/*
* The default CLS is used if arch didn't set CLS explicitly and not
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index a919db2..4829424 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1451,12 +1451,24 @@ static int pcie_bus_configure_set(struct
pci_dev *dev, void *data)
*/
void pcie_bus_configure_settings(struct pci_bus *bus, u8 mpss)
{
if (!pci_is_pcie(bus->self))
return;

+ if (pcie_bus_config == PCIE_BUS_TUNE_OFF)
+ return;