mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Andi Kleen <andi@firstfloor.org>
To: peterz@infradead.org
Cc: dave@sr71.net, linux-kernel@vger.kernel.org, mingo@kernel.org,
	eranian@google.com, x86@kernel.org,
	Andi Kleen <ak@linux.intel.com>,
	torvalds@linux-foundation.org,
	Vitaly Mayatskikh <v.mayatskih@gmail.com>
Subject: [PATCH 2/2] x86: Only do a single page fault for copy_from_user_nmi
Date: Fri, 26 Sep 2014 16:31:17 -0700	[thread overview]
Message-ID: <1411774277-4198-3-git-send-email-andi@firstfloor.org> (raw)
In-Reply-To: <1411774277-4198-1-git-send-email-andi@firstfloor.org>

From: Andi Kleen <ak@linux.intel.com>

When copy_from_user_nmi faults the copy_user_tail code ends
up "replaying" the page faults to compute the exact tail bytes,
(added with 112958).

So we do an expensive page fault. And then we do it *again*.

This ends up being very expensive in the PMI handler for any
page fault on a stack access, and is one the more common
causes for the NMI handler exceeding its runtime limit.

  1)   0.109 us    |        copy_from_user_nmi();
  1)               |        copy_from_user_nmi() {
  1)               |          __do_page_fault() {
  1)               |            bad_area_nosemaphore() {
  1)               |              __bad_area_nosemaphore() {
  1)               |                no_context() {
  1)               |                  fixup_exception() {
  1)               |                    search_exception_tables() {
  1)   0.079 us    |                      search_extable();
  1)   0.409 us    |                    }
  1)   0.757 us    |                  }
  1)   1.106 us    |                }
  1)   1.466 us    |              }
  1)   1.793 us    |            }
  1)   2.233 us    |          }
  1)               |          copy_user_handle_tail() {
  1)               |            __do_page_fault() {
  1)               |              bad_area_nosemaphore() {
  1)               |                __bad_area_nosemaphore() {
  1)               |                  no_context() {
  1)               |                    fixup_exception() {
  1)               |                      search_exception_tables() {
  1)   0.060 us    |                        search_extable();
  1)   0.412 us    |                      }
  1)   0.764 us    |                    }
  1)   1.074 us    |                  }
  1)   1.389 us    |                }
  1)   1.665 us    |              }
  1)   2.002 us    |            }
  1)   2.784 us    |          }
  1)   6.230 us    |        }

The NMI code actually doesn't care about the exact tail value. It only
needs to know if a fault happened (!= 0)

So check for in_nmi() in copy_user_tail and don't bother with the exact
tail check. This way we save the extra ~2.7us.

In theory we could also duplicate the whole copy_*_ path for cases
where the caller doesn't care about the exact bytes. But that
seems overkill for just this issue, and I'm not sure anyone
else cares about how fast this is. The simpler check works
as well for now.

Cc: torvalds@linux-foundation.org
Cc: Vitaly Mayatskikh <v.mayatskih@gmail.com>
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 arch/x86/lib/usercopy_64.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/x86/lib/usercopy_64.c b/arch/x86/lib/usercopy_64.c
index c905e89..1b581e7 100644
--- a/arch/x86/lib/usercopy_64.c
+++ b/arch/x86/lib/usercopy_64.c
@@ -67,6 +67,9 @@ EXPORT_SYMBOL(copy_in_user);
  * Try to copy last bytes and clear the rest if needed.
  * Since protection fault in copy_from/to_user is not a normal situation,
  * it is not necessary to optimize tail handling.
+ *
+ * It can be somewhat common in NMI handlers doing backtraces.
+ * So don't bother here with returning the exact tail.
  */
 __visible unsigned long
 copy_user_handle_tail(char *to, char *from, unsigned len, unsigned zerorest)
@@ -74,6 +77,11 @@ copy_user_handle_tail(char *to, char *from, unsigned len, unsigned zerorest)
 	char c;
 	unsigned zero_len;
 
+	if (in_nmi()) {
+		len = 1; /* users only care about != 0 */
+		goto out;
+	}
+
 	for (; len; --len, to++) {
 		if (__get_user_nocheck(c, from++, sizeof(char)))
 			break;
@@ -84,6 +92,7 @@ copy_user_handle_tail(char *to, char *from, unsigned len, unsigned zerorest)
 	for (c = 0, zero_len = len; zerorest && zero_len; --zero_len)
 		if (__put_user_nocheck(c, to++, sizeof(char)))
 			break;
+out:
 	clac();
 	return len;
 }
-- 
1.9.3


  parent reply	other threads:[~2014-09-26 23:31 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-26 23:31 Optimize backtrace code for perf PMI handler Andi Kleen
2014-09-26 23:31 ` [PATCH 1/2] Use faster check for modules in backtrace on 64bit Andi Kleen
2014-09-29 11:42   ` Peter Zijlstra
2014-09-29 15:21     ` Andi Kleen
2014-09-29 20:30       ` Andi Kleen
2014-09-30  8:58         ` Peter Zijlstra
2014-09-30 20:10           ` Andi Kleen
2014-10-02 10:57             ` Peter Zijlstra
2014-10-03 23:20               ` Andi Kleen
2014-09-26 23:31 ` Andi Kleen [this message]
2014-09-29 11:56   ` [PATCH 2/2] x86: Only do a single page fault for copy_from_user_nmi Peter Zijlstra
2014-09-29 15:26     ` Andi Kleen
2014-10-03  4:53       ` Ingo Molnar
2014-10-03 23:25         ` Andi Kleen

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=1411774277-4198-3-git-send-email-andi@firstfloor.org \
    --to=andi@firstfloor.org \
    --cc=ak@linux.intel.com \
    --cc=dave@sr71.net \
    --cc=eranian@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=torvalds@linux-foundation.org \
    --cc=v.mayatskih@gmail.com \
    --cc=x86@kernel.org \
    /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

Powered by JetHome