public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@kernel.org>
To: Andy Lutomirski <luto@kernel.org>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>,
	Xen-devel <xen-devel@lists.xen.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
	x86@kernel.org, linux-kernel@vger.kernel.org,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	Boris Ostrovsky <boris.ostrovsky@oracle.com>,
	David Vrabel <david.vrabel@citrix.com>,
	Rusty Russell <rusty@rustcorp.com.au>,
	lguest@lists.ozlabs.org,
	Denys Vlasenko <vda.linux@googlemail.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Borislav Petkov <bp@alien8.de>
Subject: [RFC PATCH] x86/asm/irq: Don't use POPF but STI
Date: Tue, 21 Apr 2015 14:45:58 +0200	[thread overview]
Message-ID: <20150421124558.GA3483@gmail.com> (raw)
In-Reply-To: <55359B57.3070008@kernel.org>


* Andy Lutomirski <luto@kernel.org> wrote:

> > Another different approach would be to formally state that 
> > pv_irq_ops.save_fl() needs to return all the flags, which would 
> > make local_irq_save() safe to use in this circumstance, but that 
> > makes a hotpath longer for the sake of a single boot time check.
> 
> ...which reminds me:
> 
> Why does native_restore_fl restore anything other than IF?  A branch 
> and sti should be considerably faster than popf.

Yes, this has come up in the past, something like the patch below?

Totally untested and not signed off yet: because we'd first have to 
make sure (via irq flags debugging) that it's not used in reverse, to 
re-disable interrupts:

	local_irq_save(flags);
	local_irq_enable();
	...
	local_irq_restore(flags); /* effective local_irq_disable() */

I don't think we have many (any?) such patterns left, but it has to be 
checked first. If we have such cases then we'll have to use different 
primitives there.

But this patch should be good enough to give an good overview of the 
effects: the text impact does not look too horrible, if we decide to 
do this. The bloat of +1K on x86 defconfig is better than I first 
feared.

Thanks,

	Ingo

======================>
>From 6f01f6381e8293c360b7a89f516b8605e357d563 Mon Sep 17 00:00:00 2001
From: Ingo Molnar <mingo@kernel.org>
Date: Tue, 21 Apr 2015 13:32:13 +0200
Subject: [PATCH] x86/asm/irq: Don't use POPF but STI

So because the POPF instruction is slow and STI is faster on 
essentially all x86 CPUs that matter, instead of:

  ffffffff81891848:       9d                      popfq

we can do:

  ffffffff81661a2e:       41 f7 c4 00 02 00 00    test   $0x200,%r12d
  ffffffff81661a35:       74 01                   je     ffffffff81661a38 <snd_pcm_stream_unlock_irqrestore+0x28>
  ffffffff81661a37:       fb                      sti
  ffffffff81661a38:

This bloats the kernel a bit, by about 1K on the 64-bit defconfig:

   text    data     bss     dec     hex filename
   12258634        1812120 1085440 15156194         e743e2 vmlinux.before
   12259582        1812120 1085440 15157142         e74796 vmlinux.after

the other cost is the extra branching, adding extra pressure to the
branch prediction hardware and also potential branch misses.
---
 arch/x86/include/asm/irqflags.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/irqflags.h b/arch/x86/include/asm/irqflags.h
index b77f5edb03b0..8bc2a9bc7a06 100644
--- a/arch/x86/include/asm/irqflags.h
+++ b/arch/x86/include/asm/irqflags.h
@@ -69,7 +69,8 @@ static inline notrace unsigned long arch_local_save_flags(void)
 
 static inline notrace void arch_local_irq_restore(unsigned long flags)
 {
-	native_restore_fl(flags);
+	if (likely(flags & X86_EFLAGS_IF))
+		native_irq_enable();
 }
 
 static inline notrace void arch_local_irq_disable(void)


  parent reply	other threads:[~2015-04-21 12:46 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-20 17:09 [PATCH] [RFC] x86/cpu: Fix SMAP check in PVOPS environments Andrew Cooper
2015-04-20 17:11 ` David Vrabel
2015-04-21  0:35 ` Andy Lutomirski
2015-04-21  5:07   ` Rusty Russell
2015-04-21  8:26   ` Andrew Cooper
2015-04-21 12:45   ` Ingo Molnar [this message]
2015-04-21 13:09     ` [RFC PATCH] x86/asm/irq: Don't use POPF but STI Borislav Petkov
2015-04-21 15:22       ` Ingo Molnar
2015-04-21 16:12     ` Linus Torvalds
2015-04-21 22:39       ` Andy Lutomirski
2015-04-21  8:36 ` [Xen-devel] [PATCH] [RFC] x86/cpu: Fix SMAP check in PVOPS environments Jan Beulich

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=20150421124558.GA3483@gmail.com \
    --to=mingo@kernel.org \
    --cc=andrew.cooper3@citrix.com \
    --cc=boris.ostrovsky@oracle.com \
    --cc=bp@alien8.de \
    --cc=david.vrabel@citrix.com \
    --cc=hpa@zytor.com \
    --cc=konrad.wilk@oracle.com \
    --cc=lguest@lists.ozlabs.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mingo@redhat.com \
    --cc=rusty@rustcorp.com.au \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=vda.linux@googlemail.com \
    --cc=x86@kernel.org \
    --cc=xen-devel@lists.xen.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