Skip to contentSkip to conversation

x86 TLB flush cleanups, moving toward PCID support

PATCH Superseded Andy Lutomirski 4 participants 18 messages May 22, 2017 – May 24, 2017

Subsystems (11)

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Andy Lutomirski

author maintainer
#0 lore raw

v2 cover x86 TLB flush cleanups, moving toward PCID support

As I've been working on polishing my PCID code, a major problem I've encountered is that there are too many x86 TLB flushing code paths and that they have too many inconsequential differences. The result was that earlier versions of the PCID code were a colossal mess and very difficult to understand.

This series goes a long way toward cleaning up the mess. With all the patches applied, there is a single function that contains the meat of the code to flush the TLB on a given CPU, and all the tlb flushing APIs call it for both local and remote CPUs.

This series should only adversely affect the kernel in a couple of minor ways:

- It makes smp_mb() unconditional when flushing TLBs. We used to use the TLB flush itself to mostly avoid smp_mb() on the initiating CPU.

- On UP kernels, we lose the dubious optimization of inlining nerfed variants of all the TLB flush APIs. This bloats the kernel a tiny bit, although it should increase performance, since the SMP versions were better.

Patch 10 in here is a little bit off topic. It's a cleanup that's also needed before PCID can go in, but it's not directly about TLB flushing.

Changes from RFC:
- Fixed missing call to arch_tlbbatch_flush().
- "Be more consistent wrt PAGE_SHIFT vs PAGE_SIZE in tlb flush code" is new
- Misc typos fixed.
- Actually compiles when UV is enabled.

Andy Lutomirski (11):
x86/mm: Reimplement flush_tlb_page() using flush_tlb_mm_range() x86/mm: Reduce indentation in flush_tlb_func()
x86/mm: Make the batched unmap TLB flush API more generic x86/mm: Pass flush_tlb_info to flush_tlb_others() etc x86/mm: Change the leave_mm() condition for local TLB flushes x86/mm: Refactor flush_tlb_mm_range() to merge local and remote cases x86/mm: Use new merged flush logic in arch_tlbbatch_flush() x86/mm: Remove the UP tlbflush code; always use the formerly SMP code x86/mm: Rework lazy TLB to track the actual loaded mm x86/mm: Be more consistent wrt PAGE_SHIFT vs PAGE_SIZE in tlb flush
code
x86,kvm: Teach KVM's VMX code that CR3 isn't a constant

 arch/x86/Kconfig                      |   2 +-
 arch/x86/events/core.c                |   3 +-
 arch/x86/include/asm/hardirq.h        |   2 +-
 arch/x86/include/asm/mmu.h            |   6 -
 arch/x86/include/asm/mmu_context.h    |  21 +-
 arch/x86/include/asm/paravirt.h       |   6 +-
 arch/x86/include/asm/paravirt_types.h |   5 +-
 arch/x86/include/asm/tlbbatch.h       |  14 ++
 arch/x86/include/asm/tlbflush.h       | 116 +++------
 arch/x86/include/asm/uv/uv.h          |   9 +-
 arch/x86/kernel/ldt.c                 |   7 +-
 arch/x86/kvm/vmx.c                    |  21 +-
 arch/x86/mm/init.c                    |   4 +-
 arch/x86/mm/tlb.c                     | 429 +++++++++++++++-------------------
 arch/x86/platform/uv/tlb_uv.c         |  10 +-
 arch/x86/xen/mmu.c                    |  61 +++--
 include/linux/mm_types_task.h         |  15 +-
 mm/rmap.c                             |  16 +-
 18 files changed, 336 insertions(+), 411 deletions(-)
 create mode 100644 arch/x86/include/asm/tlbbatch.h
Signature · 7 lines
2.9.3

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
author maintainer
#1 lore raw

PATCH v2 1/11 x86/mm: Reimplement flush_tlb_page() using flush_tlb_mm_range()

flush_tlb_page() was very similar to flush_tlb_mm_range() except that it had a couple of issues:

- It was missing an smp_mb() in the case where current->active_mm != mm. (This is a longstanding bug reported by Nadav Amit.)

- It was missing tracepoints and vm counter updates.

The only reason that I can see for keeping it at as a separate function is that it could avoid a few branches that flush_tlb_mm_range() needs to decide to flush just one page. This hardly seems worthwhile. If we decide we want to get rid of those branches again, a better way would be to introduce an __flush_tlb_mm_range() helper and make both flush_tlb_page() and flush_tlb_mm_range() use it.

Cc Rik van Riel <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Nadav Amit <[email protected]>
Cc Michal Hocko <[email protected]>
Cc Andrew Morton <[email protected]>
---
 arch/x86/include/asm/tlbflush.h |  6 +++++-
 arch/x86/mm/tlb.c               | 27 ---------------------------
 2 files changed, 5 insertions(+), 28 deletions(-)
