Skip to contentSkip to conversation

PCI fixes

GIT PULL Jesse Barnes 4 participants 10 messages Jun 23, 2011 – Jun 29, 2011

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

The following changes since commit
af0d6a0a3a30946f7df69c764791f1b0643f7cd6:

Merge branch 'x86-urgent-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip (2011-06-01 02:07:22 +0900)

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/jbarnes/pci-2.6 for-linus

Just a few minor fixes. We have one open issue with the resource re-allocation code that was re-added. Ram has a fix but I'm concerned it will introduce other regressions. So at this point we have a few options:
- revert the bridge resource re-allocation code (again)
- hide the re-allocation code behind a new kernel option (and not try=!), this would at least let us experiment with new strategies and PNP resource tracking etc
- apply Ram's patch and revert it all later if things are still broken

The last option gets us the most coverage, but it's a bit late in the cycle to be experimenting like this...

David S. Miller (1):
PCI: Fix warning in drivers/pci/probe.c on sparc64

Márton Németh (1):
x86/PCI/ACPI: fix type mismatch

Randy Dunlap (1):
PCI: fix new kernel-doc warning

 arch/x86/pci/acpi.c |    2 +-
 drivers/pci/pci.c   |    2 +-
 drivers/pci/probe.c |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

Thanks,

Signature · 1 line
Jesse Barnes, Intel Open Source Technology Center
#1 lore raw
Quoted 10 lines · Jesse Barnes

Just a few minor fixes.  We have one open issue with the resource re-allocation code that was re-added.  Ram has a fix but I'm concerned it will introduce other regressions.  So at this point we have a few options:
 - revert the bridge resource re-allocation code (again)
 - hide the re-allocation code behind a new kernel option (and not try=!), this would at least let us experiment with new strategies and PNP resource tracking etc
 - apply Ram's patch and revert it all later if things are still broken

I think I've missed the regression discussion, so can you forward a description and the fix?

I have to admit that I'm inclined to try the fix if it's clean, just in the hope to get over this finally. But I'd like to see what the problem/fix is...

Maybe I've seen it and just didn't react.

Linus

#2 lore raw
Quoted 12 lines · On Fri, 24 Jun 2011 08:38:07 -0700 Linus Torvalds…
Quoted 10 lines · Jesse Barnes

Just a few minor fixes.  We have one open issue with the resource re-allocation code that was re-added.  Ram has a fix but I'm concerned it will introduce other regressions.  So at this point we have a few options:
 - revert the bridge resource re-allocation code (again)
 - hide the re-allocation code behind a new kernel option (and not try=!), this would at least let us experiment with new strategies and PNP resource tracking etc
 - apply Ram's patch and revert it all later if things are still broken

Quoted 8 lines

I think I've missed the regression discussion, so can you forward a description and the fix?

I have to admit that I'm inclined to try the fix if it's clean, just in the hope to get over this finally. But I'd like to see what the problem/fix is...

Maybe I've seen it and just didn't react.

Some discussion was under "PCI: fix cardbus and sriov regressions", which includes a patchset as well. Oliver Hartkopp's problem is the one that worries me most. The SR-IOV issue is really a BIOS bug and unlikely to affect many people anyway (SR-IOV is really a big server feature).

Signature · 1 line
Jesse Barnes, Intel Open Source Technology Center
#3 lore raw
Quoted 6 lines · Jesse Barnes

Some discussion was under "PCI: fix cardbus and sriov regressions", which includes a patchset as well. Oliver Hartkopp's problem is the one that worries me most. The SR-IOV issue is really a BIOS bug and unlikely to affect many people anyway (SR-IOV is really a big server feature).

Ouch. That is much bigger than I was hoping for. Especially that second patch doesn't look like some -rc5 candidate.

And the fact that Oliver then at the end shows an impossibly aligned resource shows that it clearly isn't even remotely correct even AFTER the patches, so that doesn't make me get all warm and fuzzy either.

So I guess we should just revert again.

Linus

#4 lore raw
Quoted 8 lines · On Fri, 24 Jun 2011 09:07:46 -0700 Linus Torvalds…
Quoted 6 lines · Jesse Barnes

