public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: tip-bot for Dave Hansen <tipbot@zytor.com>
To: linux-tip-commits@vger.kernel.org
Cc: mingo@kernel.org, linux-kernel@vger.kernel.org,
	dave.hansen@linux.intel.com, hpa@zytor.com, dave@sr71.net,
	tglx@linutronix.de
Subject: [tip:x86/urgent] x86, mpx: Fix potential performance issue on unmaps
Date: Thu, 22 Jan 2015 12:13:05 -0800	[thread overview]
Message-ID: <tip-c922228efeeefa32e57f875764bfa6ca8053a68a@git.kernel.org> (raw)
In-Reply-To: <20150108223021.AEEAB987@viggo.jf.intel.com>

Commit-ID:  c922228efeeefa32e57f875764bfa6ca8053a68a
Gitweb:     http://git.kernel.org/tip/c922228efeeefa32e57f875764bfa6ca8053a68a
Author:     Dave Hansen <dave.hansen@linux.intel.com>
AuthorDate: Thu, 8 Jan 2015 14:30:21 -0800
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 22 Jan 2015 21:11:06 +0100

x86, mpx: Fix potential performance issue on unmaps

The 3.19 merge window saw some TLB modifications merged which caused a
performance regression. They were fixed in commit 045bbb9fa.

Once that fix was applied, I also noticed that there was a small
but intermittent regression still present.  It was not present
consistently enough to bisect reliably, but I'm fairly confident
that it came from (my own) MPX patches.  The source was reading
a relatively unused field in the mm_struct via arch_unmap.

I also noted that this code was in the main instruction flow of
do_munmap() and probably had more icache impact than we want.

This patch does two things:
1. Adds a static (via Kconfig) and dynamic (via cpuid) check
   for MPX with cpu_feature_enabled().  This keeps us from
   reading that cacheline in the mm and trades it for a check
   of the global CPUID variables at least on CPUs without MPX.
2. Adds an unlikely() to ensure that the MPX call ends up out
   of the main instruction flow in do_munmap().  I've added
   a detailed comment about why this was done and why we want
   it even on systems where MPX is present.

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Cc: luto@amacapital.net
Cc: Dave Hansen <dave@sr71.net>
Link: http://lkml.kernel.org/r/20150108223021.AEEAB987@viggo.jf.intel.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/include/asm/mmu_context.h | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/mmu_context.h b/arch/x86/include/asm/mmu_context.h
index 40269a2..4b75d59 100644
--- a/arch/x86/include/asm/mmu_context.h
+++ b/arch/x86/include/asm/mmu_context.h
@@ -130,7 +130,25 @@ static inline void arch_bprm_mm_init(struct mm_struct *mm,
 static inline void arch_unmap(struct mm_struct *mm, struct vm_area_struct *vma,
 			      unsigned long start, unsigned long end)
 {
-	mpx_notify_unmap(mm, vma, start, end);
+	/*
+	 * mpx_notify_unmap() goes and reads a rarely-hot
+	 * cacheline in the mm_struct.  That can be expensive
+	 * enough to be seen in profiles.
+	 *
+	 * The mpx_notify_unmap() call and its contents have been
+	 * observed to affect munmap() performance on hardware
+	 * where MPX is not present.
+	 *
+	 * The unlikely() optimizes for the fast case: no MPX
+	 * in the CPU, or no MPX use in the process.  Even if
+	 * we get this wrong (in the unlikely event that MPX
+	 * is widely enabled on some system) the overhead of
+	 * MPX itself (reading bounds tables) is expected to
+	 * overwhelm the overhead of getting this unlikely()
+	 * consistently wrong.
+	 */
+	if (unlikely(cpu_feature_enabled(X86_FEATURE_MPX)))
+		mpx_notify_unmap(mm, vma, start, end);
 }
 
 #endif /* _ASM_X86_MMU_CONTEXT_H */

  reply	other threads:[~2015-01-22 20:13 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-08 22:30 [PATCH 0/3] x86, mpx: Fixes for 3.19 Dave Hansen
2015-01-08 22:30 ` [PATCH 1/3] x86, mpx: explicitly disable 32-bit MPX support on 64-bit kernels Dave Hansen
2015-01-22 20:12   ` [tip:x86/urgent] x86, mpx: Explicitly " tip-bot for Dave Hansen
2015-01-08 22:30 ` [PATCH 2/3] x86 mpx: fix potential performance issue on unmaps Dave Hansen
2015-01-22 20:13   ` tip-bot for Dave Hansen [this message]
2015-01-08 22:30 ` [PATCH 3/3] x86 mpx: strictly enforce empty prctl() args Dave Hansen
2015-01-22 20:13   ` [tip:x86/urgent] x86, mpx: Strictly " tip-bot for Dave Hansen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=tip-c922228efeeefa32e57f875764bfa6ca8053a68a@git.kernel.org \
    --to=tipbot@zytor.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=dave@sr71.net \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tip-commits@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox