Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Linux: Kernel

[PATCH v4 3/7] x86/flush_tlb: try flush_tlb_single one by one in flush_tlb_range

 

 

Linux kernel RSS feed   Index | Next | Previous | View Threaded


alex.shi at intel

May 9, 2012, 10:00 PM

Post #1 of 14 (173 views)
Permalink
[PATCH v4 3/7] 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 <alex.shi [at] intel>
---
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 b8e2794..75bab52 100644
--- a/arch/x86/xen/mmu.c
+++ b/arch/x86/xen/mmu.c
@@ -1239,7 +1239,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;
@@ -1251,7 +1252,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 */
@@ -1264,11 +1265,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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


bp at amd64

May 10, 2012, 12:53 AM

Post #2 of 14 (165 views)
Permalink
Re: [PATCH v4 3/7] x86/flush_tlb: try flush_tlb_single one by one in flush_tlb_range [In reply to]

On Thu, May 10, 2012 at 01:00:09PM +0800, Alex Shi wrote:
> 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 <alex.shi [at] intel>
> ---

[ … ]

> +#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;

Ok, question:

we're comparing TLB size with the amount of pages mapped by this mm
struct. AFAICT, that doesn't mean that all those mapped pages do have
respective entries in the TLB, does it?

If so, then the actual entries number is kinda inaccurate, no? We don't
really know how many TLB entries actually belong to this mm struct. Or am I
missing something?

> + if ((end - start)/PAGE_SIZE > act_entries/FLUSHALL_BAR)

Oh, in a later patch you do this:

+ if ((end - start) >> PAGE_SHIFT >
+ act_entries >> tlb_flushall_factor)

and the tlb_flushall_factor factor is 5 or 6 but the division by 16
(FLUSHALL_BAR) was a >> 4. So, is this to assume that it is not 16 but
actually more than 32 or even 64 TLB entries where a full TLB flush
makes sense and one-by-one if less?

> + 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();

Thanks.

--
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


bp at amd64

May 10, 2012, 1:42 AM

Post #3 of 14 (163 views)
Permalink
Re: [PATCH v4 3/7] x86/flush_tlb: try flush_tlb_single one by one in flush_tlb_range [In reply to]

On Thu, May 10, 2012 at 01:00:09PM +0800, Alex Shi wrote:
> 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 <alex.shi [at] intel>

[ … ]

> +
> +#define FLUSHALL_BAR 16
> +

Btw, you can save a bunch of indenting on this function, let me add
the final version here from the whole patchset so I can comment on it
easier:

> void __flush_tlb_range(struct mm_struct *mm, unsigned long start,
> unsigned long end, unsigned long vmflag)
> {
> preempt_disable();
> if (current->active_mm == mm) {

if (current->active_mm != mm)
goto flush_all;

Now this whole piece below can move one indentation level to the left.

Then you can do:

if (!current->mm)
goto leave;

and add the "leave" label below.

Now you're saving yet another indentation level, bringing the meat of
the function at 1st indentation level, which is cool and gives you much
more room so that you don't have to linebreak longer statements.

> if (current->mm) {
> unsigned long addr;
> unsigned long act_entries, tlb_entries = 0;
>
> if (end == TLB_FLUSH_ALL ||
> tlb_flushall_factor == (u16)TLB_FLUSH_ALL) {
> local_flush_tlb();
> goto flush_all;
> }
> if (vmflag & VM_EXEC)
> tlb_entries = tlb_lli_4k[ENTRIES];
> else
> tlb_entries = tlb_lld_4k[ENTRIES];
> act_entries = min(mm->total_vm, tlb_entries);
>
> if ((end - start) >> PAGE_SHIFT >
> act_entries >> tlb_flushall_factor)
> local_flush_tlb();
> else {
> if (has_large_page(mm, start, end)) {
> local_flush_tlb();
> goto flush_all;
> }
> 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());
> }
> }

leave:
leave_mm(smp_processor_id());

> 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();
> }

Thanks.

--
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


alex.shi at intel

May 10, 2012, 1:50 AM

Post #4 of 14 (164 views)
Permalink
Re: [PATCH v4 3/7] x86/flush_tlb: try flush_tlb_single one by one in flush_tlb_range [In reply to]

