[PATCH] x86/fpu: Unify the naming of the FPU register cache validity flags
From: Ingo Molnar
Date: Thu Jan 26 2017 - 10:01:00 EST
Rik pointed out that the fpu_fpregs_owner_ctx and the ->fpregs_cached
flags are still described in a confusing way.
Clarify this some more by renaming them to:
->fpregs_cached => ->fpregs_owner [bool]
fpu_fpregs_owner_ctx => fpregs_owner_ctx [ptr]
... which better expresses that they are a cache validity flag
split into two parts, where the cache can be invalidated if any
of the flags is cleared.
Also describe this relationship more accurately in the fpu/types.h header.
No change in functionality.
Reported-by: Rik van Riel <riel@xxxxxxxxxx>
Cc: Andy Lutomirski <luto@xxxxxxxxxx>
Cc: Borislav Petkov <bp@xxxxxxxxx>
Cc: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
Cc: Fenghua Yu <fenghua.yu@xxxxxxxxx>
Cc: H. Peter Anvin <hpa@xxxxxxxxx>
Cc: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
Cc: Oleg Nesterov <oleg@xxxxxxxxxx>
Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
Cc: Yu-cheng Yu <yu-cheng.yu@xxxxxxxxx>
Cc: Fenghua Yu <fenghua.yu@xxxxxxxxx>
Signed-off-by: Ingo Molnar <mingo@xxxxxxxxxx>
---
arch/x86/include/asm/fpu/internal.h | 22 +++++++++++-----------
arch/x86/include/asm/fpu/types.h | 12 ++++++++++--
arch/x86/include/asm/switch_to.h | 2 +-
arch/x86/kernel/fpu/core.c | 4 ++--
arch/x86/kernel/smpboot.c | 2 +-
5 files changed, 25 insertions(+), 17 deletions(-)
diff --git a/arch/x86/include/asm/fpu/internal.h b/arch/x86/include/asm/fpu/internal.h
index e62eee2e989e..bbee00aac864 100644
--- a/arch/x86/include/asm/fpu/internal.h
+++ b/arch/x86/include/asm/fpu/internal.h
@@ -486,11 +486,11 @@ extern int copy_fpstate_to_sigframe(void __user *buf, void __user *fp, int size)
* FPU context switch related helper methods:
*/
-DECLARE_PER_CPU(struct fpu *, fpu_fpregs_owner_ctx);
+DECLARE_PER_CPU(struct fpu *, fpregs_owner_ctx);
/*
* The in-register FPU state for an FPU context on a CPU is assumed to be
- * valid if fpu->fpregs_cached is still set, and if the fpu_fpregs_owner_ctx
+ * valid if fpu->fpregs_owner is still set, and if the fpregs_owner_ctx
* matches the FPU.
*
* If the FPU register state is valid, the kernel can skip restoring the
@@ -507,17 +507,17 @@ DECLARE_PER_CPU(struct fpu *, fpu_fpregs_owner_ctx);
*/
static inline void __cpu_invalidate_fpregs_state(void)
{
- __this_cpu_write(fpu_fpregs_owner_ctx, NULL);
+ __this_cpu_write(fpregs_owner_ctx, NULL);
}
static inline void __fpu_invalidate_fpregs_state(struct fpu *fpu)
{
- fpu->fpregs_cached = 0;
+ fpu->fpregs_owner = 0;
}
static inline int fpregs_state_valid(struct fpu *fpu, unsigned int cpu)
{
- return fpu == this_cpu_read_stable(fpu_fpregs_owner_ctx) && fpu->fpregs_cached;
+ return fpu == this_cpu_read_stable(fpregs_owner_ctx) && fpu->fpregs_owner;
}
/*
@@ -526,13 +526,13 @@ static inline int fpregs_state_valid(struct fpu *fpu, unsigned int cpu)
*/
static inline void fpregs_deactivate(struct fpu *fpu)
{
- this_cpu_write(fpu_fpregs_owner_ctx, NULL);
+ this_cpu_write(fpregs_owner_ctx, NULL);
trace_x86_fpu_regs_deactivated(fpu);
}
static inline void fpregs_activate(struct fpu *fpu)
{
- this_cpu_write(fpu_fpregs_owner_ctx, fpu);
+ this_cpu_write(fpregs_owner_ctx, fpu);
trace_x86_fpu_regs_activated(fpu);
}
@@ -552,14 +552,14 @@ switch_fpu_prepare(struct fpu *old_fpu, int cpu)
{
if (old_fpu->fpstate_active) {
if (!copy_fpregs_to_fpstate(old_fpu))
- old_fpu->fpregs_cached = 0;
+ old_fpu->fpregs_owner = 0;
else
- old_fpu->fpregs_cached = 1;
+ old_fpu->fpregs_owner = 1;
- /* But leave fpu_fpregs_owner_ctx! */
+ /* But leave fpregs_owner_ctx! */
trace_x86_fpu_regs_deactivated(old_fpu);
} else {
- old_fpu->fpregs_cached = 0;
+ old_fpu->fpregs_owner = 0;
}
}
diff --git a/arch/x86/include/asm/fpu/types.h b/arch/x86/include/asm/fpu/types.h
index 07452fbd7867..d15cbfe0e8c4 100644
--- a/arch/x86/include/asm/fpu/types.h
+++ b/arch/x86/include/asm/fpu/types.h
@@ -285,14 +285,22 @@ struct fpu {
unsigned char fpstate_active;
/*
- * @fpregs_cached:
+ * @fpregs_owner:
*
* This flag tells us whether this context is loaded into a CPU
* right now.
*
* This is set to 0 if a task is migrated to another CPU.
+ *
+ * NOTE: the fpregs_owner_ctx percpu pointer also has to point to
+ * this FPU context for the register cache to be valid. If any
+ * of these two flags is cleared then the cache is invalid.
+ * Some internals can access the context-flag more easily,
+ * others have easier access to the percpu variable. The
+ * FPU context-switching code has access to both so there's
+ * very little cost of having the cache indexed in two ways:
*/
- unsigned char fpregs_cached;
+ unsigned char fpregs_owner;
/*
* @state:
diff --git a/arch/x86/include/asm/switch_to.h b/arch/x86/include/asm/switch_to.h
index a7146dadb31d..7a4915dd0547 100644
--- a/arch/x86/include/asm/switch_to.h
+++ b/arch/x86/include/asm/switch_to.h
@@ -78,7 +78,7 @@ do { \
*/
static inline void arch_task_migrate(struct task_struct *p)
{
- p->thread.fpu.fpregs_cached = 0;
+ p->thread.fpu.fpregs_owner = 0;
}
#define arch_task_migrate arch_task_migrate
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index 217e37029585..1b3bf98072fe 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -39,7 +39,7 @@ static DEFINE_PER_CPU(bool, in_kernel_fpu);
/*
* Track which context is using the FPU on the CPU:
*/
-DEFINE_PER_CPU(struct fpu *, fpu_fpregs_owner_ctx);
+DEFINE_PER_CPU(struct fpu *, fpregs_owner_ctx);
static void kernel_fpu_disable(void)
{
@@ -189,7 +189,7 @@ EXPORT_SYMBOL_GPL(fpstate_init);
int fpu__copy(struct fpu *dst_fpu, struct fpu *src_fpu)
{
- dst_fpu->fpregs_cached = 0;
+ dst_fpu->fpregs_owner = 0;
if (!src_fpu->fpstate_active || !static_cpu_has(X86_FEATURE_FPU))
return 0;
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 46732dc3b73c..8bf24aa01878 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1118,7 +1118,7 @@ int native_cpu_up(unsigned int cpu, struct task_struct *tidle)
return err;
/* the FPU context is blank, nobody can own it */
- per_cpu(fpu_fpregs_owner_ctx, cpu) = NULL;
+ per_cpu(fpregs_owner_ctx, cpu) = NULL;
common_cpu_up(cpu, tidle);