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

[H. Peter Anvin]

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