Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1628427 > unrolled thread

[PATCH v2 2/3] x86/mm: Make flush_tlb_mm_range() more predictable

Started byAndy Lutomirski <luto@kernel.org>
First post2017-04-21 20:30 +0200
Last post2017-04-21 21:00 +0200
Articles 2 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [PATCH v2 2/3] x86/mm: Make flush_tlb_mm_range() more predictable Andy Lutomirski <luto@kernel.org> - 2017-04-21 20:30 +0200
    Re: [PATCH v2 2/3] x86/mm: Make flush_tlb_mm_range() more predictable Dave Hansen <dave.hansen@intel.com> - 2017-04-21 21:00 +0200

#1628427 — [PATCH v2 2/3] x86/mm: Make flush_tlb_mm_range() more predictable

FromAndy Lutomirski <luto@kernel.org>
Date2017-04-21 20:30 +0200
Subject[PATCH v2 2/3] x86/mm: Make flush_tlb_mm_range() more predictable
Message-ID<tyLtE-Ix-25@gated-at.bofh.it>
I'm about to rewrite the function almost completely, but first I
want to get a functional change out of the way.  Currently, if
flush_tlb_mm_range() does not flush the local TLB at all, it will
never do individual page flushes on remote CPUs.  This seems to be
an accident, and preserving it will be awkward.  Let's change it
first so that any regressions in the rewrite will be easier to
bisect and so that the rewrite can attempt to change no visible
behavior at all.

The fix is simple: we can simply avoid short-circuiting the
calculation of base_pages_to_flush.

As a side effect, this also eliminates a potential corner case: if
tlb_single_page_flush_ceiling == TLB_FLUSH_ALL, flush_tlb_mm_range()
could have ended up flushing the entire address space one page at a
time.

Cc: Rik van Riel <riel@redhat.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Nadav Amit <namit@vmware.com>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Sasha Levin <sasha.levin@oracle.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/mm/tlb.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
index 92ec37f517ab..9db9260a5e9f 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -309,6 +309,12 @@ void flush_tlb_mm_range(struct mm_struct *mm, unsigned long start,
 	unsigned long base_pages_to_flush = TLB_FLUSH_ALL;
 
 	preempt_disable();
+
+	if ((end != TLB_FLUSH_ALL) && !(vmflag & VM_HUGETLB))
+		base_pages_to_flush = (end - start) >> PAGE_SHIFT;
+	if (base_pages_to_flush > tlb_single_page_flush_ceiling)
+		base_pages_to_flush = TLB_FLUSH_ALL;
+
 	if (current->active_mm != mm) {
 		/* Synchronize with switch_mm. */
 		smp_mb();
@@ -325,15 +331,11 @@ void flush_tlb_mm_range(struct mm_struct *mm, unsigned long start,
 		goto out;
 	}
 
-	if ((end != TLB_FLUSH_ALL) && !(vmflag & VM_HUGETLB))
-		base_pages_to_flush = (end - start) >> PAGE_SHIFT;
-
 	/*
 	 * Both branches below are implicit full barriers (MOV to CR or
 	 * INVLPG) that synchronize with switch_mm.
 	 */
-	if (base_pages_to_flush > tlb_single_page_flush_ceiling) {
-		base_pages_to_flush = TLB_FLUSH_ALL;
+	if (base_pages_to_flush == TLB_FLUSH_ALL) {
 		count_vm_tlb_event(NR_TLB_LOCAL_FLUSH_ALL);
 		local_flush_tlb();
 	} else {
-- 
2.9.3

[toc] | [next] | [standalone]


#1628500

FromDave Hansen <dave.hansen@intel.com>
Date2017-04-21 21:00 +0200
Message-ID<tyLWG-Sq-19@gated-at.bofh.it>
In reply to#1628427
On 04/21/2017 11:15 AM, Andy Lutomirski wrote:
> I'm about to rewrite the function almost completely, but first I
> want to get a functional change out of the way.  Currently, if
> flush_tlb_mm_range() does not flush the local TLB at all, it will
> never do individual page flushes on remote CPUs.  This seems to be
> an accident, and preserving it will be awkward.  Let's change it
> first so that any regressions in the rewrite will be easier to
> bisect and so that the rewrite can attempt to change no visible
> behavior at all.
> 
> The fix is simple: we can simply avoid short-circuiting the
> calculation of base_pages_to_flush.

This looks sane to me.  I think it makes things more straightforward.

Acked-by: Dave Hansen <dave.hansen@intel.com>

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web