2012-05-23 14:16:53

by Alex Shi

[permalink] [raw]
Subject: [PATCH v7 0/8] x86 tlb optimisations

This version remove the 'x86/tlb: unify TLB_FLUSH_ALL definition', since
it was picked up by Ingo.
And combined with the 7th/8th patches. that were send out days ago.

Any comments are appreciated!

Thanks!
Alex Shi
[PATCH v7 1/8] x86/tlb_info: get last level TLB entry number of CPU
[PATCH v7 2/8] x86/flush_tlb: try flush_tlb_single one by one in
[PATCH v7 3/8] x86/tlb: fall back to flush all when meet a THP large
[PATCH v7 4/8] x86/tlb: add tlb_flushall_shift for specific CPU
[PATCH v7 5/8] x86/tlb: enable tlb flush range support for generic
[PATCH v7 6/8] x86/tlb: add tlb_flushall_shift knob into debugfs
[PATCH v7 7/8] x86/tlb: replace INVALIDATE_TLB_VECTOR by
[PATCH v7 8/8] x86/tlb: just do tlb flush on one of siblings of SMT


2012-05-23 14:17:04

by Alex Shi

[permalink] [raw]
Subject: [PATCH v7 1/8] x86/tlb_info: get last level TLB entry number of CPU

For 4KB pages, x86 CPU has 2 or 1 level TLB, first level is data TLB and
instruction TLB, second level is shared TLB for both data and instructions.

For hupe page TLB, usually there is just one level and seperated by 2MB/4MB
and 1GB.

Although each levels TLB size is important for performance tuning, but for
genernal and rude optimizing, last level TLB entry number is suitable. And
in fact, last level TLB always has the biggest entry number.

This patch will get the biggest TLB entry number and use it in furture TLB
optimizing.

Accroding Borislav's suggestion, except tlb_ll[i/d]_* array, other
function and data will be released after system boot up.

For all kinds of x86 vendor friendly, vendor specific code was moved to its
specific files.

Signed-off-by: Alex Shi <[email protected]>
---
arch/x86/include/asm/processor.h | 11 +++
arch/x86/kernel/cpu/common.c | 21 ++++++
arch/x86/kernel/cpu/cpu.h | 9 +++
arch/x86/kernel/cpu/intel.c | 141 ++++++++++++++++++++++++++++++++++++++
4 files changed, 182 insertions(+), 0 deletions(-)

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 4fa7dcc..797faca 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -61,6 +61,17 @@ static inline void *current_text_addr(void)
# define ARCH_MIN_MMSTRUCT_ALIGN 0
#endif

+enum tlb_infos {
+ ENTRIES,
+ NR_INFO
+};
+
+extern u16 __read_mostly tlb_lli_4k[NR_INFO];
+extern u16 __read_mostly tlb_lli_2m[NR_INFO];
+extern u16 __read_mostly tlb_lli_4m[NR_INFO];
+extern u16 __read_mostly tlb_lld_4k[NR_INFO];
+extern u16 __read_mostly tlb_lld_2m[NR_INFO];
+extern u16 __read_mostly tlb_lld_4m[NR_INFO];
/*
* CPU type and hardware bug flags. Kept separately for each CPU.
* Members of this structure are referenced in head.S, so think twice
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index cf79302..0152082 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -452,6 +452,25 @@ void __cpuinit cpu_detect_cache_sizes(struct cpuinfo_x86 *c)
c->x86_cache_size = l2size;
}

+u16 __read_mostly tlb_lli_4k[NR_INFO];
+u16 __read_mostly tlb_lli_2m[NR_INFO];
+u16 __read_mostly tlb_lli_4m[NR_INFO];
+u16 __read_mostly tlb_lld_4k[NR_INFO];
+u16 __read_mostly tlb_lld_2m[NR_INFO];
+u16 __read_mostly tlb_lld_4m[NR_INFO];
+
+void __cpuinit cpu_detect_tlb(struct cpuinfo_x86 *c)
+{
+ if (c->x86_vendor == X86_VENDOR_INTEL)
+ intel_cpu_detect_tlb(c);
+
+ printk(KERN_INFO "Last level iTLB entries: 4KB %d, 2MB %d, 4MB %d\n" \
+ "Last level dTLB entries: 4KB %d, 2MB %d, 4MB %d\n",
+ tlb_lli_4k[ENTRIES], tlb_lli_2m[ENTRIES],
+ tlb_lli_4m[ENTRIES], tlb_lld_4k[ENTRIES],
+ tlb_lld_2m[ENTRIES], tlb_lld_4m[ENTRIES]);
+}
+
void __cpuinit detect_ht(struct cpuinfo_x86 *c)
{
#ifdef CONFIG_X86_HT
@@ -911,6 +930,8 @@ void __init identify_boot_cpu(void)
#else
vgetcpu_set_mode();
#endif
+ if (boot_cpu_data.cpuid_level >= 2)
+ cpu_detect_tlb(&boot_cpu_data);
}

void __cpuinit identify_secondary_cpu(struct cpuinfo_x86 *c)
diff --git a/arch/x86/kernel/cpu/cpu.h b/arch/x86/kernel/cpu/cpu.h
index 8bacc78..78db1d9 100644
--- a/arch/x86/kernel/cpu/cpu.h
+++ b/arch/x86/kernel/cpu/cpu.h
@@ -24,6 +24,14 @@ struct cpu_dev {
int c_x86_vendor;
};

+struct _tlb_table {
+ unsigned char descriptor;
+ char tlb_type;
+ unsigned int entries;
+ /* unsigned int ways; */
+ char info[128];
+};
+
#define cpu_dev_register(cpu_devX) \
static const struct cpu_dev *const __cpu_dev_##cpu_devX __used \
__attribute__((__section__(".x86_cpu_dev.init"))) = \
@@ -34,4 +42,5 @@ extern const struct cpu_dev *const __x86_cpu_dev_start[],

extern void get_cpu_cap(struct cpuinfo_x86 *c);
extern void cpu_detect_cache_sizes(struct cpuinfo_x86 *c);
+extern void intel_cpu_detect_tlb(struct cpuinfo_x86 *c) __cpuinit;
#endif /* ARCH_X86_CPU_H */
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index 3e6ff6c..28ecd1b 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -491,6 +491,147 @@ static unsigned int __cpuinit intel_size_cache(struct cpuinfo_x86 *c, unsigned i
}
#endif

+#define TLB_INST_4K 0x01
+#define TLB_INST_4M 0x02
+#define TLB_INST_2M_4M 0x03
+
+#define TLB_INST_ALL 0x05
+#define TLB_INST_1G 0x06
+
+#define TLB_DATA_4K 0x11
+#define TLB_DATA_4M 0x12
+#define TLB_DATA_2M_4M 0x13
+#define TLB_DATA_4K_4M 0x14
+
+#define TLB_DATA_1G 0x16
+
+#define TLB_DATA0_4K 0x21
+#define TLB_DATA0_4M 0x22
+#define TLB_DATA0_2M_4M 0x23
+
+#define STLB_4K 0x41
+
+static const struct _tlb_table intel_tlb_table[] __cpuinitconst = {
+ { 0x01, TLB_INST_4K, 32, " TLB_INST 4 KByte pages, 4-way set associative" },
+ { 0x02, TLB_INST_4M, 2, " TLB_INST 4 MByte pages, full associative" },
+ { 0x03, TLB_DATA_4K, 64, " TLB_DATA 4 KByte pages, 4-way set associative" },
+ { 0x04, TLB_DATA_4M, 8, " TLB_DATA 4 MByte pages, 4-way set associative" },
+ { 0x05, TLB_DATA_4M, 32, " TLB_DATA 4 MByte pages, 4-way set associative" },
+ { 0x0b, TLB_INST_4M, 4, " TLB_INST 4 MByte pages, 4-way set associative" },
+ { 0x4f, TLB_INST_4K, 32, " TLB_INST 4 KByte pages */" },
+ { 0x50, TLB_INST_ALL, 64, " TLB_INST 4 KByte and 2-MByte or 4-MByte pages" },
+ { 0x51, TLB_INST_ALL, 128, " TLB_INST 4 KByte and 2-MByte or 4-MByte pages" },
+ { 0x52, TLB_INST_ALL, 256, " TLB_INST 4 KByte and 2-MByte or 4-MByte pages" },
+ { 0x55, TLB_INST_2M_4M, 7, " TLB_INST 2-MByte or 4-MByte pages, fully associative" },
+ { 0x56, TLB_DATA0_4M, 16, " TLB_DATA0 4 MByte pages, 4-way set associative" },
+ { 0x57, TLB_DATA0_4K, 16, " TLB_DATA0 4 KByte pages, 4-way associative" },
+ { 0x59, TLB_DATA0_4K, 16, " TLB_DATA0 4 KByte pages, fully associative" },
+ { 0x5a, TLB_DATA0_2M_4M, 32, " TLB_DATA0 2-MByte or 4 MByte pages, 4-way set associative" },
+ { 0x5b, TLB_DATA_4K_4M, 64, " TLB_DATA 4 KByte and 4 MByte pages" },
+ { 0x5c, TLB_DATA_4K_4M, 128, " TLB_DATA 4 KByte and 4 MByte pages" },
+ { 0x5d, TLB_DATA_4K_4M, 256, " TLB_DATA 4 KByte and 4 MByte pages" },
+ { 0xb0, TLB_INST_4K, 128, " TLB_INST 4 KByte pages, 4-way set associative" },
+ { 0xb1, TLB_INST_2M_4M, 4, " TLB_INST 2M pages, 4-way, 8 entries or 4M pages, 4-way entries" },
+ { 0xb2, TLB_INST_4K, 64, " TLB_INST 4KByte pages, 4-way set associative" },
+ { 0xb3, TLB_DATA_4K, 128, " TLB_DATA 4 KByte pages, 4-way set associative" },
+ { 0xb4, TLB_DATA_4K, 256, " TLB_DATA 4 KByte pages, 4-way associative" },
+ { 0xba, TLB_DATA_4K, 64, " TLB_DATA 4 KByte pages, 4-way associative" },
+ { 0xc0, TLB_DATA_4K_4M, 8, " TLB_DATA 4 KByte and 4 MByte pages, 4-way associative" },
+ { 0xca, STLB_4K, 512, " STLB 4 KByte pages, 4-way associative" },
+ { 0x00, 0, 0 }
+};
+
+static void __cpuinit intel_tlb_lookup(const unsigned char desc)
+{
+ unsigned char k;
+ if (desc == 0)
+ return;
+
+ /* look up this descriptor in the table */
+ for (k = 0; intel_tlb_table[k].descriptor != desc && \
+ intel_tlb_table[k].descriptor != 0; k++)
+ ;
+
+ if (intel_tlb_table[k].tlb_type == 0)
+ return;
+
+ switch (intel_tlb_table[k].tlb_type) {
+ case STLB_4K:
+ if (tlb_lli_4k[ENTRIES] < intel_tlb_table[k].entries)
+ tlb_lli_4k[ENTRIES] = intel_tlb_table[k].entries;
+ if (tlb_lld_4k[ENTRIES] < intel_tlb_table[k].entries)
+ tlb_lld_4k[ENTRIES] = intel_tlb_table[k].entries;
+ break;
+ case TLB_INST_ALL:
+ if (tlb_lli_4k[ENTRIES] < intel_tlb_table[k].entries)
+ tlb_lli_4k[ENTRIES] = intel_tlb_table[k].entries;
+ if (tlb_lli_2m[ENTRIES] < intel_tlb_table[k].entries)
+ tlb_lli_2m[ENTRIES] = intel_tlb_table[k].entries;
+ if (tlb_lli_4m[ENTRIES] < intel_tlb_table[k].entries)
+ tlb_lli_4m[ENTRIES] = intel_tlb_table[k].entries;
+ break;
+ case TLB_INST_4K:
+ if (tlb_lli_4k[ENTRIES] < intel_tlb_table[k].entries)
+ tlb_lli_4k[ENTRIES] = intel_tlb_table[k].entries;
+ break;
+ case TLB_INST_4M:
+ if (tlb_lli_4m[ENTRIES] < intel_tlb_table[k].entries)
+ tlb_lli_4m[ENTRIES] = intel_tlb_table[k].entries;
+ break;
+ case TLB_INST_2M_4M:
+ if (tlb_lli_2m[ENTRIES] < intel_tlb_table[k].entries)
+ tlb_lli_2m[ENTRIES] = intel_tlb_table[k].entries;
+ if (tlb_lli_4m[ENTRIES] < intel_tlb_table[k].entries)
+ tlb_lli_4m[ENTRIES] = intel_tlb_table[k].entries;
+ break;
+ case TLB_DATA_4K:
+ case TLB_DATA0_4K:
+ if (tlb_lld_4k[ENTRIES] < intel_tlb_table[k].entries)
+ tlb_lld_4k[ENTRIES] = intel_tlb_table[k].entries;
+ break;
+ case TLB_DATA_4M:
+ case TLB_DATA0_4M:
+ if (tlb_lld_4m[ENTRIES] < intel_tlb_table[k].entries)
+ tlb_lld_4m[ENTRIES] = intel_tlb_table[k].entries;
+ break;
+ case TLB_DATA_2M_4M:
+ case TLB_DATA0_2M_4M:
+ if (tlb_lld_2m[ENTRIES] < intel_tlb_table[k].entries)
+ tlb_lld_2m[ENTRIES] = intel_tlb_table[k].entries;
+ if (tlb_lld_4m[ENTRIES] < intel_tlb_table[k].entries)
+ tlb_lld_4m[ENTRIES] = intel_tlb_table[k].entries;
+ break;
+ case TLB_DATA_4K_4M:
+ if (tlb_lld_4k[ENTRIES] < intel_tlb_table[k].entries)
+ tlb_lld_4k[ENTRIES] = intel_tlb_table[k].entries;
+ if (tlb_lld_4m[ENTRIES] < intel_tlb_table[k].entries)
+ tlb_lld_4m[ENTRIES] = intel_tlb_table[k].entries;
+ break;
+ }
+}
+
+void __cpuinit intel_cpu_detect_tlb(struct cpuinfo_x86 *c)
+{
+ int i, j, n;
+ unsigned int regs[4];
+ unsigned char *desc = (unsigned char *)regs;
+ /* Number of times to iterate */
+ n = cpuid_eax(2) & 0xFF;
+
+ for (i = 0 ; i < n ; i++) {
+ cpuid(2, &regs[0], &regs[1], &regs[2], &regs[3]);
+
+ /* If bit 31 is set, this is an unknown format */
+ for (j = 0 ; j < 3 ; j++)
+ if (regs[j] & (1 << 31))
+ regs[j] = 0;
+
+ /* Byte 0 is level count, not a descriptor */
+ for (j = 1 ; j < 16 ; j++)
+ intel_tlb_lookup(desc[j]);
+ }
+}
+
static const struct cpu_dev __cpuinitconst intel_cpu_dev = {
.c_vendor = "Intel",
.c_ident = { "GenuineIntel" },
--
1.7.5.4

2012-05-23 14:17:14

by Alex Shi

[permalink] [raw]
Subject: [PATCH v7 2/8] x86/flush_tlb: try flush_tlb_single one by one in flush_tlb_range

x86 has no flush_tlb_range support in instruction level. Currently the
flush_tlb_range just implemented by flushing all page table. That is not
the best solution for all scenarios. In fact, if we just use 'invlpg' to
flush few lines from TLB, we can get the performance gain from later
remain TLB lines accessing.

But the 'invlpg' instruction costs much of time. Its execution time can
compete with cr3 rewriting, and even a bit more on SNB CPU.

So, on a 512 4KB TLB entries CPU, the balance points is at:
(512 - X) * 100ns(assumed TLB refill cost) =
X(TLB flush entries) * 100ns(assumed invlpg cost)

Here, X is 256, that is 1/2 of 512 entries.

But with the mysterious CPU pre-fetcher and page miss handler Unit, the
assumed TLB refill cost is far lower then 100ns in sequential access. And
2 HT siblings in one core makes the memory access more faster if they are
accessing the same memory. So, in the patch, I just do the change when
the target entries is less than 1/16 of whole active tlb entries.
Actually, I have no data support for the percentage '1/16', so any
suggestions are welcomed.

As to hugetlb, guess due to smaller page table, and smaller active TLB
entries, I didn't see benefit via my benchmark, so no optimizing now.

My macro benchmark show in ideal scenarios, the performance improves 70
percent in reading. And in worst scenario, the reading/writing
performance is similar with unpatched 3.4-rc4 kernel.

Here is the reading data on my 2P * 4cores *HT NHM EP machine, with THP
'always':

multi thread testing, '-t' paramter is thread number:
with patch unpatched 3.4-rc4
./mprotect -t 1 14ns 24ns
./mprotect -t 2 13ns 22ns
./mprotect -t 4 12ns 19ns
./mprotect -t 8 14ns 16ns
./mprotect -t 16 28ns 26ns
./mprotect -t 32 54ns 51ns
./mprotect -t 128 200ns 199ns

Single process with sequencial flushing and memory accessing:

with patch unpatched 3.4-rc4
./mprotect 7ns 11ns
./mprotect -p 4096 -l 8 -n 10240
21ns 21ns

I also tried other benchmarks on Intel core2/NHM/SNB EP and NHM EX machine.
No clear performance change on specjbb2005 with openjdk, and oltp reading.

Signed-off-by: Alex Shi <[email protected]>
---
arch/x86/include/asm/paravirt.h | 5 +-
arch/x86/include/asm/paravirt_types.h | 3 +-
arch/x86/include/asm/tlbflush.h | 23 +++-----
arch/x86/include/asm/uv/uv.h | 5 +-
arch/x86/mm/tlb.c | 97 +++++++++++++++++++++++++++------
arch/x86/platform/uv/tlb_uv.c | 6 +-
arch/x86/xen/mmu.c | 9 ++--
include/trace/events/xen.h | 12 +++--
8 files changed, 113 insertions(+), 47 deletions(-)

diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index aa0f913..03da4ab 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -397,9 +397,10 @@ static inline void __flush_tlb_single(unsigned long addr)

static inline void flush_tlb_others(const struct cpumask *cpumask,
struct mm_struct *mm,
- unsigned long va)
+ unsigned long start,
+ unsigned long end)
{
- PVOP_VCALL3(pv_mmu_ops.flush_tlb_others, cpumask, mm, va);
+ PVOP_VCALL4(pv_mmu_ops.flush_tlb_others, cpumask, mm, start, end);
}

static inline int paravirt_pgd_alloc(struct mm_struct *mm)
diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
index 8e8b9a4..600a5fcac9 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -250,7 +250,8 @@ struct pv_mmu_ops {
void (*flush_tlb_single)(unsigned long addr);
void (*flush_tlb_others)(const struct cpumask *cpus,
struct mm_struct *mm,
- unsigned long va);
+ unsigned long start,
+ unsigned long end);

/* Hooks for allocating and freeing a pagetable top-level */
int (*pgd_alloc)(struct mm_struct *mm);
diff --git a/arch/x86/include/asm/tlbflush.h b/arch/x86/include/asm/tlbflush.h
index 7e8a096..c39c94e 100644
--- a/arch/x86/include/asm/tlbflush.h
+++ b/arch/x86/include/asm/tlbflush.h
@@ -73,14 +73,10 @@ static inline void __flush_tlb_one(unsigned long addr)
* - flush_tlb_page(vma, vmaddr) flushes one page
* - flush_tlb_range(vma, start, end) flushes a range of pages
* - flush_tlb_kernel_range(start, end) flushes a range of kernel pages
- * - flush_tlb_others(cpumask, mm, va) flushes TLBs on other cpus
+ * - flush_tlb_others(cpumask, mm, start, end) flushes TLBs on other cpus
*
* ..but the i386 has somewhat limited tlb flushing capabilities,
* and page-granular flushes are available only on i486 and up.
- *
- * x86-64 can only flush individual pages or full VMs. For a range flush
- * we always do the full VM. Might be worth trying if for a small
- * range a few INVLPGs in a row are a win.
*/

#ifndef CONFIG_SMP
@@ -111,7 +107,8 @@ static inline void flush_tlb_range(struct vm_area_struct *vma,

static inline void native_flush_tlb_others(const struct cpumask *cpumask,
struct mm_struct *mm,
- unsigned long va)
+ unsigned long start,
+ unsigned long end)
{
}

@@ -129,17 +126,14 @@ extern void flush_tlb_all(void);
extern void flush_tlb_current_task(void);
extern void flush_tlb_mm(struct mm_struct *);
extern void flush_tlb_page(struct vm_area_struct *, unsigned long);
+extern void flush_tlb_range(struct vm_area_struct *vma,
+ unsigned long start, unsigned long end);

#define flush_tlb() flush_tlb_current_task()

-static inline void flush_tlb_range(struct vm_area_struct *vma,
- unsigned long start, unsigned long end)
-{
- flush_tlb_mm(vma->vm_mm);
-}
-
void native_flush_tlb_others(const struct cpumask *cpumask,
- struct mm_struct *mm, unsigned long va);
+ struct mm_struct *mm,
+ unsigned long start, unsigned long end);