>

> Ok, question:
>
> we're comparing TLB size with the amount of pages mapped by this mm
> struct. AFAICT, that doesn't mean that all those mapped pages do have
> respective entries in the TLB, does it?
>
> If so, then the actual entries number is kinda inaccurate, no? We don't
> really know how many TLB entries actually belong to this mm struct. Or am I
> missing something?


No, we can not know the exactly TLB entires for. But usually, when you
process is doing the mprotect/munmap etc system call, your process has
taken much of memory and already filled lots of TLB entries.

This point is considered imply in the balance point calculation.
checking following equation
(512 - X) * 100ns(assumed TLB refill cost) =
X(TLB flush entries) * 100ns(assumed invlpg cost)

The X value we got is far lower then theory value. That means remain TLB
entries is may not so much, or TLB refill cost is much lower due to
hardware pre-fetcher.

>

>> + if ((end - start)/PAGE_SIZE > act_entries/FLUSHALL_BAR)
>
> Oh, in a later patch you do this:
>
> + if ((end - start) >> PAGE_SHIFT >
> + act_entries >> tlb_flushall_factor)
>
> and the tlb_flushall_factor factor is 5 or 6 but the division by 16
> (FLUSHALL_BAR) was a >> 4. So, is this to assume that it is not 16 but
> actually more than 32 or even 64 TLB entries where a full TLB flush
> makes sense and one-by-one if less?


Yes, the FLUSHALL_BAR is just a guessing value here. And take your
advice, I modify the macro benchmark a little and get the more sensible
value in later patch.

BTW, I found 8% performance increase on kbuild on SNB EP from the
average multiple testing, while result variation is up to 15%.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


alex.shi at intel

May 10, 2012, 2:04 AM

Post #5 of 14 (164 views)
Permalink
Re: [PATCH v4 3/7] x86/flush_tlb: try flush_tlb_single one by one in flush_tlb_range [In reply to]

