Fixes for batched TLB flushes

PATCH In review Nadav Amit 2 participants 6 messages Mar 28, 2016

Subsystems (4)

Memory management x86 x86 MM MM rmap

lore

Share & tools

lore

Nadav Amit

author

2016-03-28 16:42 UTC #0 lore raw

v2 cover Fixes for batched TLB flushes

The recent introduction of batched TLB flushes causes accounting problems in vmstat and misses some tracepoints. In addition, I am afraid it might cause problems in some platforms (Xen, UV) since it uses non-standard APIs.

v1->v2
* Added second patch to use standard api for invalidations
* cc'ing linux-mm

Nadav Amit (2):
x86/mm: TLB_REMOTE_SEND_IPI should count pages mm/rmap: batched invalidations should use existing api

 arch/x86/include/asm/tlbflush.h |  6 ------
 arch/x86/mm/tlb.c               | 14 ++++++++++----
 mm/rmap.c                       | 28 +++++++---------------------
 3 files changed, 17 insertions(+), 31 deletions(-)

Signature · 7 lines

2.5.0

--
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>

Nadav Amit

author

2016-03-28 16:42 UTC #1 lore raw

PATCH v2 1/2 x86/mm: TLB_REMOTE_SEND_IPI should count pages

TLB_REMOTE_SEND_IPI was recently introduced, but it counts bytes instead of pages. In addition, it does not report correctly the case in which flush_tlb_page flushes a page. Fix it to be consistent with other TLB counters.

Fixes 4595f9620cda8a1e973588e743cf5f8436dd20c6

Signed-off-by Nadav Amit <[email protected]>

---
 arch/x86/mm/tlb.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

arch/x86/mm/tlb.c +9 -3

index 8f4cc3d..5fb6ada 100644
@@ -106,8 +106,6 @@ static void flush_tlb_func(void *info)
 
 	if (f->flush_mm != this_cpu_read(cpu_tlbstate.active_mm))
 		return;