#define TLBSTATE_OK 1
#define TLBSTATE_LAZY 2
@@ -159,7 +153,8 @@ static inline void reset_lazy_tlbstate(void)
#endif /* SMP */

#ifndef CONFIG_PARAVIRT
-#define flush_tlb_others(mask, mm, va) native_flush_tlb_others(mask, mm, va)
+#define flush_tlb_others(mask, mm, start, end) \
+ native_flush_tlb_others(mask, mm, start, end)
#endif

static inline void flush_tlb_kernel_range(unsigned long start,
diff --git a/arch/x86/include/asm/uv/uv.h b/arch/x86/include/asm/uv/uv.h
index 3bb9491..b47c2a8 100644
--- a/arch/x86/include/asm/uv/uv.h
+++ b/arch/x86/include/asm/uv/uv.h
@@ -15,7 +15,8 @@ extern void uv_nmi_init(void);
extern void uv_system_init(void);
extern const struct cpumask *uv_flush_tlb_others(const struct cpumask *cpumask,
struct mm_struct *mm,
- unsigned long va,
+ unsigned long start,
+ unsigned end,
unsigned int cpu);

#else /* X86_UV */
@@ -26,7 +27,7 @@ static inline void uv_cpu_init(void) { }
static inline void uv_system_init(void) { }
static inline const struct cpumask *
uv_flush_tlb_others(const struct cpumask *cpumask, struct mm_struct *mm,
- unsigned long va, unsigned int cpu)
+ unsigned long start, unsigned long end, unsigned int cpu)
{ return cpumask; }

#endif /* X86_UV */
diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
index d6c0418..7d92079 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -41,7 +41,8 @@ DEFINE_PER_CPU_SHARED_ALIGNED(struct tlb_state, cpu_tlbstate)
union smp_flush_state {
struct {
struct mm_struct *flush_mm;
- unsigned long flush_va;
+ unsigned long flush_start;
+ unsigned long flush_end;
raw_spinlock_t tlbstate_lock;
DECLARE_BITMAP(flush_cpumask, NR_CPUS);
};
@@ -154,10 +155,19 @@ void smp_invalidate_interrupt(struct pt_regs *regs)

if (f->flush_mm == percpu_read(cpu_tlbstate.active_mm)) {
if (percpu_read(cpu_tlbstate.state) == TLBSTATE_OK) {
- if (f->flush_va == TLB_FLUSH_ALL)
+ if (f->flush_end == TLB_FLUSH_ALL
+ || !cpu_has_invlpg)
local_flush_tlb();
- else
- __flush_tlb_one(f->flush_va);
+ else if (!f->flush_end)
+ __flush_tlb_single(f->flush_start);
+ else {
+ unsigned long addr;
+ addr = f->flush_start;
+ while (addr <= f->flush_end) {
+ __flush_tlb_single(addr);
+ addr += PAGE_SIZE;
+ }
+ }
} else
leave_mm(cpu);
}
@@ -170,7 +180,8 @@ out:
}