Some discussion was under "PCI: fix cardbus and sriov regressions", which includes a patchset as well. Oliver Hartkopp's problem is the one that worries me most. The SR-IOV issue is really a BIOS bug and unlikely to affect many people anyway (SR-IOV is really a big server feature).

Quoted 8 lines

Ouch. That is much bigger than I was hoping for. Especially that second patch doesn't look like some -rc5 candidate.

And the fact that Oliver then at the end shows an impossibly aligned resource shows that it clearly isn't even remotely correct even AFTER the patches, so that doesn't make me get all warm and fuzzy either.

So I guess we should just revert again.

How about a boot param? We've done similar things for _CRS, and it might give Yinghai and Ram some more flexibility in improving our dynamic resource allocation before enabling it again (if ever).

Thanks,

Signature · 1 line
Jesse Barnes, Intel Open Source Technology Center
#5 lore raw
Quoted 4 lines · Jesse Barnes

How about a boot param? We've done similar things for _CRS, and it might give Yinghai and Ram some more flexibility in improving our dynamic resource allocation before enabling it again (if ever).

I'm OK with that, but then I'd suggest against the currently not yet merged series that doesn't seem to work anyway...

IOW, just a simple patch that disables the resource re-allocation, and enables it with some command line option.

Linus

#6 lore raw
Quoted 5 lines · On Fri, 24 Jun 2011 15:56:42 -0700 Linus Torvalds…
Quoted 1 line · Jesse Barnes
Quoted 7 lines
Quoted 3 lines

How about a boot param? We've done similar things for _CRS, and it might give Yinghai and Ram some more flexibility in improving our dynamic resource allocation before enabling it again (if ever).

I'm OK with that, but then I'd suggest against the currently not yet merged series that doesn't seem to work anyway...

IOW, just a simple patch that disables the resource re-allocation, and enables it with some command line option.

Ok sounds good, thanks.

Signature · 1 line
Jesse Barnes, Intel Open Source Technology Center
#7 lore raw
Quoted 12 lines · Jesse Barnes
Quoted 9 lines · On Fri, 24 Jun 2011 15:56:42 -0700 Linus Torvalds…
Quoted 4 lines · Jesse Barnes

How about a boot param? We've done similar things for _CRS, and it might give Yinghai and Ram some more flexibility in improving our dynamic resource allocation before enabling it again (if ever).

Quoted 5 lines
Quoted 3 lines

I'm OK with that, but then I'd suggest against the currently not yet merged series that doesn't seem to work anyway...

IOW, just a simple patch that disables the resource re-allocation, and enables it with some command line option.

Ok sounds good, thanks.

please check the draft one, not sure if you like
pci=try=2
or
pci=reallocate_bridge

[PATCH] pci: disable pci trying to reallocate pci bridge by default.

| PCI: update bridge resources to get more big ranges when allocating space (again)
| da7822e5ad71ec9b745b412639f1e5e0ba795a20

still too risky and cause some systems have problem.

Disable reallocating feature, So other user could use command line enable it to enable it with "pci=try=2"

---
 Documentation/kernel-parameters.txt |    6 ++++++
 drivers/pci/pci.c                   |    4 ++++
 drivers/pci/pci.h                   |    2 ++
 drivers/pci/setup-bus.c             |   15 +++++++++------
 4 files changed, 21 insertions(+), 6 deletions(-)
Documentation/kernel-parameters.txt +6 -0
Index: linux-2.6/Documentation/kernel-parameters.txt
===================================================================
@@ -1980,6 +1980,12 @@ bytes respectively. Such letter suffixes
 				for broken drivers that don't call it.
 		skip_isa_align	[X86] do not align io start addr, so can
 				handle more pci cards
+		try=n		set the pci_try_num to reallocate the pci bridge resource
+				1: default
+				2: will set the num to max_depth, and try to reallocate res
+				   to get big range for the bridge. assume the pci peer root
+				   resource is right from _CRS or from hostbridge pci reg
+				   reading out.
 		firmware	[ARM] Do not re-enumerate the bus but instead
 				just use the configuration from the
 				bootloader. This is currently used on