arch/x86/include/asm/tlbflush.h +5 -1
index 6ed9ea469b48..b9db0f8fef55 100644
@@ -307,11 +307,15 @@ static inline void flush_tlb_kernel_range(unsigned long start,
 		flush_tlb_mm_range(vma->vm_mm, start, end, vma->vm_flags)
 
 extern void flush_tlb_all(void);
-extern void flush_tlb_page(struct vm_area_struct *, unsigned long);
 extern void flush_tlb_mm_range(struct mm_struct *mm, unsigned long start,
 				unsigned long end, unsigned long vmflag);
 extern void flush_tlb_kernel_range(unsigned long start, unsigned long end);
 
+static inline void flush_tlb_page(struct vm_area_struct *vma, unsigned long a)
+{
+	flush_tlb_mm_range(vma->vm_mm, a, a + PAGE_SIZE, VM_NONE);
+}
+
 void native_flush_tlb_others(const struct cpumask *cpumask,
 				struct mm_struct *mm,
 				unsigned long start, unsigned long end);
arch/x86/mm/tlb.c +0 -27
index 6e7bedf69af7..fe6471132ea3 100644
@@ -354,33 +354,6 @@ void flush_tlb_mm_range(struct mm_struct *mm, unsigned long start,
 	preempt_enable();
 }
 
-void flush_tlb_page(struct vm_area_struct *vma, unsigned long start)
-{
-	struct mm_struct *mm = vma->vm_mm;
-
-	preempt_disable();
-
-	if (current->active_mm == mm) {
-		if (current->mm) {
-			/*
-			 * Implicit full barrier (INVLPG) that synchronizes
-			 * with switch_mm.
-			 */
-			__flush_tlb_one(start);
-		} else {
-			leave_mm(smp_processor_id());
-
-			/* Synchronize with switch_mm. */
-			smp_mb();
-		}
-	}
-
-	if (cpumask_any_but(mm_cpumask(mm), smp_processor_id()) < nr_cpu_ids)
-		flush_tlb_others(mm_cpumask(mm), mm, start, start + PAGE_SIZE);
-
-	preempt_enable();
-}
-
 static void do_flush_tlb_all(void *info)
 {
 	count_vm_tlb_event(NR_TLB_REMOTE_FLUSH_RECEIVED);
Signature · 7 lines
2.9.3

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#2 lore raw

[tip:x86/mm] x86/mm: Reimplement flush_tlb_page() using flush_tlb_mm_range()

x86/mm: Reimplement flush_tlb_page() using flush_tlb_mm_range()
Commit-ID:  ca6c99c0794875c6d1db6e22f246699691ab7e6b
Gitweb:     http://git.kernel.org/tip/ca6c99c0794875c6d1db6e22f246699691ab7e6b
Author:     Andy Lutomirski <[email protected]>
AuthorDate: Mon, 22 May 2017 15:30:01 -0700
Committer:  Ingo Molnar <[email protected]>
CommitDate: Wed, 24 May 2017 10:18:27 +0200

x86/mm: Reimplement flush_tlb_page() using flush_tlb_mm_range()

flush_tlb_page() was very similar to flush_tlb_mm_range() except that it had a couple of issues:

- It was missing an smp_mb() in the case where
current->active_mm != mm. (This is a longstanding bug reported by Nadav Amit)

- It was missing tracepoints and vm counter updates.

The only reason that I can see for keeping it at as a separate function is that it could avoid a few branches that flush_tlb_mm_range() needs to decide to flush just one page. This hardly seems worthwhile. If we decide we want to get rid of those branches again, a better way would be to introduce an __flush_tlb_mm_range() helper and make both flush_tlb_page() and flush_tlb_mm_range() use it.

---
 arch/x86/include/asm/tlbflush.h |  6 +++++-
 arch/x86/mm/tlb.c               | 27 ---------------------------
 2 files changed, 5 insertions(+), 28 deletions(-)
arch/x86/include/asm/tlbflush.h +5 -1
index 6ed9ea4..b9db0f8fe 100644
@@ -307,11 +307,15 @@ static inline void flush_tlb_kernel_range(unsigned long start,
 		flush_tlb_mm_range(vma->vm_mm, start, end, vma->vm_flags)
 
 extern void flush_tlb_all(void);
-extern void flush_tlb_page(struct vm_area_struct *, unsigned long);
 extern void flush_tlb_mm_range(struct mm_struct *mm, unsigned long start,
 				unsigned long end, unsigned long vmflag);
 extern void flush_tlb_kernel_range(unsigned long start, unsigned long end);
 
+static inline void flush_tlb_page(struct vm_area_struct *vma, unsigned long a)
+{
+	flush_tlb_mm_range(vma->vm_mm, a, a + PAGE_SIZE, VM_NONE);
+}
+
 void native_flush_tlb_others(const struct cpumask *cpumask,
 				struct mm_struct *mm,
 				unsigned long start, unsigned long end);
arch/x86/mm/tlb.c +0 -27
index 6e7bedf..fe64711 100644
@@ -354,33 +354,6 @@ out:
 	preempt_enable();
 }
 
-void flush_tlb_page(struct vm_area_struct *vma, unsigned long start)
-{
-	struct mm_struct *mm = vma->vm_mm;
-
-	preempt_disable();
-
-	if (current->active_mm == mm) {
-		if (current->mm) {
-			/*
-			 * Implicit full barrier (INVLPG) that synchronizes
-			 * with switch_mm.
-			 */
-			__flush_tlb_one(start);
-		} else {
-			leave_mm(smp_processor_id());
-
-			/* Synchronize with switch_mm. */
-			smp_mb();
-		}
-	}
-
-	if (cpumask_any_but(mm_cpumask(mm), smp_processor_id()) < nr_cpu_ids)
-		flush_tlb_others(mm_cpumask(mm), mm, start, start + PAGE_SIZE);
-
-	preempt_enable();
-}
-
 static void do_flush_tlb_all(void *info)
 {
 	count_vm_tlb_event(NR_TLB_REMOTE_FLUSH_RECEIVED);
author maintainer
#3 lore raw

PATCH v2 2/11 x86/mm: Reduce indentation in flush_tlb_func()

In reply to Andy Lutomirski

The leave_mm() case can just exit the function early so we don't need to indent the entire remainder of the function.

Cc Rik van Riel <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Nadav Amit <[email protected]>
Cc Michal Hocko <[email protected]>
Cc Andrew Morton <[email protected]>
---
 arch/x86/mm/tlb.c | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)
arch/x86/mm/tlb.c +18 -16
index fe6471132ea3..4d303864b310 100644
@@ -237,24 +237,26 @@ static void flush_tlb_func(void *info)
 		return;
 
 	count_vm_tlb_event(NR_TLB_REMOTE_FLUSH_RECEIVED);
-	if (this_cpu_read(cpu_tlbstate.state) == TLBSTATE_OK) {
-		if (f->flush_end == TLB_FLUSH_ALL) {
-			local_flush_tlb();
-			trace_tlb_flush(TLB_REMOTE_SHOOTDOWN, TLB_FLUSH_ALL);
-		} else {
-			unsigned long addr;
-			unsigned long nr_pages =
-				(f->flush_end - f->flush_start) / PAGE_SIZE;
-			addr = f->flush_start;
-			while (addr < f->flush_end) {
-				__flush_tlb_single(addr);
-				addr += PAGE_SIZE;
-			}
-			trace_tlb_flush(TLB_REMOTE_SHOOTDOWN, nr_pages);
-		}
-	} else
+
+	if (this_cpu_read(cpu_tlbstate.state) != TLBSTATE_OK) {
 		leave_mm(smp_processor_id());
+		return;
+	}
 
+	if (f->flush_end == TLB_FLUSH_ALL) {
+		local_flush_tlb();
+		trace_tlb_flush(TLB_REMOTE_SHOOTDOWN, TLB_FLUSH_ALL);
+	} else {
+		unsigned long addr;
+		unsigned long nr_pages =
+			(f->flush_end - f->flush_start) / PAGE_SIZE;
+		addr = f->flush_start;
+		while (addr < f->flush_end) {
+			__flush_tlb_single(addr);
+			addr += PAGE_SIZE;
+		}
+		trace_tlb_flush(TLB_REMOTE_SHOOTDOWN, nr_pages);
+	}
 }
 
 void native_flush_tlb_others(const struct cpumask *cpumask,
Signature · 7 lines
2.9.3

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#4 lore raw

[tip:x86/mm] x86/mm: Reduce indentation in flush_tlb_func()

x86/mm: Reduce indentation in flush_tlb_func()
Commit-ID:  b3b90e5af7976e46541f5029a369c9c38c5e4cea
Gitweb:     http://git.kernel.org/tip/b3b90e5af7976e46541f5029a369c9c38c5e4cea
Author:     Andy Lutomirski <[email protected]>
AuthorDate: Mon, 22 May 2017 15:30:02 -0700
Committer:  Ingo Molnar <[email protected]>
CommitDate: Wed, 24 May 2017 10:18:27 +0200

x86/mm: Reduce indentation in flush_tlb_func()

The leave_mm() case can just exit the function early so we don't need to indent the entire remainder of the function.

---
 arch/x86/mm/tlb.c | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)
arch/x86/mm/tlb.c +18 -16
index fe64711..4d30386 100644
@@ -237,24 +237,26 @@ static void flush_tlb_func(void *info)
 		return;
 
 	count_vm_tlb_event(NR_TLB_REMOTE_FLUSH_RECEIVED);
-	if (this_cpu_read(cpu_tlbstate.state) == TLBSTATE_OK) {
-		if (f->flush_end == TLB_FLUSH_ALL) {
-			local_flush_tlb();
-			trace_tlb_flush(TLB_REMOTE_SHOOTDOWN, TLB_FLUSH_ALL);
-		} else {
-			unsigned long addr;
-			unsigned long nr_pages =
-				(f->flush_end - f->flush_start) / PAGE_SIZE;
-			addr = f->flush_start;
-			while (addr < f->flush_end) {
-				__flush_tlb_single(addr);
-				addr += PAGE_SIZE;
-			}
-			trace_tlb_flush(TLB_REMOTE_SHOOTDOWN, nr_pages);
-		}
-	} else
+
+	if (this_cpu_read(cpu_tlbstate.state) != TLBSTATE_OK) {
 		leave_mm(smp_processor_id());
+		return;
+	}
 
+	if (f->flush_end == TLB_FLUSH_ALL) {
+		local_flush_tlb();
+		trace_tlb_flush(TLB_REMOTE_SHOOTDOWN, TLB_FLUSH_ALL);
+	} else {
+		unsigned long addr;
+		unsigned long nr_pages =
+			(f->flush_end - f->flush_start) / PAGE_SIZE;
+		addr = f->flush_start;
+		while (addr < f->flush_end) {
+			__flush_tlb_single(addr);
+			addr += PAGE_SIZE;
+		}
+		trace_tlb_flush(TLB_REMOTE_SHOOTDOWN, nr_pages);
+	}
 }
 
 void native_flush_tlb_others(const struct cpumask *cpumask,
author maintainer
#5 lore raw

PATCH v2 3/11 x86/mm: Make the batched unmap TLB flush API more generic

In reply to Andy Lutomirski

try_to_unmap_flush() used to open-code a rather x86-centric flush
sequence: local_flush_tlb() + flush_tlb_others(). Rearrange the code so that the arch (only x86 for now) provides arch_tlbbatch_add_mm() and arch_tlbbatch_flush() and the core code calls those functions instead.

I'll want this for x86 because, to enable address space ids, I can't support the flush_tlb_others() mode used by exising try_to_unmap_flush() implementation with good performance. I can support the new API fairly easily, though.

I imagine that other architectures may be in a similar position. Architectures with strong remote flush primitives (arm64?) may have even worse performance problems with flush_tlb_others() the way that try_to_unmap_flush() uses it.

Cc Mel Gorman <[email protected]>
Cc Rik van Riel <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Nadav Amit <[email protected]>
Cc Michal Hocko <[email protected]>
Cc Sasha Levin <[email protected]>
Cc Andrew Morton <[email protected]>
---
 arch/x86/include/asm/tlbbatch.h | 16 ++++++++++++++++
 arch/x86/include/asm/tlbflush.h |  8 ++++++++
 arch/x86/mm/tlb.c               | 17 +++++++++++++++++
 include/linux/mm_types_task.h   | 15 +++++++++++----
 mm/rmap.c                       | 16 ++--------------
 5 files changed, 54 insertions(+), 18 deletions(-)
 create mode 100644 arch/x86/include/asm/tlbbatch.h
arch/x86/include/asm/tlbbatch.h new file +16 -0
new file mode 100644
index 000000000000..01a6de16fb96
@@ -0,0 +1,16 @@
+#ifndef _ARCH_X86_TLBBATCH_H
+#define _ARCH_X86_TLBBATCH_H
+
+#include <linux/cpumask.h>
+
+#ifdef CONFIG_SMP
+struct arch_tlbflush_unmap_batch {
+	/*
+	 * Each bit set is a CPU that potentially has a TLB entry for one of
+	 * the PFNs being flushed..
+	 */
+	struct cpumask cpumask;
+};
+#endif
+
+#endif /* _ARCH_X86_TLBBATCH_H */
arch/x86/include/asm/tlbflush.h +8 -0
index b9db0f8fef55..8f6e2f87511b 100644
@@ -329,6 +329,14 @@ static inline void reset_lazy_tlbstate(void)
 	this_cpu_write(cpu_tlbstate.active_mm, &init_mm);
 }
 
