Bug 189951
Summary: | Enabling ASPM causes NIC performance issue | ||
---|---|---|---|
Product: | Drivers | Reporter: | Mathias Koehrer (mathias.koehrer) |
Component: | PCI | Assignee: | drivers_pci (drivers_pci) |
Status: | NEEDINFO --- | ||
Severity: | normal | CC: | bjorn, bugzilla.pci |
Priority: | P1 | ||
Hardware: | Intel | ||
OS: | Linux | ||
Kernel Version: | 4.1, 4.4, 4.8, 4.9 | Subsystem: | |
Regression: | Yes | Bisected commit-id: | |
Attachments: |
Output of lspci for 3.18 and 4.8
Output of dmesg for 4.8.13 v4.8.13 dmesg log (uncompressed) v4.8.13 lspci (uncompressed) v3.18.42 lspci (uncompressed) allow drivers to disable ASPM v4.9.4 lspci -vvv output v4.9.4 dmesg output v4.9.4-rt2: Output of dmesg with PCIe power management disabled in BIOS v4.9.4-rt2: Output of lspci -vvv with PCIe power management disabled in BIOS |
Description
Mathias Koehrer
2016-12-09 09:41:19 UTC
Created attachment 247301 [details]
Output of dmesg for 4.8.13
Here is the output of dmesg for the kernel 4.8.13
Created attachment 247311 [details]
v4.8.13 dmesg log (uncompressed)
Created attachment 247321 [details]
v4.8.13 lspci (uncompressed)
Created attachment 247331 [details]
v3.18.42 lspci (uncompressed)
Marking this as a regression since v4.0 worked fine and the performance issue appeared in v4.1. Mathias bisected this to 387d37577fdd ("PCI: Don't clear ASPM bits when the FADT declares it's unsupported"). Proposed patch at https://patchwork.ozlabs.org/patch/690339/ Created attachment 251601 [details]
allow drivers to disable ASPM
The BIOS claims the platform doesn't support ASPM. Linux currently interprets that to mean that it shouldn't touch the ASPM configuration at all, even though e1000e tries to disable ASPM for its device:
ACPI FADT declares the system doesn't support PCIe ASPM, so disable it
e1000e 0000:03:00.0: Disabling ASPM L0s L1
e1000e 0000:03:00.0: can't disable ASPM; OS doesn't have ASPM control
This is a patch to allow drivers to disable ASPM. I'd want to audit the rest of the ASPM code to make sure we've set up enough to make this safe, but it's possible this is all we need to do.
Mathias, is there any chance you can test the patch from comment #6? If that resolves the problem, I will try to merge it for v4.10. Created attachment 253101 [details]
v4.9.4 lspci -vvv output
Created attachment 253111 [details]
v4.9.4 dmesg output
(In reply to Bjorn Helgaas from comment #7) > Mathias, is there any chance you can test the patch from comment #6? If > that resolves the problem, I will try to merge it for v4.10. I have applied the patch to kernel 4.9.4-rt2. There is no difference in the output of "lspci -vvv" with or without the applied patch. The state of the ASPM on the Intel IGB based devices (04:00.0, 04:00.1) is in both cases: 04:00.0 Ethernet controller: Intel Corporation I350 Gigabit Network Connection (rev 01) Capabilities: [a0] Express (v2) Endpoint, MSI 00 ... LnkCtl: ASPM L1 Enabled; RCB 64 bytes Disabled- CommClk+ ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt- Even if the NICs have been configured to be in use, the ASPM is still enabled. Please see the attachments 253101 and 253111 for details. They reflect the situation after having applied the patch. I'm very sorry; I wasn't paying enough attention. I was focused on the e1000e device because we complained about "e1000e 0000:03:00.0: can't disable ASPM; OS doesn't have ASPM control", but that's a red herring. The problem is actually with the igb devices at 04:00.0 and .1. Your system advertises ACPI_FADT_NO_ASPM. Prior to 387d37577fdd ("PCI: Don't clear ASPM bits when the FADT declares it's unsupported"), we actively disabled ASPM on all PCIe links, including the link to 04:00.x. After 387d37577fdd, we leave ASPM alone, so if the BIOS enabled it, it will remain enabled. My patch in comment #6 doesn't make any difference because igb doesn't call pci_disable_link_state(). So let me start over. When you enable the BIOS energy saving feature, BIOS enables ASPM on the 04:00.x link and advertises ACPI_FADT_NO_ASPM, which we think means "don't touch it". What happens if you turn off the BIOS energy saving feature? Does your system still advertise ACPI_FADT_NO_ASPM, i.e., does the dmesg log still contain "ACPI FADT declares the system doesn't support PCIe ASPM, so disable it"? Created attachment 253161 [details]
v4.9.4-rt2: Output of dmesg with PCIe power management disabled in BIOS
v4.9.4-rt2: Output of dmesg with PCEe power management disabled in BIOS.
The patch is still applied
Created attachment 253171 [details]
v4.9.4-rt2: Output of lspci -vvv with PCIe power management disabled in BIOS
I have disabled the PCI Express Power Management in BIOS. The patch from comment #6 is still applied. The state of the ASPM on the Intel IGB based devices (04:00.0, 04:00.1) is now: 04:00.0 Ethernet controller: Intel Corporation I350 Gigabit Network Connection (rev 01) Capabilities: [a0] Express (v2) Endpoint, MSI 00 ... LnkCtl: ASPM Disabled; RCB 64 bytes Disabled- CommClk+ ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt- dmesg | grep -i aspm returns [ 0.561622] ACPI FADT declares the system doesn't support PCIe ASPM, so disable it [ 0.579437] acpi PNP0A08:00: _OSC: OS supports [ExtendedConfig ASPM ClockPM Segments MSI] [ 0.579671] acpi PNP0A08:00: FADT indicates ASPM is unsupported, using BIOS configuration Please find the full output of "lspci -vvv" in attachment #253171 [details] (comment #13), the output of dmesg in this case can be found in attachment #253161 [details] (comment #12). BIOS energy saving feature enabled: - BIOS enables ASPM - BIOS advertises ACPI_FADT_NO_ASPM - igb performance is reduced BIOS energy saving feature disabled: - BIOS disables ASPM - BIOS advertises ACPI_FADT_NO_ASPM - igb full performance Per 387d37577fdd, our current understanding is that when ACPI_FADT_NO_ASPM is set, the OS should not touch the ASPM configuration. I wish we had a clearer statement in the spec about that, but with the information we have now, I think it's a reasonable interpretation. I think there's some tension between the OS and the "platform", i.e., BIOS and other non-OS management entities. The platform folks want to provide management features like datacenter-scale power management, without requiring OS support. Of course Linux has no knowledge of anything like that. I suspect that this may be a case of the platform deciding that it wants to be in control of power management, and ACPI_FADT_NO_ASPM is its way of keeping the OS's mitts off it. I don't know a good way around it -- we can't change what the BIOS does, so if the BIOS decides it wants to control a feature, it's likely to cause conflicts if the OS also controls it. However, I think we *can* improve our documentation, e.g., Kconfig help and dmesg logging. It's certainly the case that enabling ASPM has some performance impact -- if it didn't, we would *always* enable it. So maybe we should hint at that in the PCI core dmesg logging, and possibly even in performance-sensitive drivers, e.g., maybe igb should notice if ASPM is enabled and warn that its performance will be affected. And the Kconfig help should probably acknowledge the possibility that even if we enable PCIEASPM and PCIEASPM_PERFORMANCE, the platform may still prevent us from using the highest-performance settings. What about introducing another kernel configuration option PCIEASPM_FORCE_PERFORMANCE which may be used to disable all ASPM features as good as possible? Even in cases where the BIOS has enabled them before. This should restore the behaviour as it was in earlier kernel version before 387d37577fdd ("PCI: Don't clear ASPM bits when the FADT declares it's unsupported"). |