mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Frederic Weisbecker <fweisbec@gmail.com>
To: Ingo Molnar <mingo@kernel.org>, Andy Lutomirski <luto@kernel.org>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Christoph Lameter <cl@linux.com>,
	Chris Metcalf <cmetcalf@ezchip.com>
Subject: [Regression full nohz] [PATCH] x86: Don't call context tracking APIs on IRQs
Date: Tue, 13 Oct 2015 18:01:39 +0200	[thread overview]
Message-ID: <20151013160138.GD14876@lerouge> (raw)

I did rant about this before the merge window but this got basically ignored,
as all my concerns about x86 context tracking calls that are now based on
regs and not context tracking internal states, making it more fragile.

Don't get me wrong, I love this x86 entry code rework but please don't
ignore other's concerns.

Yes we could optimize IRQ context tracking calls by pulling them on
low level IRQ code, but only if we manage to spare the current calls
on irq_enter/irq_exit. Otherwise they just double the context tracking
calls and result in avoidable overhead.


---
From: Frederic Weisbecker <fweisbec@gmail.com>
Date: Sat, 3 Oct 2015 01:18:09 +0200
Subject: [PATCH] x86: Don't call context tracking APIs on IRQs

IRQs already call irq_enter() and irq_exit() which take care of RCU
and vtime needs. There is no need to call user_enter() / user_exit()
on IRQs except on IRQ exit time if we schedule out or handle signals.

This may result in performance regression when context tracking is
enabled, not to mention that enter_from_user_mode() is called all the
time on IRQ entry when CONFIG_CONTEXT_TRACKING=y (which is enabled on
many distros) even though context tracking is actually not running,
breaking the static key optimizations.

This could be optimized with pulling irq_enter/exit to low level irq
code but that requires more thoughts.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 arch/x86/entry/common.c   | 11 ++++++++++-
 arch/x86/entry/entry_64.S | 11 ++++++-----
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
index 80dcc92..1b7a866 100644
--- a/arch/x86/entry/common.c
+++ b/arch/x86/entry/common.c
@@ -229,6 +229,7 @@ __visible void prepare_exit_to_usermode(struct pt_regs *regs)
 	 * work to clear some of the flags can sleep.
 	 */
 	while (true) {
+		enum ctx_state prev_state;
 		u32 cached_flags =
 			READ_ONCE(pt_regs_to_thread_info(regs)->flags);
 
@@ -237,8 +238,10 @@ __visible void prepare_exit_to_usermode(struct pt_regs *regs)
 				      _TIF_USER_RETURN_NOTIFY)))
 			break;
 
+
 		/* We have work to do. */
 		local_irq_enable();
+		prev_state = exception_enter();
 
 		if (cached_flags & _TIF_NEED_RESCHED)
 			schedule();
@@ -258,10 +261,16 @@ __visible void prepare_exit_to_usermode(struct pt_regs *regs)
 		if (cached_flags & _TIF_USER_RETURN_NOTIFY)
 			fire_user_return_notifiers();
 
+		exception_exit(prev_state);
+
 		/* Disable IRQs and retry */
 		local_irq_disable();
 	}
+}
 
+__visible void prepare_exit_to_usermode_track(struct pt_regs *regs)
+{
+	prepare_exit_to_usermode(regs);
 	user_enter();
 }
 
@@ -314,5 +323,5 @@ __visible void syscall_return_slowpath(struct pt_regs *regs)
 #endif
 
 	local_irq_disable();
-	prepare_exit_to_usermode(regs);
+	prepare_exit_to_usermode_track(regs);
 }
diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 055a01d..f10b2c4 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -513,10 +513,6 @@ END(irq_entries_start)
 	 * tracking that we're in kernel mode.
 	 */
 	SWAPGS
-#ifdef CONFIG_CONTEXT_TRACKING
-	call enter_from_user_mode
-#endif
-
 1:
 	/*
 	 * Save previous stack pointer, optionally switch to interrupt stack.
@@ -1123,7 +1119,12 @@ ENTRY(error_exit)
 	TRACE_IRQS_OFF
 	testl	%eax, %eax
 	jnz	retint_kernel
-	jmp	retint_user
+	/* like retint_user with the call to context tracking */
+	mov	%rsp,%rdi
+	call	prepare_exit_to_usermode_track
+	TRACE_IRQS_IRETQ
+	SWAPGS
+	jmp	restore_regs_and_iret
 END(error_exit)
 
 /* Runs on exception stack */
-- 
2.5.3



             reply	other threads:[~2015-10-13 16:01 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-13 16:01 Frederic Weisbecker [this message]
2015-10-13 16:31 ` Andy Lutomirski
2015-10-16 15:05   ` Frederic Weisbecker

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=20151013160138.GD14876@lerouge \
    --to=fweisbec@gmail.com \
    --cc=cl@linux.com \
    --cc=cmetcalf@ezchip.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mingo@kernel.org \
    --cc=paulmck@linux.vnet.ibm.com \
    /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

all inboxes | Powered by JetHome®