+static inline void arch_tlbbatch_add_mm(struct arch_tlbflush_unmap_batch *batch,
+					struct mm_struct *mm)
+{
+	cpumask_or(&batch->cpumask, &batch->cpumask, mm_cpumask(mm));
+}
+
+extern void arch_tlbbatch_flush(struct arch_tlbflush_unmap_batch *batch);
+
 #endif	/* SMP */
 
 #ifndef CONFIG_PARAVIRT
arch/x86/mm/tlb.c +17 -0
index 4d303864b310..743e4c6b4529 100644
@@ -395,6 +395,23 @@ void flush_tlb_kernel_range(unsigned long start, unsigned long end)
 	}
 }
 
+void arch_tlbbatch_flush(struct arch_tlbflush_unmap_batch *batch)
+{
+	int cpu = get_cpu();
+
+	if (cpumask_test_cpu(cpu, &batch->cpumask)) {
+		count_vm_tlb_event(NR_TLB_LOCAL_FLUSH_ALL);
+		local_flush_tlb();
+		trace_tlb_flush(TLB_LOCAL_SHOOTDOWN, TLB_FLUSH_ALL);
+	}
+
+	if (cpumask_any_but(&batch->cpumask, cpu) < nr_cpu_ids)
+		flush_tlb_others(&batch->cpumask, NULL, 0, TLB_FLUSH_ALL);
+	cpumask_clear(&batch->cpumask);
+
+	put_cpu();
+}
+
 static ssize_t tlbflush_read_file(struct file *file, char __user *user_buf,
 			     size_t count, loff_t *ppos)
 {
include/linux/mm_types_task.h +11 -4
index 136dfdf63ba1..fc412fbd80bd 100644
@@ -14,6 +14,10 @@
 
 #include <asm/page.h>
 
+#ifdef CONFIG_ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH
+#include <asm/tlbbatch.h>
+#endif
+
 #define USE_SPLIT_PTE_PTLOCKS	(NR_CPUS >= CONFIG_SPLIT_PTLOCK_CPUS)
 #define USE_SPLIT_PMD_PTLOCKS	(USE_SPLIT_PTE_PTLOCKS && \
 		IS_ENABLED(CONFIG_ARCH_ENABLE_SPLIT_PMD_PTLOCK))
@@ -67,12 +71,15 @@ struct page_frag {
 struct tlbflush_unmap_batch {
 #ifdef CONFIG_ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH
 	/*
-	 * Each bit set is a CPU that potentially has a TLB entry for one of
-	 * the PFNs being flushed. See set_tlb_ubc_flush_pending().
+	 * The arch code makes the following promise: generic code can modify a
+	 * PTE, then call arch_tlbbatch_add_mm() (which internally provides all
+	 * needed barriers), then call arch_tlbbatch_flush(), and the entries
+	 * will be flushed on all CPUs by the time that arch_tlbbatch_flush()
+	 * returns.
 	 */
-	struct cpumask cpumask;
+	struct arch_tlbflush_unmap_batch arch;
 
-	/* True if any bit in cpumask is set */
+	/* True if a flush is needed. */
 	bool flush_required;
 
 	/*
mm/rmap.c +2 -14
index f6838015810f..f29d725528d2 100644
@@ -579,25 +579,13 @@ void page_unlock_anon_vma_read(struct anon_vma *anon_vma)
 void try_to_unmap_flush(void)
 {
 	struct tlbflush_unmap_batch *tlb_ubc = &current->tlb_ubc;
-	int cpu;
 
 	if (!tlb_ubc->flush_required)
 		return;
 
-	cpu = get_cpu();
-
-	if (cpumask_test_cpu(cpu, &tlb_ubc->cpumask)) {
-		count_vm_tlb_event(NR_TLB_LOCAL_FLUSH_ALL);
-		local_flush_tlb();
-		trace_tlb_flush(TLB_LOCAL_SHOOTDOWN, TLB_FLUSH_ALL);
-	}
-
-	if (cpumask_any_but(&tlb_ubc->cpumask, cpu) < nr_cpu_ids)
-		flush_tlb_others(&tlb_ubc->cpumask, NULL, 0, TLB_FLUSH_ALL);
-	cpumask_clear(&tlb_ubc->cpumask);
+	arch_tlbbatch_flush(&tlb_ubc->arch);
 	tlb_ubc->flush_required = false;
 	tlb_ubc->writable = false;
-	put_cpu();
 }
 
 /* Flush iff there are potentially writable TLB entries that can race with IO */
@@ -613,7 +601,7 @@ static void set_tlb_ubc_flush_pending(struct mm_struct *mm, bool writable)
 {
 	struct tlbflush_unmap_batch *tlb_ubc = &current->tlb_ubc;
 
-	cpumask_or(&tlb_ubc->cpumask, &tlb_ubc->cpumask, mm_cpumask(mm));
+	arch_tlbbatch_add_mm(&tlb_ubc->arch, mm);
 	tlb_ubc->flush_required = true;
 
 	/*
Signature · 7 lines
2.9.3

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#6 lore raw

[tip:x86/mm] mm, x86/mm: Make the batched unmap TLB flush API more generic

mm, x86/mm: Make the batched unmap TLB flush API more generic
Commit-ID:  e73ad5ff2f76da25390e9607cb549691639330c3
Gitweb:     http://git.kernel.org/tip/e73ad5ff2f76da25390e9607cb549691639330c3
Author:     Andy Lutomirski <[email protected]>
AuthorDate: Mon, 22 May 2017 15:30:03 -0700
Committer:  Ingo Molnar <[email protected]>
CommitDate: Wed, 24 May 2017 10:18:27 +0200

mm, x86/mm: Make the batched unmap TLB flush API more generic

try_to_unmap_flush() used to open-code a rather x86-centric flush
sequence: local_flush_tlb() + flush_tlb_others(). Rearrange the code so that the arch (only x86 for now) provides arch_tlbbatch_add_mm() and arch_tlbbatch_flush() and the core code calls those functions instead.

I'll want this for x86 because, to enable address space ids, I can't support the flush_tlb_others() mode used by exising try_to_unmap_flush() implementation with good performance. I can support the new API fairly easily, though.

I imagine that other architectures may be in a similar position. Architectures with strong remote flush primitives (arm64?) may have even worse performance problems with flush_tlb_others() the way that try_to_unmap_flush() uses it.

---
 arch/x86/include/asm/tlbbatch.h | 16 ++++++++++++++++
 arch/x86/include/asm/tlbflush.h |  8 ++++++++
 arch/x86/mm/tlb.c               | 17 +++++++++++++++++
 include/linux/mm_types_task.h   | 15 +++++++++++----
 mm/rmap.c                       | 16 ++--------------
 5 files changed, 54 insertions(+), 18 deletions(-)
arch/x86/include/asm/tlbbatch.h new file +16 -0
new file mode 100644
index 0000000..01a6de1
@@ -0,0 +1,16 @@
+#ifndef _ARCH_X86_TLBBATCH_H
+#define _ARCH_X86_TLBBATCH_H
+
+#include <linux/cpumask.h>
+
+#ifdef CONFIG_SMP
+struct arch_tlbflush_unmap_batch {
+	/*
+	 * Each bit set is a CPU that potentially has a TLB entry for one of
+	 * the PFNs being flushed..
+	 */
+	struct cpumask cpumask;
+};
+#endif
+
+#endif /* _ARCH_X86_TLBBATCH_H */
arch/x86/include/asm/tlbflush.h +8 -0
index b9db0f8fe..8f6e2f8 100644
@@ -329,6 +329,14 @@ static inline void reset_lazy_tlbstate(void)
 	this_cpu_write(cpu_tlbstate.active_mm, &init_mm);
 }
 
+static inline void arch_tlbbatch_add_mm(struct arch_tlbflush_unmap_batch *batch,
+					struct mm_struct *mm)
+{
+	cpumask_or(&batch->cpumask, &batch->cpumask, mm_cpumask(mm));
+}
+
+extern void arch_tlbbatch_flush(struct arch_tlbflush_unmap_batch *batch);
+
 #endif	/* SMP */
 
 #ifndef CONFIG_PARAVIRT
arch/x86/mm/tlb.c +17 -0
index 4d30386..743e4c6 100644
@@ -395,6 +395,23 @@ void flush_tlb_kernel_range(unsigned long start, unsigned long end)
 	}
 }
 
