All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] Use maximum latency when determining L1 ASPM
@ 2020-07-29 22:27 Bjorn Helgaas
  2020-08-03 14:58 ` [PATCH] Use maximum latency when determining L1/L0s ASPM v2 Ian Kumlien
  0 siblings, 1 reply; 33+ messages in thread
From: Bjorn Helgaas @ 2020-07-29 22:27 UTC (permalink / raw)
  To: Ian Kumlien; +Cc: linux-pci, Kai-Heng Feng, Mika Westerberg

On Mon, Jul 27, 2020 at 11:30:45PM +0200, Ian Kumlien wrote:
> Currently we check the maximum latency of upstream and downstream
> per link, not the maximum for the path
> 
> This would work if all links have the same latency, but:
> endpoint -> c -> b -> a -> root  (in the order we walk the path)
> 
> If c or b has the higest latency, it will not register
> 
> Fix this by maintaining the maximum latency value for the path
> 
> This change fixes a regression introduced by:
> 66ff14e59e8a (PCI/ASPM: Allow ASPM on links to PCIe-to-PCI/PCI-X Bridges)

Hi Ian,

Sorry about the regression, and thank you very much for doing the
hard work of debugging and fixing it!

My guess is that 66ff14e59e8a isn't itself buggy, but it allowed ASPM
to be enabled on a longer path, and we weren't computing the maximum
latency correctly, so ASPM on that longer path exceeded the amount we
could tolerate.  If that's the case, 66ff14e59e8a probably just
exposed an existing problem that could occur in other topologies even
without 66ff14e59e8a.

I'd like to work through this latency code with concrete examples.
Can you collect the "sudo lspci -vv" output and attach it to an entry
at https://bugzilla.kernel.org?  If it's convenient, it would be
really nice to compare it with similar output from before this patch.

Bjorn

> Signed-off-by: Ian Kumlien <ian.kumlien@gmail.com>
> ---
>  drivers/pci/pcie/aspm.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
> index b17e5ffd31b1..bd53fba7f382 100644
> --- a/drivers/pci/pcie/aspm.c
> +++ b/drivers/pci/pcie/aspm.c
> @@ -434,7 +434,7 @@ static void pcie_get_aspm_reg(struct pci_dev *pdev,
>  
>  static void pcie_aspm_check_latency(struct pci_dev *endpoint)
>  {
> -	u32 latency, l1_switch_latency = 0;
> +	u32 latency, l1_max_latency = 0, l1_switch_latency = 0;
>  	struct aspm_latency *acceptable;
>  	struct pcie_link_state *link;
>  
> @@ -470,8 +470,9 @@ static void pcie_aspm_check_latency(struct pci_dev *endpoint)
>  		 * substate latencies (and hence do not do any check).
>  		 */
>  		latency = max_t(u32, link->latency_up.l1, link->latency_dw.l1);
> +		l1_max_latency = max_t(u32, latency, l1_max_latency);
>  		if ((link->aspm_capable & ASPM_STATE_L1) &&
> -		    (latency + l1_switch_latency > acceptable->l1))
> +		    (l1_max_latency + l1_switch_latency > acceptable->l1))
>  			link->aspm_capable &= ~ASPM_STATE_L1;
>  		l1_switch_latency += 1000;
>  
> -- 
> 2.27.0
> 

^ permalink raw reply	[flat|nested] 33+ messages in thread
end of thread, other threads:[~2020-10-07 13:03 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CAA85sZvrPApeAYPVSYdVuKnp84xCpLBLf+f32e=R9tdPC0dvOw@mail.gmail.com>
2020-09-25 15:49 ` [PATCH] Use maximum latency when determining L1/L0s ASPM v2 Bjorn Helgaas
2020-09-25 22:26   ` Ian Kumlien
2020-09-28  0:06     ` Bjorn Helgaas
2020-09-28 10:24       ` Ian Kumlien
2020-09-28 17:09         ` Bjorn Helgaas
2020-09-28 17:41           ` Ian Kumlien
2020-09-28 19:53             ` Alexander Duyck
2020-09-28 20:04               ` Ian Kumlien
2020-09-28 20:33                 ` Ian Kumlien
2020-09-28 23:30                   ` Alexander Duyck
2020-09-29 12:51                     ` Ian Kumlien
2020-09-29 16:23                       ` Alexander Duyck
2020-09-29 21:13                         ` Ian Kumlien
2020-09-28 21:43               ` Ian Kumlien
2020-09-28 18:10           ` Alexander Duyck
2020-07-29 22:27 [PATCH] Use maximum latency when determining L1 ASPM Bjorn Helgaas
2020-08-03 14:58 ` [PATCH] Use maximum latency when determining L1/L0s ASPM v2 Ian Kumlien
2020-08-15 19:39   ` Ian Kumlien
2020-09-18 22:47   ` Ian Kumlien
2020-09-22 20:19   ` Bjorn Helgaas
2020-09-22 21:02     ` Ian Kumlien
2020-09-22 23:00       ` Bjorn Helgaas
2020-09-22 23:29         ` Ian Kumlien
2020-09-22 23:31           ` Ian Kumlien
2020-09-23 21:23           ` Bjorn Helgaas
2020-09-23 21:36             ` Ian Kumlien
2020-09-23 21:48               ` Ian Kumlien
2020-09-24 16:24               ` Bjorn Helgaas
2020-09-25  8:06                 ` Ian Kumlien
2020-10-05 18:31   ` Bjorn Helgaas
2020-10-05 18:38     ` Ian Kumlien
2020-10-05 19:09       ` Bjorn Helgaas
2020-10-07 11:31         ` Ian Kumlien
2020-10-07 13:03           ` Bjorn Helgaas

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.