>> +
>> +#define FLUSHALL_BAR 16
>> +
>
> Btw, you can save a bunch of indenting on this function, let me add
> the final version here from the whole patchset so I can comment on it
> easier:
>
>> void __flush_tlb_range(struct mm_struct *mm, unsigned long start,
>> unsigned long end, unsigned long vmflag)
>> {
>> preempt_disable();
>> if (current->active_mm == mm) {
>
> if (current->active_mm != mm)
> goto flush_all;
>
> Now this whole piece below can move one indentation level to the left.
>
> Then you can do:
>
> if (!current->mm)
> goto leave;
>
> and add the "leave" label below.
>
> Now you're saving yet another indentation level, bringing the meat of
> the function at 1st indentation level, which is cool and gives you much
> more room so that you don't have to linebreak longer statements.
>


sure, thanks! :)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


rob at landley

May 10, 2012, 2:42 PM

Post #6 of 14 (160 views)
Permalink
Re: [PATCH v4 3/7] x86/flush_tlb: try flush_tlb_single one by one in flush_tlb_range [In reply to]

On 05/10/2012 03:50 AM, Alex Shi wrote:
>>
>
>> Ok, question:
>>
>> we're comparing TLB size with the amount of pages mapped by this mm
>> struct. AFAICT, that doesn't mean that all those mapped pages do have
>> respective entries in the TLB, does it?
>>
>> If so, then the actual entries number is kinda inaccurate, no? We don't
>> really know how many TLB entries actually belong to this mm struct. Or am I
>> missing something?
>
> No, we can not know the exactly TLB entires for. But usually, when you
> process is doing the mprotect/munmap etc system call, your process has
> taken much of memory and already filled lots of TLB entries.

$ strace true 2>&1 | grep mprotect
mprotect(0x7f67a934b000, 2093056, PROT_NONE) = 0
mprotect(0x7f67a954a000, 16384, PROT_READ) = 0
mprotect(0x607000, 4096, PROT_READ) = 0
mprotect(0x7f67a9773000, 4096, PROT_READ) = 0

This appears to be part of glibc process setup. Define "usually".

Rob
--
GNU/Linux isn't: Linux=GPLv2, GNU=GPLv3+, they can't share code.
Either it's "mere aggregation", or a license violation. Pick one.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


alex.shi at intel

May 12, 2012, 1:01 AM

Post #7 of 14 (156 views)
Permalink
Re: [PATCH v4 3/7] x86/flush_tlb: try flush_tlb_single one by one in flush_tlb_range [In reply to]

On 05/10/2012 05:04 PM, Alex Shi wrote:

>
>>> +
>>> +#define FLUSHALL_BAR 16
>>> +
>>
>> Btw, you can save a bunch of indenting on this function, let me add
>> the final version here from the whole patchset so I can comment on it
>> easier:
>>
>>> void __flush_tlb_range(struct mm_struct *mm, unsigned long start,
>>> unsigned long end, unsigned long vmflag)
>>> {
>>> preempt_disable();
>>> if (current->active_mm == mm) {
>>
>> if (current->active_mm != mm)
>> goto flush_all;
>>
>> Now this whole piece below can move one indentation level to the left.


that is helpful and not imply logical too much.

>>
>> Then you can do:
>>
>> if (!current->mm)
>> goto leave;
>>
>> and add the "leave" label below.


I tried this, found too many goto and label is worse than line breaking. :(

>>
>> Now you're saving yet another indentation level, bringing the meat of
>> the function at 1st indentation level, which is cool and gives you much
>> more room so that you don't have to linebreak longer statements.
>>
>
>
> sure, thanks! :)


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


bp at amd64

May 13, 2012, 4:13 AM

Post #8 of 14 (156 views)
Permalink
Re: [PATCH v4 3/7] x86/flush_tlb: try flush_tlb_single one by one in flush_tlb_range [In reply to]

On Sat, May 12, 2012 at 04:01:20PM +0800, Alex Shi wrote:
> I tried this, found too many goto and label is worse than line breaking. :(

I know exactly what you're saying, I just tried it too. It looks pretty
ugly and unreadable.

Maybe you can carve out the meat of the function into another helper,
see below.

Dunno, it kinda looks ok if I haven't fat-fingered all the return paths
and it could use a bunch of comments and maybe even better naming to
explain what happens:

---
static bool __flush_tlb_range(unsigned int cpu, struct mm_struct *mm,
unsigned long start, unsigned long end,
unsigned long vmflag)
{
unsigned long addr;
unsigned long act_entries, tlb_entries = 0;

if (end == TLB_FLUSH_ALL || tlb_flushall_factor == (u16)TLB_FLUSH_ALL)
goto flush_out;

tlb_entries = (vmflag & VM_EXEC ? tlb_lli_4k[ENTRIES]
: tlb_lld_4k[ENTRIES]);

act_entries = min(mm->total_vm, tlb_entries);

if ((end - start) >> PAGE_SHIFT > act_entries >> tlb_flushall_factor)
goto flush_out;

if (has_large_page(mm, start, end))
goto flush_out;

for (addr = start; addr <= end; addr += PAGE_SIZE)
__flush_tlb_single(addr);

if (cpumask_any_but(mm_cpumask(mm), cpu) < nr_cpu_ids)
flush_tlb_others(mm_cpumask(mm), mm, start, end);

return true;

flush_out:
local_flush_tlb();
return false;
}

void _flush_tlb_range(struct mm_struct *mm, unsigned long start,
unsigned long end, unsigned long vmflag)
{
unsigned int cpu = smp_processor_id();

preempt_disable();

if (current->active_mm != mm)
goto flush_all;

if (!current->mm) {
leave_mm(cpu);
goto flush_all;
}

if (__flush_tlb_range(cpu, mm, start, end, vmflag))
goto out;

flush_all:
if (cpumask_any_but(mm_cpumask(mm), cpu) < nr_cpu_ids)
flush_tlb_others(mm_cpumask(mm), mm, 0UL, TLB_FLUSH_ALL);

out:
preempt_enable();
}
--

Oh well, enough games :-).

--
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


alex.shi at intel

May 14, 2012, 6:06 PM

Post #9 of 14 (156 views)
Permalink
Re: [PATCH v4 3/7] x86/flush_tlb: try flush_tlb_single one by one in flush_tlb_range [In reply to]

> --
>
> Oh well, enough games :-).
>




Thanks for your input. but actually, total 3 labels doesn't looks good.
Maybe the following lines can meet your expectation.

---
void __flush_tlb_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();
if (current->active_mm != mm)
goto flush_all;

if (!current->mm) {
leave_mm(smp_processor_id());
goto flush_all;
}

if (end == TLB_FLUSH_ALL ||
tlb_flushall_shift == (u16)TLB_FLUSH_ALL) {
local_flush_tlb();
goto flush_all;
}

if (vmflag & VM_EXEC)
tlb_entries = tlb_lli_4k[ENTRIES];
else
tlb_entries = tlb_lld_4k[ENTRIES];
act_entries = mm->total_vm > tlb_entries ? tlb_entries : mm->total_vm;

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;
}
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();
}


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