+void arch_tlbbatch_flush(struct arch_tlbflush_unmap_batch *batch)
+{
+	int cpu = get_cpu();
+
+	if (cpumask_test_cpu(cpu, &batch->cpumask)) {
+		count_vm_tlb_event(NR_TLB_LOCAL_FLUSH_ALL);
+		local_flush_tlb();
+		trace_tlb_flush(TLB_LOCAL_SHOOTDOWN, TLB_FLUSH_ALL);
+	}
+
+	if (cpumask_any_but(&batch->cpumask, cpu) < nr_cpu_ids)
+		flush_tlb_others(&batch->cpumask, NULL, 0, TLB_FLUSH_ALL);
+	cpumask_clear(&batch->cpumask);
+
+	put_cpu();
+}
+
 static ssize_t tlbflush_read_file(struct file *file, char __user *user_buf,
 			     size_t count, loff_t *ppos)
 {
include/linux/mm_types_task.h +11 -4
index 136dfdf..fc412fb 100644
@@ -14,6 +14,10 @@
 
 #include <asm/page.h>
 
+#ifdef CONFIG_ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH
+#include <asm/tlbbatch.h>
+#endif
+
 #define USE_SPLIT_PTE_PTLOCKS	(NR_CPUS >= CONFIG_SPLIT_PTLOCK_CPUS)
 #define USE_SPLIT_PMD_PTLOCKS	(USE_SPLIT_PTE_PTLOCKS && \
 		IS_ENABLED(CONFIG_ARCH_ENABLE_SPLIT_PMD_PTLOCK))
@@ -67,12 +71,15 @@ struct page_frag {
 struct tlbflush_unmap_batch {
 #ifdef CONFIG_ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH
 	/*
-	 * Each bit set is a CPU that potentially has a TLB entry for one of
-	 * the PFNs being flushed. See set_tlb_ubc_flush_pending().
+	 * The arch code makes the following promise: generic code can modify a
+	 * PTE, then call arch_tlbbatch_add_mm() (which internally provides all
+	 * needed barriers), then call arch_tlbbatch_flush(), and the entries
+	 * will be flushed on all CPUs by the time that arch_tlbbatch_flush()
+	 * returns.
 	 */
-	struct cpumask cpumask;
+	struct arch_tlbflush_unmap_batch arch;
 
-	/* True if any bit in cpumask is set */
+	/* True if a flush is needed. */
 	bool flush_required;
 
 	/*
mm/rmap.c +2 -14
index d405f0e..130c238 100644
@@ -579,25 +579,13 @@ void page_unlock_anon_vma_read(struct anon_vma *anon_vma)
 void try_to_unmap_flush(void)
 {
 	struct tlbflush_unmap_batch *tlb_ubc = &current->tlb_ubc;
-	int cpu;
 
 	if (!tlb_ubc->flush_required)
 		return;
 
-	cpu = get_cpu();
-
-	if (cpumask_test_cpu(cpu, &tlb_ubc->cpumask)) {
-		count_vm_tlb_event(NR_TLB_LOCAL_FLUSH_ALL);
-		local_flush_tlb();
-		trace_tlb_flush(TLB_LOCAL_SHOOTDOWN, TLB_FLUSH_ALL);
-	}
-
-	if (cpumask_any_but(&tlb_ubc->cpumask, cpu) < nr_cpu_ids)
-		flush_tlb_others(&tlb_ubc->cpumask, NULL, 0, TLB_FLUSH_ALL);
-	cpumask_clear(&tlb_ubc->cpumask);
+	arch_tlbbatch_flush(&tlb_ubc->arch);
 	tlb_ubc->flush_required = false;
 	tlb_ubc->writable = false;
-	put_cpu();
 }
 
 /* Flush iff there are potentially writable TLB entries that can race with IO */
@@ -613,7 +601,7 @@ static void set_tlb_ubc_flush_pending(struct mm_struct *mm, bool writable)
 {
 	struct tlbflush_unmap_batch *tlb_ubc = &current->tlb_ubc;
 
-	cpumask_or(&tlb_ubc->cpumask, &tlb_ubc->cpumask, mm_cpumask(mm));
+	arch_tlbbatch_add_mm(&tlb_ubc->arch, mm);
 	tlb_ubc->flush_required = true;
 
 	/*
author maintainer
#7 lore raw

PATCH v2 4/11 x86/mm: Pass flush_tlb_info to flush_tlb_others() etc

In reply to Andy Lutomirski

Rather than passing all the contents of flush_tlb_info to flush_tlb_others(), pass a pointer to the structure directly. For consistency, this also removes the unnecessary cpu parameter from uv_flush_tlb_others() to make its signature match the other *flush_tlb_others() functions.

This serves two purposes:

- It will dramatically simplify future patches that change struct flush_tlb_info, which I'm planning to do.

- struct flush_tlb_info is an adequate description of what to do for a local flush, too, so by reusing it we can remove duplicated code between local and remove flushes in a future patch.

Cc Rik van Riel <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Nadav Amit <[email protected]>
Cc Michal Hocko <[email protected]>
Cc Andrew Morton <[email protected]>
---
 arch/x86/include/asm/paravirt.h       |  6 ++--
 arch/x86/include/asm/paravirt_types.h |  5 ++-
 arch/x86/include/asm/tlbflush.h       | 19 ++++++-----
 arch/x86/include/asm/uv/uv.h          |  9 ++---
 arch/x86/mm/tlb.c                     | 64 +++++++++++++++++------------------
 arch/x86/platform/uv/tlb_uv.c         | 10 +++---
 arch/x86/xen/mmu.c                    | 10 +++---
 7 files changed, 59 insertions(+), 64 deletions(-)
arch/x86/include/asm/paravirt.h +2 -4
index 55fa56fe4e45..9a15739d9f4b 100644
@@ -312,11 +312,9 @@ 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 start,
-				    unsigned long end)
+				    const struct flush_tlb_info *info)
 {
-	PVOP_VCALL4(pv_mmu_ops.flush_tlb_others, cpumask, mm, start, end);
+	PVOP_VCALL2(pv_mmu_ops.flush_tlb_others, cpumask, info);
 }
 
 static inline int paravirt_pgd_alloc(struct mm_struct *mm)
arch/x86/include/asm/paravirt_types.h +2 -3
index 7465d6fe336f..cb976bab6299 100644
@@ -51,6 +51,7 @@ struct mm_struct;
 struct desc_struct;
 struct task_struct;
 struct cpumask;
+struct flush_tlb_info;
 
 /*
  * Wrapper type for pointers to code which uses the non-standard
@@ -223,9 +224,7 @@ struct pv_mmu_ops {
 	void (*flush_tlb_kernel)(void);
 	void (*flush_tlb_single)(unsigned long addr);
 	void (*flush_tlb_others)(const struct cpumask *cpus,
-				 struct mm_struct *mm,
-				 unsigned long start,
-				 unsigned long end);
+				 const struct flush_tlb_info *info);
 
 	/* Hooks for allocating and freeing a pagetable top-level */
 	int  (*pgd_alloc)(struct mm_struct *mm);
arch/x86/include/asm/tlbflush.h +11 -8
index 8f6e2f87511b..6f439ac92026 100644
@@ -220,12 +220,18 @@ 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, start, end) flushes TLBs on other cpus
+ *  - flush_tlb_others(cpumask, info) 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.
  */
 
+struct flush_tlb_info {
+	struct mm_struct *mm;
+	unsigned long start;
+	unsigned long end;
+};
+
 #ifndef CONFIG_SMP
 
 /* "_up" is for UniProcessor.
@@ -279,9 +285,7 @@ static inline void flush_tlb_mm_range(struct mm_struct *mm,
 }
 
 static inline void native_flush_tlb_others(const struct cpumask *cpumask,
-					   struct mm_struct *mm,
-					   unsigned long start,
-					   unsigned long end)
+					   const struct flush_tlb_info *info)
 {
 }
 
@@ -317,8 +321,7 @@ static inline void flush_tlb_page(struct vm_area_struct *vma, unsigned long a)
 }
 
 void native_flush_tlb_others(const struct cpumask *cpumask,
-				struct mm_struct *mm,
-				unsigned long start, unsigned long end);
+			     const struct flush_tlb_info *info);
 
 #define TLBSTATE_OK	1
 #define TLBSTATE_LAZY	2
@@ -340,8 +343,8 @@ extern void arch_tlbbatch_flush(struct arch_tlbflush_unmap_batch *batch);
 #endif	/* SMP */
 
 #ifndef CONFIG_PARAVIRT
-#define flush_tlb_others(mask, mm, start, end)	\
-	native_flush_tlb_others(mask, mm, start, end)
+#define flush_tlb_others(mask, info)	\
+	native_flush_tlb_others(mask, info)
 #endif
 
 #endif /* _ASM_X86_TLBFLUSH_H */
arch/x86/include/asm/uv/uv.h +3 -6
index 6686820feae9..d2db4517930c 100644
@@ -15,10 +15,7 @@ extern void uv_cpu_init(void);
 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 start,
-						 unsigned long end,
-						 unsigned int cpu);
+						 const struct flush_tlb_info *info);
 
 #else	/* X86_UV */
 