drivers/pci/pci.c +4 -0
Index: linux-2.6/drivers/pci/pci.c
===================================================================
@@ -3485,6 +3485,10 @@ static int __init pci_setup(char *str)
 				pci_no_aer();
 			} else if (!strcmp(str, "nodomains")) {
 				pci_no_domains();
+			} else if (!strncmp(str, "try=", 4)) {
+				int try_num = memparse(str + 4, &str);
+				if (try_num > 0)
+					pci_try_num = try_num;
 			} else if (!strncmp(str, "cbiosize=", 9)) {
 				pci_cardbus_io_size = memparse(str + 9, &str);
 			} else if (!strncmp(str, "cbmemsize=", 10)) {
drivers/pci/pci.h +2 -0
Index: linux-2.6/drivers/pci/pci.h
===================================================================
@@ -223,6 +223,8 @@ static inline int pci_ari_enabled(struct
 	return bus->self && bus->self->ari_enabled;
 }
 
+extern int pci_try_num;
+
 #ifdef CONFIG_PCI_QUIRKS
 extern int pci_is_reassigndev(struct pci_dev *dev);
 resource_size_t pci_specified_resource_alignment(struct pci_dev *dev);
drivers/pci/setup-bus.c +9 -6
Index: linux-2.6/drivers/pci/setup-bus.c
===================================================================
@@ -1030,6 +1030,7 @@ static int __init pci_get_max_depth(void
  * second  and later try will clear small leaf bridge res
  * will stop till to the max  deepth if can not find good one
  */
+int pci_try_num = 1;
 void __init
 pci_assign_unassigned_resources(void)
 {
@@ -1042,16 +1043,18 @@ pci_assign_unassigned_resources(void)
 	unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM |
 				  IORESOURCE_PREFETCH;
 	unsigned long failed_type;
-	int max_depth = pci_get_max_depth();
-	int pci_try_num;
-
 
 	head.next = NULL;
 	add_list.next = NULL;
 
-	pci_try_num = max_depth + 1;
-	printk(KERN_DEBUG "PCI: max bus depth: %d pci_try_num: %d\n",
-		 max_depth, pci_try_num);
+	if (pci_try_num > 1) {
+		int max_depth = pci_get_max_depth();
+
+		if (max_depth + 1 > pci_try_num)
+			pci_try_num = max_depth + 1;
+		printk(KERN_DEBUG "PCI: max bus depth: %d pci_try_num: %d\n",
+			max_depth, pci_try_num);
+	}
 
 again:
 	/* Depth first, calculate sizes and alignments of all
#8 lore raw
In reply to Linus Torvalds
Quoted 5 lines · On Fri, 24 Jun 2011 15:56:42 -0700 Linus Torvalds…
Quoted 1 line · Jesse Barnes
Quoted 7 lines
Quoted 3 lines

How about a boot param? We've done similar things for _CRS, and it might give Yinghai and Ram some more flexibility in improving our dynamic resource allocation before enabling it again (if ever).

I'm OK with that, but then I'd suggest against the currently not yet merged series that doesn't seem to work anyway...

IOW, just a simple patch that disables the resource re-allocation, and enables it with some command line option.

Ram or Yinghai, want to come up with something? (And
note: pci=bridge_realloc or something would be a lot more descriptive than pci=try=2!)

Thanks,

Signature · 1 line
Jesse Barnes, Intel Open Source Technology Center
#9 lore raw
Quoted 12 lines · Jesse Barnes
Quoted 9 lines · On Fri, 24 Jun 2011 15:56:42 -0700 Linus Torvalds…
Quoted 4 lines · Jesse Barnes

How about a boot param? We've done similar things for _CRS, and it might give Yinghai and Ram some more flexibility in improving our dynamic resource allocation before enabling it again (if ever).

Quoted 7 lines
Quoted 3 lines

I'm OK with that, but then I'd suggest against the currently not yet merged series that doesn't seem to work anyway...

IOW, just a simple patch that disables the resource re-allocation, and enables it with some command line option.

Ram or Yinghai, want to come up with something? (And
note: pci=bridge_realloc or something would be a lot more descriptive than pci=try=2!)

Jesse,

Will have something your way soon.

RP