Re: [RFC PATCH] x86, fpu: Use eagerfpu by default on all CPUs

From: Andy Lutomirski <luto@amacapital.net>
To: Rik van Riel <riel@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>,
	"Maciej W. Rozycki" <macro@linux-mips.org>,
	Ingo Molnar <mingo@kernel.org>, Oleg Nesterov <oleg@redhat.com>,
	X86 ML <x86@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [RFC PATCH] x86, fpu: Use eagerfpu by default on all CPUs
Date: Sun, 22 Feb 2015 21:22:04 -0800	[thread overview]
Message-ID: <CALCETrUU_V+Hyp2Fz=DtEzdEFAOtuXWL4OS+rP-yKWBFJ413HA@mail.gmail.com> (raw)
In-Reply-To: <54EA8641.6040609@redhat.com>

On Sun, Feb 22, 2015 at 5:45 PM, Rik van Riel <riel@redhat.com> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 02/22/2015 06:06 AM, Borislav Petkov wrote:
>> On Sat, Feb 21, 2015 at 06:18:01PM -0800, Andy Lutomirski wrote:
>>> That's true.  The question is whether there are enough of them,
>>> and whether twiddling TS is fast enough, that it's worth it.
>>
>> Yes, and let me make it clear what I'm trying to do here: I want to
>> make sure that eager FPU handling (both allocation and switching -
>> and no, I'm not confusing the concepts) *doesn't* *hurt* any
>> relevant workload. If it does, then we'll stop wasting time right
>> there.
>>
>> But(!), if the CR0.TS lazy dance turns out to be really slow and
>> the eager handling doesn't bring a noticeable slowdown, in
>> comparison, we should consider doing the eager thing by default.
>> After running a lot more benchmarks, of course.
>>
>> Which brings me to the main reason why we're doing this: code
>> simplification. If we switch to eager, we'll kill a lot of
>> non-trivial code and the FPU handling in the kernel becomes dumb
>> and nice again.
>
> Currently the FPU handling does something really dumb for
> KVM VCPU threads.  Specifically, every time we enter a
> KVM guest, we save the userspace FPU state of the VCPU
> thread, and every time we leave the KVM guest, we load
> the userspace FPU state of the VCPU thread.
>
> This is done for a thread that hardly ever exits to
> userspace, and generally just switches between kernel
> and guest mode.
>
> The reason for this acrobatics is that at every
> context switch time, the userspace FPU state is
> saved & loaded.
>
> I am working on a patch series to avoid that needless
> FPU save & restore, by moving the point at which the
> user FPU state is loaded out to the point where we
> return to userspace, in do_notify_resume.
>
> One implication of this is that in kernel mode, we
> can no longer just assume that the user space FPU
> state is always loaded, and we need to check for that
> (like the lazy FPU code does today).  I would really
> like to keep that code around, for obvious reasons :)

I like that stuff, except for the fact that it still has code that
depends on whether we're in eager or lazy mode, even though eager is a
little less eager with your patches.  Ideally I'd like to see your
patches applied *and* lazy mode removed.

--Andy