Re: [PATCH 1/4] pci: Allow lockless access path to PCI mmconfig

From: H. Peter Anvin
Date: Tue Mar 14 2017 - 13:29:21 EST


On 03/02/17 15:21, Andi Kleen wrote:
> From: Andi Kleen <ak@xxxxxxxxxxxxxxx>
>
> The Intel uncore driver can do a lot of PCI config accesses to read
> performance counters. I had a situation on a 4S system where it
> was spending 40+% of CPU time grabbing the pci_cfg_lock due to that.
>
> For 64bit x86 with MMCONFIG there isn't really any reason to take
> a lock. The access is directly mapped to an underlying MMIO area,
> which can fully operate lockless.
>
> Add a new flag that allows the PCI mid layer to skip the lock
> and set it for the 64bit mmconfig code.
>
> There's a small risk that someone relies on this lock for synchronization,
> but I think that's unlikely because there isn't really any useful
> synchronization at this individual operation level. Any useful
> synchronization would likely need to protect at least a
> read-modify-write or similar. So I made it unconditional without opt-in.
>
> Signed-off-by: Andi Kleen <ak@xxxxxxxxxxxxxxx>
> ---
> arch/x86/pci/mmconfig_64.c | 1 +
> drivers/pci/access.c | 14 ++++++++++----
> include/linux/pci.h | 2 ++
> 3 files changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/pci/mmconfig_64.c b/arch/x86/pci/mmconfig_64.c
> index bea52496aea6..8bf10f41e626 100644
> --- a/arch/x86/pci/mmconfig_64.c
> +++ b/arch/x86/pci/mmconfig_64.c
> @@ -121,6 +121,7 @@ int __init pci_mmcfg_arch_init(void)
> }
>
> raw_pci_ext_ops = &pci_mmcfg;
> + pci_root_ops.ll_allowed = true;
>

"ll_allowed" is pretty awful naming... you spend almost all the
characters telling us nothing. I spend several seconds trying to figure
out what "ll" stood for, and without the context of the patch I'd have
had to go a massive grep. Just call it "lockless" or something.

-hpa