Re: [patch 17/24] x86/speculation: Move IBPB control out of switch_mm()

From: Thomas Gleixner <tglx@linutronix.de>
To: Jiri Kosina <jikos@kernel.org>
Cc: Andi Kleen <ak@linux.intel.com>,
	LKML <linux-kernel@vger.kernel.org>,
	x86@kernel.org, Peter Zijlstra <peterz@infradead.org>,
	Andy Lutomirski <luto@kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Tom Lendacky <thomas.lendacky@amd.com>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	Andrea Arcangeli <aarcange@redhat.com>,
	David Woodhouse <dwmw@amazon.co.uk>,
	Dave Hansen <dave.hansen@intel.com>,
	Casey Schaufler <casey.schaufler@intel.com>,
	Asit Mallick <asit.k.mallick@intel.com>,
	Arjan van de Ven <arjan@linux.intel.com>,
	Jon Masters <jcm@redhat.com>, Waiman Long <longman9394@gmail.com>,
	Greg KH <gregkh@linuxfoundation.org>,
	Dave Stewart <david.c.stewart@intel.com>,
	Kees Cook <keescook@chromium.org>
Subject: Re: [patch 17/24] x86/speculation: Move IBPB control out of switch_mm()
Date: Thu, 22 Nov 2018 10:18:43 +0100 (CET)	[thread overview]
Message-ID: <alpine.DEB.2.21.1811221015140.1571@nanos.tec.linutronix.de> (raw)
In-Reply-To: <nycvar.YFH.7.76.1811220838410.21108@cbobk.fhfr.pm>

On Thu, 22 Nov 2018, Jiri Kosina wrote:
> On Wed, 21 Nov 2018, Andi Kleen wrote:
> 
> > > +		 * This could be optimized by keeping track of the last
> > > +		 * user task per cpu and avoiding the barrier when the task
> > > +		 * is immediately scheduled back and the thread inbetween
> > > +		 * was a kernel thread. It's dubious whether that'd be
> > > +		 * worth the extra load/store and conditional operations.
> > > +		 * Keep it optimized for the common case where the TIF bit
> > > +		 * is not set.
> > > +		 */
> > 
> > The optimization was there before and you removed it?
> > 
> > It's quite important for switching to idle and back. With your variant short IOs
> > that do short idle waits will be badly impacted. 
> 
> The question is what scenario to optimize for.
> 
> Either you penalize everybody in the default prctl+seccomp setup 
> (irrespective of it's TIF flag value), as you have the extra overhead on 
> each and every switch_to() (to check exactly for this back-to-back 
> scheduling), or you penalize only those tasks that are penalized anyway by 
> the IBPB flush.
> 
> I think the latter (which is what this patch implements) makes more sense. 

That was my rationale. The back to back thing makes sense for unconditional
mode, but not so much for the prctl/seccomp case which is default.

In unconditional mode we could add the extra overhead, but then performance
is down the drain already with STIBP.

Thanks,

	tglx