static void flush_tlb_others_ipi(const struct cpumask *cpumask,
- struct mm_struct *mm, unsigned long va)
+ struct mm_struct *mm, unsigned long start,
+ unsigned long end)
{
unsigned int sender;
union smp_flush_state *f;
@@ -183,7 +194,8 @@ static void flush_tlb_others_ipi(const struct cpumask *cpumask,
raw_spin_lock(&f->tlbstate_lock);

f->flush_mm = mm;
- f->flush_va = va;
+ f->flush_start = start;
+ f->flush_end = end;
if (cpumask_andnot(to_cpumask(f->flush_cpumask), cpumask, cpumask_of(smp_processor_id()))) {
/*
* We have to send the IPI only to
@@ -197,24 +209,26 @@ static void flush_tlb_others_ipi(const struct cpumask *cpumask,
}

f->flush_mm = NULL;
- f->flush_va = 0;
+ f->flush_start = 0;
+ f->flush_end = 0;
if (nr_cpu_ids > NUM_INVALIDATE_TLB_VECTORS)
raw_spin_unlock(&f->tlbstate_lock);
}

void native_flush_tlb_others(const struct cpumask *cpumask,
- struct mm_struct *mm, unsigned long va)
+ struct mm_struct *mm, unsigned long start,
+ unsigned long end)
{
if (is_uv_system()) {
unsigned int cpu;

cpu = smp_processor_id();
- cpumask = uv_flush_tlb_others(cpumask, mm, va, cpu);
+ cpumask = uv_flush_tlb_others(cpumask, mm, start, end, cpu);
if (cpumask)
- flush_tlb_others_ipi(cpumask, mm, va);
+ flush_tlb_others_ipi(cpumask, mm, start, end);
return;
}
- flush_tlb_others_ipi(cpumask, mm, va);
+ flush_tlb_others_ipi(cpumask, mm, start, end);
}

static void __cpuinit calculate_tlb_offset(void)
@@ -280,7 +294,7 @@ void flush_tlb_current_task(void)

local_flush_tlb();
if (cpumask_any_but(mm_cpumask(mm), smp_processor_id()) < nr_cpu_ids)
- flush_tlb_others(mm_cpumask(mm), mm, TLB_FLUSH_ALL);
+ flush_tlb_others(mm_cpumask(mm), mm, 0UL, TLB_FLUSH_ALL);
preempt_enable();
}

@@ -295,12 +309,63 @@ void flush_tlb_mm(struct mm_struct *mm)
leave_mm(smp_processor_id());
}
if (cpumask_any_but(mm_cpumask(mm), smp_processor_id()) < nr_cpu_ids)
- flush_tlb_others(mm_cpumask(mm), mm, TLB_FLUSH_ALL);
+ flush_tlb_others(mm_cpumask(mm), mm, 0UL, TLB_FLUSH_ALL);
+
+ preempt_enable();
+}
+
+#define FLUSHALL_BAR 16
+
+void flush_tlb_range(struct vm_area_struct *vma,
+ unsigned long start, unsigned long end)
+{
+ struct mm_struct *mm;
+
+ if (!cpu_has_invlpg || vma->vm_flags & VM_HUGETLB) {
+ flush_tlb_mm(vma->vm_mm);
+ return;
+ }
+
+ preempt_disable();
+ mm = vma->vm_mm;
+ if (current->active_mm == mm) {
+ if (current->mm) {
+ unsigned long addr, vmflag = vma->vm_flags;
+ unsigned act_entries, tlb_entries = 0;
+
+ if (vmflag & VM_EXEC)
+ tlb_entries = tlb_lli_4k[ENTRIES];
+ else
+ tlb_entries = tlb_lld_4k[ENTRIES];
+
+ act_entries = tlb_entries > mm->total_vm ?
+ mm->total_vm : tlb_entries;

+ if ((end - start)/PAGE_SIZE > act_entries/FLUSHALL_BAR)
+ local_flush_tlb();
+ else {
+ for (addr = start; addr <= end;
+ addr += PAGE_SIZE)
+ __flush_tlb_single(addr);
+
+ if (cpumask_any_but(mm_cpumask(mm),
+ smp_processor_id()) < nr_cpu_ids)
+ flush_tlb_others(mm_cpumask(mm), mm,
+ start, end);
+ preempt_enable();
+ return;
+ }
+ } else {
+ leave_mm(smp_processor_id());
+ }
+ }
+ if (cpumask_any_but(mm_cpumask(mm), smp_processor_id()) < nr_cpu_ids)
+ flush_tlb_others(mm_cpumask(mm), mm, 0UL, TLB_FLUSH_ALL);
preempt_enable();
}

-void flush_tlb_page(struct vm_area_struct *vma, unsigned long va)
+
+void flush_tlb_page(struct vm_area_struct *vma, unsigned long start)
{
struct mm_struct *mm = vma->vm_mm;

@@ -308,13 +373,13 @@ void flush_tlb_page(struct vm_area_struct *vma, unsigned long va)

if (current->active_mm == mm) {
if (current->mm)
- __flush_tlb_one(va);
+ __flush_tlb_one(start);
else
leave_mm(smp_processor_id());
}

if (cpumask_any_but(mm_cpumask(mm), smp_processor_id()) < nr_cpu_ids)
- flush_tlb_others(mm_cpumask(mm), mm, va);
+ flush_tlb_others(mm_cpumask(mm), mm, start, 0UL);

preempt_enable();
}
diff --git a/arch/x86/platform/uv/tlb_uv.c b/arch/x86/platform/uv/tlb_uv.c
index 3ae0e61..0df5ad2 100644
--- a/arch/x86/platform/uv/tlb_uv.c
+++ b/arch/x86/platform/uv/tlb_uv.c
@@ -1068,8 +1068,8 @@ static int set_distrib_bits(struct cpumask *flush_mask, struct bau_control *bcp,
* done. The returned pointer is valid till preemption is re-enabled.
*/
const struct cpumask *uv_flush_tlb_others(const struct cpumask *cpumask,
- struct mm_struct *mm, unsigned long va,
- unsigned int cpu)
+ struct mm_struct *mm, unsigned long start,
+ unsigned end, unsigned int cpu)
{
int locals = 0;
int remotes = 0;
@@ -1112,7 +1112,7 @@ const struct cpumask *uv_flush_tlb_others(const struct cpumask *cpumask,

record_send_statistics(stat, locals, hubs, remotes, bau_desc);

- bau_desc->payload.address = va;
+ bau_desc->payload.address = start;
bau_desc->payload.sending_cpu = cpu;
/*
* uv_flush_send_and_wait returns 0 if all cpu's were messaged,
diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
index 69f5857..900d91e 100644
--- a/arch/x86/xen/mmu.c
+++ b/arch/x86/xen/mmu.c
@@ -1244,7 +1244,8 @@ static void xen_flush_tlb_single(unsigned long addr)
}

static void xen_flush_tlb_others(const struct cpumask *cpus,
- struct mm_struct *mm, unsigned long va)
+ struct mm_struct *mm, unsigned long start,
+ unsigned long end)
{
struct {
struct mmuext_op op;
@@ -1256,7 +1257,7 @@ static void xen_flush_tlb_others(const struct cpumask *cpus,
} *args;
struct multicall_space mcs;

- trace_xen_mmu_flush_tlb_others(cpus, mm, va);
+ trace_xen_mmu_flush_tlb_others(cpus, mm, start, end);

if (cpumask_empty(cpus))
return; /* nothing to do */
@@ -1269,11 +1270,11 @@ static void xen_flush_tlb_others(const struct cpumask *cpus,
cpumask_and(to_cpumask(args->mask), cpus, cpu_online_mask);
cpumask_clear_cpu(smp_processor_id(), to_cpumask(args->mask));

- if (va == TLB_FLUSH_ALL) {
+ if (start == TLB_FLUSH_ALL) {
args->op.cmd = MMUEXT_TLB_FLUSH_MULTI;
} else {
args->op.cmd = MMUEXT_INVLPG_MULTI;
- args->op.arg1.linear_addr = va;
+ args->op.arg1.linear_addr = start;
}

MULTI_mmuext_op(mcs.mc, &args->op, 1, NULL, DOMID_SELF);
diff --git a/include/trace/events/xen.h b/include/trace/events/xen.h
index 92f1a79..15ba03b 100644
--- a/include/trace/events/xen.h
+++ b/include/trace/events/xen.h
@@ -397,18 +397,20 @@ TRACE_EVENT(xen_mmu_flush_tlb_single,

TRACE_EVENT(xen_mmu_flush_tlb_others,
TP_PROTO(const struct cpumask *cpus, struct mm_struct *mm,
- unsigned long addr),
- TP_ARGS(cpus, mm, addr),
+ unsigned long addr, unsigned long end),
+ TP_ARGS(cpus, mm, addr, end),
TP_STRUCT__entry(
__field(unsigned, ncpus)
__field(struct mm_struct *, mm)
__field(unsigned long, addr)
+ __field(unsigned long, end)
),
TP_fast_assign(__entry->ncpus = cpumask_weight(cpus);
__entry->mm = mm;
- __entry->addr = addr),
- TP_printk("ncpus %d mm %p addr %lx",
- __entry->ncpus, __entry->mm, __entry->addr)
+ __entry->addr = addr,
+ __entry->end = end),
+ TP_printk("ncpus %d mm %p addr %lx, end %lx",
+ __entry->ncpus, __entry->mm, __entry->addr, __entry->end)
);

TRACE_EVENT(xen_mmu_write_cr3,
--
1.7.5.4

2012-05-23 14:17:25

by Alex Shi

[permalink] [raw]
Subject: [PATCH v7 3/8] x86/tlb: fall back to flush all when meet a THP large page

We don't need to flush large pages by PAGE_SIZE step, that just waste
time. and actually, large page don't need 'invlpg' optimizing according
to our macro benchmark. So, just flush whole TLB is enough for them.

The following result is tested on a 2CPU * 4cores * 2HT NHM EP machine,
with THP 'always' setting.

Multi-thread testing, '-t' paramter is thread number:
without this patch with this patch
./mprotect -t 1 14ns 13ns
./mprotect -t 2 13ns 13ns
./mprotect -t 4 12ns 11ns
./mprotect -t 8 14ns 10ns
./mprotect -t 16 28ns 28ns
./mprotect -t 32 54ns 52ns
./mprotect -t 128 200ns 200ns

Signed-off-by: Alex Shi <[email protected]>
---
arch/x86/mm/tlb.c | 34 ++++++++++++++++++++++++++++++++++
1 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
index 7d92079..22e5bb1 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -316,12 +316,42 @@ void flush_tlb_mm(struct mm_struct *mm)

#define FLUSHALL_BAR 16

+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+static inline int has_large_page(struct mm_struct *mm,
+ unsigned long start, unsigned long end)
+{
+ pgd_t *pgd;
+ pud_t *pud;
+ pmd_t *pmd;
+ unsigned long addr = ALIGN(start, HPAGE_SIZE);
+ for (; addr < end; addr += HPAGE_SIZE) {
+ pgd = pgd_offset(mm, addr);
+ if (likely(!pgd_none(*pgd))) {
+ pud = pud_offset(pgd, addr);
+ if (likely(!pud_none(*pud))) {
+ pmd = pmd_offset(pud, addr);
+ if (likely(!pmd_none(*pmd)))
+ if (pmd_large(*pmd))
+ return 1;
+ }
+ }
+ }
+ return 0;
+}
+#else
+static inline int has_large_page(struct mm_struct *mm,
+ unsigned long start, unsigned long end)
+{
+ return 0;
+}
+#endif
void flush_tlb_range(struct vm_area_struct *vma,
unsigned long start, unsigned long end)
{
struct mm_struct *mm;

if (!cpu_has_invlpg || vma->vm_flags & VM_HUGETLB) {
+flush_all:
flush_tlb_mm(vma->vm_mm);
return;
}
@@ -344,6 +374,10 @@ void flush_tlb_range(struct vm_area_struct *vma,
if ((end - start)/PAGE_SIZE > act_entries/FLUSHALL_BAR)
local_flush_tlb();
else {
+ if (has_large_page(mm, start, end)) {
+ preempt_enable();
+ goto flush_all;
+ }
for (addr = start; addr <= end;
addr += PAGE_SIZE)
__flush_tlb_single(addr);
--
1.7.5.4

2012-05-23 14:17:34

by Alex Shi

[permalink] [raw]
Subject: [PATCH v7 4/8] x86/tlb: add tlb_flushall_shift for specific CPU

Testing show different CPU type(micro architectures and NUMA mode) has
different balance points between the TLB flush all and multiple invlpg.
And there also has cases the tlb flush change has no any help.

This patch give a interface to let x86 vendor developers have a chance
to set different shift for different CPU type.

like some machine in my hands, balance points is 16 entries on
Romely-EP; while it is at 8 entries on Bloomfield NHM-EP; and is 256 on
IVB mobile CPU. but on model 15 core2 Xeon using invlpg has nothing
help.

For untested machine, do a conservative optimization, same as NHM CPU.

Signed-off-by: Alex Shi <[email protected]>
---
arch/x86/include/asm/processor.h | 2 ++
arch/x86/kernel/cpu/common.c | 14 ++++++++++++--
arch/x86/kernel/cpu/intel.c | 34 ++++++++++++++++++++++++++++++++++
arch/x86/mm/tlb.c | 7 +++----
include/asm-generic/tlb.h | 3 ++-
5 files changed, 53 insertions(+), 7 deletions(-)

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 797faca..a1667f0 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -72,6 +72,8 @@ extern u16 __read_mostly tlb_lli_4m[NR_INFO];
extern u16 __read_mostly tlb_lld_4k[NR_INFO];
extern u16 __read_mostly tlb_lld_2m[NR_INFO];
extern u16 __read_mostly tlb_lld_4m[NR_INFO];
+extern s8 __read_mostly tlb_flushall_shift;
+
/*
* CPU type and hardware bug flags. Kept separately for each CPU.
* Members of this structure are referenced in head.S, so think twice
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 0152082..690e95d 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -459,16 +459,26 @@ u16 __read_mostly tlb_lld_4k[NR_INFO];
u16 __read_mostly tlb_lld_2m[NR_INFO];
u16 __read_mostly tlb_lld_4m[NR_INFO];

+/*
+ * tlb_flushall_shift shows the balance point in replacing cr3 write
+ * with multiple 'invlpg'. It will do this replacement when
+ * flush_tlb_lines <= active_lines/2^tlb_flushall_shift.
+ * If tlb_flushall_shift is -1, means the replacement will be disabled.
+ */
+s8 __read_mostly tlb_flushall_shift;
+
void __cpuinit cpu_detect_tlb(struct cpuinfo_x86 *c)
{
if (c->x86_vendor == X86_VENDOR_INTEL)
intel_cpu_detect_tlb(c);

printk(KERN_INFO "Last level iTLB entries: 4KB %d, 2MB %d, 4MB %d\n" \
- "Last level dTLB entries: 4KB %d, 2MB %d, 4MB %d\n",
+ "Last level dTLB entries: 4KB %d, 2MB %d, 4MB %d\n" \
+ "tlb_flushall_shift is 0x%x\n",
tlb_lli_4k[ENTRIES], tlb_lli_2m[ENTRIES],
tlb_lli_4m[ENTRIES], tlb_lld_4k[ENTRIES],
- tlb_lld_2m[ENTRIES], tlb_lld_4m[ENTRIES]);
+ tlb_lld_2m[ENTRIES], tlb_lld_4m[ENTRIES],
+ tlb_flushall_shift);
}

void __cpuinit detect_ht(struct cpuinfo_x86 *c)
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index 28ecd1b..bb90754 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -610,6 +610,39 @@ static void __cpuinit intel_tlb_lookup(const unsigned char desc)
}
}

+static void __cpuinit intel_tlb_flushall_shift_set(struct cpuinfo_x86 *c)
+{
+ if (!cpu_has_invlpg) {
+ tlb_flushall_shift = -1;
+ return;
+ }
+ switch ((c->x86 << 8) + c->x86_model) {
+ case 0x60f: /* original 65 nm celeron/pentium/core2/xeon, "Merom"/"Conroe" */
+ case 0x616: /* single-core 65 nm celeron/core2solo "Merom-L"/"Conroe-L" */
+ case 0x617: /* current 45 nm celeron/core2/xeon "Penryn"/"Wolfdale" */
+ case 0x61d: /* six-core 45 nm xeon "Dunnington" */
+ tlb_flushall_shift = -1;
+ break;
+ case 0x61a: /* 45 nm nehalem, "Bloomfield" */
+ case 0x61e: /* 45 nm nehalem, "Lynnfield" */
+ case 0x625: /* 32 nm nehalem, "Clarkdale" */
+ case 0x62c: /* 32 nm nehalem, "Gulftown" */
+ case 0x62e: /* 45 nm nehalem-ex, "Beckton" */
+ case 0x62f: /* 32 nm Xeon E7 */
+ tlb_flushall_shift = 6;
+ break;
+ case 0x62a: /* SandyBridge */
+ case 0x62d: /* SandyBridge, "Romely-EP" */
+ tlb_flushall_shift = 5;
+ break;
+ case 0x63a: /* Ivybridge */
+ tlb_flushall_shift = 1;
+ break;
+ default:
+ tlb_flushall_shift = 6;
+ }
+}
+
void __cpuinit intel_cpu_detect_tlb(struct cpuinfo_x86 *c)
{
int i, j, n;
@@ -630,6 +663,7 @@ void __cpuinit intel_cpu_detect_tlb(struct cpuinfo_x86 *c)
for (j = 1 ; j < 16 ; j++)
intel_tlb_lookup(desc[j]);
}
+ intel_tlb_flushall_shift_set(c);
}

static const struct cpu_dev __cpuinitconst intel_cpu_dev = {
diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
index 22e5bb1..6a7ca47 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -314,8 +314,6 @@ void flush_tlb_mm(struct mm_struct *mm)
preempt_enable();
}

-#define FLUSHALL_BAR 16
-
#ifdef CONFIG_TRANSPARENT_HUGEPAGE
static inline int has_large_page(struct mm_struct *mm,
unsigned long start, unsigned long end)
@@ -350,7 +348,7 @@ void flush_tlb_range(struct vm_area_struct *vma,
{
struct mm_struct *mm;

- if (!cpu_has_invlpg || vma->vm_flags & VM_HUGETLB) {
+ if (vma->vm_flags & VM_HUGETLB || tlb_flushall_shift == -1) {
flush_all:
flush_tlb_mm(vma->vm_mm);
return;
@@ -371,7 +369,8 @@ flush_all:
act_entries = tlb_entries > mm->total_vm ?
mm->total_vm : tlb_entries;

- if ((end - start)/PAGE_SIZE > act_entries/FLUSHALL_BAR)
+ if ((end - start) >> PAGE_SHIFT >
+ act_entries >> tlb_flushall_shift)
local_flush_tlb();
else {
if (has_large_page(mm, start, end)) {
diff --git a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h
index f96a5b5..75e888b 100644
--- a/include/asm-generic/tlb.h
+++ b/include/asm-generic/tlb.h
@@ -113,7 +113,8 @@ static inline int tlb_fast_mode(struct mmu_gather *tlb)

void tlb_gather_mmu(struct mmu_gather *tlb, struct mm_struct *mm, bool fullmm);
void tlb_flush_mmu(struct mmu_gather *tlb);
-void tlb_finish_mmu(struct mmu_gather *tlb, unsigned long start, unsigned long end);
+void tlb_finish_mmu(struct mmu_gather *tlb, unsigned long start,
+ unsigned long end);
int __tlb_remove_page(struct mmu_gather *tlb, struct page *page);

/* tlb_remove_page
--
1.7.5.4

2012-05-23 14:17:46

by Alex Shi

[permalink] [raw]
Subject: [PATCH v7 5/8] x86/tlb: enable tlb flush range support for generic mmu and x86

Not every tlb_flush execution moment is really need to evacuate all
TLB entries, like in munmap, just few 'invlpg' is better for whole
process performance, since it leaves most of TLB entries for later
accessing.

All of tlb interfaces in mm/memory.c are reused by all architectures
except few of them for generic mmu_gather were protected under
HAVE_GENERIC_MMU_GATHER.
For x86, need to re-implement 'tlb_flush'.

This patch also rewrite flush_tlb_range for 2 purposes:
1, split it out to get flush_blt_mm_range function.
2, clean up to reduce line breaking, thanks for Borislav's input.

My macro benchmark 'mummap' http://lkml.org/lkml/2012/5/17/59
show that the random memory access on other CPU has 0~50% speed up
on a 2P * 4cores * HT NHM EP.

Thanks for Peter Zijlstra time and time reminder for multiple
architecture code safe!

Signed-off-by: Alex Shi <[email protected]>
---
arch/x86/include/asm/tlb.h | 9 +++-
arch/x86/include/asm/tlbflush.h | 9 +++
arch/x86/mm/tlb.c | 123 ++++++++++++++++++--------------------
include/asm-generic/tlb.h | 2 +
mm/memory.c | 9 +++
5 files changed, 86 insertions(+), 66 deletions(-)

diff --git a/arch/x86/include/asm/tlb.h b/arch/x86/include/asm/tlb.h
index 829215f..4fef207 100644
--- a/arch/x86/include/asm/tlb.h
+++ b/arch/x86/include/asm/tlb.h
@@ -4,7 +4,14 @@
#define tlb_start_vma(tlb, vma) do { } while (0)
#define tlb_end_vma(tlb, vma) do { } while (0)
#define __tlb_remove_tlb_entry(tlb, ptep, address) do { } while (0)
-#define tlb_flush(tlb) flush_tlb_mm((tlb)->mm)
+
+#define tlb_flush(tlb) \
+{ \
+ if (tlb->fullmm == 0) \
+ flush_tlb_mm_range(tlb->mm, tlb->start, tlb->end, 0UL); \
+ else \
+ flush_tlb_mm_range(tlb->mm, 0UL, TLB_FLUSH_ALL, 0UL); \
+}

#include <asm-generic/tlb.h>

diff --git a/arch/x86/include/asm/tlbflush.h b/arch/x86/include/asm/tlbflush.h
index c39c94e..1562ed0 100644
--- a/arch/x86/include/asm/tlbflush.h
+++ b/arch/x86/include/asm/tlbflush.h
@@ -105,6 +105,13 @@ static inline void flush_tlb_range(struct vm_area_struct *vma,
__flush_tlb();
}

+static inline void flush_tlb_mm_range(struct vm_area_struct *vma,
+ unsigned long start, unsigned long end, unsigned long vmflag)
+{
+ if (mm == current->active_mm)
+ __flush_tlb();
+}
+
static inline void native_flush_tlb_others(const struct cpumask *cpumask,
struct mm_struct *mm,
unsigned long start,
@@ -128,6 +135,8 @@ extern void flush_tlb_mm(struct mm_struct *);
extern void flush_tlb_page(struct vm_area_struct *, unsigned long);
extern void flush_tlb_range(struct vm_area_struct *vma,
unsigned long start, unsigned long end);
+extern void flush_tlb_mm_range(struct mm_struct *mm, unsigned long start,
+ unsigned long end, unsigned long vmflag);

#define flush_tlb() flush_tlb_current_task()

diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
index 6a7ca47..d0ab80e 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -298,23 +298,6 @@ void flush_tlb_current_task(void)
preempt_enable();
}

-void flush_tlb_mm(struct mm_struct *mm)
-{
- preempt_disable();
-
- if (current->active_mm == mm) {
- if (current->mm)
- local_flush_tlb();
- else
- leave_mm(smp_processor_id());
- }
- if (cpumask_any_but(mm_cpumask(mm), smp_processor_id()) < nr_cpu_ids)
- flush_tlb_others(mm_cpumask(mm), mm, 0UL, TLB_FLUSH_ALL);
-
- preempt_enable();
-}
-
-#ifdef CONFIG_TRANSPARENT_HUGEPAGE
static inline int has_large_page(struct mm_struct *mm,
unsigned long start, unsigned long end)
{
@@ -336,67 +319,77 @@ static inline int has_large_page(struct mm_struct *mm,
}
return 0;
}
-#else
-static inline int has_large_page(struct mm_struct *mm,
- unsigned long start, unsigned long end)
-{
- return 0;
-}
-#endif
-void flush_tlb_range(struct vm_area_struct *vma,
- unsigned long start, unsigned long end)
-{
- struct mm_struct *mm;

- if (vma->vm_flags & VM_HUGETLB || tlb_flushall_shift == -1) {
-flush_all:
- flush_tlb_mm(vma->vm_mm);
- return;
- }
+void flush_tlb_mm_range(struct mm_struct *mm, unsigned long start,
+ unsigned long end, unsigned long vmflag)
+{
+ unsigned long addr;
+ unsigned act_entries, tlb_entries = 0;

preempt_disable();
- mm = vma->vm_mm;
- if (current->active_mm == mm) {
- if (current->mm) {
- unsigned long addr, vmflag = vma->vm_flags;
- unsigned act_entries, tlb_entries = 0;
-
- if (vmflag & VM_EXEC)
- tlb_entries = tlb_lli_4k[ENTRIES];
- else
- tlb_entries = tlb_lld_4k[ENTRIES];
+ if (current->active_mm != mm)
+ goto flush_all;

- act_entries = tlb_entries > mm->total_vm ?
- mm->total_vm : tlb_entries;
+ if (!current->mm) {
+ leave_mm(smp_processor_id());
+ goto flush_all;
+ }

- if ((end - start) >> PAGE_SHIFT >
- act_entries >> tlb_flushall_shift)
- local_flush_tlb();
- else {
- if (has_large_page(mm, start, end)) {
- preempt_enable();
- goto flush_all;
- }
- for (addr = start; addr <= end;
- addr += PAGE_SIZE)
- __flush_tlb_single(addr);
+ if (end == TLB_FLUSH_ALL || tlb_flushall_shift == -1) {
+ local_flush_tlb();
+ goto flush_all;
+ }

- if (cpumask_any_but(mm_cpumask(mm),
- smp_processor_id()) < nr_cpu_ids)
- flush_tlb_others(mm_cpumask(mm), mm,
- start, end);
- preempt_enable();
- return;
- }
- } else {
- leave_mm(smp_processor_id());
+ /* In modern CPU, last level tlb used for both data/ins */
+ if (vmflag & VM_EXEC)
+ tlb_entries = tlb_lli_4k[ENTRIES];
+ else
+ tlb_entries = tlb_lld_4k[ENTRIES];
+ /* Assume all of TLB entries was occupied by this task */
+ act_entries = mm->total_vm > tlb_entries ? tlb_entries : mm->total_vm;
+
+ /* tlb_flushall_shift is on balance point, details in commit log */
+ if ((end - start) >> PAGE_SHIFT > act_entries >> tlb_flushall_shift)
+ local_flush_tlb();
+ else {
+ if (has_large_page(mm, start, end)) {
+ local_flush_tlb();
+ goto flush_all;
}
+ /* flush range by one by one 'invlpg' */
+ for (addr = start; addr <= end; addr += PAGE_SIZE)
+ __flush_tlb_single(addr);
+
+ if (cpumask_any_but(mm_cpumask(mm),
+ smp_processor_id()) < nr_cpu_ids)
+ flush_tlb_others(mm_cpumask(mm), mm, start, end);
+ preempt_enable();
+ return;
}
+
+flush_all:
if (cpumask_any_but(mm_cpumask(mm), smp_processor_id()) < nr_cpu_ids)
flush_tlb_others(mm_cpumask(mm), mm, 0UL, TLB_FLUSH_ALL);
preempt_enable();
}

+void flush_tlb_mm(struct mm_struct *mm)
+{
+ flush_tlb_mm_range(mm, 0UL, TLB_FLUSH_ALL, 0UL);
+}
+
+void flush_tlb_range(struct vm_area_struct *vma,
+ unsigned long start, unsigned long end)
+{
+ struct mm_struct *mm = vma->vm_mm;
+ unsigned long vmflag = vma->vm_flags;
+
+ if (!cpu_has_invlpg || vma->vm_flags & VM_HUGETLB)
+ flush_tlb_mm_range(mm, 0UL, TLB_FLUSH_ALL, 0UL);
+ else
+ flush_tlb_mm_range(mm, start, end, vmflag);
+}
+

void flush_tlb_page(struct vm_area_struct *vma, unsigned long start)
{
diff --git a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h
index 75e888b..ed6642a 100644
--- a/include/asm-generic/tlb.h
+++ b/include/asm-generic/tlb.h
@@ -86,6 +86,8 @@ struct mmu_gather {
#ifdef CONFIG_HAVE_RCU_TABLE_FREE
struct mmu_table_batch *batch;
#endif
+ unsigned long start;
+ unsigned long end;
unsigned int need_flush : 1, /* Did free PTEs */
fast_mode : 1; /* No batching */

diff --git a/mm/memory.c b/mm/memory.c
index 1e77da6..c22d9f5 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -206,6 +206,8 @@ void tlb_gather_mmu(struct mmu_gather *tlb, struct mm_struct *mm, bool fullmm)
tlb->mm = mm;

tlb->fullmm = fullmm;
+ tlb->start = -1UL;
+ tlb->end = 0;
tlb->need_flush = 0;
tlb->fast_mode = (num_possible_cpus() == 1);
tlb->local.next = NULL;
@@ -248,6 +250,8 @@ void tlb_finish_mmu(struct mmu_gather *tlb, unsigned long start, unsigned long e
{
struct mmu_gather_batch *batch, *next;

+ tlb->start = start;
+ tlb->end = end;
tlb_flush_mmu(tlb);

/* keep the page table cache within bounds */
@@ -1204,6 +1208,11 @@ again:
*/
if (force_flush) {
force_flush = 0;
+
+#ifdef HAVE_GENERIC_MMU_GATHER
+ tlb->start = addr;
+ tlb->end = end;
+#endif
tlb_flush_mmu(tlb);
if (addr != end)
goto again;
--
1.7.5.4

2012-05-23 14:17:52

by Alex Shi

[permalink] [raw]
Subject: [PATCH v7 6/8] x86/tlb: add tlb_flushall_shift knob into debugfs

kernel will replace cr3 rewrite with invlpg when
tlb_flush_entries <= active_tlb_entries / 2^tlb_flushall_factor
if tlb_flushall_factor is -1, kernel won't do this replacement.

User can modify its value according to specific CPU/applications.

Thanks for Borislav providing the help message of
CONFIG_DEBUG_TLBFLUSH.

Signed-off-by: Alex Shi <[email protected]>
---
arch/x86/Kconfig.debug | 19 +++++++++++++++++
arch/x86/mm/tlb.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 70 insertions(+), 0 deletions(-)

diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug
index e46c214..b322f12 100644
--- a/arch/x86/Kconfig.debug
+++ b/arch/x86/Kconfig.debug
@@ -129,6 +129,25 @@ config DOUBLEFAULT
option saves about 4k and might cause you much additional grey
hair.

+config DEBUG_TLBFLUSH
+ bool "Set upper limit of TLB entries to flush one-by-one"
+ depends on DEBUG_KERNEL && (X86_64 || X86_INVLPG)
+ ---help---
+
+ X86-only for now.
+
+ This option allows the user to tune the amount of TLB entries the
+ kernel flushes one-by-one instead of doing a full TLB flush. In
+ certain situations, the former is cheaper. This is controlled by the
+ tlb_flushall_shift knob under /sys/kernel/debug/x86. If you set it
+ to -1, the code flushes the whole TLB unconditionally. Otherwise,
+ for positive values of it, the kernel will use single TLB entry
+ invalidating instructions according to the following formula:
+
+ flush_entries <= active_tlb_entries / 2^tlb_flushall_shift
+
+ If in doubt, say "N".
+
config IOMMU_DEBUG
bool "Enable IOMMU debugging"
depends on GART_IOMMU && DEBUG_KERNEL
diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
index d0ab80e..0f17b04 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -12,6 +12,7 @@
#include <asm/cache.h>
#include <asm/apic.h>
#include <asm/uv/uv.h>
+#include <linux/debugfs.h>

DEFINE_PER_CPU_SHARED_ALIGNED(struct tlb_state, cpu_tlbstate)
= { &init_mm, 0, };
@@ -421,3 +422,53 @@ void flush_tlb_all(void)
{
on_each_cpu(do_flush_tlb_all, NULL, 1);
}
+
+#ifdef CONFIG_DEBUG_TLBFLUSH
+static ssize_t tlbflush_read_file(struct file *file, char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ char buf[32];
+ unsigned int len;
+
+ len = sprintf(buf, "%hd\n", tlb_flushall_shift);
+ return simple_read_from_buffer(user_buf, count, ppos, buf, len);
+}
+
+static ssize_t tlbflush_write_file(struct file *file,
+ const char __user *user_buf, size_t count, loff_t *ppos)
+{
+ char buf[32];
+ ssize_t len;
+ s8 shift;
+
+ len = min(count, sizeof(buf) - 1);
+ if (copy_from_user(buf, user_buf, len))
+ return -EFAULT;
+
+ buf[len] = '\0';
+ if (kstrtos8(buf, 0, &shift))
+ return -EINVAL;
+
+ if (shift > 64)
+ return -EINVAL;
+
+ tlb_flushall_shift = shift;
+ return count;
+}
+
+static const struct file_operations fops_tlbflush = {
+ .read = tlbflush_read_file,
+ .write = tlbflush_write_file,
+ .llseek = default_llseek,
+};
+
+static int __cpuinit create_tlb_flushall_shift(void)
+{
+ if (cpu_has_invlpg) {
+ debugfs_create_file("tlb_flushall_shift", S_IRUSR | S_IWUSR,
+ arch_debugfs_dir, NULL, &fops_tlbflush);
+ }
+ return 0;
+}
+late_initcall(create_tlb_flushall_shift);
+#endif
--
1.7.5.4

2012-05-23 14:18:05

by Alex Shi

[permalink] [raw]
Subject: [PATCH v7 7/8] x86/tlb: replace INVALIDATE_TLB_VECTOR by CALL_FUNCTION_VECTOR

There are only 32 INVALIDATE_TLB_VECTOR now in kernel.
but modern x86 sever has more cpu number. That causes heavy
lock contentation in TLB flushing.

Now, useing generic smp call function to replace it.
In the NHM EX machine 4P * 8cores * HT = 64 CPUs, hackbench pthread
has 3% performance increase.
And no clear performance changes on NHM EP(16CPUs), WSM EP(24CPU)
and SNB EP(32CPU) machines.

Signed-off-by: Alex Shi <[email protected]>
---
arch/x86/include/asm/entry_arch.h | 9 --
arch/x86/include/asm/irq_vectors.h | 11 --
arch/x86/kernel/entry_64.S | 18 ---
arch/x86/kernel/irqinit.c | 73 -----------
arch/x86/mm/tlb.c | 250 +++++-------------------------------
5 files changed, 33 insertions(+), 328 deletions(-)

diff --git a/arch/x86/include/asm/entry_arch.h b/arch/x86/include/asm/entry_arch.h
index 0baa628..40afa00 100644
--- a/arch/x86/include/asm/entry_arch.h
+++ b/arch/x86/include/asm/entry_arch.h
@@ -15,15 +15,6 @@ BUILD_INTERRUPT(call_function_interrupt,CALL_FUNCTION_VECTOR)
BUILD_INTERRUPT(call_function_single_interrupt,CALL_FUNCTION_SINGLE_VECTOR)
BUILD_INTERRUPT(irq_move_cleanup_interrupt,IRQ_MOVE_CLEANUP_VECTOR)
BUILD_INTERRUPT(reboot_interrupt,REBOOT_VECTOR)
-
-.irp idx,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15, \
- 16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31
-.if NUM_INVALIDATE_TLB_VECTORS > \idx
-BUILD_INTERRUPT3(invalidate_interrupt\idx,
- (INVALIDATE_TLB_VECTOR_START)+\idx,
- smp_invalidate_interrupt)
-.endif
-.endr
#endif

BUILD_INTERRUPT(x86_platform_ipi, X86_PLATFORM_IPI_VECTOR)
diff --git a/arch/x86/include/asm/irq_vectors.h b/arch/x86/include/asm/irq_vectors.h
index 4b44487..1508e51 100644
--- a/arch/x86/include/asm/irq_vectors.h
+++ b/arch/x86/include/asm/irq_vectors.h
@@ -119,17 +119,6 @@
*/
#define LOCAL_TIMER_VECTOR 0xef

-/* up to 32 vectors used for spreading out TLB flushes: */
-#if NR_CPUS <= 32
-# define NUM_INVALIDATE_TLB_VECTORS (NR_CPUS)
-#else
-# define NUM_INVALIDATE_TLB_VECTORS (32)
-#endif
-
-#define INVALIDATE_TLB_VECTOR_END (0xee)
-#define INVALIDATE_TLB_VECTOR_START \
- (INVALIDATE_TLB_VECTOR_END-NUM_INVALIDATE_TLB_VECTORS+1)
-
#define NR_VECTORS 256

#define FPU_IRQ 13
diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index cdc79b5..0fa34c9 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -1015,24 +1015,6 @@ apicinterrupt LOCAL_TIMER_VECTOR \
apicinterrupt X86_PLATFORM_IPI_VECTOR \
x86_platform_ipi smp_x86_platform_ipi

-#ifdef CONFIG_SMP
- ALIGN
- INTR_FRAME
-.irp idx,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15, \
- 16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31
-.if NUM_INVALIDATE_TLB_VECTORS > \idx
-ENTRY(invalidate_interrupt\idx)
- pushq_cfi $~(INVALIDATE_TLB_VECTOR_START+\idx)
- jmp .Lcommon_invalidate_interrupt0
- CFI_ADJUST_CFA_OFFSET -8
-END(invalidate_interrupt\idx)
-.endif
-.endr
- CFI_ENDPROC
-apicinterrupt INVALIDATE_TLB_VECTOR_START, \
- invalidate_interrupt0, smp_invalidate_interrupt
-#endif
-
apicinterrupt THRESHOLD_APIC_VECTOR \
threshold_interrupt smp_threshold_interrupt
apicinterrupt THERMAL_APIC_VECTOR \
diff --git a/arch/x86/kernel/irqinit.c b/arch/x86/kernel/irqinit.c
index 252981a..6e03b0d 100644
--- a/arch/x86/kernel/irqinit.c
+++ b/arch/x86/kernel/irqinit.c
@@ -171,79 +171,6 @@ static void __init smp_intr_init(void)
*/
alloc_intr_gate(RESCHEDULE_VECTOR, reschedule_interrupt);

- /* IPIs for invalidation */
-#define ALLOC_INVTLB_VEC(NR) \
- alloc_intr_gate(INVALIDATE_TLB_VECTOR_START+NR, \
- invalidate_interrupt##NR)
-
- switch (NUM_INVALIDATE_TLB_VECTORS) {
- default:
- ALLOC_INVTLB_VEC(31);
- case 31:
- ALLOC_INVTLB_VEC(30);
- case 30:
- ALLOC_INVTLB_VEC(29);
- case 29:
- ALLOC_INVTLB_VEC(28);
- case 28:
- ALLOC_INVTLB_VEC(27);
- case 27:
- ALLOC_INVTLB_VEC(26);
- case 26:
- ALLOC_INVTLB_VEC(25);
- case 25:
- ALLOC_INVTLB_VEC(24);
- case 24:
- ALLOC_INVTLB_VEC(23);
- case 23:
- ALLOC_INVTLB_VEC(22);
- case 22:
- ALLOC_INVTLB_VEC(21);
- case 21:
- ALLOC_INVTLB_VEC(20);
- case 20:
- ALLOC_INVTLB_VEC(19);
- case 19:
- ALLOC_INVTLB_VEC(18);
- case 18:
- ALLOC_INVTLB_VEC(17);
- case 17:
- ALLOC_INVTLB_VEC(16);
- case 16:
- ALLOC_INVTLB_VEC(15);
- case 15:
- ALLOC_INVTLB_VEC(14);
- case 14:
- ALLOC_INVTLB_VEC(13);
- case 13:
- ALLOC_INVTLB_VEC(12);
- case 12:
- ALLOC_INVTLB_VEC(11);
- case 11:
- ALLOC_INVTLB_VEC(10);
- case 10:
- ALLOC_INVTLB_VEC(9);
- case 9:
- ALLOC_INVTLB_VEC(8);
- case 8:
- ALLOC_INVTLB_VEC(7);
- case 7:
- ALLOC_INVTLB_VEC(6);
- case 6:
- ALLOC_INVTLB_VEC(5);
- case 5:
- ALLOC_INVTLB_VEC(4);
- case 4:
- ALLOC_INVTLB_VEC(3);
- case 3:
- ALLOC_INVTLB_VEC(2);
- case 2:
- ALLOC_INVTLB_VEC(1);
- case 1:
- ALLOC_INVTLB_VEC(0);
- break;
- }
-
/* IPI for generic function call */
alloc_intr_gate(CALL_FUNCTION_VECTOR, call_function_interrupt);

diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
index 0f17b04..0232e24 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -28,34 +28,14 @@ DEFINE_PER_CPU_SHARED_ALIGNED(struct tlb_state, cpu_tlbstate)
*
* More scalable flush, from Andi Kleen
*
- * To avoid global state use 8 different call vectors.
- * Each CPU uses a specific vector to trigger flushes on other
- * CPUs. Depending on the received vector the target CPUs look into
- * the right array slot for the flush data.
- *
- * With more than 8 CPUs they are hashed to the 8 available
- * vectors. The limited global vector space forces us to this right now.
- * In future when interrupts are split into per CPU domains this could be
- * fixed, at the cost of triggering multiple IPIs in some cases.
+ * Implemented by smp_call_function_many, Alex Shi
*/

-union smp_flush_state {
- struct {
- struct mm_struct *flush_mm;
- unsigned long flush_start;
- unsigned long flush_end;
- raw_spinlock_t tlbstate_lock;
- DECLARE_BITMAP(flush_cpumask, NR_CPUS);
- };
- char pad[INTERNODE_CACHE_BYTES];
-} ____cacheline_internodealigned_in_smp;
-
-/* State is put into the per CPU data section, but padded
- to a full cache line because other CPUs can access it and we don't
- 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);
+struct flush_tlb_info {
+ struct mm_struct *flush_mm;
+ unsigned long flush_start;
+ unsigned long flush_end;
+};

/*
* We cannot call mmdrop() because we are in interrupt context,
@@ -72,220 +52,56 @@ void leave_mm(int cpu)
EXPORT_SYMBOL_GPL(leave_mm);

/*
- *
- * The flush IPI assumes that a thread switch happens in this order:
- * [cpu0: the cpu that switches]
- * 1) switch_mm() either 1a) or 1b)
- * 1a) thread switch to a different mm
- * 1a1) cpu_clear(cpu, old_mm->cpu_vm_mask);
- * Stop ipi delivery for the old mm. This is not synchronized with
- * the other cpus, but smp_invalidate_interrupt ignore flush ipis
- * for the wrong mm, and in the worst case we perform a superfluous
- * tlb flush.
- * 1a2) set cpu mmu_state to TLBSTATE_OK
- * Now the smp_invalidate_interrupt won't call leave_mm if cpu0
- * was in lazy tlb mode.
- * 1a3) update cpu active_mm
- * Now cpu0 accepts tlb flushes for the new mm.
- * 1a4) cpu_set(cpu, new_mm->cpu_vm_mask);
- * Now the other cpus will send tlb flush ipis.
- * 1a4) change cr3.
- * 1b) thread switch without mm change
- * cpu active_mm is correct, cpu0 already handles
- * flush ipis.
- * 1b1) set cpu mmu_state to TLBSTATE_OK
- * 1b2) test_and_set the cpu bit in cpu_vm_mask.
- * Atomically set the bit [other cpus will start sending flush ipis],
- * and test the bit.
- * 1b3) if the bit was 0: leave_mm was called, flush the tlb.
- * 2) switch %%esp, ie current
- *
- * The interrupt must handle 2 special cases:
- * - cr3 is changed before %%esp, ie. it cannot use current->{active_,}mm.
- * - the cpu performs speculative tlb reads, i.e. even if the cpu only
- * runs in kernel space, the cpu could load tlb entries for user space
- * pages.
- *
- * The good news is that cpu mmu_state is local to each cpu, no
- * write/read ordering problems.
- */
-
-/*
- * TLB flush IPI:
- *
+ * TLB flush funcation:
* 1) Flush the tlb entries if the cpu uses the mm that's being flushed.
* 2) Leave the mm if we are in the lazy tlb mode.
- *
- * Interrupts are disabled.
*/
-
-/*
- * FIXME: use of asmlinkage is not consistent. On x86_64 it's noop
- * but still used for documentation purpose but the usage is slightly
- * inconsistent. On x86_32, asmlinkage is regparm(0) but interrupt
- * entry calls in with the first parameter in %eax. Maybe define
- * intrlinkage?
- */
-#ifdef CONFIG_X86_64
-asmlinkage
-#endif
-void smp_invalidate_interrupt(struct pt_regs *regs)
+static void flush_tlb_func(void *info)
{
- unsigned int cpu;
- unsigned int sender;
- union smp_flush_state *f;
-
- cpu = smp_processor_id();
- /*
- * orig_rax contains the negated interrupt vector.
- * Use that to determine where the sender put the data.
- */
- sender = ~regs->orig_ax - INVALIDATE_TLB_VECTOR_START;
- f = &flush_state[sender];
-
- if (!cpumask_test_cpu(cpu, to_cpumask(f->flush_cpumask)))
- goto out;
- /*
- * This was a BUG() but until someone can quote me the
- * line from the intel manual that guarantees an IPI to
- * multiple CPUs is retried _only_ on the erroring CPUs
- * its staying as a return
- *
- * BUG();
- */
-
- if (f->flush_mm == percpu_read(cpu_tlbstate.active_mm)) {
- if (percpu_read(cpu_tlbstate.state) == TLBSTATE_OK) {
- if (f->flush_end == TLB_FLUSH_ALL
- || !cpu_has_invlpg)
- local_flush_tlb();
- else if (!f->flush_end)
- __flush_tlb_single(f->flush_start);
- else {
- unsigned long addr;
- addr = f->flush_start;
- while (addr <= f->flush_end) {
- __flush_tlb_single(addr);
- addr += PAGE_SIZE;
- }
- }
- } else
- leave_mm(cpu);
- }
-out:
- ack_APIC_irq();
- smp_mb__before_clear_bit();
- cpumask_clear_cpu(cpu, to_cpumask(f->flush_cpumask));
- smp_mb__after_clear_bit();
- inc_irq_stat(irq_tlb_count);
-}
+ struct flush_tlb_info *f = info;

-static void flush_tlb_others_ipi(const struct cpumask *cpumask,
- struct mm_struct *mm, unsigned long start,
- unsigned long end)
-{
- unsigned int sender;
- union smp_flush_state *f;
-
- /* Caller has disabled preemption */
- sender = this_cpu_read(tlb_vector_offset);
- f = &flush_state[sender];
-
- if (nr_cpu_ids > NUM_INVALIDATE_TLB_VECTORS)
- raw_spin_lock(&f->tlbstate_lock);
-
- f->flush_mm = mm;
- f->flush_start = start;
- f->flush_end = end;
- if (cpumask_andnot(to_cpumask(f->flush_cpumask), cpumask, cpumask_of(smp_processor_id()))) {
- /*
- * We have to send the IPI only to
- * CPUs affected.
- */
- apic->send_IPI_mask(to_cpumask(f->flush_cpumask),
- INVALIDATE_TLB_VECTOR_START + sender);
-
- while (!cpumask_empty(to_cpumask(f->flush_cpumask)))
- cpu_relax();
- }
+ if (f->flush_mm != percpu_read(cpu_tlbstate.active_mm))
+ return;
+
+ if (percpu_read(cpu_tlbstate.state) == TLBSTATE_OK) {
+ if (f->flush_end == TLB_FLUSH_ALL || !cpu_has_invlpg)
+ local_flush_tlb();
+ else if (!f->flush_end)
+ __flush_tlb_single(f->flush_start);
+ else {
+ unsigned long addr;
+ addr = f->flush_start;
+ while (addr <= f->flush_end) {
+ __flush_tlb_single(addr);
+ addr += PAGE_SIZE;
+ }
+ }
+ } else
+ leave_mm(smp_processor_id());

- f->flush_mm = NULL;
- f->flush_start = 0;
- f->flush_end = 0;
- if (nr_cpu_ids > NUM_INVALIDATE_TLB_VECTORS)
- raw_spin_unlock(&f->tlbstate_lock);
}

void native_flush_tlb_others(const struct cpumask *cpumask,
struct mm_struct *mm, unsigned long start,
unsigned long end)
{
+ struct flush_tlb_info info;
+ info.flush_mm = mm;
+ info.flush_start = start;
+ info.flush_end = end;
+
if (is_uv_system()) {
unsigned int cpu;

cpu = smp_processor_id();
cpumask = uv_flush_tlb_others(cpumask, mm, start, end, cpu);
if (cpumask)
- flush_tlb_others_ipi(cpumask, mm, start, end);
+ smp_call_function_many(cpumask, flush_tlb_func,
+ &info, 1);
return;
}
- flush_tlb_others_ipi(cpumask, mm, start, end);
-}
-
-static void __cpuinit calculate_tlb_offset(void)
-{
- int cpu, node, nr_node_vecs, idx = 0;
- /*
- * we are changing tlb_vector_offset for each CPU in runtime, but this
- * will not cause inconsistency, as the write is atomic under X86. we
- * might see more lock contentions in a short time, but after all CPU's
- * tlb_vector_offset are changed, everything should go normal
- *
- * Note: if NUM_INVALIDATE_TLB_VECTORS % nr_online_nodes !=0, we might
- * waste some vectors.
- **/
- if (nr_online_nodes > NUM_INVALIDATE_TLB_VECTORS)
- nr_node_vecs = 1;
- else
- nr_node_vecs = NUM_INVALIDATE_TLB_VECTORS/nr_online_nodes;
-
- for_each_online_node(node) {
- int node_offset = (idx % NUM_INVALIDATE_TLB_VECTORS) *
- nr_node_vecs;
- int cpu_offset = 0;
- for_each_cpu(cpu, cpumask_of_node(node)) {
- per_cpu(tlb_vector_offset, cpu) = node_offset +
- cpu_offset;
- cpu_offset++;
- cpu_offset = cpu_offset % nr_node_vecs;
- }
- idx++;
- }
-}
-
-static int __cpuinit tlb_cpuhp_notify(struct notifier_block *n,
- unsigned long action, void *hcpu)
-{
- switch (action & 0xf) {
- case CPU_ONLINE:
- case CPU_DEAD:
- calculate_tlb_offset();
- }
- return NOTIFY_OK;
-}
-
-static int __cpuinit init_smp_flush(void)
-{
- int i;
-
- for (i = 0; i < ARRAY_SIZE(flush_state); i++)
- raw_spin_lock_init(&flush_state[i].tlbstate_lock);
-
- calculate_tlb_offset();
- hotcpu_notifier(tlb_cpuhp_notify, 0);
- return 0;
+ smp_call_function_many(cpumask, flush_tlb_func, &info, 1);
}
-core_initcall(init_smp_flush);

void flush_tlb_current_task(void)
{
--
1.7.5.4

2012-05-23 14:18:10

by Alex Shi

[permalink] [raw]
Subject: [PATCH v7 8/8] x86/tlb: just do tlb flush on one of siblings of SMT

According to Intel's SDM, flush tlb on both of siblings of SMT is
just wasting time, no any benefit and hurt performance. Because SMT
siblings share the all levels TLB and page structure caches.

Random flush sibling can make mulitiple thread run more balance.
Here rand calculated from jiffies, that is a bit less heavy than
random32()(save 2/3 time on my NHM EP, and 1/2 on my SNB EP)

The patched tested with my macro benchmark munmap, that sent
to lkml before. http://lkml.org/lkml/2012/5/17/59

On my 2P * 4 cores * HT NHM EP machine, munmap system call speed
increased 10~15%, while average random memory access speed on other
LCPUs increase 12%.

On my 2P * 8 cores * HT SNB EP machine, munmap system call speed
increased 10~13%, while average random memory access speed on other
LCPUs increase 4~20%.

Signed-off-by: Alex Shi <[email protected]>
---
arch/x86/mm/tlb.c | 30 +++++++++++++++++++++++++++---
1 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
index 0232e24..bc0a6fc 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -85,22 +85,46 @@ void native_flush_tlb_others(const struct cpumask *cpumask,
struct mm_struct *mm, unsigned long start,
unsigned long end)
{
+ int cpu;
+ unsigned long rand;
struct flush_tlb_info info;
+ cpumask_t flush_mask, *sblmask;
+
info.flush_mm = mm;
info.flush_start = start;
info.flush_end = end;

+ /* doing flush on both siblings of SMT is just wasting time */
+ cpumask_copy(&flush_mask, cpumask);
+ if (likely(smp_num_siblings > 1)) {
+ rand = jiffies;
+ /* See "Numerical Recipes in C", second edition, p. 284 */
+ rand = rand * 1664525L + 1013904223L;
+ rand &= 0x1;
+
+ for_each_cpu(cpu, &flush_mask) {
+ sblmask = cpu_sibling_mask(cpu);
+ if (cpumask_subset(sblmask, &flush_mask)) {
+ if (rand == 0)
+ cpu_clear(cpu, flush_mask);
+ else
+ cpu_clear(cpumask_next(cpu, sblmask),
+ flush_mask);
+ }
+ }
+ }
+
if (is_uv_system()) {
unsigned int cpu;

cpu = smp_processor_id();
- cpumask = uv_flush_tlb_others(cpumask, mm, start, end, cpu);
+ cpumask = uv_flush_tlb_others(&flush_mask, mm, start, end, cpu);
if (cpumask)
- smp_call_function_many(cpumask, flush_tlb_func,
+ smp_call_function_many(&flush_mask, flush_tlb_func,
&info, 1);
return;
}
- smp_call_function_many(cpumask, flush_tlb_func, &info, 1);
+ smp_call_function_many(&flush_mask, flush_tlb_func, &info, 1);
}

void flush_tlb_current_task(void)
--
1.7.5.4

2012-05-23 14:59:14

by Jan Beulich

[permalink] [raw]
Subject: Re: [PATCH v7 2/8] x86/flush_tlb: try flush_tlb_single one by one in flush_tlb_range

>>> On 23.05.12 at 16:15, Alex Shi <[email protected]> wrote:
> @@ -1269,11 +1270,11 @@ static void xen_flush_tlb_others(const struct cpumask *cpus,
> cpumask_and(to_cpumask(args->mask), cpus, cpu_online_mask);
> cpumask_clear_cpu(smp_processor_id(), to_cpumask(args->mask));
>
> - if (va == TLB_FLUSH_ALL) {
> + if (start == TLB_FLUSH_ALL) {
> args->op.cmd = MMUEXT_TLB_FLUSH_MULTI;
> } else {
> args->op.cmd = MMUEXT_INVLPG_MULTI;
> - args->op.arg1.linear_addr = va;
> + args->op.arg1.linear_addr = start;
> }
>
> MULTI_mmuext_op(mcs.mc, &args->op, 1, NULL, DOMID_SELF);

Unless there is an implicit assumption that 'start' and 'end' are on
the same page (which I doubt, as then it would be pointless to
add 'end' here), this one is definitely wrong - you'd either have
to issue multiple MMUEXT_INVLPG_MULTI-s, or you'd have to
also use MMUEXT_TLB_FLUSH_MULTI for the multi-page case.

The same would appear to apply to the UV case, albeit I don't
know enough about that code to be certain.

Jan

2012-05-23 15:04:55

by Jan Beulich

[permalink] [raw]
Subject: Re: [PATCH v7 8/8] x86/tlb: just do tlb flush on one of siblings of SMT

>>> On 23.05.12 at 16:15, Alex Shi <[email protected]> wrote:
> + /* doing flush on both siblings of SMT is just wasting time */
> + cpumask_copy(&flush_mask, cpumask);
> + if (likely(smp_num_siblings > 1)) {
> + rand = jiffies;
> + /* See "Numerical Recipes in C", second edition, p. 284 */
> + rand = rand * 1664525L + 1013904223L;
> + rand &= 0x1;
> +
> + for_each_cpu(cpu, &flush_mask) {
> + sblmask = cpu_sibling_mask(cpu);
> + if (cpumask_subset(sblmask, &flush_mask)) {
> + if (rand == 0)
> + cpu_clear(cpu, flush_mask);
> + else
> + cpu_clear(cpumask_next(cpu, sblmask),
> + flush_mask);
> + }
> + }
> + }
> +

There is no comment or anything else indicating that this is
suitable for dual-thread CPUs only - when there are more than
2 threads per core, the intended effect won't be achieved. I'd
recommend making the logic generic from the beginning, but if
that doesn't seem feasible to you, at least a comment stating
the limitation should be added imo.

Jan

2012-05-23 17:09:58

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH v7 8/8] x86/tlb: just do tlb flush on one of siblings of SMT

On Wed, 2012-05-23 at 16:05 +0100, Jan Beulich wrote:
> >>> On 23.05.12 at 16:15, Alex Shi <[email protected]> wrote:
> > + /* doing flush on both siblings of SMT is just wasting time */
> > + cpumask_copy(&flush_mask, cpumask);
> > + if (likely(smp_num_siblings > 1)) {
> > + rand = jiffies;
> > + /* See "Numerical Recipes in C", second edition, p. 284 */
> > + rand = rand * 1664525L + 1013904223L;
> > + rand &= 0x1;
> > +
> > + for_each_cpu(cpu, &flush_mask) {
> > + sblmask = cpu_sibling_mask(cpu);
> > + if (cpumask_subset(sblmask, &flush_mask)) {
> > + if (rand == 0)
> > + cpu_clear(cpu, flush_mask);
> > + else
> > + cpu_clear(cpumask_next(cpu, sblmask),
> > + flush_mask);
> > + }
> > + }
> > + }
> > +
>
> There is no comment or anything else indicating that this is
> suitable for dual-thread CPUs only - when there are more than
> 2 threads per core, the intended effect won't be achieved.

Why would that be? Won't higher thread count still share the same
resources just more so?

> I'd
> recommend making the logic generic from the beginning, but if
> that doesn't seem feasible to you, at least a comment stating
> the limitation should be added imo.

My objection to the whole lot is that its looks mightily expensive on
large machines, cpumask operations aren't cheap when you've got 4k cpus
etc..

Also, you very much cannot put cpumask_t on stack.

2012-05-23 17:15:48

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH v7 8/8] x86/tlb: just do tlb flush on one of siblings of SMT

On Wed, 2012-05-23 at 19:09 +0200, Peter Zijlstra wrote:
> > There is no comment or anything else indicating that this is
> > suitable for dual-thread CPUs only - when there are more than
> > 2 threads per core, the intended effect won't be achieved.
>
> Why would that be? Won't higher thread count still share the same
> resources just more so?

Ah, I see, you're saying his code is buggy for >2 threads. Agreed.

2012-05-24 01:47:20

by Andrew Lutomirski

[permalink] [raw]
Subject: Re: [PATCH v7 8/8] x86/tlb: just do tlb flush on one of siblings of SMT

On Wed, May 23, 2012 at 10:15 AM, Peter Zijlstra <[email protected]> wrote:
> On Wed, 2012-05-23 at 19:09 +0200, Peter Zijlstra wrote:
>> > There is no comment or anything else indicating that this is
>> > suitable for dual-thread CPUs only - when there are more than
>> > 2 threads per core, the intended effect won't be achieved.
>>
>> Why would that be? Won't higher thread count still share the same
>> resources just more so?
>
> Ah, I see, you're saying his code is buggy for >2 threads. Agreed.
>

An evil knob to statically choose which SMT sibling gets the interrupt
would be nice. Then my compute-intensive thread could be (mostly)
unaffected by the other thread on a different core that calls munmap
frequently.

--Andy

2012-05-24 05:14:29

by Alex Shi

[permalink] [raw]
Subject: Re: [PATCH v7 8/8] x86/tlb: just do tlb flush on one of siblings of SMT

On 05/24/2012 09:46 AM, Andrew Lutomirski wrote:

> On Wed, May 23, 2012 at 10:15 AM, Peter Zijlstra <[email protected]> wrote:
>> On Wed, 2012-05-23 at 19:09 +0200, Peter Zijlstra wrote:
>>>> There is no comment or anything else indicating that this is
>>>> suitable for dual-thread CPUs only - when there are more than
>>>> 2 threads per core, the intended effect won't be achieved.
>>>
>>> Why would that be? Won't higher thread count still share the same
>>> resources just more so?
>>
>> Ah, I see, you're saying his code is buggy for >2 threads. Agreed.
>>
>
> An evil knob to statically choose which SMT sibling gets the interrupt
> would be nice. Then my compute-intensive thread could be (mostly)
> unaffected by the other thread on a different core that calls munmap
> frequently.


How to know we are in such situation? :)

>
> --Andy

2012-05-24 06:04:24

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v7 8/8] x86/tlb: just do tlb flush on one of siblings of SMT

On Thu, May 24, 2012 at 01:12:46PM +0800, Alex Shi wrote:
> On 05/24/2012 09:46 AM, Andrew Lutomirski wrote:
>
> > On Wed, May 23, 2012 at 10:15 AM, Peter Zijlstra <[email protected]> wrote:
> >> On Wed, 2012-05-23 at 19:09 +0200, Peter Zijlstra wrote:
> >>>> There is no comment or anything else indicating that this is
> >>>> suitable for dual-thread CPUs only - when there are more than
> >>>> 2 threads per core, the intended effect won't be achieved.
> >>>
> >>> Why would that be? Won't higher thread count still share the same
> >>> resources just more so?
> >>
> >> Ah, I see, you're saying his code is buggy for >2 threads. Agreed.
> >>
> >
> > An evil knob to statically choose which SMT sibling gets the interrupt
> > would be nice. Then my compute-intensive thread could be (mostly)
> > unaffected by the other thread on a different core that calls munmap
> > frequently.
>
> How to know we are in such situation? :)

Please, not yet another knob, we have too many as it is.

Can't you figure this out from the topology, smt siblings or whatever
this is called:

/sys/devices/system/cpu/cpu0/topology/thread_siblings

?

--
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

2012-05-24 06:43:29

by Alex Shi

[permalink] [raw]
Subject: Re: [PATCH v7 2/8] x86/flush_tlb: try flush_tlb_single one by one in flush_tlb_range

On 05/23/2012 10:51 PM, Jan Beulich wrote:

>>>> On 23.05.12 at 16:15, Alex Shi <[email protected]> wrote:
>> @@ -1269,11 +1270,11 @@ static void xen_flush_tlb_others(const struct cpumask *cpus,
>> cpumask_and(to_cpumask(args->mask), cpus, cpu_online_mask);
>> cpumask_clear_cpu(smp_processor_id(), to_cpumask(args->mask));
>>
>> - if (va == TLB_FLUSH_ALL) {
>> + if (start == TLB_FLUSH_ALL) {
>> args->op.cmd = MMUEXT_TLB_FLUSH_MULTI;
>> } else {
>> args->op.cmd = MMUEXT_INVLPG_MULTI;
>> - args->op.arg1.linear_addr = va;
>> + args->op.arg1.linear_addr = start;
>> }
>>
>> MULTI_mmuext_op(mcs.mc, &args->op, 1, NULL, DOMID_SELF);
>
> Unless there is an implicit assumption that 'start' and 'end' are on
> the same page (which I doubt, as then it would be pointless to
> add 'end' here), this one is definitely wrong - you'd either have
> to issue multiple MMUEXT_INVLPG_MULTI-s, or you'd have to
> also use MMUEXT_TLB_FLUSH_MULTI for the multi-page case.






Thanks comments!
So, the following change should be more safe for PV?

- if (va == TLB_FLUSH_ALL) {
- args->op.cmd = MMUEXT_TLB_FLUSH_MULTI;
- } else {
- args->op.cmd = MMUEXT_INVLPG_MULTI;
- args->op.arg1.linear_addr = va;
- }
+ args->op.cmd = MMUEXT_TLB_FLUSH_MULTI;
+ if (start != TLB_FLUSH_ALL)
+ args->op.arg1.linear_addr = start;

MULTI_mmuext_op(mcs.mc, &args->op, 1, NULL, DOMID_SELF);

Do you know why the old patch is past the performance testing of Yongjie?
https://lkml.org/lkml/2012/5/4/20

---

>From a43af44448039f15427de1901f9c2c7bf3cca698 Mon Sep 17 00:00:00 2001
From: Alex Shi <[email protected]>
Date: Wed, 9 May 2012 15:09:08 -0600
Subject: [PATCH 2/8] x86/flush_tlb: try flush_tlb_single one by one in
flush_tlb_range

x86 has no flush_tlb_range support in instruction level. Currently the
flush_tlb_range just implemented by flushing all page table. That is not
the best solution for all scenarios. In fact, if we just use 'invlpg' to
flush few lines from TLB, we can get the performance gain from later
remain TLB lines accessing.

But the 'invlpg' instruction costs much of time. Its execution time can
compete with cr3 rewriting, and even a bit more on SNB CPU.

So, on a 512 4KB TLB entries CPU, the balance points is at:
(512 - X) * 100ns(assumed TLB refill cost) =
X(TLB flush entries) * 100ns(assumed invlpg cost)

Here, X is 256, that is 1/2 of 512 entries.

But with the mysterious CPU pre-fetcher and page miss handler Unit, the
assumed TLB refill cost is far lower then 100ns in sequential access. And
2 HT siblings in one core makes the memory access more faster if they are
accessing the same memory. So, in the patch, I just do the change when
the target entries is less than 1/16 of whole active tlb entries.
Actually, I have no data support for the percentage '1/16', so any
suggestions are welcomed.

As to hugetlb, guess due to smaller page table, and smaller active TLB
entries, I didn't see benefit via my benchmark, so no optimizing now.

My macro benchmark show in ideal scenarios, the performance improves 70
percent in reading. And in worst scenario, the reading/writing
performance is similar with unpatched 3.4-rc4 kernel.

Here is the reading data on my 2P * 4cores *HT NHM EP machine, with THP
'always':

multi thread testing, '-t' paramter is thread number:
with patch unpatched 3.4-rc4
./mprotect -t 1 14ns 24ns
./mprotect -t 2 13ns 22ns
./mprotect -t 4 12ns 19ns
./mprotect -t 8 14ns 16ns
./mprotect -t 16 28ns 26ns
./mprotect -t 32 54ns 51ns
./mprotect -t 128 200ns 199ns

Single process with sequencial flushing and memory accessing:

with patch unpatched 3.4-rc4
./mprotect 7ns 11ns
./mprotect -p 4096 -l 8 -n 10240
21ns 21ns

I also tried other benchmarks on Intel core2/NHM/SNB EP and NHM EX machine.
No clear performance change on specjbb2005 with openjdk, and oltp reading.

Signed-off-by: Alex Shi <[email protected]>
---
arch/x86/include/asm/paravirt.h | 5 +-
arch/x86/include/asm/paravirt_types.h | 3 +-
arch/x86/include/asm/tlbflush.h | 23 +++-----
arch/x86/include/asm/uv/uv.h | 5 +-
arch/x86/mm/tlb.c | 97 +++++++++++++++++++++++++++------
arch/x86/platform/uv/tlb_uv.c | 6 +-
arch/x86/xen/mmu.c | 14 ++---
include/trace/events/xen.h | 12 +++--
8 files changed, 114 insertions(+), 51 deletions(-)

diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index aa0f913..03da4ab 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -397,9 +397,10 @@ static inline void __flush_tlb_single(unsigned long addr)

static inline void flush_tlb_others(const struct cpumask *cpumask,
struct mm_struct *mm,
- unsigned long va)
+ unsigned long start,
+ unsigned long end)
{
- PVOP_VCALL3(pv_mmu_ops.flush_tlb_others, cpumask, mm, va);
+ PVOP_VCALL4(pv_mmu_ops.flush_tlb_others, cpumask, mm, start, end);
}

static inline int paravirt_pgd_alloc(struct mm_struct *mm)
diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
index 8e8b9a4..600a5fcac9 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -250,7 +250,8 @@ struct pv_mmu_ops {
void (*flush_tlb_single)(unsigned long addr);
void (*flush_tlb_others)(const struct cpumask *cpus,
struct mm_struct *mm,
- unsigned long va);
+ unsigned long start,
+ unsigned long end);

/* Hooks for allocating and freeing a pagetable top-level */
int (*pgd_alloc)(struct mm_struct *mm);
diff --git a/arch/x86/include/asm/tlbflush.h b/arch/x86/include/asm/tlbflush.h
index c0e108e..51f8b1c 100644
--- a/arch/x86/include/asm/tlbflush.h
+++ b/arch/x86/include/asm/tlbflush.h
@@ -77,14 +77,10 @@ static inline void __flush_tlb_one(unsigned long addr)
* - flush_tlb_page(vma, vmaddr) flushes one page
* - flush_tlb_range(vma, start, end) flushes a range of pages
* - flush_tlb_kernel_range(start, end) flushes a range of kernel pages
- * - flush_tlb_others(cpumask, mm, va) flushes TLBs on other cpus
+ * - flush_tlb_others(cpumask, mm, start, end) flushes TLBs on other cpus
*
* ..but the i386 has somewhat limited tlb flushing capabilities,
* and page-granular flushes are available only on i486 and up.
- *
- * x86-64 can only flush individual pages or full VMs. For a range flush
- * we always do the full VM. Might be worth trying if for a small
- * range a few INVLPGs in a row are a win.
*/

#ifndef CONFIG_SMP
@@ -115,7 +111,8 @@ static inline void flush_tlb_range(struct vm_area_struct *vma,

static inline void native_flush_tlb_others(const struct cpumask *cpumask,
struct mm_struct *mm,
- unsigned long va)
+ unsigned long start,
+ unsigned long end)
{
}

@@ -133,17 +130,14 @@ extern void flush_tlb_all(void);
extern void flush_tlb_current_task(void);
extern void flush_tlb_mm(struct mm_struct *);
extern void flush_tlb_page(struct vm_area_struct *, unsigned long);
+extern void flush_tlb_range(struct vm_area_struct *vma,
+ unsigned long start, unsigned long end);

#define flush_tlb() flush_tlb_current_task()

-static inline void flush_tlb_range(struct vm_area_struct *vma,
- unsigned long start, unsigned long end)
-{
- flush_tlb_mm(vma->vm_mm);
-}
-
void native_flush_tlb_others(const struct cpumask *cpumask,
- struct mm_struct *mm, unsigned long va);
+ struct mm_struct *mm,
+ unsigned long start, unsigned long end);

#define TLBSTATE_OK 1
#define TLBSTATE_LAZY 2
@@ -163,7 +157,8 @@ static inline void reset_lazy_tlbstate(void)
#endif /* SMP */

#ifndef CONFIG_PARAVIRT
-#define flush_tlb_others(mask, mm, va) native_flush_tlb_others(mask, mm, va)
+#define flush_tlb_others(mask, mm, start, end) \
+ native_flush_tlb_others(mask, mm, start, end)
#endif

static inline void flush_tlb_kernel_range(unsigned long start,
diff --git a/arch/x86/include/asm/uv/uv.h b/arch/x86/include/asm/uv/uv.h
index 3bb9491..b47c2a8 100644
--- a/arch/x86/include/asm/uv/uv.h
+++ b/arch/x86/include/asm/uv/uv.h
@@ -15,7 +15,8 @@ extern void uv_nmi_init(void);
extern void uv_system_init(void);
extern const struct cpumask *uv_flush_tlb_others(const struct cpumask *cpumask,
struct mm_struct *mm,
- unsigned long va,
+ unsigned long start,
+ unsigned end,
unsigned int cpu);

#else /* X86_UV */
@@ -26,7 +27,7 @@ static inline void uv_cpu_init(void) { }
static inline void uv_system_init(void) { }
static inline const struct cpumask *
uv_flush_tlb_others(const struct cpumask *cpumask, struct mm_struct *mm,
- unsigned long va, unsigned int cpu)
+ unsigned long start, unsigned long end, unsigned int cpu)
{ return cpumask; }

#endif /* X86_UV */
diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
index d6c0418..7d92079 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -41,7 +41,8 @@ DEFINE_PER_CPU_SHARED_ALIGNED(struct tlb_state, cpu_tlbstate)
union smp_flush_state {
struct {
struct mm_struct *flush_mm;
- unsigned long flush_va;
+ unsigned long flush_start;
+ unsigned long flush_end;
raw_spinlock_t tlbstate_lock;
DECLARE_BITMAP(flush_cpumask, NR_CPUS);
};
@@ -154,10 +155,19 @@ void smp_invalidate_interrupt(struct pt_regs *regs)

if (f->flush_mm == percpu_read(cpu_tlbstate.active_mm)) {
if (percpu_read(cpu_tlbstate.state) == TLBSTATE_OK) {
- if (f->flush_va == TLB_FLUSH_ALL)
+ if (f->flush_end == TLB_FLUSH_ALL
+ || !cpu_has_invlpg)
local_flush_tlb();
- else
- __flush_tlb_one(f->flush_va);
+ else if (!f->flush_end)
+ __flush_tlb_single(f->flush_start);
+ else {
+ unsigned long addr;
+ addr = f->flush_start;
+ while (addr <= f->flush_end) {
+ __flush_tlb_single(addr);
+ addr += PAGE_SIZE;
+ }
+ }
} else
leave_mm(cpu);
}
@@ -170,7 +180,8 @@ out:
}

static void flush_tlb_others_ipi(const struct cpumask *cpumask,
- struct mm_struct *mm, unsigned long va)
+ struct mm_struct *mm, unsigned long start,
+ unsigned long end)
{
unsigned int sender;
union smp_flush_state *f;
@@ -183,7 +194,8 @@ static void flush_tlb_others_ipi(const struct cpumask *cpumask,
raw_spin_lock(&f->tlbstate_lock);

f->flush_mm = mm;
- f->flush_va = va;
+ f->flush_start = start;
+ f->flush_end = end;
if (cpumask_andnot(to_cpumask(f->flush_cpumask), cpumask, cpumask_of(smp_processor_id()))) {
/*
* We have to send the IPI only to
@@ -197,24 +209,26 @@ static void flush_tlb_others_ipi(const struct cpumask *cpumask,
}

f->flush_mm = NULL;
- f->flush_va = 0;
+ f->flush_start = 0;
+ f->flush_end = 0;
if (nr_cpu_ids > NUM_INVALIDATE_TLB_VECTORS)
raw_spin_unlock(&f->tlbstate_lock);
}

void native_flush_tlb_others(const struct cpumask *cpumask,
- struct mm_struct *mm, unsigned long va)
+ struct mm_struct *mm, unsigned long start,
+ unsigned long end)
{
if (is_uv_system()) {
unsigned int cpu;

cpu = smp_processor_id();
- cpumask = uv_flush_tlb_others(cpumask, mm, va, cpu);
+ cpumask = uv_flush_tlb_others(cpumask, mm, start, end, cpu);
if (cpumask)
- flush_tlb_others_ipi(cpumask, mm, va);
+ flush_tlb_others_ipi(cpumask, mm, start, end);
return;
}
- flush_tlb_others_ipi(cpumask, mm, va);
+ flush_tlb_others_ipi(cpumask, mm, start, end);
}

static void __cpuinit calculate_tlb_offset(void)
@@ -280,7 +294,7 @@ void flush_tlb_current_task(void)

local_flush_tlb();
if (cpumask_any_but(mm_cpumask(mm), smp_processor_id()) < nr_cpu_ids)
- flush_tlb_others(mm_cpumask(mm), mm, TLB_FLUSH_ALL);
+ flush_tlb_others(mm_cpumask(mm), mm, 0UL, TLB_FLUSH_ALL);
preempt_enable();
}

@@ -295,12 +309,63 @@ void flush_tlb_mm(struct mm_struct *mm)
leave_mm(smp_processor_id());
}
if (cpumask_any_but(mm_cpumask(mm), smp_processor_id()) < nr_cpu_ids)
- flush_tlb_others(mm_cpumask(mm), mm, TLB_FLUSH_ALL);
+ flush_tlb_others(mm_cpumask(mm), mm, 0UL, TLB_FLUSH_ALL);
+
+ preempt_enable();
+}
+
+#define FLUSHALL_BAR 16
+
+void flush_tlb_range(struct vm_area_struct *vma,
+ unsigned long start, unsigned long end)
+{
+ struct mm_struct *mm;
+
+ if (!cpu_has_invlpg || vma->vm_flags & VM_HUGETLB) {
+ flush_tlb_mm(vma->vm_mm);
+ return;
+ }
+
+ preempt_disable();
+ mm = vma->vm_mm;
+ if (current->active_mm == mm) {
+ if (current->mm) {
+ unsigned long addr, vmflag = vma->vm_flags;
+ unsigned act_entries, tlb_entries = 0;
+
+ if (vmflag & VM_EXEC)
+ tlb_entries = tlb_lli_4k[ENTRIES];
+ else
+ tlb_entries = tlb_lld_4k[ENTRIES];
+
+ act_entries = tlb_entries > mm->total_vm ?
+ mm->total_vm : tlb_entries;

+ if ((end - start)/PAGE_SIZE > act_entries/FLUSHALL_BAR)
+ local_flush_tlb();
+ else {
+ for (addr = start; addr <= end;
+ addr += PAGE_SIZE)
+ __flush_tlb_single(addr);
+
+ if (cpumask_any_but(mm_cpumask(mm),
+ smp_processor_id()) < nr_cpu_ids)
+ flush_tlb_others(mm_cpumask(mm), mm,
+ start, end);
+ preempt_enable();
+ return;
+ }
+ } else {
+ leave_mm(smp_processor_id());
+ }
+ }
+ if (cpumask_any_but(mm_cpumask(mm), smp_processor_id()) < nr_cpu_ids)
+ flush_tlb_others(mm_cpumask(mm), mm, 0UL, TLB_FLUSH_ALL);
preempt_enable();
}

-void flush_tlb_page(struct vm_area_struct *vma, unsigned long va)
+
+void flush_tlb_page(struct vm_area_struct *vma, unsigned long start)
{
struct mm_struct *mm = vma->vm_mm;

@@ -308,13 +373,13 @@ void flush_tlb_page(struct vm_area_struct *vma, unsigned long va)

if (current->active_mm == mm) {
if (current->mm)
- __flush_tlb_one(va);
+ __flush_tlb_one(start);
else
leave_mm(smp_processor_id());
}

if (cpumask_any_but(mm_cpumask(mm), smp_processor_id()) < nr_cpu_ids)
- flush_tlb_others(mm_cpumask(mm), mm, va);
+ flush_tlb_others(mm_cpumask(mm), mm, start, 0UL);

preempt_enable();
}
diff --git a/arch/x86/platform/uv/tlb_uv.c b/arch/x86/platform/uv/tlb_uv.c
index 3ae0e61..0df5ad2 100644
--- a/arch/x86/platform/uv/tlb_uv.c
+++ b/arch/x86/platform/uv/tlb_uv.c
@@ -1068,8 +1068,8 @@ static int set_distrib_bits(struct cpumask *flush_mask, struct bau_control *bcp,
* done. The returned pointer is valid till preemption is re-enabled.
*/
const struct cpumask *uv_flush_tlb_others(const struct cpumask *cpumask,
- struct mm_struct *mm, unsigned long va,
- unsigned int cpu)
+ struct mm_struct *mm, unsigned long start,
+ unsigned end, unsigned int cpu)
{
int locals = 0;
int remotes = 0;
@@ -1112,7 +1112,7 @@ const struct cpumask *uv_flush_tlb_others(const struct cpumask *cpumask,

record_send_statistics(stat, locals, hubs, remotes, bau_desc);

- bau_desc->payload.address = va;
+ bau_desc->payload.address = start;
bau_desc->payload.sending_cpu = cpu;
/*
* uv_flush_send_and_wait returns 0 if all cpu's were messaged,
diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
index 69f5857..34c9bad 100644
--- a/arch/x86/xen/mmu.c
+++ b/arch/x86/xen/mmu.c
@@ -1244,7 +1244,8 @@ static void xen_flush_tlb_single(unsigned long addr)
}

static void xen_flush_tlb_others(const struct cpumask *cpus,
- struct mm_struct *mm, unsigned long va)
+ struct mm_struct *mm, unsigned long start,
+ unsigned long end)
{
struct {
struct mmuext_op op;
@@ -1256,7 +1257,7 @@ static void xen_flush_tlb_others(const struct cpumask *cpus,
} *args;
struct multicall_space mcs;

- trace_xen_mmu_flush_tlb_others(cpus, mm, va);
+ trace_xen_mmu_flush_tlb_others(cpus, mm, start, end);

if (cpumask_empty(cpus))
return; /* nothing to do */
@@ -1269,12 +1270,9 @@ static void xen_flush_tlb_others(const struct cpumask *cpus,
cpumask_and(to_cpumask(args->mask), cpus, cpu_online_mask);
cpumask_clear_cpu(smp_processor_id(), to_cpumask(args->mask));

- if (va == TLB_FLUSH_ALL) {
- args->op.cmd = MMUEXT_TLB_FLUSH_MULTI;
- } else {
- args->op.cmd = MMUEXT_INVLPG_MULTI;
- args->op.arg1.linear_addr = va;
- }
+ args->op.cmd = MMUEXT_TLB_FLUSH_MULTI;
+ if (start != TLB_FLUSH_ALL)
+ args->op.arg1.linear_addr = start;

MULTI_mmuext_op(mcs.mc, &args->op, 1, NULL, DOMID_SELF);

diff --git a/include/trace/events/xen.h b/include/trace/events/xen.h
index 92f1a79..15ba03b 100644
--- a/include/trace/events/xen.h
+++ b/include/trace/events/xen.h
@@ -397,18 +397,20 @@ TRACE_EVENT(xen_mmu_flush_tlb_single,

TRACE_EVENT(xen_mmu_flush_tlb_others,
TP_PROTO(const struct cpumask *cpus, struct mm_struct *mm,
- unsigned long addr),
- TP_ARGS(cpus, mm, addr),
+ unsigned long addr, unsigned long end),
+ TP_ARGS(cpus, mm, addr, end),
TP_STRUCT__entry(
__field(unsigned, ncpus)
__field(struct mm_struct *, mm)
__field(unsigned long, addr)
+ __field(unsigned long, end)
),
TP_fast_assign(__entry->ncpus = cpumask_weight(cpus);
__entry->mm = mm;
- __entry->addr = addr),
- TP_printk("ncpus %d mm %p addr %lx",
- __entry->ncpus, __entry->mm, __entry->addr)
+ __entry->addr = addr,
+ __entry->end = end),
+ TP_printk("ncpus %d mm %p addr %lx, end %lx",
+ __entry->ncpus, __entry->mm, __entry->addr, __entry->end)
);

TRACE_EVENT(xen_mmu_write_cr3,
--
1.7.5.4

2012-05-24 07:40:48

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH v7 8/8] x86/tlb: just do tlb flush on one of siblings of SMT

On Wed, 2012-05-23 at 18:46 -0700, Andrew Lutomirski wrote:
> On Wed, May 23, 2012 at 10:15 AM, Peter Zijlstra <[email protected]> wrote:
> > On Wed, 2012-05-23 at 19:09 +0200, Peter Zijlstra wrote:
> >> > There is no comment or anything else indicating that this is
> >> > suitable for dual-thread CPUs only - when there are more than
> >> > 2 threads per core, the intended effect won't be achieved.
> >>
> >> Why would that be? Won't higher thread count still share the same
> >> resources just more so?
> >
> > Ah, I see, you're saying his code is buggy for >2 threads. Agreed.
> >
>
> An evil knob to statically choose which SMT sibling gets the interrupt
> would be nice. Then my compute-intensive thread could be (mostly)
> unaffected by the other thread on a different core that calls munmap
> frequently.

Just make sure the two workloads never share a core and this should
already happen since TLB invalidates are only broadcast to the mm
cpumask.

2012-05-24 08:11:40

by Jan Beulich

[permalink] [raw]
Subject: Re: [PATCH v7 2/8] x86/flush_tlb: try flush_tlb_single one by one in flush_tlb_range

>>> On 24.05.12 at 08:41, Alex Shi <[email protected]> wrote:
> On 05/23/2012 10:51 PM, Jan Beulich wrote:
>> Unless there is an implicit assumption that 'start' and 'end' are on
>> the same page (which I doubt, as then it would be pointless to
>> add 'end' here), this one is definitely wrong - you'd either have
>> to issue multiple MMUEXT_INVLPG_MULTI-s, or you'd have to
>> also use MMUEXT_TLB_FLUSH_MULTI for the multi-page case.
>
> Thanks comments!
> So, the following change should be more safe for PV?
>
> - if (va == TLB_FLUSH_ALL) {
> - args->op.cmd = MMUEXT_TLB_FLUSH_MULTI;
> - } else {
> - args->op.cmd = MMUEXT_INVLPG_MULTI;
> - args->op.arg1.linear_addr = va;
> - }
> + args->op.cmd = MMUEXT_TLB_FLUSH_MULTI;

This would be safe ...

> + if (start != TLB_FLUSH_ALL)
> + args->op.arg1.linear_addr = start;

... and then this superfluous, but it'd result in an unconditional
full TLB flush. When start and end (or perhaps end-1, assuming
end is not inclusive) are on the same page, MMUEXT_INVLPG_MULTI
should be used; MMUEXT_TLB_FLUSH_MULTI might need to be
used in all other cases, unless you want to split multi-page, non-
global invalidations into multiple MMUEXT_INVLPG_MULTI-s (which
would appear to be what the whole patch aims at).

Perhaps the abstraction layer needs to be changed instead:
Have the low level routines (Xen, UV, native) just deal with
single pages, and do the splitting in common code (using the
TLB size metrics).

But then again these metrics will become stale after a
migration (not only on Xen, but in all virtualization scenarios), so
some additional aspects will need to be taken care of anyway.

>
> MULTI_mmuext_op(mcs.mc, &args->op, 1, NULL, DOMID_SELF);
>
> Do you know why the old patch is past the performance testing of Yongjie?
> https://lkml.org/lkml/2012/5/4/20

No - mere luck perhaps (possibly because the invalidation policy
inside the hypervisor is strict enough to paper over the flaw
here)?

Jan

2012-05-24 08:33:45

by Alex Shi

[permalink] [raw]
Subject: Re: [PATCH v7 8/8] x86/tlb: just do tlb flush on one of siblings of SMT

On 05/24/2012 01:09 AM, Peter Zijlstra wrote:

> On Wed, 2012-05-23 at 16:05 +0100, Jan Beulich wrote:
>>>>> On 23.05.12 at 16:15, Alex Shi <[email protected]> wrote:
>>> + /* doing flush on both siblings of SMT is just wasting time */
>>> + cpumask_copy(&flush_mask, cpumask);
>>> + if (likely(smp_num_siblings > 1)) {
>>> + rand = jiffies;
>>> + /* See "Numerical Recipes in C", second edition, p. 284 */
>>> + rand = rand * 1664525L + 1013904223L;
>>> + rand &= 0x1;
>>> +
>>> + for_each_cpu(cpu, &flush_mask) {
>>> + sblmask = cpu_sibling_mask(cpu);
>>> + if (cpumask_subset(sblmask, &flush_mask)) {
>>> + if (rand == 0)
>>> + cpu_clear(cpu, flush_mask);
>>> + else
>>> + cpu_clear(cpumask_next(cpu, sblmask),
>>> + flush_mask);
>>> + }
>>> + }
>>> + }
>>> +
>>
>> There is no comment or anything else indicating that this is
>> suitable for dual-thread CPUs only - when there are more than
>> 2 threads per core, the intended effect won't be achieved.
>
> Why would that be? Won't higher thread count still share the same
> resources just more so?
>
>> I'd
>> recommend making the logic generic from the beginning, but if
>> that doesn't seem feasible to you, at least a comment stating
>> the limitation should be added imo.


Sure. but just want to know how many commercial x86 CPU uses >2 SMTs?
Write a short, quick function to do random selection in SMT is quite
complicate considering cpumask maybe just contain random number SMT
siblings in a core.

>
> My objection to the whole lot is that its looks mightily expensive on
> large machines, cpumask operations aren't cheap when you've got 4k cpus
> etc..
>
> Also, you very much cannot put cpumask_t on stack.


Sure, and do you has related data for this?

I just measured the cost of this function on my Romely EP(32 LCPUs) with
cpumask_t and NR_CPUS = 32/256/512/4096, the cost are similar with
256/512/4096 and that increased about 20% time cost from 32.

I also tried to use cpumask_var_t and alloc it in heap(use
CPUMASK_OFFSTACK), actually, it cost same time with cpumask_t in stack.
But, the allocation bring another big cost. So, I use cpumask_t in stack.
The performance gain data in commit log is getting with NR_CPUS = 256.

2012-05-24 08:43:04

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH v7 8/8] x86/tlb: just do tlb flush on one of siblings of SMT

On Thu, 2012-05-24 at 16:32 +0800, Alex Shi wrote:
> So, I use cpumask_t in stack.

cpumask_t is 512 bytes with NR_CPUS=4096, that's generally considered
too big to be on stack.

A number of people spend a lot of time removing cpumask_t from stacks a
while ago, I'm very sure they'll not be happy if you're going to add it
back.

Rusty, what was the plan on removing cpumask_t altogether?

2012-05-24 08:43:43

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH v7 8/8] x86/tlb: just do tlb flush on one of siblings of SMT

On Thu, 2012-05-24 at 16:32 +0800, Alex Shi wrote:
> The performance gain data in commit log is getting with NR_CPUS = 256.

Remember that most disto kernels are build with NR_CPUS=4096, so that is
what most people will run with.

2012-05-24 08:47:47

by Jan Beulich

[permalink] [raw]
Subject: Re: [PATCH v7 8/8] x86/tlb: just do tlb flush on one of siblings of SMT

>>> On 24.05.12 at 10:32, Alex Shi <[email protected]> wrote:
> On 05/24/2012 01:09 AM, Peter Zijlstra wrote:
>
>> On Wed, 2012-05-23 at 16:05 +0100, Jan Beulich wrote:
>>>>>> On 23.05.12 at 16:15, Alex Shi <[email protected]> wrote:
>>>> + /* doing flush on both siblings of SMT is just wasting time */
>>>> + cpumask_copy(&flush_mask, cpumask);
>>>> + if (likely(smp_num_siblings > 1)) {
>>>> + rand = jiffies;
>>>> + /* See "Numerical Recipes in C", second edition, p. 284 */
>>>> + rand = rand * 1664525L + 1013904223L;
>>>> + rand &= 0x1;
>>>> +
>>>> + for_each_cpu(cpu, &flush_mask) {
>>>> + sblmask = cpu_sibling_mask(cpu);
>>>> + if (cpumask_subset(sblmask, &flush_mask)) {
>>>> + if (rand == 0)
>>>> + cpu_clear(cpu, flush_mask);
>>>> + else
>>>> + cpu_clear(cpumask_next(cpu, sblmask),
>>>> + flush_mask);
>>>> + }
>>>> + }
>>>> + }
>>>> +
>>>
>>> There is no comment or anything else indicating that this is
>>> suitable for dual-thread CPUs only - when there are more than
>>> 2 threads per core, the intended effect won't be achieved.
>>
>> Why would that be? Won't higher thread count still share the same
>> resources just more so?
>>
>>> I'd
>>> recommend making the logic generic from the beginning, but if
>>> that doesn't seem feasible to you, at least a comment stating
>>> the limitation should be added imo.
>
>
> Sure. but just want to know how many commercial x86 CPU uses >2 SMTs?
> Write a short, quick function to do random selection in SMT is quite
> complicate considering cpumask maybe just contain random number SMT
> siblings in a core.

Which is why I wrote that a second best solution would be to
merely document the restriction in the source.

However, picking one out of more than 2 siblings shouldn't be
_that_ difficult.

>> My objection to the whole lot is that its looks mightily expensive on
>> large machines, cpumask operations aren't cheap when you've got 4k cpus
>> etc..
>>
>> Also, you very much cannot put cpumask_t on stack.
>
>
> Sure, and do you has related data for this?
>
> I just measured the cost of this function on my Romely EP(32 LCPUs) with
> cpumask_t and NR_CPUS = 32/256/512/4096, the cost are similar with
> 256/512/4096 and that increased about 20% time cost from 32.
>
> I also tried to use cpumask_var_t and alloc it in heap(use
> CPUMASK_OFFSTACK), actually, it cost same time with cpumask_t in stack.
> But, the allocation bring another big cost. So, I use cpumask_t in stack.
> The performance gain data in commit log is getting with NR_CPUS = 256.

Perhaps using a per-CPU cpumask would be the better choice here
(I can't see how preemption could validly be enabled when this
code is utilized).

Jan

2012-05-24 08:50:21

by Alex Shi

[permalink] [raw]
Subject: Re: [PATCH v7 8/8] x86/tlb: just do tlb flush on one of siblings of SMT

On 05/24/2012 04:42 PM, Peter Zijlstra wrote:

> On Thu, 2012-05-24 at 16:32 +0800, Alex Shi wrote:
>> So, I use cpumask_t in stack.
>
> cpumask_t is 512 bytes with NR_CPUS=4096, that's generally considered
> too big to be on stack.
>
> A number of people spend a lot of time removing cpumask_t from stacks a
> while ago, I'm very sure they'll not be happy if you're going to add it
> back.


In my testing, allocate a cpumask_var_t is more worse than cpumask_t.
So, another choice is using percpu pre-allocatd cpumask for this, but I
am wondering if it is acceptable. What's suggestion for this point?

>
> Rusty, what was the plan on removing cpumask_t altogether?
>

2012-05-24 08:57:41

by Alex Shi

[permalink] [raw]
Subject: Re: [PATCH v7 2/8] x86/flush_tlb: try flush_tlb_single one by one in flush_tlb_range

On 05/24/2012 04:12 PM, Jan Beulich wrote:

>>>> On 24.05.12 at 08:41, Alex Shi <[email protected]> wrote:
>> On 05/23/2012 10:51 PM, Jan Beulich wrote:
>>> Unless there is an implicit assumption that 'start' and 'end' are on
>>> the same page (which I doubt, as then it would be pointless to
>>> add 'end' here), this one is definitely wrong - you'd either have
>>> to issue multiple MMUEXT_INVLPG_MULTI-s, or you'd have to
>>> also use MMUEXT_TLB_FLUSH_MULTI for the multi-page case.
>>
>> Thanks comments!
>> So, the following change should be more safe for PV?
>>
>> - if (va == TLB_FLUSH_ALL) {
>> - args->op.cmd = MMUEXT_TLB_FLUSH_MULTI;
>> - } else {
>> - args->op.cmd = MMUEXT_INVLPG_MULTI;
>> - args->op.arg1.linear_addr = va;
>> - }
>> + args->op.cmd = MMUEXT_TLB_FLUSH_MULTI;
>
> This would be safe ...
>
>> + if (start != TLB_FLUSH_ALL)
>> + args->op.arg1.linear_addr = start;
>
> ... and then this superfluous, but it'd result in an unconditional
> full TLB flush. When start and end (or perhaps end-1, assuming
> end is not inclusive) are on the same page, MMUEXT_INVLPG_MULTI
> should be used; MMUEXT_TLB_FLUSH_MULTI might need to be
> used in all other cases, unless you want to split multi-page, non-
> global invalidations into multiple MMUEXT_INVLPG_MULTI-s (which
> would appear to be what the whole patch aims at).


args->op.cmd = MMUEXT_TLB_FLUSH_MULTI;
- if (start != TLB_FLUSH_ALL)
+ if (start != TLB_FLUSH_ALL && (end - start) < PAGE_SIZE) {
+ args->op.cmd = MMUEXT_INVLPG_MULTI;
args->op.arg1.linear_addr = start;
+ }

So, above it correct code for xen?
As to the xen optimisation of flush range, it is may better to be done
in a separate patch.

>
> Perhaps the abstraction layer needs to be changed instead:
> Have the low level routines (Xen, UV, native) just deal with
> single pages, and do the splitting in common code (using the
> TLB size metrics).
>
> But then again these metrics will become stale after a
> migration (not only on Xen, but in all virtualization scenarios), so
> some additional aspects will need to be taken care of anyway.


Sure, thanks for your care!

2012-05-24 09:04:01

by Alex Shi

[permalink] [raw]
Subject: Re: [PATCH v7 8/8] x86/tlb: just do tlb flush on one of siblings of SMT


>>
>> I just measured the cost of this function on my Romely EP(32 LCPUs) with
>> cpumask_t and NR_CPUS = 32/256/512/4096, the cost are similar with
>> 256/512/4096 and that increased about 20% time cost from 32.
>>
>> I also tried to use cpumask_var_t and alloc it in heap(use
>> CPUMASK_OFFSTACK), actually, it cost same time with cpumask_t in stack.
>> But, the allocation bring another big cost. So, I use cpumask_t in stack.
>> The performance gain data in commit log is getting with NR_CPUS = 256.
>
> Perhaps using a per-CPU cpumask would be the better choice here


See.

> (I can't see how preemption could validly be enabled when this
> code is utilized).


Sorry, What's your meaning here?, the function is always in pre-empt
safe mode.

>
> Jan
>

2012-05-24 09:29:27

by Alex Shi

[permalink] [raw]
Subject: Re: [PATCH v7 8/8] x86/tlb: just do tlb flush on one of siblings of SMT

On 05/24/2012 04:42 PM, Peter Zijlstra wrote:

> On Thu, 2012-05-24 at 16:32 +0800, Alex Shi wrote:
>> So, I use cpumask_t in stack.
>
> cpumask_t is 512 bytes with NR_CPUS=4096, that's generally considered
> too big to be on stack.


I am not consistent with cpumask_t.
I just want to know why it is too big on stack? Since if it causes
trouble in using, that mean current kernel will fail to run on 4096 CPU
system.

2012-05-24 09:42:57

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH v7 8/8] x86/tlb: just do tlb flush on one of siblings of SMT

On Thu, 2012-05-24 at 17:27 +0800, Alex Shi wrote:

> I just want to know why it is too big on stack? Since if it causes
> trouble in using, that mean current kernel will fail to run on 4096 CPU
> system.

Because the stack is only 8k or so. If you add functions with 512+ bytes
of footprint you're done very very quickly indeed.

Now if you were able to build a call-graph using a static analysis tool
and were able to weight each edge with the stack usage of every
function, finding the actual max stack would be doable. Lacking this its
a matter of policy and 'luck'.

Our policy is to use as little stack as possible and 512 bytes is
definitely out.

Our 'luck' is that every so often people over-run their stack and we get
to spend time on figuring out wtf happened.

2012-05-24 09:44:16

by Jan Beulich

[permalink] [raw]
Subject: Re: [PATCH v7 2/8] x86/flush_tlb: try flush_tlb_single one by one in flush_tlb_range

>>> On 24.05.12 at 10:55, Alex Shi <[email protected]> wrote:
> On 05/24/2012 04:12 PM, Jan Beulich wrote:
>
>>>>> On 24.05.12 at 08:41, Alex Shi <[email protected]> wrote:
>>> So, the following change should be more safe for PV?
>>>
>>> - if (va == TLB_FLUSH_ALL) {
>>> - args->op.cmd = MMUEXT_TLB_FLUSH_MULTI;
>>> - } else {
>>> - args->op.cmd = MMUEXT_INVLPG_MULTI;
>>> - args->op.arg1.linear_addr = va;
>>> - }
>>> + args->op.cmd = MMUEXT_TLB_FLUSH_MULTI;
>>
>> This would be safe ...
>>
>>> + if (start != TLB_FLUSH_ALL)
>>> + args->op.arg1.linear_addr = start;
>>
>> ... and then this superfluous, but it'd result in an unconditional
>> full TLB flush. When start and end (or perhaps end-1, assuming
>> end is not inclusive) are on the same page, MMUEXT_INVLPG_MULTI
>> should be used; MMUEXT_TLB_FLUSH_MULTI might need to be
>> used in all other cases, unless you want to split multi-page, non-
>> global invalidations into multiple MMUEXT_INVLPG_MULTI-s (which
>> would appear to be what the whole patch aims at).
>
>
> args->op.cmd = MMUEXT_TLB_FLUSH_MULTI;
> - if (start != TLB_FLUSH_ALL)
> + if (start != TLB_FLUSH_ALL && (end - start) < PAGE_SIZE) {

Assuming 'end' is not inclusive (you didn't clarify that),

if (start != TLB_FLUSH_ALL && (end - start) <= PAGE_SIZE) {

(and only if 'start' is guaranteed to be page aligned).

> + args->op.cmd = MMUEXT_INVLPG_MULTI;
> args->op.arg1.linear_addr = start;
> + }
>
> So, above it correct code for xen?
> As to the xen optimisation of flush range, it is may better to be done
> in a separate patch.

Agreed.

Jan

2012-05-24 09:45:07

by Jan Beulich

[permalink] [raw]
Subject: Re: [PATCH v7 8/8] x86/tlb: just do tlb flush on one of siblings of SMT

>>> On 24.05.12 at 11:02, Alex Shi <[email protected]> wrote:

>>>
>>> I just measured the cost of this function on my Romely EP(32 LCPUs) with
>>> cpumask_t and NR_CPUS = 32/256/512/4096, the cost are similar with
>>> 256/512/4096 and that increased about 20% time cost from 32.
>>>
>>> I also tried to use cpumask_var_t and alloc it in heap(use
>>> CPUMASK_OFFSTACK), actually, it cost same time with cpumask_t in stack.
>>> But, the allocation bring another big cost. So, I use cpumask_t in stack.
>>> The performance gain data in commit log is getting with NR_CPUS = 256.
>>
>> Perhaps using a per-CPU cpumask would be the better choice here
>
>
> See.
>
>> (I can't see how preemption could validly be enabled when this
>> code is utilized).
>
>
> Sorry, What's your meaning here?, the function is always in pre-empt
> safe mode.

That's exactly what I implied (I was merely pointing this out to
prove that using per-CPU data here is possible).

Jan

2012-05-24 09:46:21

by Jan Beulich

[permalink] [raw]
Subject: Re: [PATCH v7 8/8] x86/tlb: just do tlb flush on one of siblings of SMT

>>> On 24.05.12 at 11:27, Alex Shi <[email protected]> wrote:
> On 05/24/2012 04:42 PM, Peter Zijlstra wrote:
>
>> On Thu, 2012-05-24 at 16:32 +0800, Alex Shi wrote:
>>> So, I use cpumask_t in stack.
>>
>> cpumask_t is 512 bytes with NR_CPUS=4096, that's generally considered
>> too big to be on stack.
>
>
> I am not consistent with cpumask_t.
> I just want to know why it is too big on stack? Since if it causes
> trouble in using, that mean current kernel will fail to run on 4096 CPU
> system.

Why? Did you spot critical left-over instances of on-stack
cpumask_t-s anywhere?

Jan

2012-05-24 12:08:07

by Rusty Russell

[permalink] [raw]
Subject: Re: [PATCH v7 8/8] x86/tlb: just do tlb flush on one of siblings of SMT

On Thu, 24 May 2012 16:48:37 +0800, Alex Shi <[email protected]> wrote:
> On 05/24/2012 04:42 PM, Peter Zijlstra wrote:
>
> > On Thu, 2012-05-24 at 16:32 +0800, Alex Shi wrote:
> >> So, I use cpumask_t in stack.
> >
> > cpumask_t is 512 bytes with NR_CPUS=4096, that's generally considered
> > too big to be on stack.
> >
> > A number of people spend a lot of time removing cpumask_t from stacks a
> > while ago, I'm very sure they'll not be happy if you're going to add it
> > back.
>
>
> In my testing, allocate a cpumask_var_t is more worse than cpumask_t.
> So, another choice is using percpu pre-allocatd cpumask for this, but I
> am wondering if it is acceptable. What's suggestion for this point?

Thanks for the ping Peter!

Please don't use cpus_ operations: they're deprecated. Use cpumask_.
Similarly, avoid cpumask_t.

And yes, if you configure for thousands of CPUs, it's not free! If it's
a significant, you will want to use a per-cpu cpumask_var_t.

My other thought: your patch seems optimal as far as avoiding IPIs goes,
but I wonder how often it folds down to a single CPU? That case is
easier to fast-path without using a new cpumask.

Cheers,
Rusty.

2012-05-24 13:19:43

by Andrew Lutomirski

[permalink] [raw]
Subject: Re: [PATCH v7 8/8] x86/tlb: just do tlb flush on one of siblings of SMT

On Thu, May 24, 2012 at 12:40 AM, Peter Zijlstra <[email protected]> wrote:
> On Wed, 2012-05-23 at 18:46 -0700, Andrew Lutomirski wrote:
>> On Wed, May 23, 2012 at 10:15 AM, Peter Zijlstra <[email protected]> wrote:
>> > On Wed, 2012-05-23 at 19:09 +0200, Peter Zijlstra wrote:
>> >> > There is no comment or anything else indicating that this is
>> >> > suitable for dual-thread CPUs only - when there are more than
>> >> > 2 threads per core, the intended effect won't be achieved.
>> >>
>> >> Why would that be? Won't higher thread count still share the same
>> >> resources just more so?
>> >
>> > Ah, I see, you're saying his code is buggy for >2 threads. Agreed.
>> >
>>
>> An evil knob to statically choose which SMT sibling gets the interrupt
>> would be nice. ?Then my compute-intensive thread could be (mostly)
>> unaffected by the other thread on a different core that calls munmap
>> frequently.
>
> Just make sure the two workloads never share a core and this should
> already happen since TLB invalidates are only broadcast to the mm
> cpumask.
>

I already do that. But these two workloads share the same mm (they're
threads in the same process), so I'd gain performance if the
invalidations were sent from the core that called munmap to the SMT
sibling of the other thread.

A decent heuristic might be to prefer idle SMT siblings for TLB
invalidation. I don't know what effect that would have on power
consumption (it would be rather bad if idling one SMT thread while the
other one is busy saves much power).

--Andy

2012-05-24 13:23:42

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH v7 8/8] x86/tlb: just do tlb flush on one of siblings of SMT

On Thu, 2012-05-24 at 06:19 -0700, Andrew Lutomirski wrote:
>
> A decent heuristic might be to prefer idle SMT siblings for TLB
> invalidation. I don't know what effect that would have on power
> consumption (it would be rather bad if idling one SMT thread while the
> other one is busy saves much power).

Right, I've never really understood how C-states and SMT go together.
Arjan recently implied waking a thread sibling from C-state was
'expensive' which on first thought seems daft, the core is running
already.


2012-05-24 13:39:20

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [PATCH v7 8/8] x86/tlb: just do tlb flush on one of siblings of SMT

On 5/24/2012 6:23 AM, Peter Zijlstra wrote:
> On Thu, 2012-05-24 at 06:19 -0700, Andrew Lutomirski wrote:
>>
>> A decent heuristic might be to prefer idle SMT siblings for TLB
>> invalidation. I don't know what effect that would have on power
>> consumption (it would be rather bad if idling one SMT thread while the
>> other one is busy saves much power).

we really really shouldn't do flushing of tlb's on only one half of SMT.
SMT sibblings have their own TLB pool at least on some of Intels chips.

Also, note that on anything sane, we flush the tlb's in software before
going to an Idle state, so that we don't have to wake idle cpus up to
flush their TLBs (except for "global tlbs", but those change very very
very rarely hopefully)

>
> Right, I've never really understood how C-states and SMT go together.
> Arjan recently implied waking a thread sibling from C-state was
> 'expensive' which on first thought seems daft, the core is running
> already.

in order to wake *anything* you need to send an IPI to it, it has to
exit the idle loop etc etc. It's not expensive-expensive, but it
certainly is not free either.

2012-05-24 13:54:45

by Alex Shi

[permalink] [raw]
Subject: Re: [PATCH v7 8/8] x86/tlb: just do tlb flush on one of siblings of SMT

On 05/24/2012 09:39 PM, Arjan van de Ven wrote:

> On 5/24/2012 6:23 AM, Peter Zijlstra wrote:
>> On Thu, 2012-05-24 at 06:19 -0700, Andrew Lutomirski wrote:
>>>
>>> A decent heuristic might be to prefer idle SMT siblings for TLB
>>> invalidation. I don't know what effect that would have on power
>>> consumption (it would be rather bad if idling one SMT thread while the
>>> other one is busy saves much power).
>
> we really really shouldn't do flushing of tlb's on only one half of SMT.
> SMT sibblings have their own TLB pool at least on some of Intels chips.


That is also the biggest question I want to know. Actually, some
documents, wiki said the SMT sibling just has process registers and
interrupt part, no any tlb/l1 cache etc, (like intel's doc
vol6iss1_hyper_threading_technology.pdf). And the patch runs well on
NHM EP/WSM EP/NHM EX/SNB EP CPUs.

But hard to get such clearly per cpu info of SMT/HT, so, what the
detailed Intel chips has 'TLB pool' on SMT?

>
> Also, note that on anything sane, we flush the tlb's in software before
> going to an Idle state, so that we don't have to wake idle cpus up to
> flush their TLBs (except for "global tlbs", but those change very very
> very rarely hopefully)
>
>>
>> Right, I've never really understood how C-states and SMT go together.
>> Arjan recently implied waking a thread sibling from C-state was
>> 'expensive' which on first thought seems daft, the core is running
>> already.
>
> in order to wake *anything* you need to send an IPI to it, it has to
> exit the idle loop etc etc. It's not expensive-expensive, but it
> certainly is not free either.

2012-05-24 14:04:04

by Alex Shi

[permalink] [raw]
Subject: Re: [PATCH v7 8/8] x86/tlb: just do tlb flush on one of siblings of SMT

On 05/24/2012 07:35 PM, Rusty Russell wrote:

> On Thu, 24 May 2012 16:48:37 +0800, Alex Shi <[email protected]> wrote:
>> On 05/24/2012 04:42 PM, Peter Zijlstra wrote:
>>
>>> On Thu, 2012-05-24 at 16:32 +0800, Alex Shi wrote:
>>>> So, I use cpumask_t in stack.
>>>
>>> cpumask_t is 512 bytes with NR_CPUS=4096, that's generally considered
>>> too big to be on stack.
>>>
>>> A number of people spend a lot of time removing cpumask_t from stacks a
>>> while ago, I'm very sure they'll not be happy if you're going to add it
>>> back.
>>
>>
>> In my testing, allocate a cpumask_var_t is more worse than cpumask_t.
>> So, another choice is using percpu pre-allocatd cpumask for this, but I
>> am wondering if it is acceptable. What's suggestion for this point?
>
> Thanks for the ping Peter!
>
> Please don't use cpus_ operations: they're deprecated. Use cpumask_.
> Similarly, avoid cpumask_t.


Thanks, Rusty and Peter!

>
> And yes, if you configure for thousands of CPUs, it's not free! If it's
> a significant, you will want to use a per-cpu cpumask_var_t.


I see.

>
> My other thought: your patch seems optimal as far as avoiding IPIs goes,
> but I wonder how often it folds down to a single CPU? That case is
> easier to fast-path without using a new cpumask.


It will be quite often if threads number of user level APP is more than
a half of LCPUs.
It needs a new cpumask because we can not remove SMT bit on
mm->cpu_vm_mask_var directly.

>
> Cheers,
> Rusty.

2012-05-24 14:07:10

by Alex Shi

[permalink] [raw]
Subject: Re: [PATCH v7 8/8] x86/tlb: just do tlb flush on one of siblings of SMT

On 05/24/2012 05:46 PM, Jan Beulich wrote:

>>>> On 24.05.12 at 11:27, Alex Shi <[email protected]> wrote:
>> On 05/24/2012 04:42 PM, Peter Zijlstra wrote:
>>
>>> On Thu, 2012-05-24 at 16:32 +0800, Alex Shi wrote:
>>>> So, I use cpumask_t in stack.
>>>
>>> cpumask_t is 512 bytes with NR_CPUS=4096, that's generally considered
>>> too big to be on stack.
>>
>>
>> I am not consistent with cpumask_t.
>> I just want to know why it is too big on stack? Since if it causes
>> trouble in using, that mean current kernel will fail to run on 4096 CPU
>> system.
>
> Why? Did you spot critical left-over instances of on-stack
> cpumask_t-s anywhere?


I aware it is a stupid question just after the e-mail sent out.
Thanks for you and PeterZ's nice reply!

>
> Jan
>

2012-05-24 14:18:38

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [PATCH v7 8/8] x86/tlb: just do tlb flush on one of siblings of SMT

On 5/24/2012 6:54 AM, Alex Shi wrote:
> On 05/24/2012 09:39 PM, Arjan van de Ven wrote:
>
>> On 5/24/2012 6:23 AM, Peter Zijlstra wrote:
>>> On Thu, 2012-05-24 at 06:19 -0700, Andrew Lutomirski wrote:
>>>>
>>>> A decent heuristic might be to prefer idle SMT siblings for TLB
>>>> invalidation. I don't know what effect that would have on power
>>>> consumption (it would be rather bad if idling one SMT thread while the
>>>> other one is busy saves much power).
>>
>> we really really shouldn't do flushing of tlb's on only one half of SMT.
>> SMT sibblings have their own TLB pool at least on some of Intels chips.
>
>
> That is also the biggest question I want to know. Actually, some
> documents, wiki said the SMT sibling just has process registers and
> interrupt part, no any tlb/l1 cache etc, (like intel's doc
> vol6iss1_hyper_threading_technology.pdf). And the patch runs well on
> NHM EP/WSM EP/NHM EX/SNB EP CPUs.
>
> But hard to get such clearly per cpu info of SMT/HT, so, what the
> detailed Intel chips has 'TLB pool' on SMT?

all of them.

the TLB pool is shared as physical resource (dynamic or static, that
depends), but each tlb entry will be tagged for which of the two HT
pairs it's for, and on a logical level, they are completely separate as
a result (as they should be)

2012-05-24 14:32:30

by Alex Shi

[permalink] [raw]
Subject: Re: [PATCH v7 8/8] x86/tlb: just do tlb flush on one of siblings of SMT

>>> we really really shouldn't do flushing of tlb's on only one half of SMT.

>>> SMT sibblings have their own TLB pool at least on some of Intels chips.
>>
>>
>> That is also the biggest question I want to know. Actually, some
>> documents, wiki said the SMT sibling just has process registers and
>> interrupt part, no any tlb/l1 cache etc, (like intel's doc
>> vol6iss1_hyper_threading_technology.pdf). And the patch runs well on
>> NHM EP/WSM EP/NHM EX/SNB EP CPUs.
>>
>> But hard to get such clearly per cpu info of SMT/HT, so, what the
>> detailed Intel chips has 'TLB pool' on SMT?
>
> all of them.
>
> the TLB pool is shared as physical resource (dynamic or static, that
> depends), but each tlb entry will be tagged for which of the two HT
> pairs it's for, and on a logical level, they are completely separate as
> a result (as they should be)


But, why just flush part of SMT doesn't crash kernel on many benchmarks
testing? Does it means flush tlb without PCID (doesn't enable in current
kernel) will flush both of 'TLB pool'?

Oh, lots of questions of the TLB pool details. :) Could you like share
the URL of related documents?


>

2012-05-24 14:36:26

by Alex Shi

[permalink] [raw]
Subject: Re: [PATCH v7 2/8] x86/flush_tlb: try flush_tlb_single one by one in flush_tlb_range

On 05/24/2012 05:44 PM, Jan Beulich wrote:

>>>> On 24.05.12 at 10:55, Alex Shi <[email protected]> wrote:
>> On 05/24/2012 04:12 PM, Jan Beulich wrote:
>>
>>>>>> On 24.05.12 at 08:41, Alex Shi <[email protected]> wrote:
>>>> So, the following change should be more safe for PV?
>>>>
>>>> - if (va == TLB_FLUSH_ALL) {
>>>> - args->op.cmd = MMUEXT_TLB_FLUSH_MULTI;
>>>> - } else {
>>>> - args->op.cmd = MMUEXT_INVLPG_MULTI;
>>>> - args->op.arg1.linear_addr = va;
>>>> - }
>>>> + args->op.cmd = MMUEXT_TLB_FLUSH_MULTI;
>>>
>>> This would be safe ...
>>>
>>>> + if (start != TLB_FLUSH_ALL)
>>>> + args->op.arg1.linear_addr = start;
>>>
>>> ... and then this superfluous, but it'd result in an unconditional
>>> full TLB flush. When start and end (or perhaps end-1, assuming
>>> end is not inclusive) are on the same page, MMUEXT_INVLPG_MULTI
>>> should be used; MMUEXT_TLB_FLUSH_MULTI might need to be
>>> used in all other cases, unless you want to split multi-page, non-
>>> global invalidations into multiple MMUEXT_INVLPG_MULTI-s (which
>>> would appear to be what the whole patch aims at).
>>
>>
>> args->op.cmd = MMUEXT_TLB_FLUSH_MULTI;
>> - if (start != TLB_FLUSH_ALL)
>> + if (start != TLB_FLUSH_ALL && (end - start) < PAGE_SIZE) {
>
> Assuming 'end' is not inclusive (you didn't clarify that),
>
> if (start != TLB_FLUSH_ALL && (end - start) <= PAGE_SIZE) {
>
> (and only if 'start' is guaranteed to be page aligned).


I still a idiot for xen and a newbie for kernel. :)
AFAIK, if system just flush one page, it won't set end there. But anyway
using '<=' has no problem.

>
>> + args->op.cmd = MMUEXT_INVLPG_MULTI;
>> args->op.arg1.linear_addr = start;
>> + }
>>
>> So, above it correct code for xen?
>> As to the xen optimisation of flush range, it is may better to be done
>> in a separate patch.
>
> Agreed.


Thanks for all comments!

>
> Jan
>

2012-05-24 15:04:49

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH v7 8/8] x86/tlb: just do tlb flush on one of siblings of SMT

On 05/24/2012 07:32 AM, Alex Shi wrote:
>>
>> the TLB pool is shared as physical resource (dynamic or static, that
>> depends), but each tlb entry will be tagged for which of the two HT
>> pairs it's for, and on a logical level, they are completely separate as
>> a result (as they should be)
>
> But, why just flush part of SMT doesn't crash kernel on many benchmarks
> testing? Does it means flush tlb without PCID (doesn't enable in current
> kernel) will flush both of 'TLB pool'?
>
> Oh, lots of questions of the TLB pool details. :) Could you like share
> the URL of related documents?
>

Hang on here... there is a huge difference between what a particular CPU
implementation does and what is architecturally guaranteed.

Both wearing my Linux x86 maintainer hat, and wearing my Intel employee
hat, I want to categorically state that Linux cannot rely on behavior
that isn't architecturally guaranteed. Unless we can get an
architectural guarantee that this elision is safe, it cannot go in. It
doesn't work the other way -- the burden of proof is to prove that the
change is safe, not that the change cannot be proven unsafe.

-hpa

2012-05-24 15:06:23

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH v7 8/8] x86/tlb: just do tlb flush on one of siblings of SMT

On 05/24/2012 01:32 AM, Alex Shi wrote:
>
> Sure. but just want to know how many commercial x86 CPU uses >2 SMTs?
> Write a short, quick function to do random selection in SMT is quite
> complicate considering cpumask maybe just contain random number SMT
> siblings in a core.
>

Wrong question. This is a forward compatibility issue. You have to
take into account potential future CPUs as well.

And no, it isn't hard.

-hpa

2012-05-24 16:08:40

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [PATCH v7 8/8] x86/tlb: just do tlb flush on one of siblings of SMT

On 5/24/2012 7:32 AM, Alex Shi wrote:
>>>> we really really shouldn't do flushing of tlb's on only one half of SMT.
>
>>>> SMT sibblings have their own TLB pool at least on some of Intels chips.
>>>
>>>
>>> That is also the biggest question I want to know. Actually, some
>>> documents, wiki said the SMT sibling just has process registers and
>>> interrupt part, no any tlb/l1 cache etc, (like intel's doc
>>> vol6iss1_hyper_threading_technology.pdf). And the patch runs well on
>>> NHM EP/WSM EP/NHM EX/SNB EP CPUs.
>>>
>>> But hard to get such clearly per cpu info of SMT/HT, so, what the
>>> detailed Intel chips has 'TLB pool' on SMT?
>>
>> all of them.
>>
>> the TLB pool is shared as physical resource (dynamic or static, that
>> depends), but each tlb entry will be tagged for which of the two HT
>> pairs it's for, and on a logical level, they are completely separate as
>> a result (as they should be)
>
>
> But, why just flush part of SMT doesn't crash kernel on many benchmarks
> testing?

stale tlb's don't crash the kernel
they do random weird **** to userspace processes.

you REALLY don't want to be debugging those.

There is absolutely NO GUARANTEE that a full tlbflush on one thread
flushes the other one. (in fact I'd be surprised if it actually did).

Also remember that there are several levels of TLB and tlb caches, and
you HAVE to flush all.
Intel put out a very clear document about the rules around TLBs
(it's on the same URL as the SDM, if it hasn't been absorbed into the
SDM already)... and we have to follow those rules.
(Linux already followed those rules even prior to that doc existing,
but the rules are at least very explicit now)

2012-05-25 00:26:22

by Alex Shi

[permalink] [raw]
Subject: Re: [PATCH v7 8/8] x86/tlb: just do tlb flush on one of siblings of SMT

On 05/24/2012 11:03 PM, H. Peter Anvin wrote:

> On 05/24/2012 07:32 AM, Alex Shi wrote:
>>>
>>> the TLB pool is shared as physical resource (dynamic or static, that
>>> depends), but each tlb entry will be tagged for which of the two HT
>>> pairs it's for, and on a logical level, they are completely separate as
>>> a result (as they should be)
>>
>> But, why just flush part of SMT doesn't crash kernel on many benchmarks
>> testing? Does it means flush tlb without PCID (doesn't enable in current
>> kernel) will flush both of 'TLB pool'?
>>
>> Oh, lots of questions of the TLB pool details. :) Could you like share
>> the URL of related documents?
>>
>
> Hang on here... there is a huge difference between what a particular CPU
> implementation does and what is architecturally guaranteed.
>
> Both wearing my Linux x86 maintainer hat, and wearing my Intel employee
> hat, I want to categorically state that Linux cannot rely on behavior
> that isn't architecturally guaranteed. Unless we can get an
> architectural guarantee that this elision is safe, it cannot go in. It
> doesn't work the other way -- the burden of proof is to prove that the
> change is safe, not that the change cannot be proven unsafe.


Understand and thanks for all of your time!

>
> -hpa
>

2012-05-25 00:30:13

by Alex Shi

[permalink] [raw]
Subject: Re: [PATCH v7 8/8] x86/tlb: just do tlb flush on one of siblings of SMT

On 05/25/2012 12:08 AM, Arjan van de Ven wrote:

> On 5/24/2012 7:32 AM, Alex Shi wrote:
>>>>> we really really shouldn't do flushing of tlb's on only one half of SMT.
>>
>>>>> SMT sibblings have their own TLB pool at least on some of Intels chips.
>>>>
>>>>
>>>> That is also the biggest question I want to know. Actually, some
>>>> documents, wiki said the SMT sibling just has process registers and
>>>> interrupt part, no any tlb/l1 cache etc, (like intel's doc
>>>> vol6iss1_hyper_threading_technology.pdf). And the patch runs well on
>>>> NHM EP/WSM EP/NHM EX/SNB EP CPUs.
>>>>
>>>> But hard to get such clearly per cpu info of SMT/HT, so, what the
>>>> detailed Intel chips has 'TLB pool' on SMT?
>>>
>>> all of them.
>>>
>>> the TLB pool is shared as physical resource (dynamic or static, that
>>> depends), but each tlb entry will be tagged for which of the two HT
>>> pairs it's for, and on a logical level, they are completely separate as
>>> a result (as they should be)
>>
>>
>> But, why just flush part of SMT doesn't crash kernel on many benchmarks
>> testing?
>
> stale tlb's don't crash the kernel
> they do random weird **** to userspace processes.
>
> you REALLY don't want to be debugging those.
>
> There is absolutely NO GUARANTEE that a full tlbflush on one thread
> flushes the other one. (in fact I'd be surprised if it actually did).
>
> Also remember that there are several levels of TLB and tlb caches, and
> you HAVE to flush all.


Thanks for comments!
BTW,
As my limited knowledge, rewrite cr3 or invlpg will flush all levels TLB
entries in CPU. So, what's you mean of 'HAVE to flush all'?

2012-05-25 00:46:49

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [PATCH v7 8/8] x86/tlb: just do tlb flush on one of siblings of SMT

On
> As my limited knowledge, rewrite cr3 or invlpg will flush all levels TLB
> entries in CPU. So, what's you mean of 'HAVE to flush all'?

neither of these will flush global tlb entries.

(see the "global" bit in the page table format)

2012-05-25 02:45:27

by Alex Shi

[permalink] [raw]
Subject: Re: [PATCH v7 2/8] x86/flush_tlb: try flush_tlb_single one by one in flush_tlb_range

>

> Assuming 'end' is not inclusive (you didn't clarify that),
>
> if (start != TLB_FLUSH_ALL && (end - start) <= PAGE_SIZE) {
>
> (and only if 'start' is guaranteed to be page aligned).
>
>> + args->op.cmd = MMUEXT_INVLPG_MULTI;
>> args->op.arg1.linear_addr = start;
>> + }
>>
>> So, above it correct code for xen?
>> As to the xen optimisation of flush range, it is may better to be done
>> in a separate patch.
>
> Agreed.
>
> Jan
>


Thanks for comments.

this the updated patch according to your suggestion.

-----

>From 82dfb90a26e9a27e7a073b2072684049aa2f5e12 Mon Sep 17 00:00:00 2001
From: Alex Shi <[email protected]>
Date: Wed, 9 May 2012 15:09:08 -0600
Subject: [PATCH 2/8] x86/flush_tlb: try flush_tlb_single one by one in
flush_tlb_range

x86 has no flush_tlb_range support in instruction level. Currently the
flush_tlb_range just implemented by flushing all page table. That is not
the best solution for all scenarios. In fact, if we just use 'invlpg' to
flush few lines from TLB, we can get the performance gain from later
remain TLB lines accessing.

But the 'invlpg' instruction costs much of time. Its execution time can
compete with cr3 rewriting, and even a bit more on SNB CPU.

So, on a 512 4KB TLB entries CPU, the balance points is at:
(512 - X) * 100ns(assumed TLB refill cost) =
X(TLB flush entries) * 100ns(assumed invlpg cost)

Here, X is 256, that is 1/2 of 512 entries.

But with the mysterious CPU pre-fetcher and page miss handler Unit, the
assumed TLB refill cost is far lower then 100ns in sequential access. And
2 HT siblings in one core makes the memory access more faster if they are
accessing the same memory. So, in the patch, I just do the change when
the target entries is less than 1/16 of whole active tlb entries.
Actually, I have no data support for the percentage '1/16', so any
suggestions are welcomed.

As to hugetlb, guess due to smaller page table, and smaller active TLB
entries, I didn't see benefit via my benchmark, so no optimizing now.

My macro benchmark show in ideal scenarios, the performance improves 70
percent in reading. And in worst scenario, the reading/writing
performance is similar with unpatched 3.4-rc4 kernel.

Here is the reading data on my 2P * 4cores *HT NHM EP machine, with THP
'always':

multi thread testing, '-t' paramter is thread number:
with patch unpatched 3.4-rc4
./mprotect -t 1 14ns 24ns
./mprotect -t 2 13ns 22ns
./mprotect -t 4 12ns 19ns
./mprotect -t 8 14ns 16ns
./mprotect -t 16 28ns 26ns
./mprotect -t 32 54ns 51ns
./mprotect -t 128 200ns 199ns

Single process with sequencial flushing and memory accessing:

with patch unpatched 3.4-rc4
./mprotect 7ns 11ns
./mprotect -p 4096 -l 8 -n 10240
21ns 21ns

I also tried other benchmarks on Intel core2/NHM/SNB EP and NHM EX machine.
No clear performance change on specjbb2005 with openjdk, and oltp reading.

Signed-off-by: Alex Shi <[email protected]>
---
arch/x86/include/asm/paravirt.h | 5 +-
arch/x86/include/asm/paravirt_types.h | 3 +-
arch/x86/include/asm/tlbflush.h | 23 +++-----
arch/x86/include/asm/uv/uv.h | 5 +-
arch/x86/mm/tlb.c | 97 +++++++++++++++++++++++++++------
arch/x86/platform/uv/tlb_uv.c | 6 +-
arch/x86/xen/mmu.c | 12 ++--
include/trace/events/xen.h | 12 +++--
8 files changed, 114 insertions(+), 49 deletions(-)

diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index aa0f913..03da4ab 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -397,9 +397,10 @@ static inline void __flush_tlb_single(unsigned long addr)

static inline void flush_tlb_others(const struct cpumask *cpumask,
struct mm_struct *mm,
- unsigned long va)
+ unsigned long start,
+ unsigned long end)
{
- PVOP_VCALL3(pv_mmu_ops.flush_tlb_others, cpumask, mm, va);
+ PVOP_VCALL4(pv_mmu_ops.flush_tlb_others, cpumask, mm, start, end);
}

static inline int paravirt_pgd_alloc(struct mm_struct *mm)
diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
index 8e8b9a4..600a5fcac9 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -250,7 +250,8 @@ struct pv_mmu_ops {
void (*flush_tlb_single)(unsigned long addr);
void (*flush_tlb_others)(const struct cpumask *cpus,
struct mm_struct *mm,
- unsigned long va);
+ unsigned long start,
+ unsigned long end);

/* Hooks for allocating and freeing a pagetable top-level */
int (*pgd_alloc)(struct mm_struct *mm);
diff --git a/arch/x86/include/asm/tlbflush.h b/arch/x86/include/asm/tlbflush.h
index c0e108e..51f8b1c 100644
--- a/arch/x86/include/asm/tlbflush.h
+++ b/arch/x86/include/asm/tlbflush.h
@@ -77,14 +77,10 @@ static inline void __flush_tlb_one(unsigned long addr)
* - flush_tlb_page(vma, vmaddr) flushes one page
* - flush_tlb_range(vma, start, end) flushes a range of pages
* - flush_tlb_kernel_range(start, end) flushes a range of kernel pages
- * - flush_tlb_others(cpumask, mm, va) flushes TLBs on other cpus
+ * - flush_tlb_others(cpumask, mm, start, end) flushes TLBs on other cpus
*
* ..but the i386 has somewhat limited tlb flushing capabilities,
* and page-granular flushes are available only on i486 and up.
- *
- * x86-64 can only flush individual pages or full VMs. For a range flush
- * we always do the full VM. Might be worth trying if for a small
- * range a few INVLPGs in a row are a win.
*/

#ifndef CONFIG_SMP
@@ -115,7 +111,8 @@ static inline void flush_tlb_range(struct vm_area_struct *vma,

static inline void native_flush_tlb_others(const struct cpumask *cpumask,
struct mm_struct *mm,
- unsigned long va)
+ unsigned long start,
+ unsigned long end)
{
}

@@ -133,17 +130,14 @@ extern void flush_tlb_all(void);
extern void flush_tlb_current_task(void);
extern void flush_tlb_mm(struct mm_struct *);
extern void flush_tlb_page(struct vm_area_struct *, unsigned long);
+extern void flush_tlb_range(struct vm_area_struct *vma,
+ unsigned long start, unsigned long end);

#define flush_tlb() flush_tlb_current_task()

-static inline void flush_tlb_range(struct vm_area_struct *vma,
- unsigned long start, unsigned long end)
-{
- flush_tlb_mm(vma->vm_mm);
-}
-
void native_flush_tlb_others(const struct cpumask *cpumask,
- struct mm_struct *mm, unsigned long va);
+ struct mm_struct *mm,
+ unsigned long start, unsigned long end);

#define TLBSTATE_OK 1
#define TLBSTATE_LAZY 2
@@ -163,7 +157,8 @@ static inline void reset_lazy_tlbstate(void)
#endif /* SMP */

#ifndef CONFIG_PARAVIRT
-#define flush_tlb_others(mask, mm, va) native_flush_tlb_others(mask, mm, va)
+#define flush_tlb_others(mask, mm, start, end) \
+ native_flush_tlb_others(mask, mm, start, end)
#endif

static inline void flush_tlb_kernel_range(unsigned long start,
diff --git a/arch/x86/include/asm/uv/uv.h b/arch/x86/include/asm/uv/uv.h
index 3bb9491..b47c2a8 100644
--- a/arch/x86/include/asm/uv/uv.h
+++ b/arch/x86/include/asm/uv/uv.h
@@ -15,7 +15,8 @@ extern void uv_nmi_init(void);
extern void uv_system_init(void);
extern const struct cpumask *uv_flush_tlb_others(const struct cpumask *cpumask,
struct mm_struct *mm,
- unsigned long va,
+ unsigned long start,
+ unsigned end,
unsigned int cpu);

#else /* X86_UV */
@@ -26,7 +27,7 @@ static inline void uv_cpu_init(void) { }
static inline void uv_system_init(void) { }
static inline const struct cpumask *
uv_flush_tlb_others(const struct cpumask *cpumask, struct mm_struct *mm,
- unsigned long va, unsigned int cpu)
+ unsigned long start, unsigned long end, unsigned int cpu)
{ return cpumask; }

#endif /* X86_UV */
diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
index d6c0418..7d92079 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -41,7 +41,8 @@ DEFINE_PER_CPU_SHARED_ALIGNED(struct tlb_state, cpu_tlbstate)
union smp_flush_state {
struct {
struct mm_struct *flush_mm;
- unsigned long flush_va;
+ unsigned long flush_start;
+ unsigned long flush_end;
raw_spinlock_t tlbstate_lock;
DECLARE_BITMAP(flush_cpumask, NR_CPUS);
};
@@ -154,10 +155,19 @@ void smp_invalidate_interrupt(struct pt_regs *regs)

if (f->flush_mm == percpu_read(cpu_tlbstate.active_mm)) {
if (percpu_read(cpu_tlbstate.state) == TLBSTATE_OK) {
- if (f->flush_va == TLB_FLUSH_ALL)
+ if (f->flush_end == TLB_FLUSH_ALL
+ || !cpu_has_invlpg)
local_flush_tlb();
- else
- __flush_tlb_one(f->flush_va);
+ else if (!f->flush_end)
+ __flush_tlb_single(f->flush_start);
+ else {
+ unsigned long addr;
+ addr = f->flush_start;
+ while (addr <= f->flush_end) {
+ __flush_tlb_single(addr);
+ addr += PAGE_SIZE;
+ }
+ }
} else
leave_mm(cpu);
}
@@ -170,7 +180,8 @@ out:
}

static void flush_tlb_others_ipi(const struct cpumask *cpumask,
- struct mm_struct *mm, unsigned long va)
+ struct mm_struct *mm, unsigned long start,
+ unsigned long end)
{
unsigned int sender;
union smp_flush_state *f;
@@ -183,7 +194,8 @@ static void flush_tlb_others_ipi(const struct cpumask *cpumask,
raw_spin_lock(&f->tlbstate_lock);

f->flush_mm = mm;
- f->flush_va = va;
+ f->flush_start = start;
+ f->flush_end = end;
if (cpumask_andnot(to_cpumask(f->flush_cpumask), cpumask, cpumask_of(smp_processor_id()))) {
/*
* We have to send the IPI only to
@@ -197,24 +209,26 @@ static void flush_tlb_others_ipi(const struct cpumask *cpumask,
}

f->flush_mm = NULL;
- f->flush_va = 0;
+ f->flush_start = 0;
+ f->flush_end = 0;
if (nr_cpu_ids > NUM_INVALIDATE_TLB_VECTORS)
raw_spin_unlock(&f->tlbstate_lock);
}

void native_flush_tlb_others(const struct cpumask *cpumask,
- struct mm_struct *mm, unsigned long va)
+ struct mm_struct *mm, unsigned long start,
+ unsigned long end)
{
if (is_uv_system()) {
unsigned int cpu;

cpu = smp_processor_id();
- cpumask = uv_flush_tlb_others(cpumask, mm, va, cpu);
+ cpumask = uv_flush_tlb_others(cpumask, mm, start, end, cpu);
if (cpumask)
- flush_tlb_others_ipi(cpumask, mm, va);
+ flush_tlb_others_ipi(cpumask, mm, start, end);
return;
}
- flush_tlb_others_ipi(cpumask, mm, va);
+ flush_tlb_others_ipi(cpumask, mm, start, end);
}

static void __cpuinit calculate_tlb_offset(void)
@@ -280,7 +294,7 @@ void flush_tlb_current_task(void)

local_flush_tlb();
if (cpumask_any_but(mm_cpumask(mm), smp_processor_id()) < nr_cpu_ids)
- flush_tlb_others(mm_cpumask(mm), mm, TLB_FLUSH_ALL);
+ flush_tlb_others(mm_cpumask(mm), mm, 0UL, TLB_FLUSH_ALL);
preempt_enable();
}

@@ -295,12 +309,63 @@ void flush_tlb_mm(struct mm_struct *mm)
leave_mm(smp_processor_id());
}
if (cpumask_any_but(mm_cpumask(mm), smp_processor_id()) < nr_cpu_ids)
- flush_tlb_others(mm_cpumask(mm), mm, TLB_FLUSH_ALL);
+ flush_tlb_others(mm_cpumask(mm), mm, 0UL, TLB_FLUSH_ALL);
+
+ preempt_enable();
+}
+
+#define FLUSHALL_BAR 16
+
+void flush_tlb_range(struct vm_area_struct *vma,
+ unsigned long start, unsigned long end)
+{
+ struct mm_struct *mm;
+
+ if (!cpu_has_invlpg || vma->vm_flags & VM_HUGETLB) {
+ flush_tlb_mm(vma->vm_mm);
+ return;
+ }
+
+ preempt_disable();
+ mm = vma->vm_mm;
+ if (current->active_mm == mm) {
+ if (current->mm) {
+ unsigned long addr, vmflag = vma->vm_flags;
+ unsigned act_entries, tlb_entries = 0;
+
+ if (vmflag & VM_EXEC)
+ tlb_entries = tlb_lli_4k[ENTRIES];
+ else
+ tlb_entries = tlb_lld_4k[ENTRIES];
+
+ act_entries = tlb_entries > mm->total_vm ?
+ mm->total_vm : tlb_entries;

+ if ((end - start)/PAGE_SIZE > act_entries/FLUSHALL_BAR)
+ local_flush_tlb();
+ else {
+ for (addr = start; addr <= end;
+ addr += PAGE_SIZE)
+ __flush_tlb_single(addr);
+
+ if (cpumask_any_but(mm_cpumask(mm),
+ smp_processor_id()) < nr_cpu_ids)
+ flush_tlb_others(mm_cpumask(mm), mm,
+ start, end);
+ preempt_enable();
+ return;
+ }
+ } else {
+ leave_mm(smp_processor_id());
+ }
+ }
+ if (cpumask_any_but(mm_cpumask(mm), smp_processor_id()) < nr_cpu_ids)
+ flush_tlb_others(mm_cpumask(mm), mm, 0UL, TLB_FLUSH_ALL);
preempt_enable();
}

-void flush_tlb_page(struct vm_area_struct *vma, unsigned long va)
+
+void flush_tlb_page(struct vm_area_struct *vma, unsigned long start)
{
struct mm_struct *mm = vma->vm_mm;

@@ -308,13 +373,13 @@ void flush_tlb_page(struct vm_area_struct *vma, unsigned long va)

if (current->active_mm == mm) {
if (current->mm)
- __flush_tlb_one(va);
+ __flush_tlb_one(start);
else
leave_mm(smp_processor_id());
}

if (cpumask_any_but(mm_cpumask(mm), smp_processor_id()) < nr_cpu_ids)
- flush_tlb_others(mm_cpumask(mm), mm, va);
+ flush_tlb_others(mm_cpumask(mm), mm, start, 0UL);

preempt_enable();
}
diff --git a/arch/x86/platform/uv/tlb_uv.c b/arch/x86/platform/uv/tlb_uv.c
index 3ae0e61..0df5ad2 100644
--- a/arch/x86/platform/uv/tlb_uv.c
+++ b/arch/x86/platform/uv/tlb_uv.c
@@ -1068,8 +1068,8 @@ static int set_distrib_bits(struct cpumask *flush_mask, struct bau_control *bcp,
* done. The returned pointer is valid till preemption is re-enabled.
*/
const struct cpumask *uv_flush_tlb_others(const struct cpumask *cpumask,
- struct mm_struct *mm, unsigned long va,
- unsigned int cpu)
+ struct mm_struct *mm, unsigned long start,
+ unsigned end, unsigned int cpu)
{
int locals = 0;
int remotes = 0;
@@ -1112,7 +1112,7 @@ const struct cpumask *uv_flush_tlb_others(const struct cpumask *cpumask,

record_send_statistics(stat, locals, hubs, remotes, bau_desc);

- bau_desc->payload.address = va;
+ bau_desc->payload.address = start;
bau_desc->payload.sending_cpu = cpu;
/*
* uv_flush_send_and_wait returns 0 if all cpu's were messaged,
diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
index 69f5857..e731140 100644
--- a/arch/x86/xen/mmu.c
+++ b/arch/x86/xen/mmu.c
@@ -1244,7 +1244,8 @@ static void xen_flush_tlb_single(unsigned long addr)
}

static void xen_flush_tlb_others(const struct cpumask *cpus,
- struct mm_struct *mm, unsigned long va)
+ struct mm_struct *mm, unsigned long start,
+ unsigned long end)
{
struct {
struct mmuext_op op;
@@ -1256,7 +1257,7 @@ static void xen_flush_tlb_others(const struct cpumask *cpus,
} *args;
struct multicall_space mcs;

- trace_xen_mmu_flush_tlb_others(cpus, mm, va);
+ trace_xen_mmu_flush_tlb_others(cpus, mm, start, end);

if (cpumask_empty(cpus))
return; /* nothing to do */
@@ -1269,11 +1270,10 @@ static void xen_flush_tlb_others(const struct cpumask *cpus,
cpumask_and(to_cpumask(args->mask), cpus, cpu_online_mask);
cpumask_clear_cpu(smp_processor_id(), to_cpumask(args->mask));

- if (va == TLB_FLUSH_ALL) {
- args->op.cmd = MMUEXT_TLB_FLUSH_MULTI;
- } else {
+ args->op.cmd = MMUEXT_TLB_FLUSH_MULTI;
+ if (start != TLB_FLUSH_ALL && (end - start) <= PAGE_SIZE) {
args->op.cmd = MMUEXT_INVLPG_MULTI;
- args->op.arg1.linear_addr = va;
+ args->op.arg1.linear_addr = start;
}

MULTI_mmuext_op(mcs.mc, &args->op, 1, NULL, DOMID_SELF);
diff --git a/include/trace/events/xen.h b/include/trace/events/xen.h
index 92f1a79..15ba03b 100644
--- a/include/trace/events/xen.h
+++ b/include/trace/events/xen.h
@@ -397,18 +397,20 @@ TRACE_EVENT(xen_mmu_flush_tlb_single,

TRACE_EVENT(xen_mmu_flush_tlb_others,
TP_PROTO(const struct cpumask *cpus, struct mm_struct *mm,
- unsigned long addr),
- TP_ARGS(cpus, mm, addr),
+ unsigned long addr, unsigned long end),
+ TP_ARGS(cpus, mm, addr, end),
TP_STRUCT__entry(
__field(unsigned, ncpus)
__field(struct mm_struct *, mm)
__field(unsigned long, addr)
+ __field(unsigned long, end)
),
TP_fast_assign(__entry->ncpus = cpumask_weight(cpus);
__entry->mm = mm;
- __entry->addr = addr),
- TP_printk("ncpus %d mm %p addr %lx",
- __entry->ncpus, __entry->mm, __entry->addr)
+ __entry->addr = addr,
+ __entry->end = end),
+ TP_printk("ncpus %d mm %p addr %lx, end %lx",
+ __entry->ncpus, __entry->mm, __entry->addr, __entry->end)
);

TRACE_EVENT(xen_mmu_write_cr3,
--
1.7.5.4