Re: [RFC][PATCH] performance walnuts

On Mon, 11 Feb 2019, speck for Peter Zijlstra wrote:
> On Fri, Feb 08, 2019 at 10:07:53AM -0800, speck for Andi Kleen wrote:
> > On Fri, Feb 08, 2019 at 11:53:18AM +0100, speck for Peter Zijlstra wrote:
> 
> > >  	 * From here on, the constraint is dynamic.
> > > @@ -3345,6 +3387,26 @@ glp_get_event_constraints(struct cpu_hw_events *cpuc, int idx,
> > >  	return c;
> > >  }
> > >  
> > > +static bool allow_tsx_force_abort = true;
> > 
> > The default needs more discussion.
> 
> This is what Thomas wants and makes sense to me. I'm done talking about
> this.

Again. The broken functionality is TSX. As the cure for TSX requires to
steal a performance counter, the ucode default of preferring TSX causes a
regression on the perf side for no good reason. This is already bad enough
for updates where the ucode comes independent of the kernel update.

But with the kernel we surely have no reason to follow the Intel marketing
decision and make the same unreasonable default. There is one known
workload which depends on TSX and it's very reasonable to make this one add
the command line option and not punish everyone else.

> > > +	/*
> > > +	 * Without TFA we must not use PMC3.
> > > +	 */
> > > +	if (!allow_tsx_force_abort && test_bit(3, c->idxmsk)) {
> > 
> > This still needs the extra changes in my patchkit to allow user/kvm opt-in/out.
> 
> It needs no such thing. Userspace has the one knob.
> 
> And I want to hear from Thomas and Paolo on what KVM wants; Andrew
> already said he doesn't want this crap exposed to virt. And the less I
> have to worry about virt the happier I am.

For virt the issue is even worse because letting the guest enable TSX
causes the host to have a counter stolen. So it's a host decision in the
first place. If the host decides that TSX has to abort, then the guest has
no choice whatsoever. If the host already decided to lose the counter then
it can allow the guest to fiddle with the abort state. Whether we want to
go there is a different question. Paolo?

> I've yet to see compelling arguments for making this more complicated.
> 
> Minimal and correct are the name of the game here; we need to backport
> this because that fscking ucode default screws us over.
> 
> If you want complicated; you can try arguing that after we've gone
> public.

Keep it simple. End of story.

Thanks,

	tglx