x86/fpu: remove lazy FPU mode & various FPU cleanups
Subsystems (4)
cover x86/fpu: remove lazy FPU mode & various FPU cleanups
This series removes lazy FPU mode, and cleans up various bits and pieces around the FPU code.
I have run this through a basic floating point test that involves about 1.5 billion context switches and 45 minutes of swapping at 250MB/second.
This seems to tease out bugs fairly well, though I would not mind an actual floating point test suite...
PATCH 1/9 x86/crypto: Remove X86_FEATURE_EAGER_FPU ifdef from the crc32c code
From: Rik van Riel <[email protected]>
Quoted 1 line
From : Andy Lutomirski <[email protected]>
The crypto code was checking both use_eager_fpu() and defined(X86_FEATURE_EAGER_FPU). The latter was nonsensical, so remove it. This will avoid breakage when we remove X86_FEATURE_EAGER_FPU.
--- arch/x86/crypto/crc32c-intel_glue.c | 5 ----- 1 file changed, 5 deletions(-)
arch/x86/crypto/crc32c-intel_glue.c +0 -5
@@ -58,16 +58,11 @@ asmlinkage unsigned int crc_pcl(const u8 *buffer, int len,unsigned int crc_init);static int crc32c_pcl_breakeven =CRC32C_PCL_BREAKEVEN_EAGERFPU ; -#if defined(X86_FEATURE_EAGER_FPU )#define set_pcl_breakeven_point () \ do { \ if (!use_eager_fpu()) \ crc32c_pcl_breakeven = CRC32C_PCL_BREAKEVEN_NOEAGERFPU; \ } while (0) -#else -#define set_pcl_breakeven_point () \ - (crc32c_pcl_breakeven = CRC32C_PCL_BREAKEVEN_NOEAGERFPU) -#endif #endif /* CONFIG_X86_64 */ static u32 crc32c_intel_le_hw_byte (u32 crc,unsigned char const *data,size_t length)
Signature · 1 line
2.7.4
Re: [PATCH 1/9] x86/crypto: Remove X86_FEATURE_EAGER_FPU ifdef from the crc32c code
Quoted 3 lines · On Tue, 2016-10-04 at 20:34 -0400, [email protected] wrote:
From: Rik van Riel <[email protected]>
From : Andy Lutomirski <[email protected]>
OK, I am not sure why "guilt patchbomb" (which invokes git patchbomb) would do that :(
Obviously these first four patches are Andy's, not my own.
Signature · 1 line
All Rights Reversed.
tip-bot for Andy Lutomirski
merge bot[tip:x86/fpu] x86/crypto, x86/fpu: Remove X86_FEATURE_EAGER_FPU #ifdef from the crc32c code
In reply to [email protected]x86/crypto, x86/fpu: Remove X86_FEATURE_EAGER_FPU #ifdef from the crc32c code
Commit-ID: 02f39b2379fb81557ae864ec8f85421c0250c954 Gitweb: http://git.kernel.org/tip/02f39b2379fb81557ae864ec8f85421c0250c954 Author: Andy Lutomirski <[email protected]> AuthorDate: Tue, 4 Oct 2016 20:34:30 -0400 Committer: Ingo Molnar <[email protected]> CommitDate: Fri, 7 Oct 2016 11:14:06 +0200
x86/crypto, x86/fpu: Remove X86_FEATURE_EAGER_FPU #ifdef from the crc32c code
The crypto code was checking both use_eager_fpu() and defined(X86_FEATURE_EAGER_FPU). The latter was nonsensical, so remove it. This will avoid breakage when we remove X86_FEATURE_EAGER_FPU.
--- arch/x86/crypto/crc32c-intel_glue.c | 5 ----- 1 file changed, 5 deletions(-)
arch/x86/crypto/crc32c-intel_glue.c +0 -5
@@ -58,16 +58,11 @@ asmlinkage unsigned int crc_pcl(const u8 *buffer, int len,unsigned int crc_init);static int crc32c_pcl_breakeven =CRC32C_PCL_BREAKEVEN_EAGERFPU ; -#if defined(X86_FEATURE_EAGER_FPU )#define set_pcl_breakeven_point () \ do { \ if (!use_eager_fpu()) \ crc32c_pcl_breakeven = CRC32C_PCL_BREAKEVEN_NOEAGERFPU; \ } while (0) -#else -#define set_pcl_breakeven_point () \ - (crc32c_pcl_breakeven = CRC32C_PCL_BREAKEVEN_NOEAGERFPU) -#endif #endif /* CONFIG_X86_64 */ static u32 crc32c_intel_le_hw_byte (u32 crc,unsigned char const *data,size_t length)
PATCH 2/9 x86/fpu: Hard-disable lazy fpu mode
In reply to [email protected]From: Andy Lutomirski <[email protected]>
Since commit 58122bf1d856 ("x86/fpu: Default eagerfpu=on on all CPUs") in Linux 4.6, eager FPU mode has been the default on all x86 systems, and no one has reported any regressions.
This patch removes the ability to enable lazy mode: use_eager_fpu() becomes "return true" and all of the FPU mode selection machinery is removed.
--- arch/x86/include/asm/cpufeatures.h | 2 +- arch/x86/include/asm/fpu/internal.h | 2 +- arch/x86/kernel/fpu/init.c | 91 ++----------------------------------- 3 files changed, 5 insertions(+), 90 deletions(-)
arch/x86/include/asm/cpufeatures.h +1 -1
@@ -104,7 +104,7 @@#define X86_FEATURE_EXTD_APICID ( 3*32+26)/* has extended APICID (8 bits) */ #define X86_FEATURE_AMD_DCM ( 3*32+27)/* multi-node processor */ #define X86_FEATURE_APERFMPERF ( 3*32+28)/* APERFMPERF */ -#define X86_FEATURE_EAGER_FPU ( 3*32+29)/* "eagerfpu" Non lazy FPU restore */ +/* free, was #define X86_FEATURE_EAGER_FPU ( 3*32+29) * "eagerfpu" Non lazy FPU restore */ #define X86_FEATURE_NONSTOP_TSC_S3 ( 3*32+30)/* TSC doesn't stop in S3 state */ /* Intel-defined CPU features, CPUID level 0x00000001 (ecx), word 4 */
arch/x86/include/asm/fpu/internal.h +1 -1
@@ -62,7 +62,7 @@ extern u64 fpu__get_supported_xfeatures_mask(void);*/ static __always_inline __pure bool use_eager_fpu(void) { -return static_cpu_has (X86_FEATURE_EAGER_FPU ); +return true; } static __always_inline __pure bool use_xsaveopt(void)
arch/x86/kernel/fpu/init.c +3 -88
123 lines not on this page · raw message
Signature · 1 line
2.7.4
Paolo Bonzini
maintainerRe: [PATCH 2/9] x86/fpu: Hard-disable lazy fpu mode
Quoted 9 lines · On 05/10/2016 02:34, [email protected] wrote:
From: Andy Lutomirski <[email protected]>
Since commit
58122bf1d856("x86/fpu: Default eagerfpu=on on all CPUs") in Linux 4.6, eager FPU mode has been the default on all x86 systems, and no one has reported any regressions.This patch removes the ability to enable lazy mode:
use_eager_fpu()becomes "return true" and all of the FPU mode selection machinery is removed.
I haven't quite followed up on my promise to benchmark lazy vs. eager FPU, but I probably should do that now...
I see two possible issues with this. First, AMD as far as I know does not have XSAVEOPT. Second, when using virtualization, depending on how you configure your cluster it's enough to have one pre-SandyBridge Intel machine to force no XSAVE on all machines.
Thanks,
Paolo
Re: [PATCH 2/9] x86/fpu: Hard-disable lazy fpu mode
Quoted 17 lines · Paolo Bonzini
Quoted 11 lines · On 05/10/2016 02:34, [email protected] wrote:
From: Andy Lutomirski <[email protected]>
Since commit
58122bf1d856("x86/fpu: Default eagerfpu=on on all CPUs") in Linux 4.6, eager FPU mode has been the default on all x86 systems, and no one has reported any regressions.This patch removes the ability to enable lazy mode:
use_eager_fpu()becomes "return true" and all of the FPU mode selection machinery is
removed.I haven't quite followed up on my promise to benchmark lazy vs. eager FPU, but I probably should do that now...
Quoted 7 lines
I see two possible issues with this. First, AMD as far as I know does
not have XSAVEOPT. Second, when using virtualization, depending on how
you configure your cluster it's enough to have one pre-SandyBridge Intel
machine to force no XSAVE on all machines.
The "OPT" part of XSAVEOPT does not work across the host/guest boundary, anyway.
One of the items used in the tuple that determines whether the optimization can be used is whether or not the system is in the VMX root, or in a guest.
In other words, across a VMEXIT / VMENTER boundary, it does full saves & restores, if I am reading the manual right.
Signature · 1 line
All Rights Reversed.
Paolo Bonzini
maintainerRe: [PATCH 2/9] x86/fpu: Hard-disable lazy fpu mode
Quoted 18 lines · Rik van Riel
Quoted 17 lines · Paolo Bonzini
Quoted 11 lines · On 05/10/2016 02:34, [email protected] wrote:
From: Andy Lutomirski <[email protected]>
Since commit
58122bf1d856("x86/fpu: Default eagerfpu=on on all CPUs") in Linux 4.6, eager FPU mode has been the default on all x86 systems, and no one has reported any regressions.This patch removes the ability to enable lazy mode:
use_eager_fpu()becomes "return true" and all of the FPU mode selection machinery is
removed.I haven't quite followed up on my promise to benchmark lazy vs. eager FPU, but I probably should do that now...
Quoted 7 lines
Quoted 4 lines
I see two possible issues with this. First, AMD as far as I know does not have XSAVEOPT. Second, when using virtualization, depending on how you configure your cluster it's enough to have one pre-SandyBridge Intel machine to force no XSAVE on all machines.
The "OPT" part of XSAVEOPT does not work across the host/guest boundary, anyway.
Yes, but it works for bare metal (and in fact eager FPU was keyed on XSAVEOPT before 58122bf1d856, not XSAVE).
I'm not talking about KVM here; I am just saying that the lazy FPU code might be used more than we'd like to, because of AMD machines and of cases where XSAVE is hidden altogether from guests. Of course it is quite unlikely that it be reported as a regression, since things just work. But as far as I know 58122bf1d856 went in without any substantial (or not-so-substantial) benchmarking.
Paolo
Re: [PATCH 2/9] x86/fpu: Hard-disable lazy fpu mode
Quoted 32 lines · Paolo Bonzini
Quoted 18 lines · Rik van Riel
Quoted 17 lines · Paolo Bonzini
Quoted 11 lines · On 05/10/2016 02:34, [email protected] wrote:
From: Andy Lutomirski <[email protected]>
Since commit
58122bf1d856("x86/fpu: Default eagerfpu=on on all CPUs") in Linux 4.6, eager FPU mode has been the default on all x86 systems, and no one has reported any regressions.This patch removes the ability to enable lazy mode:
use_eager_fpu()becomes "return true" and all of the FPU mode selection machinery is
removed.I haven't quite followed up on my promise to benchmark lazy vs. eager FPU, but I probably should do that now...
Quoted 7 lines
Quoted 4 lines
I see two possible issues with this. First, AMD as far as I know does not have XSAVEOPT. Second, when using virtualization, depending on how you configure your cluster it's enough to have one pre-SandyBridge Intel machine to force no XSAVE on all machines.
The "OPT" part of XSAVEOPT does not work across the host/guest boundary, anyway.
Yes, but it works for bare metal (and in fact eager FPU was keyed on XSAVEOPT before
58122bf1d856, not XSAVE).
Quoted 6 lines
I'm not talking about KVM here; I am just saying that the lazy FPU code might be used more than we'd like to, because of AMD machines and of cases where XSAVE is hidden altogether from guests. Of course it is quite unlikely that it be reported as a regression, since things just work. But as far as I know
58122bf1d856went in without any substantial (or not-so-substantial) benchmarking.
I actually benchmarked the underlying instructions quite a bit on Intel. (Not on AMD, but I doubt the results are very different.) Writes to CR0.TS are *incredibly* slow, as are device-not-available exceptions. Keep in mind that, while there's a (slow) CLTS instruction, there is no corresponding STTS instruction, so we're left with a fully serializing, slowly microcoded move to CR0. On SVM, I think it's worse, because IIRC SVM doesn't have fancy execution controls that let MOV to CR0 avoid exiting. We're talking a couple hundred cycles best case for a TS set/clear pair, and thousands of cycles if we actually take a fault.
In contrast, an unconditional XSAVE + XRSTOR was considerably faster.
This leads to the counterintuitive result that, if we switch from task A to B and back and task A is heavily using the FPU, then it's faster to unconditoinally save and restore the full state both ways than it is to set and clear TS so we can avoid it.
I would guess that the lazy mode hasn't been a win under most workloads for many years. It's worse on 64-bit CPUs, since almost all userspace uses XMM regs for memcpy. At least on 32-bit CPUs, SIMD instructions weren't always available and userspace was conservative.
--Andy
Paolo Bonzini
maintainerRe: [PATCH 2/9] x86/fpu: Hard-disable lazy fpu mode
Quoted 8 lines · Andy Lutomirski
I actually benchmarked the underlying instructions quite a bit on Intel. (Not on AMD, but I doubt the results are very different.) Writes to CR0.TS are *incredibly* slow, as are device-not-available exceptions. Keep in mind that, while there's a (slow) CLTS instruction, there is no corresponding STTS instruction, so we're left with a fully serializing, slowly microcoded move to CR0. On SVM, I think it's worse, because IIRC SVM doesn't have fancy execution controls that let MOV to CR0 avoid exiting.
SVM lets you choose whether to trap on TS and MP; update_cr0_intercept is where KVM does that (the "selective CR0 write" intercept is always on, while the "CR0 write" intercept is toggled in that function).
Quoted 5 lines
We're talking a couple
hundred cycles best case for a TS set/clear pair, and thousands of cycles if we actually take a fault.In contrast, an unconditional XSAVE + XRSTOR was considerably faster.
Did you also do a comparison against FXSAVE/FXRSTOR (on either pre- or post-SandyBridge processors)?
But yeah, it's possible that the lack of STTS screws the whole plan, despite the fpu.preload optimization in switch_fpu_prepare.
Paolo
Quoted 4 lines
This leads to the counterintuitive result that, if we switch from task A to B and back and task A is heavily using the FPU, then it's faster to unconditoinally save and restore the full state both ways than it is to set and clear TS so we can avoid it.
tip-bot for Andy Lutomirski
merge bot[tip:x86/fpu] x86/fpu: Hard-disable lazy FPU mode
In reply to [email protected]x86/fpu: Hard-disable lazy FPU mode
Commit-ID: ca6938a1cd8a1c5e861a99b67f84ac166fc2b9e7 Gitweb: http://git.kernel.org/tip/ca6938a1cd8a1c5e861a99b67f84ac166fc2b9e7 Author: Andy Lutomirski <[email protected]> AuthorDate: Tue, 4 Oct 2016 20:34:31 -0400 Committer: Ingo Molnar <[email protected]> CommitDate: Fri, 7 Oct 2016 11:14:17 +0200
x86/fpu: Hard-disable lazy FPU mode
Since commit:
58122bf1d856 ("x86/fpu: Default eagerfpu=on on all CPUs")
... in Linux 4.6, eager FPU mode has been the default on all x86 systems, and no one has reported any regressions.
This patch removes the ability to enable lazy mode: use_eager_fpu() becomes "return true" and all of the FPU mode selection machinery is removed.
--- arch/x86/include/asm/cpufeatures.h | 2 +- arch/x86/include/asm/fpu/internal.h | 2 +- arch/x86/kernel/fpu/init.c | 91 ++----------------------------------- 3 files changed, 5 insertions(+), 90 deletions(-)
arch/x86/include/asm/cpufeatures.h +1 -1
@@ -104,7 +104,7 @@#define X86_FEATURE_EXTD_APICID ( 3*32+26)/* has extended APICID (8 bits) */ #define X86_FEATURE_AMD_DCM ( 3*32+27)/* multi-node processor */ #define X86_FEATURE_APERFMPERF ( 3*32+28)/* APERFMPERF */ -#define X86_FEATURE_EAGER_FPU ( 3*32+29)/* "eagerfpu" Non lazy FPU restore */ +/* free, was #define X86_FEATURE_EAGER_FPU ( 3*32+29) * "eagerfpu" Non lazy FPU restore */ #define X86_FEATURE_NONSTOP_TSC_S3 ( 3*32+30)/* TSC doesn't stop in S3 state */ /* Intel-defined CPU features, CPUID level 0x00000001 (ecx), word 4 */
arch/x86/include/asm/fpu/internal.h +1 -1
@@ -62,7 +62,7 @@ extern u64 fpu__get_supported_xfeatures_mask(void);*/ static __always_inline __pure bool use_eager_fpu(void) { -return static_cpu_has (X86_FEATURE_EAGER_FPU ); +return true; } static __always_inline __pure bool use_xsaveopt(void)
arch/x86/kernel/fpu/init.c +3 -88
123 lines not on this page · raw message
PATCH 3/9 x86/fpu: Remove the XFEATURE_MASK_EAGER/LAZY distinction
In reply to [email protected]From: Andy Lutomirski <[email protected]>
Now that lazy mode is gone, we don't need to distinguish which xfeatures require eager mode.
--- arch/x86/include/asm/fpu/xstate.h | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-)
arch/x86/include/asm/fpu/xstate.h +5 -9
@@ -21,20 +21,16 @@/* Supervisor features */ #define XFEATURE_MASK_SUPERVISOR (XFEATURE_MASK_PT) -/* Supported features which support lazy state saving */ -#define XFEATURE_MASK_LAZY (XFEATURE_MASK_FP | \ +/* All currently supported features */ +#define XCNTXT_MASK (XFEATURE_MASK_FP | \ XFEATURE_MASK_SSE | \ XFEATURE_MASK_YMM | \ XFEATURE_MASK_OPMASK | \ XFEATURE_MASK_ZMM_Hi256 | \ XFEATURE_MASK_Hi16_ZMM | \ - XFEATURE_MASK_PKRU) - -/* Supported features which require eager state saving */ -#define XFEATURE_MASK_EAGER (XFEATURE_MASK_BNDREGS | XFEATURE_MASK_BNDCSR) - -/* All currently supported features */ -#define XCNTXT_MASK (XFEATURE_MASK_LAZY | XFEATURE_MASK_EAGER) + XFEATURE_MASK_PKRU | \ + XFEATURE_MASK_BNDREGS | \ + XFEATURE_MASK_BNDCSR)#ifdef CONFIG_X86_64 #define REX_PREFIX "0x48, "
Signature · 1 line
2.7.4
tip-bot for Andy Lutomirski
merge bot[tip:x86/fpu] x86/fpu: Remove the XFEATURE_MASK_EAGER/LAZY distinction
x86/fpu: Remove the XFEATURE_MASK_EAGER/LAZY distinction
Commit-ID: 2f7fada23549b4657e9ea92eeddebc878db569d2 Gitweb: http://git.kernel.org/tip/2f7fada23549b4657e9ea92eeddebc878db569d2 Author: Andy Lutomirski <[email protected]> AuthorDate: Tue, 4 Oct 2016 20:34:32 -0400 Committer: Ingo Molnar <[email protected]> CommitDate: Fri, 7 Oct 2016 11:14:29 +0200
x86/fpu: Remove the XFEATURE_MASK_EAGER/LAZY distinction
Now that lazy mode is gone, we don't need to distinguish which xfeatures require eager mode.
--- arch/x86/include/asm/fpu/xstate.h | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-)
arch/x86/include/asm/fpu/xstate.h +5 -9
@@ -21,20 +21,16 @@/* Supervisor features */ #define XFEATURE_MASK_SUPERVISOR (XFEATURE_MASK_PT) -/* Supported features which support lazy state saving */ -#define XFEATURE_MASK_LAZY (XFEATURE_MASK_FP | \ +/* All currently supported features */ +#define XCNTXT_MASK (XFEATURE_MASK_FP | \ XFEATURE_MASK_SSE | \ XFEATURE_MASK_YMM | \ XFEATURE_MASK_OPMASK | \ XFEATURE_MASK_ZMM_Hi256 | \ XFEATURE_MASK_Hi16_ZMM | \ - XFEATURE_MASK_PKRU) - -/* Supported features which require eager state saving */ -#define XFEATURE_MASK_EAGER (XFEATURE_MASK_BNDREGS | XFEATURE_MASK_BNDCSR) - -/* All currently supported features */ -#define XCNTXT_MASK (XFEATURE_MASK_LAZY | XFEATURE_MASK_EAGER) + XFEATURE_MASK_PKRU | \ + XFEATURE_MASK_BNDREGS | \ + XFEATURE_MASK_BNDCSR)#ifdef CONFIG_X86_64 #define REX_PREFIX "0x48, "
PATCH 4/9 x86/fpu: Remove use_eager_fpu()
In reply to [email protected]From: Andy Lutomirski <[email protected]>
This removes all the obvious code paths that depend on lazy FPU mode. It shouldn't change the generated code at all.
--- arch/x86/crypto/crc32c-intel_glue.c | 17 ++++------------- arch/x86/include/asm/fpu/internal.h | 34 +-------------------------------- arch/x86/kernel/fpu/core.c | 38 +++++-------------------------------- arch/x86/kernel/fpu/signal.c | 8 +++----- arch/x86/kernel/fpu/xstate.c | 9 --------- arch/x86/kvm/cpuid.c | 4 +--- arch/x86/kvm/x86.c | 10 ---------- 7 files changed, 14 insertions(+), 106 deletions(-)
arch/x86/crypto/crc32c-intel_glue.c +4 -13
@@ -48,21 +48,13 @@#ifdef CONFIG_X86_64 /* * use carryless multiply version of crc32c when buffer -* size is >= 512 (when eager fpu is enabled) or -* >= 1024 (when eager fpu is disabled) to account +* size is >= 512 to account * for fpu state save/restore overhead. */ -#define CRC32C_PCL_BREAKEVEN_EAGERFPU 512 -#define CRC32C_PCL_BREAKEVEN_NOEAGERFPU 1024 +#define CRC32C_PCL_BREAKEVEN 512 asmlinkage unsigned int crc_pcl(const u8 *buffer, int len,unsigned int crc_init); -static int crc32c_pcl_breakeven =CRC32C_PCL_BREAKEVEN_EAGERFPU ; -#define set_pcl_breakeven_point () \ -do { \ - if (!use_eager_fpu()) \ - crc32c_pcl_breakeven = CRC32C_PCL_BREAKEVEN_NOEAGERFPU; \ -} while (0)#endif /* CONFIG_X86_64 */ static u32 crc32c_intel_le_hw_byte (u32 crc,unsigned char const *data,size_t length)
@@ -185,7 +177,7 @@ static int crc32c_pcl_intel_update(struct shash_desc *desc, const u8 *data,* use faster PCL version if datasize is large enough to * overcome kernel fpu state save/restore overhead */ -if (len >= crc32c_pcl_breakeven &&irq_fpu_usable ()) { +if (len >=CRC32C_PCL_BREAKEVEN &&irq_fpu_usable ()) {kernel_fpu_begin (); *crcp =crc_pcl (data, len, *crcp);kernel_fpu_end ();
@@ -197,7 +189,7 @@ static int crc32c_pcl_intel_update(struct shash_desc *desc, const u8 *data,static int __crc32c_pcl_intel_finup (u32 *crcp,const u8 *data,unsigned int len,u8 *out) { -if (len >= crc32c_pcl_breakeven &&irq_fpu_usable ()) { +if (len >=CRC32C_PCL_BREAKEVEN &&irq_fpu_usable ()) {kernel_fpu_begin (); *(__le32 *)out = ~cpu_to_le32 (crc_pcl (data, len, *crcp));kernel_fpu_end ();
@@ -256,7 +248,6 @@ static int __init crc32c_intel_mod_init(void) alg.update = crc32c_pcl_intel_update; alg.finup = crc32c_pcl_intel_finup; alg.digest = crc32c_pcl_intel_digest; - set_pcl_breakeven_point(); }#endif return crypto_register_shash (&alg);
arch/x86/include/asm/fpu/internal.h +1 -33
@@ -60,11 +60,6 @@ extern u64 fpu__get_supported_xfeatures_mask(void);/* * FPU related CPU feature flag helper routines: */ -static __always_inline __pure bool use_eager_fpu(void) -{ -return true; -} - static __always_inline __pure bool use_xsaveopt(void) {return static_cpu_has (X86_FEATURE_XSAVEOPT );
@@ -501,24 +496,6 @@ static inline int fpu_want_lazy_restore(struct fpu *fpu, unsigned int cpu) } -/* -* Wrap lazy FPU TS handling in a 'hw fpregs activation/deactivation' -* idiom, which is then paired with the sw-flag (fpregs_active) later on: -*/ - -static inline void __fpregs_activate_hw (void ) -{ -if (!use_eager_fpu ()) -clts (); -} - -static inline void __fpregs_deactivate_hw (void ) -{ -if (!use_eager_fpu ()) -stts (); -} - -/* Must be paired with an 'stts' (fpregs_deactivate_hw()) after! */ static inline void __fpregs_deactivate (struct fpu *fpu) {WARN_ON_FPU (!fpu->fpregs_active);
@@ -528,7 +505,6 @@ static inline void __fpregs_deactivate(struct fpu *fpu)trace_x86_fpu_regs_deactivated (fpu); } -/* Must be paired with a 'clts' (fpregs_activate_hw()) before! */ static inline void __fpregs_activate (struct fpu *fpu) {WARN_ON_FPU (fpu->fpregs_active);
@@ -554,22 +530,17 @@ static inline int fpregs_active(void) }/* -* Encapsulate the CR0.TS handling together with the -* software flag. -* * These generally need preemption protection to work, * do try to avoid using these on their own. */ static inline void fpregs_activate (struct fpu *fpu) { -__fpregs_activate_hw ();__fpregs_activate (fpu); }static inline void fpregs_deactivate (struct fpu *fpu) {__fpregs_deactivate (fpu); -__fpregs_deactivate_hw (); }/*
@@ -596,8 +567,7 @@ switch_fpu_prepare(struct fpu *old_fpu, struct fpu *new_fpu, int cpu)* or if the past 5 consecutive context-switches used math. */ fpu.preload =static_cpu_has (X86_FEATURE_FPU ) && - new_fpu->fpstate_active && - (use_eager_fpu () || new_fpu->counter >5 ); + new_fpu->fpstate_active;if (old_fpu->fpregs_active) {if (!copy_fpregs_to_fpstate (old_fpu))
@@ -615,8 +585,6 @@ switch_fpu_prepare(struct fpu *old_fpu, struct fpu *new_fpu, int cpu)__fpregs_activate (new_fpu);trace_x86_fpu_regs_activated (new_fpu);prefetch (&new_fpu->state); - }else { -__fpregs_deactivate_hw (); } }else { old_fpu->counter =0 ;
arch/x86/kernel/fpu/core.c +5 -33
@@ -57,27 +57,9 @@ static bool kernel_fpu_disabled(void)return this_cpu_read (in_kernel_fpu); } -/* -* Were we in an interrupt that interrupted kernel mode? -* -* On others, we can do a kernel_fpu_begin/end() pair *ONLY* if that -* pair does nothing at all: the thread must not have fpu (so -* that we don't try to save the FPU state), and TS must -* be set (so that the clts/stts pair does nothing that is -* visible in the interrupted kernel thread). -* -* Except for the eagerfpu case when we return true; in the likely case -* the thread has FPU but we are not going to set/clear TS. -*/ static bool interrupted_kernel_fpu_idle (void ) { -if (kernel_fpu_disabled ()) -return false; - -if (use_eager_fpu ()) -return true; - -return !current->thread.fpu.fpregs_active && (read_cr0 () &X86_CR0_TS ); +return !kernel_fpu_disabled (); }/*
@@ -125,7 +107,6 @@ void __kernel_fpu_begin(void)copy_fpregs_to_fpstate (fpu); }else {this_cpu_write (fpu_fpregs_owner_ctx,NULL ); -__fpregs_activate_hw (); } }EXPORT_SYMBOL (__kernel_fpu_begin);
@@ -136,8 +117,6 @@ void __kernel_fpu_end(void)if (fpu->fpregs_active)copy_kernel_to_fpregs (&fpu->state); -else -__fpregs_deactivate_hw ();kernel_fpu_enable (); }
@@ -199,10 +178,7 @@ void fpu__save(struct fpu *fpu)trace_x86_fpu_before_save (fpu);if (fpu->fpregs_active) {if (!copy_fpregs_to_fpstate (fpu)) { -if (use_eager_fpu ()) -copy_kernel_to_fpregs (&fpu->state); -else -fpregs_deactivate (fpu); +copy_kernel_to_fpregs (&fpu->state); } }trace_x86_fpu_after_save (fpu);
@@ -259,8 +235,7 @@ int fpu__copy(struct fpu *dst_fpu, struct fpu *src_fpu)* Don't let 'init optimized' areas of the XSAVE area * leak into the child task: */ -if (use_eager_fpu ()) -memset (&dst_fpu->state.xsave,0 , fpu_kernel_xstate_size); +memset (&dst_fpu->state.xsave,0 , fpu_kernel_xstate_size);/* * Save current FPU registers directly into the child
@@ -282,10 +257,7 @@ int fpu__copy(struct fpu *dst_fpu, struct fpu *src_fpu)memcpy (&src_fpu->state, &dst_fpu->state, fpu_kernel_xstate_size); -if (use_eager_fpu ()) -copy_kernel_to_fpregs (&src_fpu->state); -else -fpregs_deactivate (src_fpu); +copy_kernel_to_fpregs (&src_fpu->state); }preempt_enable ();
@@ -517,7 +489,7 @@ void fpu__clear(struct fpu *fpu) {WARN_ON_FPU (fpu != ¤t->thread.fpu);/* Almost certainly an anomaly */ -if (!use_eager_fpu () || !static_cpu_has (X86_FEATURE_FPU )) { +if (!static_cpu_has (X86_FEATURE_FPU )) {/* FPU state will be reallocated lazily at the first use. */ fpu__drop (fpu); }else {
arch/x86/kernel/fpu/signal.c +3 -5
@@ -340,11 +340,9 @@ static int __fpu__restore_sig(void __user *buf, void __user *buf_fx, int size) } fpu->fpstate_active =1 ; -if (use_eager_fpu ()) { -preempt_disable (); -fpu__restore (fpu); -preempt_enable (); - } +preempt_disable (); +fpu__restore (fpu); +preempt_enable ();return err; }else {
arch/x86/kernel/fpu/xstate.c +0 -9
@@ -886,15 +886,6 @@ int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,*/ if (!boot_cpu_has (X86_FEATURE_OSPKE ))return -EINVAL ; -/* -* For most XSAVE components, this would be an arduous task: -* brining fpstate up to date with fpregs, updating fpstate, -* then re-populating fpregs. But, for components that are -* never lazily managed, we can just access the fpregs -* directly. PKRU is never managed lazily, so we can just -* manipulate it directly. Make sure it stays that way. -*/ -WARN_ON_ONCE (!use_eager_fpu ());/* Set the bits we need in PKRU: */ if (init_val &PKEY_DISABLE_ACCESS )
arch/x86/kvm/cpuid.c +1 -3
@@ -16,7 +16,6 @@#include <linux/export.h> #include <linux/vmalloc.h> #include <linux/uaccess.h> -#include <asm/fpu/internal.h> /* For use_eager_fpu. Ugh! */ #include <asm/user.h> #include <asm/fpu/xstate.h> #include "cpuid.h"
@@ -114,8 +113,7 @@ int kvm_update_cpuid(struct kvm_vcpu *vcpu)if (best && (best->eax & (F (XSAVES ) |F (XSAVEC )))) best->ebx =xstate_required_size (vcpu->arch.xcr0, true); -if (use_eager_fpu ()) - kvm_x86_ops->fpu_activate (vcpu); + kvm_x86_ops->fpu_activate (vcpu);/* * The existing code assumes virtual address is 48-bit in the canonical
arch/x86/kvm/x86.c +0 -10
@@ -7357,16 +7357,6 @@ void kvm_put_guest_fpu(struct kvm_vcpu *vcpu)copy_fpregs_to_fpstate (&vcpu->arch.guest_fpu);__kernel_fpu_end (); ++vcpu->stat.fpu_reload; -/* -* If using eager FPU mode, or if the guest is a frequent user -* of the FPU, just leave the FPU active for next time. -* Every 255 times fpu_counter rolls over to 0; a guest that uses -* the FPU in bursts will revert to loading it on demand. -*/ -if (!use_eager_fpu ()) { -if (++vcpu->fpu_counter <5 ) -kvm_make_request (KVM_REQ_DEACTIVATE_FPU , vcpu); - }trace_kvm_fpu (0 ); }
Signature · 1 line
2.7.4
tip-bot for Andy Lutomirski
merge bot[tip:x86/fpu] x86/fpu: Remove use_eager_fpu()
x86/fpu: Remove use_eager_fpu()
Commit-ID: c592b57347069abfc0dcad3b3a302cf882602597 Gitweb: http://git.kernel.org/tip/c592b57347069abfc0dcad3b3a302cf882602597 Author: Andy Lutomirski <[email protected]> AuthorDate: Tue, 4 Oct 2016 20:34:33 -0400 Committer: Ingo Molnar <[email protected]> CommitDate: Fri, 7 Oct 2016 11:14:36 +0200
x86/fpu: Remove use_eager_fpu()
This removes all the obvious code paths that depend on lazy FPU mode. It shouldn't change the generated code at all.
--- arch/x86/crypto/crc32c-intel_glue.c | 17 ++++------------- arch/x86/include/asm/fpu/internal.h | 34 +-------------------------------- arch/x86/kernel/fpu/core.c | 38 +++++-------------------------------- arch/x86/kernel/fpu/signal.c | 8 +++----- arch/x86/kernel/fpu/xstate.c | 9 --------- arch/x86/kvm/cpuid.c | 4 +--- arch/x86/kvm/x86.c | 10 ---------- 7 files changed, 14 insertions(+), 106 deletions(-)
arch/x86/crypto/crc32c-intel_glue.c +4 -13
@@ -48,21 +48,13 @@#ifdef CONFIG_X86_64 /* * use carryless multiply version of crc32c when buffer -* size is >= 512 (when eager fpu is enabled) or -* >= 1024 (when eager fpu is disabled) to account +* size is >= 512 to account * for fpu state save/restore overhead. */ -#define CRC32C_PCL_BREAKEVEN_EAGERFPU 512 -#define CRC32C_PCL_BREAKEVEN_NOEAGERFPU 1024 +#define CRC32C_PCL_BREAKEVEN 512 asmlinkage unsigned int crc_pcl(const u8 *buffer, int len,unsigned int crc_init); -static int crc32c_pcl_breakeven =CRC32C_PCL_BREAKEVEN_EAGERFPU ; -#define set_pcl_breakeven_point () \ -do { \ - if (!use_eager_fpu()) \ - crc32c_pcl_breakeven = CRC32C_PCL_BREAKEVEN_NOEAGERFPU; \ -} while (0)#endif /* CONFIG_X86_64 */ static u32 crc32c_intel_le_hw_byte (u32 crc,unsigned char const *data,size_t length)
@@ -185,7 +177,7 @@ static int crc32c_pcl_intel_update(struct shash_desc *desc, const u8 *data,* use faster PCL version if datasize is large enough to * overcome kernel fpu state save/restore overhead */ -if (len >= crc32c_pcl_breakeven &&irq_fpu_usable ()) { +if (len >=CRC32C_PCL_BREAKEVEN &&irq_fpu_usable ()) {kernel_fpu_begin (); *crcp =crc_pcl (data, len, *crcp);kernel_fpu_end ();
@@ -197,7 +189,7 @@ static int crc32c_pcl_intel_update(struct shash_desc *desc, const u8 *data,static int __crc32c_pcl_intel_finup (u32 *crcp,const u8 *data,unsigned int len,u8 *out) { -if (len >= crc32c_pcl_breakeven &&irq_fpu_usable ()) { +if (len >=CRC32C_PCL_BREAKEVEN &&irq_fpu_usable ()) {kernel_fpu_begin (); *(__le32 *)out = ~cpu_to_le32 (crc_pcl (data, len, *crcp));kernel_fpu_end ();
@@ -256,7 +248,6 @@ static int __init crc32c_intel_mod_init(void) alg.update = crc32c_pcl_intel_update; alg.finup = crc32c_pcl_intel_finup; alg.digest = crc32c_pcl_intel_digest; - set_pcl_breakeven_point(); }#endif return crypto_register_shash (&alg);
arch/x86/include/asm/fpu/internal.h +1 -33
@@ -60,11 +60,6 @@ extern u64 fpu__get_supported_xfeatures_mask(void);/* * FPU related CPU feature flag helper routines: */ -static __always_inline __pure bool use_eager_fpu(void) -{ -return true; -} - static __always_inline __pure bool use_xsaveopt(void) {return static_cpu_has (X86_FEATURE_XSAVEOPT );
@@ -501,24 +496,6 @@ static inline int fpu_want_lazy_restore(struct fpu *fpu, unsigned int cpu) } -/* -* Wrap lazy FPU TS handling in a 'hw fpregs activation/deactivation' -* idiom, which is then paired with the sw-flag (fpregs_active) later on: -*/ - -static inline void __fpregs_activate_hw (void ) -{ -if (!use_eager_fpu ()) -clts (); -} - -static inline void __fpregs_deactivate_hw (void ) -{ -if (!use_eager_fpu ()) -stts (); -} - -/* Must be paired with an 'stts' (fpregs_deactivate_hw()) after! */ static inline void __fpregs_deactivate (struct fpu *fpu) {WARN_ON_FPU (!fpu->fpregs_active);
@@ -528,7 +505,6 @@ static inline void __fpregs_deactivate(struct fpu *fpu)trace_x86_fpu_regs_deactivated (fpu); } -/* Must be paired with a 'clts' (fpregs_activate_hw()) before! */ static inline void __fpregs_activate (struct fpu *fpu) {WARN_ON_FPU (fpu->fpregs_active);
@@ -554,22 +530,17 @@ static inline int fpregs_active(void) }/* -* Encapsulate the CR0.TS handling together with the -* software flag. -* * These generally need preemption protection to work, * do try to avoid using these on their own. */ static inline void fpregs_activate (struct fpu *fpu) { -__fpregs_activate_hw ();__fpregs_activate (fpu); }static inline void fpregs_deactivate (struct fpu *fpu) {__fpregs_deactivate (fpu); -__fpregs_deactivate_hw (); }/*
@@ -596,8 +567,7 @@ switch_fpu_prepare(struct fpu *old_fpu, struct fpu *new_fpu, int cpu)* or if the past 5 consecutive context-switches used math. */ fpu.preload =static_cpu_has (X86_FEATURE_FPU ) && - new_fpu->fpstate_active && - (use_eager_fpu () || new_fpu->counter >5 ); + new_fpu->fpstate_active;if (old_fpu->fpregs_active) {if (!copy_fpregs_to_fpstate (old_fpu))
@@ -615,8 +585,6 @@ switch_fpu_prepare(struct fpu *old_fpu, struct fpu *new_fpu, int cpu)__fpregs_activate (new_fpu);trace_x86_fpu_regs_activated (new_fpu);prefetch (&new_fpu->state); - }else { -__fpregs_deactivate_hw (); } }else { old_fpu->counter =0 ;
arch/x86/kernel/fpu/core.c +5 -33
@@ -57,27 +57,9 @@ static bool kernel_fpu_disabled(void)return this_cpu_read (in_kernel_fpu); } -/* -* Were we in an interrupt that interrupted kernel mode? -* -* On others, we can do a kernel_fpu_begin/end() pair *ONLY* if that -* pair does nothing at all: the thread must not have fpu (so -* that we don't try to save the FPU state), and TS must -* be set (so that the clts/stts pair does nothing that is -* visible in the interrupted kernel thread). -* -* Except for the eagerfpu case when we return true; in the likely case -* the thread has FPU but we are not going to set/clear TS. -*/ static bool interrupted_kernel_fpu_idle (void ) { -if (kernel_fpu_disabled ()) -return false; - -if (use_eager_fpu ()) -return true; - -return !current->thread.fpu.fpregs_active && (read_cr0 () &X86_CR0_TS ); +return !kernel_fpu_disabled (); }/*
@@ -125,7 +107,6 @@ void __kernel_fpu_begin(void)copy_fpregs_to_fpstate (fpu); }else {this_cpu_write (fpu_fpregs_owner_ctx,NULL ); -__fpregs_activate_hw (); } }EXPORT_SYMBOL (__kernel_fpu_begin);
@@ -136,8 +117,6 @@ void __kernel_fpu_end(void)if (fpu->fpregs_active)copy_kernel_to_fpregs (&fpu->state); -else -__fpregs_deactivate_hw ();kernel_fpu_enable (); }
@@ -199,10 +178,7 @@ void fpu__save(struct fpu *fpu)trace_x86_fpu_before_save (fpu);if (fpu->fpregs_active) {if (!copy_fpregs_to_fpstate (fpu)) { -if (use_eager_fpu ()) -copy_kernel_to_fpregs (&fpu->state); -else -fpregs_deactivate (fpu); +copy_kernel_to_fpregs (&fpu->state); } }trace_x86_fpu_after_save (fpu);
@@ -259,8 +235,7 @@ int fpu__copy(struct fpu *dst_fpu, struct fpu *src_fpu)* Don't let 'init optimized' areas of the XSAVE area * leak into the child task: */ -if (use_eager_fpu ()) -memset (&dst_fpu->state.xsave,0 , fpu_kernel_xstate_size); +memset (&dst_fpu->state.xsave,0 , fpu_kernel_xstate_size);/* * Save current FPU registers directly into the child
@@ -282,10 +257,7 @@ int fpu__copy(struct fpu *dst_fpu, struct fpu *src_fpu)memcpy (&src_fpu->state, &dst_fpu->state, fpu_kernel_xstate_size); -if (use_eager_fpu ()) -copy_kernel_to_fpregs (&src_fpu->state); -else -fpregs_deactivate (src_fpu); +copy_kernel_to_fpregs (&src_fpu->state); }preempt_enable ();
@@ -517,7 +489,7 @@ void fpu__clear(struct fpu *fpu) {WARN_ON_FPU (fpu != ¤t->thread.fpu);/* Almost certainly an anomaly */ -if (!use_eager_fpu () || !static_cpu_has (X86_FEATURE_FPU )) { +if (!static_cpu_has (X86_FEATURE_FPU )) {/* FPU state will be reallocated lazily at the first use. */ fpu__drop (fpu); }else {
arch/x86/kernel/fpu/signal.c +3 -5
@@ -340,11 +340,9 @@ static int __fpu__restore_sig(void __user *buf, void __user *buf_fx, int size) } fpu->fpstate_active =1 ; -if (use_eager_fpu ()) { -preempt_disable (); -fpu__restore (fpu); -preempt_enable (); - } +preempt_disable (); +fpu__restore (fpu); +preempt_enable ();return err; }else {
arch/x86/kernel/fpu/xstate.c +0 -9
@@ -886,15 +886,6 @@ int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,*/ if (!boot_cpu_has (X86_FEATURE_OSPKE ))return -EINVAL ; -/* -* For most XSAVE components, this would be an arduous task: -* brining fpstate up to date with fpregs, updating fpstate, -* then re-populating fpregs. But, for components that are -* never lazily managed, we can just access the fpregs -* directly. PKRU is never managed lazily, so we can just -* manipulate it directly. Make sure it stays that way. -*/ -WARN_ON_ONCE (!use_eager_fpu ());/* Set the bits we need in PKRU: */ if (init_val &PKEY_DISABLE_ACCESS )
arch/x86/kvm/cpuid.c +1 -3
@@ -16,7 +16,6 @@#include <linux/export.h> #include <linux/vmalloc.h> #include <linux/uaccess.h> -#include <asm/fpu/internal.h> /* For use_eager_fpu. Ugh! */ #include <asm/user.h> #include <asm/fpu/xstate.h> #include "cpuid.h"
@@ -114,8 +113,7 @@ int kvm_update_cpuid(struct kvm_vcpu *vcpu)if (best && (best->eax & (F (XSAVES ) |F (XSAVEC )))) best->ebx =xstate_required_size (vcpu->arch.xcr0, true); -if (use_eager_fpu ()) - kvm_x86_ops->fpu_activate (vcpu); + kvm_x86_ops->fpu_activate (vcpu);/* * The existing code assumes virtual address is 48-bit in the canonical
arch/x86/kvm/x86.c +0 -10
@@ -7357,16 +7357,6 @@ void kvm_put_guest_fpu(struct kvm_vcpu *vcpu)copy_fpregs_to_fpstate (&vcpu->arch.guest_fpu);__kernel_fpu_end (); ++vcpu->stat.fpu_reload; -/* -* If using eager FPU mode, or if the guest is a frequent user -* of the FPU, just leave the FPU active for next time. -* Every 255 times fpu_counter rolls over to 0; a guest that uses -* the FPU in bursts will revert to loading it on demand. -*/ -if (!use_eager_fpu ()) { -if (++vcpu->fpu_counter <5 ) -kvm_make_request (KVM_REQ_DEACTIVATE_FPU , vcpu); - }trace_kvm_fpu (0 ); }
PATCH 5/9 x86/fpu: remove fpu.counter
In reply to [email protected]From: Rik van Riel <[email protected]>
With the lazy FPU code gone, we no longer use the counter field in struct fpu for anything. Get rid it.
--- arch/x86/include/asm/fpu/internal.h | 3 --- arch/x86/include/asm/fpu/types.h | 11 ----------- arch/x86/include/asm/trace/fpu.h | 5 +---- arch/x86/kernel/fpu/core.c | 3 --- 4 files changed, 1 insertion(+), 21 deletions(-)
arch/x86/include/asm/fpu/internal.h +0 -3
@@ -581,16 +581,13 @@ switch_fpu_prepare(struct fpu *old_fpu, struct fpu *new_fpu, int cpu)/* Don't change CR0.TS if we just switch! */ if (fpu.preload) { - new_fpu->counter++;__fpregs_activate (new_fpu);trace_x86_fpu_regs_activated (new_fpu);prefetch (&new_fpu->state); } }else { - old_fpu->counter =0 ; old_fpu->last_cpu =-1 ;if (fpu.preload) { - new_fpu->counter++;if (fpu_want_lazy_restore (new_fpu, cpu)) fpu.preload =0 ;else
arch/x86/include/asm/fpu/types.h +0 -11
@@ -322,17 +322,6 @@ struct fpu {unsigned char fpregs_active;/* -* @counter: -* -* This counter contains the number of consecutive context switches -* during which the FPU stays used. If this is over a threshold, the -* lazy FPU restore logic becomes eager, to save the trap overhead. -* This is an unsigned char so that after 256 iterations the counter -* wraps and the context switch behavior turns lazy again; this is to -* deal with bursty apps that only use the FPU for a short time: -*/ -unsigned char counter; -/* * @state: * * In-memory copy of all FPU registers that we save/restore
arch/x86/include/asm/trace/fpu.h +1 -4
@@ -14,7 +14,6 @@ DECLARE_EVENT_CLASS(x86_fpu, __field(struct fpu *, fpu) __field(bool, fpregs_active) __field(bool, fpstate_active) - __field(int, counter) __field(u64, xfeatures) __field(u64, xcomp_bv) ),
@@ -23,17 +22,15 @@ DECLARE_EVENT_CLASS(x86_fpu, __entry->fpu = fpu; __entry->fpregs_active = fpu->fpregs_active; __entry->fpstate_active = fpu->fpstate_active; - __entry->counter = fpu->counter;if (boot_cpu_has (X86_FEATURE_OSXSAVE )) { __entry->xfeatures = fpu->state.xsave.header.xfeatures; __entry->xcomp_bv = fpu->state.xsave.header.xcomp_bv; } ), -TP_printk ("x86/fpu: %p fpregs_active: %d fpstate_active: %d counter: %d xfeatures: %llx xcomp_bv: %llx" , +TP_printk ("x86/fpu: %p fpregs_active: %d fpstate_active: %d xfeatures: %llx xcomp_bv: %llx" , __entry->fpu, __entry->fpregs_active, __entry->fpstate_active, - __entry->counter, __entry->xfeatures, __entry->xcomp_bv )
arch/x86/kernel/fpu/core.c +0 -3
@@ -222,7 +222,6 @@ EXPORT_SYMBOL_GPL(fpstate_init);int fpu__copy (struct fpu *dst_fpu,struct fpu *src_fpu) { - dst_fpu->counter =0 ; dst_fpu->fpregs_active =0 ; dst_fpu->last_cpu =-1 ;
@@ -430,7 +429,6 @@ void fpu__restore(struct fpu *fpu)trace_x86_fpu_before_restore (fpu);fpregs_activate (fpu);copy_kernel_to_fpregs (&fpu->state); - fpu->counter++;trace_x86_fpu_after_restore (fpu);kernel_fpu_enable (); }
@@ -448,7 +446,6 @@ EXPORT_SYMBOL_GPL(fpu__restore);void fpu__drop (struct fpu *fpu) {preempt_disable (); - fpu->counter =0 ;if (fpu->fpregs_active) {/* Ignore delayed exceptions from user space */
Signature · 1 line
2.7.4
Re: [PATCH 5/9] x86/fpu: remove fpu.counter
Quoted 4 lines · On Tue, Oct 4, 2016 at 5:34 PM, <[email protected]> wrote:
From: Rik van Riel <[email protected]>
With the lazy FPU code gone, we no longer use the counter field in struct fpu for anything. Get rid it.
tip-bot for Rik van Riel
merge bot[tip:x86/fpu] x86/fpu: Remove struct fpu::counter
In reply to [email protected]x86/fpu: Remove struct fpu::counter
Commit-ID: 3913cc3507575273beb165a5e027a081913ed507 Gitweb: http://git.kernel.org/tip/3913cc3507575273beb165a5e027a081913ed507 Author: Rik van Riel <[email protected]> AuthorDate: Tue, 4 Oct 2016 20:34:34 -0400 Committer: Ingo Molnar <[email protected]> CommitDate: Fri, 7 Oct 2016 11:14:40 +0200
x86/fpu: Remove struct fpu::counter
With the lazy FPU code gone, we no longer use the counter field in struct fpu for anything. Get rid it.
--- arch/x86/include/asm/fpu/internal.h | 3 --- arch/x86/include/asm/fpu/types.h | 11 ----------- arch/x86/include/asm/trace/fpu.h | 5 +---- arch/x86/kernel/fpu/core.c | 3 --- 4 files changed, 1 insertion(+), 21 deletions(-)
arch/x86/include/asm/fpu/internal.h +0 -3
@@ -581,16 +581,13 @@ switch_fpu_prepare(struct fpu *old_fpu, struct fpu *new_fpu, int cpu)/* Don't change CR0.TS if we just switch! */ if (fpu.preload) { - new_fpu->counter++;__fpregs_activate (new_fpu);trace_x86_fpu_regs_activated (new_fpu);prefetch (&new_fpu->state); } }else { - old_fpu->counter =0 ; old_fpu->last_cpu =-1 ;if (fpu.preload) { - new_fpu->counter++;if (fpu_want_lazy_restore (new_fpu, cpu)) fpu.preload =0 ;else
arch/x86/include/asm/fpu/types.h +0 -11
@@ -322,17 +322,6 @@ struct fpu {unsigned char fpregs_active;/* -* @counter: -* -* This counter contains the number of consecutive context switches -* during which the FPU stays used. If this is over a threshold, the -* lazy FPU restore logic becomes eager, to save the trap overhead. -* This is an unsigned char so that after 256 iterations the counter -* wraps and the context switch behavior turns lazy again; this is to -* deal with bursty apps that only use the FPU for a short time: -*/ -unsigned char counter; -/* * @state: * * In-memory copy of all FPU registers that we save/restore
arch/x86/include/asm/trace/fpu.h +1 -4
@@ -14,7 +14,6 @@ DECLARE_EVENT_CLASS(x86_fpu, __field(struct fpu *, fpu) __field(bool, fpregs_active) __field(bool, fpstate_active) - __field(int, counter) __field(u64, xfeatures) __field(u64, xcomp_bv) ),
@@ -23,17 +22,15 @@ DECLARE_EVENT_CLASS(x86_fpu, __entry->fpu = fpu; __entry->fpregs_active = fpu->fpregs_active; __entry->fpstate_active = fpu->fpstate_active; - __entry->counter = fpu->counter;if (boot_cpu_has (X86_FEATURE_OSXSAVE )) { __entry->xfeatures = fpu->state.xsave.header.xfeatures; __entry->xcomp_bv = fpu->state.xsave.header.xcomp_bv; } ), -TP_printk ("x86/fpu: %p fpregs_active: %d fpstate_active: %d counter: %d xfeatures: %llx xcomp_bv: %llx" , +TP_printk ("x86/fpu: %p fpregs_active: %d fpstate_active: %d xfeatures: %llx xcomp_bv: %llx" , __entry->fpu, __entry->fpregs_active, __entry->fpstate_active, - __entry->counter, __entry->xfeatures, __entry->xcomp_bv )
arch/x86/kernel/fpu/core.c +0 -3
@@ -222,7 +222,6 @@ EXPORT_SYMBOL_GPL(fpstate_init);int fpu__copy (struct fpu *dst_fpu,struct fpu *src_fpu) { - dst_fpu->counter =0 ; dst_fpu->fpregs_active =0 ; dst_fpu->last_cpu =-1 ;
@@ -430,7 +429,6 @@ void fpu__restore(struct fpu *fpu)trace_x86_fpu_before_restore (fpu);fpregs_activate (fpu);copy_kernel_to_fpregs (&fpu->state); - fpu->counter++;trace_x86_fpu_after_restore (fpu);kernel_fpu_enable (); }
@@ -448,7 +446,6 @@ EXPORT_SYMBOL_GPL(fpu__restore);void fpu__drop (struct fpu *fpu) {preempt_disable (); - fpu->counter =0 ;if (fpu->fpregs_active) {/* Ignore delayed exceptions from user space */
PATCH 6/9 x86/fpu,kvm: remove kvm vcpu->fpu_counter
In reply to [email protected]From: Rik van Riel <[email protected]>
With the removal of the lazy FPU code, this field is no longer used. Get rid of it.
--- arch/x86/kvm/x86.c | 4 +--- include/linux/kvm_host.h | 1 - 2 files changed, 1 insertion(+), 4 deletions(-)
arch/x86/kvm/x86.c +1 -3
@@ -7348,10 +7348,8 @@ void kvm_load_guest_fpu(struct kvm_vcpu *vcpu)void kvm_put_guest_fpu (struct kvm_vcpu *vcpu) { -if (!vcpu->guest_fpu_loaded) { - vcpu->fpu_counter =0 ; +if (!vcpu->guest_fpu_loaded)return ; - } vcpu->guest_fpu_loaded =0 ;copy_fpregs_to_fpstate (&vcpu->arch.guest_fpu);
include/linux/kvm_host.h +0 -1
@@ -224,7 +224,6 @@ struct kvm_vcpu {int fpu_active;int guest_fpu_loaded, guest_xcr0_loaded; -unsigned char fpu_counter;struct swait_queue_head wq;struct pid *pid;int sigset_active;
Signature · 1 line
2.7.4
tip-bot for Rik van Riel
merge bot[tip:x86/fpu] x86/fpu, kvm: Remove KVM vcpu->fpu_counter
x86/fpu, kvm: Remove KVM vcpu->fpu_counter
Commit-ID: 3d42de25d290fdfe604835d1b389845b8cba5bff Gitweb: http://git.kernel.org/tip/3d42de25d290fdfe604835d1b389845b8cba5bff Author: Rik van Riel <[email protected]> AuthorDate: Tue, 4 Oct 2016 20:34:35 -0400 Committer: Ingo Molnar <[email protected]> CommitDate: Fri, 7 Oct 2016 11:14:41 +0200
x86/fpu, kvm: Remove KVM vcpu->fpu_counter
With the removal of the lazy FPU code, this field is no longer used. Get rid of it.
--- arch/x86/kvm/x86.c | 4 +--- include/linux/kvm_host.h | 1 - 2 files changed, 1 insertion(+), 4 deletions(-)
arch/x86/kvm/x86.c +1 -3
@@ -7348,10 +7348,8 @@ void kvm_load_guest_fpu(struct kvm_vcpu *vcpu)void kvm_put_guest_fpu (struct kvm_vcpu *vcpu) { -if (!vcpu->guest_fpu_loaded) { - vcpu->fpu_counter =0 ; +if (!vcpu->guest_fpu_loaded)return ; - } vcpu->guest_fpu_loaded =0 ;copy_fpregs_to_fpstate (&vcpu->arch.guest_fpu);
include/linux/kvm_host.h +0 -1
@@ -224,7 +224,6 @@ struct kvm_vcpu {int fpu_active;int guest_fpu_loaded, guest_xcr0_loaded; -unsigned char fpu_counter;struct swait_queue_head wq;struct pid *pid;int sigset_active;
PATCH 7/9 x86/fpu: rename lazy restore functions to "register state valid"
In reply to [email protected]From: Rik van Riel <[email protected]>
Name the functions after the state they track, rather than the function they currently enable. This should make it more obvious when we use the fpu_register_state_valid function for something else in the future.
--- arch/x86/include/asm/fpu/internal.h | 26 ++++++++++++++++++++------ arch/x86/kernel/fpu/core.c | 4 ++-- arch/x86/kernel/smpboot.c | 2 +- 3 files changed, 23 insertions(+), 9 deletions(-)
arch/x86/include/asm/fpu/internal.h +20 -6
@@ -479,18 +479,32 @@ extern int copy_fpstate_to_sigframe(void __user *buf, void __user *fp, int size) DECLARE_PER_CPU(struct fpu *, fpu_fpregs_owner_ctx);/* +* The in-register FPU state for an FPU context on a CPU is assumed to be +* valid if the fpu->last_cpu matches the CPU, and the fpu_fpregs_owner_ctx +* matches the FPU. +* +* If the FPU register state is valid, the kernel can skip restoring the +* FPU state from memory. +* +* Any code that clobbers the FPU registers or updates the in-memory +* FPU state for a task MUST let the rest of the kernel know that the +* FPU registers are no longer valid for this task. Calling either of +* these two invalidate functions is enough, use whichever is convenient. +* * Must be run with preemption disabled: this clears the fpu_fpregs_owner_ctx, * on this CPU. -* -* This will disable any lazy FPU state restore of the current FPU state, -* but if the current thread owns the FPU, it will still be saved by. */ -static inline void __cpu_disable_lazy_restore (unsigned int cpu) +static inline void __cpu_invalidate_fpregs_state (unsigned int cpu) {per_cpu (fpu_fpregs_owner_ctx, cpu) =NULL ; } -static inline int fpu_want_lazy_restore (struct fpu *fpu,unsigned int cpu) +static inline void __fpu_invalidate_fpregs_state (struct fpu *fpu) +{ + fpu->last_cpu =-1 ; +} + +static inline int fpregs_state_valid (struct fpu *fpu,unsigned int cpu) {return fpu ==this_cpu_read_stable (fpu_fpregs_owner_ctx) && cpu == fpu->last_cpu; }
@@ -588,7 +602,7 @@ switch_fpu_prepare(struct fpu *old_fpu, struct fpu *new_fpu, int cpu) }else { old_fpu->last_cpu =-1 ;if (fpu.preload) { -if (fpu_want_lazy_restore (new_fpu, cpu)) +if (fpregs_state_valid (new_fpu, cpu)) fpu.preload =0 ;else prefetch (&new_fpu->state);
arch/x86/kernel/fpu/core.c +2 -2
@@ -336,7 +336,7 @@ void fpu__activate_fpstate_write(struct fpu *fpu)if (fpu->fpstate_active) {/* Invalidate any lazy state: */ - fpu->last_cpu =-1 ; +__fpu_invalidate_fpregs_state (fpu); }else {fpstate_init (&fpu->state);trace_x86_fpu_init_state (fpu);
@@ -379,7 +379,7 @@ void fpu__current_fpstate_write_begin(void)* ensures we will not be lazy and skip a XRSTOR in the * future. */ - fpu->last_cpu =-1 ; +__fpu_invalidate_fpregs_state (fpu); }/*
arch/x86/kernel/smpboot.c +1 -1
@@ -1111,7 +1111,7 @@ int native_cpu_up(unsigned int cpu, struct task_struct *tidle)return err;/* the FPU context is blank, nobody can own it */ -__cpu_disable_lazy_restore (cpu); +__cpu_invalidate_fpregs_state (cpu);common_cpu_up (cpu, tidle);
Signature · 1 line
2.7.4
Re: [PATCH 7/9] x86/fpu: rename lazy restore functions to "register state valid"
Quoted 5 lines · On Tue, Oct 4, 2016 at 5:34 PM, <[email protected]> wrote:
From: Rik van Riel <[email protected]>
Name the functions after the state they track, rather than the function they currently enable. This should make it more obvious when we use the
fpu_register_state_validfunction for something else in the future.
I like this.
tip-bot for Rik van Riel
merge bot[tip:x86/fpu] x86/fpu: Rename lazy restore functions to "register state valid"
In reply to [email protected]x86/fpu: Rename lazy restore functions to "register state valid"
Commit-ID: 25d83b531c1aa4fca5b4e24ed10f493268f162bc Gitweb: http://git.kernel.org/tip/25d83b531c1aa4fca5b4e24ed10f493268f162bc Author: Rik van Riel <[email protected]> AuthorDate: Tue, 4 Oct 2016 20:34:36 -0400 Committer: Ingo Molnar <[email protected]> CommitDate: Fri, 7 Oct 2016 11:14:41 +0200
x86/fpu: Rename lazy restore functions to "register state valid"
Name the functions after the state they track, rather than the function they currently enable. This should make it more obvious when we use the fpu_register_state_valid() function for something else in the future.
--- arch/x86/include/asm/fpu/internal.h | 26 ++++++++++++++++++++------ arch/x86/kernel/fpu/core.c | 4 ++-- arch/x86/kernel/smpboot.c | 2 +- 3 files changed, 23 insertions(+), 9 deletions(-)
arch/x86/include/asm/fpu/internal.h +20 -6
@@ -479,18 +479,32 @@ extern int copy_fpstate_to_sigframe(void __user *buf, void __user *fp, int size) DECLARE_PER_CPU(struct fpu *, fpu_fpregs_owner_ctx);/* +* The in-register FPU state for an FPU context on a CPU is assumed to be +* valid if the fpu->last_cpu matches the CPU, and the fpu_fpregs_owner_ctx +* matches the FPU. +* +* If the FPU register state is valid, the kernel can skip restoring the +* FPU state from memory. +* +* Any code that clobbers the FPU registers or updates the in-memory +* FPU state for a task MUST let the rest of the kernel know that the +* FPU registers are no longer valid for this task. Calling either of +* these two invalidate functions is enough, use whichever is convenient. +* * Must be run with preemption disabled: this clears the fpu_fpregs_owner_ctx, * on this CPU. -* -* This will disable any lazy FPU state restore of the current FPU state, -* but if the current thread owns the FPU, it will still be saved by. */ -static inline void __cpu_disable_lazy_restore (unsigned int cpu) +static inline void __cpu_invalidate_fpregs_state (unsigned int cpu) {per_cpu (fpu_fpregs_owner_ctx, cpu) =NULL ; } -static inline int fpu_want_lazy_restore (struct fpu *fpu,unsigned int cpu) +static inline void __fpu_invalidate_fpregs_state (struct fpu *fpu) +{ + fpu->last_cpu =-1 ; +} + +static inline int fpregs_state_valid (struct fpu *fpu,unsigned int cpu) {return fpu ==this_cpu_read_stable (fpu_fpregs_owner_ctx) && cpu == fpu->last_cpu; }
@@ -588,7 +602,7 @@ switch_fpu_prepare(struct fpu *old_fpu, struct fpu *new_fpu, int cpu) }else { old_fpu->last_cpu =-1 ;if (fpu.preload) { -if (fpu_want_lazy_restore (new_fpu, cpu)) +if (fpregs_state_valid (new_fpu, cpu)) fpu.preload =0 ;else prefetch (&new_fpu->state);
arch/x86/kernel/fpu/core.c +2 -2
@@ -336,7 +336,7 @@ void fpu__activate_fpstate_write(struct fpu *fpu)if (fpu->fpstate_active) {/* Invalidate any lazy state: */ - fpu->last_cpu =-1 ; +__fpu_invalidate_fpregs_state (fpu); }else {fpstate_init (&fpu->state);trace_x86_fpu_init_state (fpu);
@@ -379,7 +379,7 @@ void fpu__current_fpstate_write_begin(void)* ensures we will not be lazy and skip a XRSTOR in the * future. */ - fpu->last_cpu =-1 ; +__fpu_invalidate_fpregs_state (fpu); }/*
arch/x86/kernel/smpboot.c +1 -1
@@ -1111,7 +1111,7 @@ int native_cpu_up(unsigned int cpu, struct task_struct *tidle)return err;/* the FPU context is blank, nobody can own it */ -__cpu_disable_lazy_restore (cpu); +__cpu_invalidate_fpregs_state (cpu);common_cpu_up (cpu, tidle);
PATCH 8/9 x86/fpu: remove __fpregs_(de)activate
In reply to [email protected]From: Rik van Riel <[email protected]>
Now that fpregs_activate and fpregs_deactivate do nothing except call the double underscored versions of themselves, we can get rid of the double underscore version.
--- arch/x86/include/asm/fpu/internal.h | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-)
arch/x86/include/asm/fpu/internal.h +7 -18
@@ -509,8 +509,11 @@ static inline int fpregs_state_valid(struct fpu *fpu, unsigned int cpu)return fpu ==this_cpu_read_stable (fpu_fpregs_owner_ctx) && cpu == fpu->last_cpu; } - -static inline void __fpregs_deactivate (struct fpu *fpu) +/* +* These generally need preemption protection to work, +* do try to avoid using these on their own. +*/ +static inline void fpregs_deactivate (struct fpu *fpu) {WARN_ON_FPU (!fpu->fpregs_active);
@@ -519,7 +522,7 @@ static inline void __fpregs_deactivate(struct fpu *fpu)trace_x86_fpu_regs_deactivated (fpu); } -static inline void __fpregs_activate (struct fpu *fpu) +static inline void fpregs_activate (struct fpu *fpu) {WARN_ON_FPU (fpu->fpregs_active);
@@ -544,20 +547,6 @@ static inline int fpregs_active(void) }/* -* These generally need preemption protection to work, -* do try to avoid using these on their own. -*/ -static inline void fpregs_activate (struct fpu *fpu) -{ -__fpregs_activate (fpu); -} - -static inline void fpregs_deactivate (struct fpu *fpu) -{ -__fpregs_deactivate (fpu); -} - -/* * FPU state switching for scheduling. * * This is a two-stage process:
@@ -595,7 +584,7 @@ switch_fpu_prepare(struct fpu *old_fpu, struct fpu *new_fpu, int cpu)/* Don't change CR0.TS if we just switch! */ if (fpu.preload) { -__fpregs_activate (new_fpu); +fpregs_activate (new_fpu);trace_x86_fpu_regs_activated (new_fpu);prefetch (&new_fpu->state); }
Signature · 1 line
2.7.4
Re: [PATCH 8/9] x86/fpu: remove __fpregs_(de)activate
Quoted 5 lines · On Tue, Oct 4, 2016 at 5:34 PM, <[email protected]> wrote:
From: Rik van Riel <[email protected]>
Now that
fpregs_activateandfpregs_deactivatedo nothing except call the double underscored versions of themselves, we can get rid of the double underscore version.
tip-bot for Rik van Riel
merge bot[tip:x86/fpu] x86/fpu: Remove __fpregs_(de)activate()
In reply to [email protected]x86/fpu: Remove __fpregs_(de)activate()
Commit-ID: 66f314efca3843a8874405ab015e354d041f86dd Gitweb: http://git.kernel.org/tip/66f314efca3843a8874405ab015e354d041f86dd Author: Rik van Riel <[email protected]> AuthorDate: Tue, 4 Oct 2016 20:34:37 -0400 Committer: Ingo Molnar <[email protected]> CommitDate: Fri, 7 Oct 2016 11:14:42 +0200
x86/fpu: Remove __fpregs_(de)activate()
Now that fpregs_activate() and fpregs_deactivate() do nothing except call the double underscored versions of themselves, we can get rid of the double underscore version.
--- arch/x86/include/asm/fpu/internal.h | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-)
arch/x86/include/asm/fpu/internal.h +7 -18
@@ -509,8 +509,11 @@ static inline int fpregs_state_valid(struct fpu *fpu, unsigned int cpu)return fpu ==this_cpu_read_stable (fpu_fpregs_owner_ctx) && cpu == fpu->last_cpu; } - -static inline void __fpregs_deactivate (struct fpu *fpu) +/* +* These generally need preemption protection to work, +* do try to avoid using these on their own: +*/ +static inline void fpregs_deactivate (struct fpu *fpu) {WARN_ON_FPU (!fpu->fpregs_active);
@@ -519,7 +522,7 @@ static inline void __fpregs_deactivate(struct fpu *fpu)trace_x86_fpu_regs_deactivated (fpu); } -static inline void __fpregs_activate (struct fpu *fpu) +static inline void fpregs_activate (struct fpu *fpu) {WARN_ON_FPU (fpu->fpregs_active);
@@ -544,20 +547,6 @@ static inline int fpregs_active(void) }/* -* These generally need preemption protection to work, -* do try to avoid using these on their own. -*/ -static inline void fpregs_activate (struct fpu *fpu) -{ -__fpregs_activate (fpu); -} - -static inline void fpregs_deactivate (struct fpu *fpu) -{ -__fpregs_deactivate (fpu); -} - -/* * FPU state switching for scheduling. * * This is a two-stage process:
@@ -595,7 +584,7 @@ switch_fpu_prepare(struct fpu *old_fpu, struct fpu *new_fpu, int cpu)/* Don't change CR0.TS if we just switch! */ if (fpu.preload) { -__fpregs_activate (new_fpu); +fpregs_activate (new_fpu);trace_x86_fpu_regs_activated (new_fpu);prefetch (&new_fpu->state); }
PATCH 9/9 x86/fpu: split old & new fpu code paths
In reply to [email protected]From: Rik van Riel <[email protected]>
Now that CR0.TS is no longer being manipulated, we can simplify switch_fpu_prepare by no longer nesting the handling of the new fpu inside the two branches for the old FPU.
--- arch/x86/include/asm/fpu/internal.h | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-)
arch/x86/include/asm/fpu/internal.h +8 -14
@@ -581,23 +581,17 @@ switch_fpu_prepare(struct fpu *old_fpu, struct fpu *new_fpu, int cpu)/* But leave fpu_fpregs_owner_ctx! */ old_fpu->fpregs_active =0 ;trace_x86_fpu_regs_deactivated (old_fpu); + }else + old_fpu->last_cpu =-1 ; -/* Don't change CR0.TS if we just switch! */ -if (fpu.preload) { -fpregs_activate (new_fpu); -trace_x86_fpu_regs_activated (new_fpu); +if (fpu.preload) { +if (fpregs_state_valid (new_fpu, cpu)) + fpu.preload =0 ; +else prefetch (&new_fpu->state); - } - }else { - old_fpu->last_cpu =-1 ; -if (fpu.preload) { -if (fpregs_state_valid (new_fpu, cpu)) - fpu.preload =0 ; -else -prefetch (&new_fpu->state); -fpregs_activate (new_fpu); - } +fpregs_activate (new_fpu); } +return fpu; }
Signature · 1 line
2.7.4
tip-bot for Rik van Riel
merge bot[tip:x86/fpu] x86/fpu: Split old & new FPU code paths
x86/fpu: Split old & new FPU code paths
Commit-ID: 9ad93fe35aff616fca4e2b9581fdeed498605f9e Gitweb: http://git.kernel.org/tip/9ad93fe35aff616fca4e2b9581fdeed498605f9e Author: Rik van Riel <[email protected]> AuthorDate: Tue, 4 Oct 2016 20:34:38 -0400 Committer: Ingo Molnar <[email protected]> CommitDate: Fri, 7 Oct 2016 11:14:43 +0200
x86/fpu: Split old & new FPU code paths
Now that CR0.TS is no longer being manipulated, we can simplify switch_fpu_prepare() by no longer nesting the handling of new_fpu inside the two branches for the old_fpu.
--- arch/x86/include/asm/fpu/internal.h | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-)
arch/x86/include/asm/fpu/internal.h +8 -14
@@ -581,23 +581,17 @@ switch_fpu_prepare(struct fpu *old_fpu, struct fpu *new_fpu, int cpu)/* But leave fpu_fpregs_owner_ctx! */ old_fpu->fpregs_active =0 ;trace_x86_fpu_regs_deactivated (old_fpu); + }else + old_fpu->last_cpu =-1 ; -/* Don't change CR0.TS if we just switch! */ -if (fpu.preload) { -fpregs_activate (new_fpu); -trace_x86_fpu_regs_activated (new_fpu); +if (fpu.preload) { +if (fpregs_state_valid (new_fpu, cpu)) + fpu.preload =0 ; +else prefetch (&new_fpu->state); - } - }else { - old_fpu->last_cpu =-1 ; -if (fpu.preload) { -if (fpregs_state_valid (new_fpu, cpu)) - fpu.preload =0 ; -else -prefetch (&new_fpu->state); -fpregs_activate (new_fpu); - } +fpregs_activate (new_fpu); } +return fpu; }