bp at amd64

May 15, 2012, 3:33 AM

Post #10 of 14 (158 views)
Permalink
Re: [PATCH v4 3/7] x86/flush_tlb: try flush_tlb_single one by one in flush_tlb_range [In reply to]

On Tue, May 15, 2012 at 09:06:11AM +0800, Alex Shi wrote:
> Thanks for your input. but actually, total 3 labels doesn't looks
> good. Maybe the following lines can meet your expectation.

Yeah, a bit better.

--
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


a.p.zijlstra at chello

May 15, 2012, 4:16 AM

Post #11 of 14 (163 views)
Permalink
Re: [PATCH v4 3/7] x86/flush_tlb: try flush_tlb_single one by one in flush_tlb_range [In reply to]

On Tue, 2012-05-15 at 12:33 +0200, Borislav Petkov wrote:
> On Tue, May 15, 2012 at 09:06:11AM +0800, Alex Shi wrote:
> > Thanks for your input. but actually, total 3 labels doesn't looks
> > good. Maybe the following lines can meet your expectation.
>
> Yeah, a bit better.

Wouldn't all this become much prettier if you rip out that multi-vector
stuff?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


bp at amd64

May 15, 2012, 4:56 AM

Post #12 of 14 (159 views)
Permalink
Re: [PATCH v4 3/7] x86/flush_tlb: try flush_tlb_single one by one in flush_tlb_range [In reply to]

On Tue, May 15, 2012 at 01:16:22PM +0200, Peter Zijlstra wrote:
> Wouldn't all this become much prettier if you rip out that
> multi-vector stuff?

multi-vector stuff? Please elaborate.

--
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


a.p.zijlstra at chello

May 15, 2012, 5:00 AM

Post #13 of 14 (159 views)
Permalink
Re: [PATCH v4 3/7] x86/flush_tlb: try flush_tlb_single one by one in flush_tlb_range [In reply to]

On Tue, 2012-05-15 at 13:56 +0200, Borislav Petkov wrote:
> On Tue, May 15, 2012 at 01:16:22PM +0200, Peter Zijlstra wrote:
> > Wouldn't all this become much prettier if you rip out that
> > multi-vector stuff?
>
> multi-vector stuff? Please elaborate.

The INVALIDATE_TLB_VECTOR muck:

apic->send_IPI_mask(to_cpumask(f->flush_cpumask),
INVALIDATE_TLB_VECTOR_START + sender);

and simply use:

smp_call_function()

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


alex.shi at intel

May 15, 2012, 6:58 AM

Post #14 of 14 (160 views)
Permalink
Re: [PATCH v4 3/7] x86/flush_tlb: try flush_tlb_single one by one in flush_tlb_range [In reply to]

On 05/15/2012 08:00 PM, Peter Zijlstra wrote:

> On Tue, 2012-05-15 at 13:56 +0200, Borislav Petkov wrote:
>> On Tue, May 15, 2012 at 01:16:22PM +0200, Peter Zijlstra wrote:
>>> Wouldn't all this become much prettier if you rip out that
>>> multi-vector stuff?
>>
>> multi-vector stuff? Please elaborate.
>
> The INVALIDATE_TLB_VECTOR muck:
>
> apic->send_IPI_mask(to_cpumask(f->flush_cpumask),
> INVALIDATE_TLB_VECTOR_START + sender);
>
> and simply use:
>
> smp_call_function()
>


I am trying this following the patchset.
but that should another work thread.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Linux kernel RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.