@@ -28,8 +25,8 @@ static inline int is_uv_hubless(void)	{ return 0; }
 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 start, unsigned long end, unsigned int cpu)
+uv_flush_tlb_others(const struct cpumask *cpumask,
+		    const struct flush_tlb_info *info)
 { return cpumask; }
 
 #endif	/* X86_UV */
arch/x86/mm/tlb.c +32 -32

152 lines not on this page · raw message

arch/x86/platform/uv/tlb_uv.c +4 -6
index f25982cdff90..cf621e8a4ea9 100644
@@ -1109,11 +1109,9 @@ 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 start,
-						unsigned long end,
-						unsigned int cpu)
+					  const struct flush_tlb_info *info)
 {
+	unsigned int cpu = smp_processor_id();
 	int locals = 0;
 	int remotes = 0;
 	int hubs = 0;
@@ -1170,8 +1168,8 @@ const struct cpumask *uv_flush_tlb_others(const struct cpumask *cpumask,
 
 	record_send_statistics(stat, locals, hubs, remotes, bau_desc);
 
-	if (!end || (end - start) <= PAGE_SIZE)
-		bau_desc->payload.address = start;
+	if (!info->end || (info->end - info->start) <= PAGE_SIZE)
+		bau_desc->payload.address = info->start;
 	else
 		bau_desc->payload.address = TLB_FLUSH_ALL;
 	bau_desc->payload.sending_cpu = cpu;
arch/x86/xen/mmu.c +5 -5
index f226038a39ca..894daefd6958 100644
@@ -1427,8 +1427,7 @@ 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 start,
-				 unsigned long end)
+				 const struct flush_tlb_info *info)
 {
 	struct {
 		struct mmuext_op op;
@@ -1440,7 +1439,7 @@ static void xen_flush_tlb_others(const struct cpumask *cpus,
 	} *args;
 	struct multicall_space mcs;
 
-	trace_xen_mmu_flush_tlb_others(cpus, mm, start, end);
+	trace_xen_mmu_flush_tlb_others(cpus, info->mm, info->start, info->end);
 
 	if (cpumask_empty(cpus))
 		return;		/* nothing to do */
@@ -1454,9 +1453,10 @@ static void xen_flush_tlb_others(const struct cpumask *cpus,
 	cpumask_clear_cpu(smp_processor_id(), to_cpumask(args->mask));
 
 	args->op.cmd = MMUEXT_TLB_FLUSH_MULTI;
-	if (end != TLB_FLUSH_ALL && (end - start) <= PAGE_SIZE) {
+	if (info->end != TLB_FLUSH_ALL &&
+	    (info->end - info->start) <= PAGE_SIZE) {
 		args->op.cmd = MMUEXT_INVLPG_MULTI;
-		args->op.arg1.linear_addr = start;
+		args->op.arg1.linear_addr = info->start;
 	}
 
 	MULTI_mmuext_op(mcs.mc, &args->op, 1, NULL, DOMID_SELF);
Signature · 7 lines
2.9.3

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#8 lore raw

Re: [PATCH v2 04/11] x86/mm: Pass flush_tlb_info to flush_tlb_others() etc

Quoted 21 lines · Andy Lutomirski

Rather than passing all the contents of flush_tlb_info to flush_tlb_others(), pass a pointer to the structure directly. For consistency, this also removes the unnecessary cpu parameter from uv_flush_tlb_others() to make its signature match the other *flush_tlb_others() functions.

This serves two purposes:

- It will dramatically simplify future patches that change struct flush_tlb_info, which I'm planning to do.

- struct flush_tlb_info is an adequate description of what to do for a local flush, too, so by reusing it we can remove duplicated code between local and remove flushes in a future patch.

Cc Rik van Riel <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Nadav Amit <[email protected]>
Cc Michal Hocko <[email protected]>
Cc Andrew Morton <[email protected]>
Quoted 9 lines
---
 arch/x86/include/asm/paravirt.h       |  6 ++--
 arch/x86/include/asm/paravirt_types.h |  5 ++-
 arch/x86/include/asm/tlbflush.h       | 19 ++++++-----
 arch/x86/include/asm/uv/uv.h          |  9 ++---
 arch/x86/mm/tlb.c                     | 64 +++++++++++++++++------------------
 arch/x86/platform/uv/tlb_uv.c         | 10 +++---
 arch/x86/xen/mmu.c                    | 10 +++---
 7 files changed, 59 insertions(+), 64 deletions(-)

I've picked up the first three patches, but this patch apparently clashes with v4.12 changes:

patching file arch/x86/platform/uv/tlb_uv.c
Hunk #1 FAILED at 1109.
Hunk #2 FAILED at 1170.
2 out of 2 hunks FAILED -- rejects in file arch/x86/platform/uv/tlb_uv.c patching file arch/x86/xen/mmu.c
Hunk #1 FAILED at 1427.
Hunk #2 FAILED at 1440.
Hunk #3 FAILED at 1454.
3 out of 3 hunks FAILED -- rejects in file arch/x86/xen/mmu.c

Patch 10 is broken and patch 11 needs an Ack from the KVM guys so I'll wait for a new series on top of the new x86/mm branch. (Which I'll push out once it passes testing.)

Thanks,

Ingo

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

author maintainer
#9 lore raw

PATCH v2 5/11 x86/mm: Change the leave_mm() condition for local TLB flushes

In reply to Andy Lutomirski

On a remote TLB flush, we leave_mm() if we're TLBSTATE_LAZY. For a local flush_tlb_mm_range(), we leave_mm() if !current->mm. These are approximately the same condition -- the scheduler sets lazy TLB mode when switching to a thread with no mm.

I'm about to merge the local and remote flush code, but for ease of verifying and bisecting the patch, I want the local and remote flush behavior to match first. This patch changes the local code to match the remote code.

Cc Rik van Riel <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Nadav Amit <[email protected]>
Cc Michal Hocko <[email protected]>
Cc Arjan van de Ven <[email protected]>
---
 arch/x86/mm/tlb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/mm/tlb.c +1 -1
index 776469cc54e0..3143c9a180e5 100644
@@ -311,7 +311,7 @@ void flush_tlb_mm_range(struct mm_struct *mm, unsigned long start,
 		goto out;
 	}
 
-	if (!current->mm) {
+	if (this_cpu_read(cpu_tlbstate.state) != TLBSTATE_OK) {
 		leave_mm(smp_processor_id());
 
 		/* Synchronize with switch_mm. */
Signature · 7 lines
2.9.3

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
author maintainer
#10 lore raw

PATCH v2 6/11 x86/mm: Refactor flush_tlb_mm_range() to merge local and remote cases

In reply to Andy Lutomirski

The local flush path is very similar to the remote flush path. Merge them.

This is intended to make no difference to behavior whatsoever. It removes some code and will make future changes to the flushing mechanics simpler.

This patch does remove one small optimization: flush_tlb_mm_range() now has an unconditional smp_mb() instead of using MOV to CR3 or INVLPG as a full barrier when applicable. I think this is okay for a few reasons. First, smp_mb() is quite cheap compared to the cost of a TLB flush. Second, this rearrangement makes a bigger optimization available: with some work on the SMP function call code, we could do the local and remote flushes in parallel. Third, I'm planning a rework of the TLB flush algorithm that will require an atomic operation at the beginning of each flush, and that operation will replace the smp_mb().

Cc Rik van Riel <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Nadav Amit <[email protected]>
Cc Michal Hocko <[email protected]>
Cc Andrew Morton <[email protected]>
Cc Arjan van de Ven <[email protected]>
---
 arch/x86/include/asm/tlbflush.h |   1 -
 arch/x86/mm/tlb.c               | 113 +++++++++++++++++-----------------------
 2 files changed, 48 insertions(+), 66 deletions(-)
arch/x86/include/asm/tlbflush.h +0 -1
index 6f439ac92026..9934c7c99213 100644
@@ -225,7 +225,6 @@ static inline void __flush_tlb_one(unsigned long addr)
  * ..but the i386 has somewhat limited tlb flushing capabilities,
  * and page-granular flushes are available only on i486 and up.
  */
-
 struct flush_tlb_info {
 	struct mm_struct *mm;
 	unsigned long start;
arch/x86/mm/tlb.c +48 -65

159 lines not on this page · raw message

Signature · 7 lines
2.9.3

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
author maintainer
#11 lore raw

PATCH v2 7/11 x86/mm: Use new merged flush logic in arch_tlbbatch_flush()

In reply to Andy Lutomirski

Now there's only one copy of the local tlb flush logic for non-kernel pages on SMP kernels.

The only functional change is that arch_tlbbatch_flush() will now leave_mm() on the local CPU if that CPU is in the batch and is in TLBSTATE_LAZY mode.

Cc Rik van Riel <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Nadav Amit <[email protected]>
Cc Michal Hocko <[email protected]>
Cc Arjan van de Ven <[email protected]>
Cc Andrew Morton <[email protected]>
---
 arch/x86/mm/tlb.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)
arch/x86/mm/tlb.c +2 -6
index 12b8812e8926..c03b4a0ce58c 100644
@@ -382,12 +382,8 @@ void arch_tlbbatch_flush(struct arch_tlbflush_unmap_batch *batch)
 
 	int cpu = get_cpu();
 
-	if (cpumask_test_cpu(cpu, &batch->cpumask)) {
-		count_vm_tlb_event(NR_TLB_LOCAL_FLUSH_ALL);
-		local_flush_tlb();
-		trace_tlb_flush(TLB_LOCAL_SHOOTDOWN, TLB_FLUSH_ALL);
-	}
-
+	if (cpumask_test_cpu(cpu, &batch->cpumask))
+		flush_tlb_func_local(&info, TLB_LOCAL_SHOOTDOWN);
 	if (cpumask_any_but(&batch->cpumask, cpu) < nr_cpu_ids)
 		flush_tlb_others(&batch->cpumask, &info);
 	cpumask_clear(&batch->cpumask);
Signature · 7 lines
2.9.3

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
author maintainer
#12 lore raw

PATCH v2 8/11 x86/mm: Remove the UP tlbflush code; always use the formerly SMP code

In reply to Andy Lutomirski

The UP tlbflush generates somewhat nicer code than the SMP version. Aside from that, it's fallen quite a bit behind the SMP code:

- flush_tlb_mm_range() didn't flush individual pages if the range was small.

- The lazy TLB code was much weaker. This usually wouldn't matter, but, if a kernel thread flushed its lazy "active_mm" more than once (due to reclaim or similar), it wouldn't be unlazied and would instead pointlessly flush repeatedly.

- Tracepoints were missing.

Aside from that, simply having the UP code around was a maintanence burden, since it means that any change to the TLB flush code had to make sure not to break it.

Simplify everything by deleting the UP code.

Cc Rik van Riel <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Nadav Amit <[email protected]>
Cc Michal Hocko <[email protected]>
Cc Andrew Morton <[email protected]>
Cc Arjan van de Ven <[email protected]>
---
 arch/x86/Kconfig                   |  2 +-
 arch/x86/include/asm/hardirq.h     |  2 +-
 arch/x86/include/asm/mmu.h         |  6 ---
 arch/x86/include/asm/mmu_context.h |  2 -
 arch/x86/include/asm/tlbbatch.h    |  2 -
 arch/x86/include/asm/tlbflush.h    | 76 +-------------------------------------
 arch/x86/mm/init.c                 |  2 -
 arch/x86/mm/tlb.c                  | 17 +--------
 8 files changed, 5 insertions(+), 104 deletions(-)
arch/x86/Kconfig +1 -1
index 2bde14451e54..1b781bcde1a0 100644
@@ -69,7 +69,7 @@ config X86
 	select ARCH_USE_BUILTIN_BSWAP
 	select ARCH_USE_QUEUED_RWLOCKS
 	select ARCH_USE_QUEUED_SPINLOCKS
-	select ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH if SMP
+	select ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH
 	select ARCH_WANT_FRAME_POINTERS
 	select ARCH_WANTS_DYNAMIC_TASK_STRUCT
 	select BUILDTIME_EXTABLE_SORT
arch/x86/include/asm/hardirq.h +1 -1
index 59405a248fc2..9b76cd331990 100644
@@ -22,8 +22,8 @@ typedef struct {
 #ifdef CONFIG_SMP
 	unsigned int irq_resched_count;
 	unsigned int irq_call_count;
-	unsigned int irq_tlb_count;
 #endif
+	unsigned int irq_tlb_count;
 #ifdef CONFIG_X86_THERMAL_VECTOR
 	unsigned int irq_thermal_count;
 #endif
arch/x86/include/asm/mmu.h +0 -6
index f9813b6d8b80..79b647a7ebd0 100644
@@ -37,12 +37,6 @@ typedef struct {
 #endif
 } mm_context_t;
 
-#ifdef CONFIG_SMP
 void leave_mm(int cpu);
-#else
-static inline void leave_mm(int cpu)
-{
-}
-#endif
 
 #endif /* _ASM_X86_MMU_H */
arch/x86/include/asm/mmu_context.h +0 -2
index 68b329d77b3a..187c39470a0b 100644
@@ -99,10 +99,8 @@ static inline void load_mm_ldt(struct mm_struct *mm)
 
 static inline void enter_lazy_tlb(struct mm_struct *mm, struct task_struct *tsk)
 {
-#ifdef CONFIG_SMP
 	if (this_cpu_read(cpu_tlbstate.state) == TLBSTATE_OK)
 		this_cpu_write(cpu_tlbstate.state, TLBSTATE_LAZY);
-#endif
 }
 
 static inline int init_new_context(struct task_struct *tsk,
arch/x86/include/asm/tlbbatch.h +0 -2
index 01a6de16fb96..f4a6ff352a0e 100644
@@ -3,7 +3,6 @@
 
 #include <linux/cpumask.h>
 
-#ifdef CONFIG_SMP
 struct arch_tlbflush_unmap_batch {
 	/*
 	 * Each bit set is a CPU that potentially has a TLB entry for one of
@@ -11,6 +10,5 @@ struct arch_tlbflush_unmap_batch {
 	 */
 	struct cpumask cpumask;
 };
-#endif
 
 #endif /* _ARCH_X86_TLBBATCH_H */
arch/x86/include/asm/tlbflush.h +1 -75

102 lines not on this page · raw message

arch/x86/mm/init.c +0 -2
index 2193799ca800..078972129885 100644
@@ -792,10 +792,8 @@ void __init zone_sizes_init(void)
 }
 
 DEFINE_PER_CPU_SHARED_ALIGNED(struct tlb_state, cpu_tlbstate) = {
-#ifdef CONFIG_SMP
 	.active_mm = &init_mm,
 	.state = 0,
-#endif
 	.cr4 = ~0UL,	/* fail hard if we screw up cr4 shadow initialization */
 };
 EXPORT_SYMBOL_GPL(cpu_tlbstate);
arch/x86/mm/tlb.c +2 -15
index c03b4a0ce58c..da1416c77bfb 100644
@@ -15,7 +15,7 @@
 #include <linux/debugfs.h>
 
 /*
- *	Smarter SMP flushing macros.
+ *	TLB flushing, formerly SMP-only
  *		c/o Linus Torvalds.
  *
  *	These mean you can really definitely utterly forget about
@@ -28,8 +28,6 @@
  *	Implement flush IPI by CALL_FUNCTION_VECTOR, Alex Shi
  */
 
-#ifdef CONFIG_SMP
-
 /*
  * We cannot call mmdrop() because we are in interrupt context,
  * instead update mm->cpu_vm_mask.
@@ -53,8 +51,6 @@ void leave_mm(int cpu)
 }
 EXPORT_SYMBOL_GPL(leave_mm);
 
-#endif /* CONFIG_SMP */
-
 void switch_mm(struct mm_struct *prev, struct mm_struct *next,
 	       struct task_struct *tsk)
 {
@@ -85,10 +81,8 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
 				set_pgd(pgd, init_mm.pgd[stack_pgd_index]);
 		}
 
-#ifdef CONFIG_SMP
 		this_cpu_write(cpu_tlbstate.state, TLBSTATE_OK);
 		this_cpu_write(cpu_tlbstate.active_mm, next);
-#endif
 
 		cpumask_set_cpu(cpu, mm_cpumask(next));
 
@@ -146,9 +140,7 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
 		if (unlikely(prev->context.ldt != next->context.ldt))
 			load_mm_ldt(next);
 #endif
-	}
-#ifdef CONFIG_SMP
-	  else {
+	} else {
 		this_cpu_write(cpu_tlbstate.state, TLBSTATE_OK);
 		BUG_ON(this_cpu_read(cpu_tlbstate.active_mm) != next);
 
@@ -175,11 +167,8 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
 			load_mm_ldt(next);
 		}
 	}
-#endif
 }
 
-#ifdef CONFIG_SMP
-
 /*
  * The flush IPI assumes that a thread switch happens in this order:
  * [cpu0: the cpu that switches]
@@ -436,5 +425,3 @@ static int __init create_tlb_single_page_flush_ceiling(void)
 	return 0;
 }
 late_initcall(create_tlb_single_page_flush_ceiling);
-
-#endif /* CONFIG_SMP */
Signature · 7 lines
2.9.3

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
author maintainer
#13 lore raw

PATCH v2 9/11 x86/mm: Rework lazy TLB to track the actual loaded mm

In reply to Andy Lutomirski

Lazy TLB state is current managed in a rather baroque manner. AFAICT, there are three possible states:

- Non-lazy. This means that we're running a user thread or a kernel thread that has called use_mm(). current->mm == current->active_mm == cpu_tlbstate.active_mm and cpu_tlbstate.state == TLBSTATE_OK.

- Lazy with user mm. We're running a kernel thread without an mm and we're borrowing an mm_struct. We have current->mm == NULL, current->active_mm == cpu_tlbstate.active_mm, cpu_tlbstate.state != TLBSTATE_OK (i.e. TLBSTATE_LAZY or 0). The current cpu is set in mm_cpumask(current->active_mm). CR3 points to current->active_mm->pgd. The TLB is up to date.

- Lazy with init_mm. This happens when we call leave_mm(). We have current->mm == NULL, current->active_mm == cpu_tlbstate.active_mm, but that mm is only relelvant insofar as the scheduler is tracking it for refcounting. cpu_tlbstate.state != TLBSTATE_OK. The current cpu is clear in mm_cpumask(current->active_mm). CR3 points to swapper_pg_dir, i.e. init_mm->pgd.

This patch simplifies the situation. Other than perf, x86 stops caring about current->active_mm at all. We have cpu_tlbstate.loaded_mm pointing to the mm that CR3 references. The TLB is always up to date for that mm. leave_mm() just switches us to init_mm. There are no longer any special cases for mm_cpumask, and switch_mm() switches mms without worrying about laziness.

After this patch, cpu_tlbstate.state serves only to tell the TLB flush code whether it may switch to init_mm instead of doing a normal flush.

This makes fairly extensive changes to xen_exit_mmap(), which used to look a bit like black magic.

Perf is unchanged. With or without this change, perf may behave a bit erratically if it tries to read user memory in kernel thread context. We should build on this patch to teach perf to never look at user memory when cpu_tlbstate.loaded_mm != current->mm.

Cc Rik van Riel <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Nadav Amit <[email protected]>
Cc Michal Hocko <[email protected]>
Cc Andrew Morton <[email protected]>
Cc Arjan van de Ven <[email protected]>
---
 arch/x86/events/core.c          |   3 +-
 arch/x86/include/asm/tlbflush.h |  12 ++-
 arch/x86/kernel/ldt.c           |   7 +-
 arch/x86/mm/init.c              |   2 +-
 arch/x86/mm/tlb.c               | 208 ++++++++++++++++++++--------------------
 arch/x86/xen/mmu.c              |  51 +++++-----
 6 files changed, 143 insertions(+), 140 deletions(-)
arch/x86/events/core.c +1 -2
index 580b60f5ac83..77a33096728d 100644
@@ -2101,8 +2101,7 @@ static int x86_pmu_event_init(struct perf_event *event)
 
 static void refresh_pce(void *ignored)
 {
-	if (current->active_mm)
-		load_mm_cr4(current->active_mm);
+	load_mm_cr4(this_cpu_read(cpu_tlbstate.loaded_mm));
 }
 
 static void x86_pmu_event_mapped(struct perf_event *event)
arch/x86/include/asm/tlbflush.h +10 -2
index dbb5a9f0fed8..388c2463fde6 100644
@@ -66,7 +66,13 @@ static inline void invpcid_flush_all_nonglobals(void)
 #endif
 
 struct tlb_state {
-	struct mm_struct *active_mm;
+	/*
+	 * cpu_tlbstate.loaded_mm should match CR3 whenever interrupts
+	 * are on.  This means that it may not match current->active_mm,
+	 * which will contain the previous user mm when we're in lazy TLB
+	 * mode even if we've already switched back to swapper_pg_dir.
+	 */
+	struct mm_struct *loaded_mm;
 	int state;
 
 	/*
@@ -256,7 +262,9 @@ void native_flush_tlb_others(const struct cpumask *cpumask,
 static inline void reset_lazy_tlbstate(void)
 {
 	this_cpu_write(cpu_tlbstate.state, 0);
-	this_cpu_write(cpu_tlbstate.active_mm, &init_mm);
+	this_cpu_write(cpu_tlbstate.loaded_mm, &init_mm);
+
+	WARN_ON(read_cr3() != __pa_symbol(swapper_pg_dir));
 }
 
 static inline void arch_tlbbatch_add_mm(struct arch_tlbflush_unmap_batch *batch,
arch/x86/kernel/ldt.c +4 -3
index d4a15831ac58..de503e7a64ad 100644
@@ -22,14 +22,15 @@
 #include <asm/syscalls.h>
 
 /* context.lock is held for us, so we don't need any locking. */
-static void flush_ldt(void *current_mm)
+static void flush_ldt(void *__mm)
 {
+	struct mm_struct *mm = __mm;
 	mm_context_t *pc;
 
-	if (current->active_mm != current_mm)
+	if (this_cpu_read(cpu_tlbstate.loaded_mm) != mm)
 		return;
 
-	pc = &current->active_mm->context;
+	pc = &mm->context;
 	set_ldt(pc->ldt->entries, pc->ldt->size);
 }
 
arch/x86/mm/init.c +1 -1
index 078972129885..47453ce2374e 100644
@@ -792,7 +792,7 @@ void __init zone_sizes_init(void)
 }
 
 DEFINE_PER_CPU_SHARED_ALIGNED(struct tlb_state, cpu_tlbstate) = {
-	.active_mm = &init_mm,
+	.loaded_mm = &init_mm,
 	.state = 0,
 	.cr4 = ~0UL,	/* fail hard if we screw up cr4 shadow initialization */
 };
arch/x86/mm/tlb.c +104 -104

246 lines not on this page · raw message

arch/x86/xen/mmu.c +23 -28

91 lines not on this page · raw message

Signature · 7 lines
2.9.3

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
author maintainer
#14 lore raw

PATCH v2 10/11 x86/mm: Be more consistent wrt PAGE_SHIFT vs PAGE_SIZE in tlb flush code

In reply to Andy Lutomirski

Nadav pointed out that some code used PAGE_SIZE and other code used PAGE_SHIFT. Use PAGE_SHIFT instead of multiplying or dividing by PAGE_SIZE.

Cc Rik van Riel <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Nadav Amit <[email protected]>
Cc Michal Hocko <[email protected]>
Cc Arjan van de Ven <[email protected]>
Cc Andrew Morton <[email protected]>
---
 arch/x86/mm/tlb.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
arch/x86/mm/tlb.c +2 -3
index 4bfadb869a1e..20659edd9347 100644
@@ -220,8 +220,7 @@ static void flush_tlb_func_common(const struct flush_tlb_info *f,
 		trace_tlb_flush(reason, TLB_FLUSH_ALL);
 	} else {
 		unsigned long addr;
-		unsigned long nr_pages =
-			(f->end - f->start) / PAGE_SIZE;
+		unsigned long nr_pages = (f->end - f->start) >> PAGE_SHIFT;
 		addr = f->start;
 		while (addr < f->end) {
 			__flush_tlb_single(addr);
@@ -351,7 +350,7 @@ void flush_tlb_kernel_range(unsigned long start, unsigned long end)
 
 	/* Balance as user space task's flush, a bit conservative */
 	if (end == TLB_FLUSH_ALL ||
-	    (end - start) > tlb_single_page_flush_ceiling * PAGE_SIZE) {
+	    (end - start) > tlb_single_page_flush_ceiling >> PAGE_SHIFT) {
 		on_each_cpu(do_flush_tlb_all, NULL, 1);
 	} else {
 		struct flush_tlb_info info;
Signature · 7 lines
2.9.3

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#15 lore raw

Re: [PATCH v2 10/11] x86/mm: Be more consistent wrt PAGE_SHIFT vs PAGE_SIZE in tlb flush code

arch/x86/mm/tlb.c void flush_tlb_kernel_range(unsigned long start, unsigned long end) · line 353

Quoted 5 lines
	/* Balance as user space task's flush, a bit conservative */
	if (end == TLB_FLUSH_ALL ||
arch/x86/mm/tlb.c +1 -1
-	    (end - start) > tlb_single_page_flush_ceiling * PAGE_SIZE) {
+	    (end - start) > tlb_single_page_flush_ceiling >> PAGE_SHIFT) {

Shouldn’t it be << ?

Nadav

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

author maintainer
#16 lore raw

Re: [PATCH v2 10/11] x86/mm: Be more consistent wrt PAGE_SHIFT vs PAGE_SIZE in tlb flush code

Quoted 7 lines · Nadav Amit

arch/x86/mm/tlb.c void flush_tlb_kernel_range(unsigned long start, unsigned long end) · line 353

Quoted 5 lines
      /* Balance as user space task's flush, a bit conservative */
      if (end == TLB_FLUSH_ALL ||
arch/x86/mm/tlb.c +1 -1
-         (end - start) > tlb_single_page_flush_ceiling * PAGE_SIZE) {
+         (end - start) > tlb_single_page_flush_ceiling >> PAGE_SHIFT) {

Shouldn’t it be << ?

Gah, that's embarrassing.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

author maintainer
#17 lore raw

PATCH v2 11/11 x86,kvm: Teach KVM's VMX code that CR3 isn't a constant

In reply to Andy Lutomirski

When PCID is enabled, CR3's PCID bits can change during context switches, so KVM won't be able to treat CR3 as a per-mm constant any more.

I structured this like the existing CR4 handling. Under ordinary circumstances (PCID disabled or if the current PCID and the value that's already in the VMCS match), then we won't do an extra VMCS write, and we'll never do an extra direct CR3 read. The overhead should be minimal.

I disallowed using the new helper in non-atomic context because PCID support will cause CR3 to stop being constant in non-atomic process context.

(Frankly, it also scares me a bit that KVM ever treated CR3 as constant, but it looks like it was okay before.)

Cc Paolo Bonzini <[email protected]>
Cc Radim Krčmář <[email protected]>
Cc Rik van Riel <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Nadav Amit <[email protected]>
Cc Michal Hocko <[email protected]>
Cc Andrew Morton <[email protected]>
Cc Arjan van de Ven <[email protected]>
---
 arch/x86/include/asm/mmu_context.h | 19 +++++++++++++++++++
 arch/x86/kvm/vmx.c                 | 21 ++++++++++++++++++---
 2 files changed, 37 insertions(+), 3 deletions(-)
arch/x86/include/asm/mmu_context.h +19 -0
index 187c39470a0b..f20d7ea47095 100644
@@ -266,4 +266,23 @@ static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,
 	return __pkru_allows_pkey(vma_pkey(vma), write);
 }
 
+
+/*
+ * This can be used from process context to figure out what the value of
+ * CR3 is without needing to do a (slow) read_cr3().
+ *
+ * It's intended to be used for code like KVM that sneakily changes CR3
+ * and needs to restore it.  It needs to be used very carefully.
+ */
+static inline unsigned long __get_current_cr3_fast(void)
+{
+	unsigned long cr3 = __pa(this_cpu_read(cpu_tlbstate.loaded_mm)->pgd);
+
+	/* For now, be very restrictive about when this can be called. */
+	VM_WARN_ON(in_nmi() || !in_atomic());
+
+	VM_BUG_ON(cr3 != read_cr3());
+	return cr3;
+}
+
 #endif /* _ASM_X86_MMU_CONTEXT_H */
arch/x86/kvm/vmx.c +18 -3
index 535cc065b844..2771235072aa 100644
@@ -48,6 +48,7 @@
 #include <asm/kexec.h>
 #include <asm/apic.h>
 #include <asm/irq_remapping.h>
+#include <asm/mmu_context.h>
 
 #include "trace.h"
 #include "pmu.h"
@@ -596,6 +597,7 @@ struct vcpu_vmx {
 		int           gs_ldt_reload_needed;
 		int           fs_reload_needed;
 		u64           msr_host_bndcfgs;
+		unsigned long vmcs_host_cr3;	/* May not match real cr3 */
 		unsigned long vmcs_host_cr4;	/* May not match real cr4 */
 	} host_state;
 	struct {
@@ -5017,12 +5019,19 @@ static void vmx_set_constant_host_state(struct vcpu_vmx *vmx)
 	u32 low32, high32;
 	unsigned long tmpl;
 	struct desc_ptr dt;
-	unsigned long cr0, cr4;
+	unsigned long cr0, cr3, cr4;
 
 	cr0 = read_cr0();
 	WARN_ON(cr0 & X86_CR0_TS);
 	vmcs_writel(HOST_CR0, cr0);  /* 22.2.3 */
-	vmcs_writel(HOST_CR3, read_cr3());  /* 22.2.3  FIXME: shadow tables */
+
+	/*
+	 * Save the most likely value for this task's CR3 in the VMCS.
+	 * We can't use __get_current_cr3_fast() because we're not atomic.
+	 */
+	cr3 = read_cr3();
+	vmcs_writel(HOST_CR3, cr3);		/* 22.2.3  FIXME: shadow tables */
+	vmx->host_state.vmcs_host_cr3 = cr3;
 
 	/* Save the most likely value for this task's CR4 in the VMCS. */
 	cr4 = cr4_read_shadow();
@@ -8927,7 +8936,7 @@ static void vmx_arm_hv_timer(struct kvm_vcpu *vcpu)
 static void __noclone vmx_vcpu_run(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_vmx *vmx = to_vmx(vcpu);
-	unsigned long debugctlmsr, cr4;
+	unsigned long debugctlmsr, cr3, cr4;
 
 	/* Record the guest's net vcpu time for enforced NMI injections. */
 	if (unlikely(!cpu_has_virtual_nmis() && vmx->soft_vnmi_blocked))
@@ -8953,6 +8962,12 @@ static void __noclone vmx_vcpu_run(struct kvm_vcpu *vcpu)
 	if (test_bit(VCPU_REGS_RIP, (unsigned long *)&vcpu->arch.regs_dirty))
 		vmcs_writel(GUEST_RIP, vcpu->arch.regs[VCPU_REGS_RIP]);
 
+	cr3 = __get_current_cr3_fast();
+	if (unlikely(cr3 != vmx->host_state.vmcs_host_cr3)) {
+		vmcs_writel(HOST_CR3, cr3);
+		vmx->host_state.vmcs_host_cr3 = cr3;
+	}
+
 	cr4 = cr4_read_shadow();
 	if (unlikely(cr4 != vmx->host_state.vmcs_host_cr4)) {
 		vmcs_writel(HOST_CR4, cr4);
Signature · 7 lines
2.9.3

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>