Re: [PATCH 2/2]x86: spread tlb flush vector between nodes

Eric Dumazet
2010-10-20T05:16:46Z

Le mercredi 20 octobre 2010 à 11:07 +0800, Shaohua Li a écrit :
> Currently flush tlb vector allocation is based on below equation:
> sender = smp_processor_id() % 8
> This isn't optimal, CPUs from different node can have the same vector, this
> causes a lot of lock contention. Instead, we can assign the same vectors to
> CPUs from the same node, while different node has different vectors. This has
> below advantages:
> a. if there is lock contention, the lock contention is between CPUs from one
> node. This should be much cheaper than the contention between nodes.
> b. completely avoid lock contention between nodes. This especially benefits
> kswapd, which is the biggest user of tlb flush, since kswapd sets its affinity
> to specific node.
>
> In my test, this could reduce > 20% CPU overhead in extreme case.The test
> machine has 4 nodes and each node has 16 CPUs. I then bind each node's kswapd
> to the first CPU of the node. I run a workload with 4 sequential mmap file
> read thread. The files are empty sparse file. This workload will trigger a
> lot of page reclaim and tlbflush. The kswapd bind is to easy trigger the
> extreme tlb flush lock contention because otherwise kswapd keeps migrating
> between CPUs of a node and I can't get stable result. Sure in real workload,
> we can't always see so big tlb flush lock contention, but it's possible.
>
> Signed-off-by: Shaohua Li <shaohua.li@intel.com>
> ---
> arch/x86/mm/tlb.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 47 insertions(+), 1 deletion(-)
>
> Index: linux/arch/x86/mm/tlb.c
> ===================================================================
> --- linux.orig/arch/x86/mm/tlb.c 2010-10-20 10:07:53.000000000 +0800
> +++ linux/arch/x86/mm/tlb.c 2010-10-20 10:09:26.000000000 +0800
> @@ -5,6 +5,7 @@
> #include <linux/smp.h>
> #include <linux/interrupt.h>
> #include <linux/module.h>
> +#include <linux/cpu.h>
>
> #include <asm/tlbflush.h>
> #include <asm/mmu_context.h>
> @@ -52,6 +53,8 @@ union smp_flush_state {
> want false sharing in the per cpu data segment. */
> static union smp_flush_state flush_state[NUM_INVALIDATE_TLB_VECTORS];
>
> +static DEFINE_PER_CPU_READ_MOSTLY(int, tlb_vector_offset);
> +
> /*
> * We cannot call mmdrop() because we are in interrupt context,
> * instead update mm->cpu_vm_mask.
> @@ -173,7 +176,7 @@ static void flush_tlb_others_ipi(const s
> union smp_flush_state *f;
>
> /* Caller has disabled preemption */
> - sender = smp_processor_id() % NUM_INVALIDATE_TLB_VECTORS;
> + sender = per_cpu(tlb_vector_offset, smp_processor_id());

sender = this_cpu_read(tlb_vector_offset);

> f = &flush_state[sender];
>
> /*
> @@ -218,6 +221,47 @@ void native_flush_tlb_others(const struc
> flush_tlb_others_ipi(cpumask, mm, va);
> }
>

Thats a pretty good patch, thanks !

Maybe we should have a per_node memory infrastructure, so that we can
lower memory needs of currently per_cpu objects.