-	if (!f->flush_end)
-		f->flush_end = f->flush_start + PAGE_SIZE;
 
 	count_vm_tlb_event(NR_TLB_REMOTE_FLUSH_RECEIVED);
 	if (this_cpu_read(cpu_tlbstate.state) == TLBSTATE_OK) {

@@ -135,12 +133,20 @@ void native_flush_tlb_others(const struct cpumask *cpumask,
 				 unsigned long end)
 {
 	struct flush_tlb_info info;
+
+	if (end == 0)
+		end = start + PAGE_SIZE;
 	info.flush_mm = mm;
 	info.flush_start = start;
 	info.flush_end = end;
 
 	count_vm_tlb_event(NR_TLB_REMOTE_FLUSH);
-	trace_tlb_flush(TLB_REMOTE_SEND_IPI, end - start);
+	if (end == TLB_FLUSH_ALL)
+		trace_tlb_flush(TLB_REMOTE_SEND_IPI, TLB_FLUSH_ALL);
+	else
+		trace_tlb_flush(TLB_REMOTE_SEND_IPI,
+				(end - start) >> PAGE_SHIFT);
+
 	if (is_uv_system()) {
 		unsigned int cpu;

Signature · 7 lines

2.5.0

--
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>

Nadav Amit

author

2016-03-28 17:44 UTC #2 lore raw

Re: [PATCH v2 1/2] x86/mm: TLB_REMOTE_SEND_IPI should count pages

VGhlIGNvbW1pdCBtZXNzYWdlIHNob3VsZCBoYXZlIHNhaWQ6DQoNCkZpeGVzOiA1Yjc0MjgzYWIy NTFiOWRiNTVjYmJlMzFkMTljYTcyNDgyMTAzMjkwDQoNCihhbmQgbm90IHdoYXQgaXQgY3VycmVu dGx5IHNheXMpLg0KTGV0IG1lIGtub3cgd2hldGhlciB0byBzdWJtaXQgdjMuDQoNCg0KTmFkYXYN Cg0KT24gMy8yNi8xNiwgMToyNSBBTSwgIk5hZGF2IEFtaXQiIDxuYW1pdEB2bXdhcmUuY29tPiB3 cm90ZToNCg0KPlRMQl9SRU1PVEVfU0VORF9JUEkgd2FzIHJlY2VudGx5IGludHJvZHVjZWQsIGJ1 dCBpdCBjb3VudHMgYnl0ZXMgaW5zdGVhZA0KPm9mIHBhZ2VzLiBJbiBhZGRpdGlvbiwgaXQgZG9l cyBub3QgcmVwb3J0IGNvcnJlY3RseSB0aGUgY2FzZSBpbiB3aGljaA0KPmZsdXNoX3RsYl9wYWdl IGZsdXNoZXMgYSBwYWdlLiBGaXggaXQgdG8gYmUgY29uc2lzdGVudCB3aXRoIG90aGVyIFRMQg0K PmNvdW50ZXJzLg0KPg0KPkZpeGVzOiA0NTk1Zjk2MjBjZGE4YTFlOTczNTg4ZTc0M2NmNWY4NDM2 ZGQyMGM2DQo+DQo+U2lnbmVkLW9mZi1ieTogTmFkYXYgQW1pdCA8bmFtaXRAdm13YXJlLmNvbT4N Cj4tLS0NCj4gYXJjaC94ODYvbW0vdGxiLmMgfCAxMiArKysrKysrKystLS0NCj4gMSBmaWxlIGNo YW5nZWQsIDkgaW5zZXJ0aW9ucygrKSwgMyBkZWxldGlvbnMoLSkNCj4NCj5kaWZmIC0tZ2l0IGEv YXJjaC94ODYvbW0vdGxiLmMgYi9hcmNoL3g4Ni9tbS90bGIuYw0KPmluZGV4IDhmNGNjM2QuLjVm YjZhZGEgMTAwNjQ0DQo+LS0tIGEvYXJjaC94ODYvbW0vdGxiLmMNCj4rKysgYi9hcmNoL3g4Ni9t bS90bGIuYw0KPkBAIC0xMDYsOCArMTA2LDYgQEAgc3RhdGljIHZvaWQgZmx1c2hfdGxiX2Z1bmMo dm9pZCAqaW5mbykNCj4gDQo+IAlpZiAoZi0+Zmx1c2hfbW0gIT0gdGhpc19jcHVfcmVhZChjcHVf dGxic3RhdGUuYWN0aXZlX21tKSkNCj4gCQlyZXR1cm47DQo+LQlpZiAoIWYtPmZsdXNoX2VuZCkN Cj4tCQlmLT5mbHVzaF9lbmQgPSBmLT5mbHVzaF9zdGFydCArIFBBR0VfU0laRTsNCj4gDQo+IAlj b3VudF92bV90bGJfZXZlbnQoTlJfVExCX1JFTU9URV9GTFVTSF9SRUNFSVZFRCk7DQo+IAlpZiAo dGhpc19jcHVfcmVhZChjcHVfdGxic3RhdGUuc3RhdGUpID09IFRMQlNUQVRFX09LKSB7DQo+QEAg LTEzNSwxMiArMTMzLDIwIEBAIHZvaWQgbmF0aXZlX2ZsdXNoX3RsYl9vdGhlcnMoY29uc3Qgc3Ry dWN0IGNwdW1hc2sgKmNwdW1hc2ssDQo+IAkJCQkgdW5zaWduZWQgbG9uZyBlbmQpDQo+IHsNCj4g CXN0cnVjdCBmbHVzaF90bGJfaW5mbyBpbmZvOw0KPisNCj4rCWlmIChlbmQgPT0gMCkNCj4rCQll bmQgPSBzdGFydCArIFBBR0VfU0laRTsNCj4gCWluZm8uZmx1c2hfbW0gPSBtbTsNCj4gCWluZm8u Zmx1c2hfc3RhcnQgPSBzdGFydDsNCj4gCWluZm8uZmx1c2hfZW5kID0gZW5kOw0KPiANCj4gCWNv dW50X3ZtX3RsYl9ldmVudChOUl9UTEJfUkVNT1RFX0ZMVVNIKTsNCj4tCXRyYWNlX3RsYl9mbHVz aChUTEJfUkVNT1RFX1NFTkRfSVBJLCBlbmQgLSBzdGFydCk7DQo+KwlpZiAoZW5kID09IFRMQl9G TFVTSF9BTEwpDQo+KwkJdHJhY2VfdGxiX2ZsdXNoKFRMQl9SRU1PVEVfU0VORF9JUEksIFRMQl9G TFVTSF9BTEwpOw0KPisJZWxzZQ0KPisJCXRyYWNlX3RsYl9mbHVzaChUTEJfUkVNT1RFX1NFTkRf SVBJLA0KPisJCQkJKGVuZCAtIHN0YXJ0KSA+PiBQQUdFX1NISUZUKTsNCj4rDQo+IAlpZiAoaXNf dXZfc3lzdGVtKCkpIHsNCj4gCQl1bnNpZ25lZCBpbnQgY3B1Ow0KPiANCj4tLSANCj4yLjUuMA0K Pg0K

--
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>

Andrew Morton

maintainer

2016-03-28 20:38 UTC #3 lore raw

Re: [PATCH v2 1/2] x86/mm: TLB_REMOTE_SEND_IPI should count pages

In reply to Nadav Amit

Quoted 6 lines · On Sat, 26 Mar 2016 01:25:04 -0700 Nadav Amit <[email protected]> wrote:

TLB_REMOTE_SEND_IPI was recently introduced, but it counts bytes instead of pages. In addition, it does not report correctly the case in which flush_tlb_page flushes a page. Fix it to be consistent with other TLB counters.
Fixes 4595f9620cda8a1e973588e743cf5f8436dd20c6

I think you mean 5b74283ab251b9 ("x86, mm: trace when an IPI is about to be sent")?

Quoted 22 lines

arch/x86/mm/tlb.c +3 -2

@@ -106,8 +106,6 @@ static void flush_tlb_func(void *info)
 
 	if (f->flush_mm != this_cpu_read(cpu_tlbstate.active_mm))
 		return;
-	if (!f->flush_end)
-		f->flush_end = f->flush_start + PAGE_SIZE;
 
 	count_vm_tlb_event(NR_TLB_REMOTE_FLUSH_RECEIVED);
 	if (this_cpu_read(cpu_tlbstate.state) == TLBSTATE_OK) {

@@ -135,12 +133,20 @@ void native_flush_tlb_others(const struct cpumask *cpumask,
 				 unsigned long end)
 {
 	struct flush_tlb_info info;
+
+	if (end == 0)
+		end = start + PAGE_SIZE;
 	info.flush_mm = mm;
 	info.flush_start = start;
 	info.flush_end = end;

arch/x86/mm/tlb.c void native_flush_tlb_others(const struct cpumask *cpumask, · line 151

Quoted 10 lines

arch/x86/mm/tlb.c +6 -1

 	count_vm_tlb_event(NR_TLB_REMOTE_FLUSH);
-	trace_tlb_flush(TLB_REMOTE_SEND_IPI, end - start);
+	if (end == TLB_FLUSH_ALL)
+		trace_tlb_flush(TLB_REMOTE_SEND_IPI, TLB_FLUSH_ALL);
+	else
+		trace_tlb_flush(TLB_REMOTE_SEND_IPI,
+				(end - start) >> PAGE_SHIFT);
+
 	if (is_uv_system()) {
 		unsigned int cpu;

Nadav Amit

author

2016-03-28 20:44 UTC #4 lore raw

Re: [PATCH v2 1/2] x86/mm: TLB_REMOTE_SEND_IPI should count pages

T24gMy8yOC8xNiwgMTozOCBQTSwgIkFuZHJldyBNb3J0b24iIDxha3BtQGxpbnV4LWZvdW5kYXRp b24ub3JnPiB3cm90ZToNCg0KDQo+T24gU2F0LCAyNiBNYXIgMjAxNiAwMToyNTowNCAtMDcwMCBO YWRhdiBBbWl0IDxuYW1pdEB2bXdhcmUuY29tPiB3cm90ZToNCj4NCj4+IFRMQl9SRU1PVEVfU0VO RF9JUEkgd2FzIHJlY2VudGx5IGludHJvZHVjZWQsIGJ1dCBpdCBjb3VudHMgYnl0ZXMgaW5zdGVh ZA0KPj4gb2YgcGFnZXMuIEluIGFkZGl0aW9uLCBpdCBkb2VzIG5vdCByZXBvcnQgY29ycmVjdGx5 IHRoZSBjYXNlIGluIHdoaWNoDQo+PiBmbHVzaF90bGJfcGFnZSBmbHVzaGVzIGEgcGFnZS4gRml4 IGl0IHRvIGJlIGNvbnNpc3RlbnQgd2l0aCBvdGhlciBUTEINCj4+IGNvdW50ZXJzLg0KPj4gDQo+ PiBGaXhlczogNDU5NWY5NjIwY2RhOGExZTk3MzU4OGU3NDNjZjVmODQzNmRkMjBjNg0KPg0KPkkg dGhpbmsgeW91IG1lYW4gNWI3NDI4M2FiMjUxYjkgKCJ4ODYsIG1tOiB0cmFjZSB3aGVuIGFuIElQ SSBpcyBhYm91dA0KPnRvIGJlIHNlbnQiKT8NCg0KSW5kZWVkLg0KDQoNCg0K

Nadav Amit

author

2016-03-28 16:42 UTC #5 lore raw

PATCH v2 2/2 mm/rmap: batched invalidations should use existing api

In reply to Nadav Amit

The recently introduced batched invalidations mechanism uses its own mechanism for shootdown. However, it does wrong accounting of interrupts (e.g., inc_irq_stat is called for local invalidations), trace-points (e.g., TLB_REMOTE_SHOOTDOWN for local invalidations) and may break some platforms as it bypasses the invalidation mechanisms of Xen and SGI UV.

This patch reuses the existing TLB flushing mechnaisms instead. We use NULL as mm to indicate a global invalidation is required.

Signed-off-by Nadav Amit <[email protected]>

---
 arch/x86/include/asm/tlbflush.h |  6 ------
 arch/x86/mm/tlb.c               |  2 +-
 mm/rmap.c                       | 28 +++++++---------------------
 3 files changed, 8 insertions(+), 28 deletions(-)

arch/x86/include/asm/tlbflush.h +0 -6

index 6df2029..cd79194 100644
@@ -261,12 +261,6 @@ static inline void reset_lazy_tlbstate(void)
 
 #endif	/* SMP */
 
-/* Not inlined due to inc_irq_stat not being defined yet */
-#define flush_tlb_local() {		\
-	inc_irq_stat(irq_tlb_count);	\
-	local_flush_tlb();		\
-}
-
 #ifndef CONFIG_PARAVIRT
 #define flush_tlb_others(mask, mm, start, end)	\
 	native_flush_tlb_others(mask, mm, start, end)

arch/x86/mm/tlb.c +1 -1

index 5fb6ada..fe9b9f7 100644
@@ -104,7 +104,7 @@ static void flush_tlb_func(void *info)
 
 	inc_irq_stat(irq_tlb_count);
 
-	if (f->flush_mm != this_cpu_read(cpu_tlbstate.active_mm))
+	if (f->flush_mm && f->flush_mm != this_cpu_read(cpu_tlbstate.active_mm))
 		return;
 
 	count_vm_tlb_event(NR_TLB_REMOTE_FLUSH_RECEIVED);

mm/rmap.c +7 ~~-21~~

index 79f3bf0..37fb08f 100644
@@ -569,19 +569,6 @@ void page_unlock_anon_vma_read(struct anon_vma *anon_vma)
 }
 
 #ifdef CONFIG_ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH
-static void percpu_flush_tlb_batch_pages(void *data)
-{
-	/*
-	 * All TLB entries are flushed on the assumption that it is
-	 * cheaper to flush all TLBs and let them be refilled than
-	 * flushing individual PFNs. Note that we do not track mm's
-	 * to flush as that might simply be multiple full TLB flushes
-	 * for no gain.
-	 */
-	count_vm_tlb_event(NR_TLB_REMOTE_FLUSH_RECEIVED);
-	flush_tlb_local();
-}
-
 /*
  * Flush TLB entries for recently unmapped pages from remote CPUs. It is
  * important if a PTE was dirty when it was unmapped that it's flushed

@@ -598,15 +585,14 @@ void try_to_unmap_flush(void)
 
 	cpu = get_cpu();
 
-	trace_tlb_flush(TLB_REMOTE_SHOOTDOWN, -1UL);
-
-	if (cpumask_test_cpu(cpu, &tlb_ubc->cpumask))
-		percpu_flush_tlb_batch_pages(&tlb_ubc->cpumask);
-
-	if (cpumask_any_but(&tlb_ubc->cpumask, cpu) < nr_cpu_ids) {
-		smp_call_function_many(&tlb_ubc->cpumask,
-			percpu_flush_tlb_batch_pages, (void *)tlb_ubc, true);
+	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);
 	tlb_ubc->flush_required = false;
 	tlb_ubc->writable = false;

Signature · 7 lines

2.5.0

--
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>