Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1567501
| From | Ingo Molnar <mingo@kernel.org> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | [PATCH] x86/fpu: Unify the naming of the FPU register cache validity flags |
| Date | 2017-01-26 16:10 +0100 |
| Message-ID | <t3TQu-uN-19@gated-at.bofh.it> (permalink) |
| References | <t3QpA-6Hl-9@gated-at.bofh.it> <t3QpB-6Hl-41@gated-at.bofh.it> <t3TdM-8u9-23@gated-at.bofh.it> <t3TGP-cu-25@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
* Ingo Molnar <mingo@kernel.org> wrote:
> What we could do is to unify the naming to explain all this a bit better - right
> now there's very little indication that ->fpregs_cached is closely related to
> fpu_fpregs_owner_ctx.
>
> For example we could rename them to:
>
> ->fpregs_cached => ->fpregs_owner [bool]
> fpu_fpregs_owner_ctx => fpregs_owner_ctx [ptr]
>
> ?
>
> Clearing ->fpregs_owner or setting fpregs_owner_ctx to NULL invalidates the
> cache and it's clear from the naming that the two values are closely related.
Something like the patch below - only minimally tested.
Thanks,
Ingo
================>
From d5b99e1e25f86d4880bf85588eb4a4769610dd47 Mon Sep 17 00:00:00 2001
From: Ingo Molnar <mingo@kernel.org>
Date: Thu, 26 Jan 2017 16:01:00 +0100
Subject: [PATCH] x86/fpu: Unify the naming of the FPU register cache validity flags
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@redhat.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Yu-cheng Yu <yu-cheng.yu@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
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);
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH 0/7] x86/fpu: Simplify the FPU state machine Ingo Molnar <mingo@kernel.org> - 2017-01-26 12:30 +0100
[PATCH 4/7] x86/fpu: Split the state handling in fpu__drop() Ingo Molnar <mingo@kernel.org> - 2017-01-26 12:30 +0100
[PATCH 7/7] x86/fpu: Remove struct fpu::fpregs_active Ingo Molnar <mingo@kernel.org> - 2017-01-26 12:30 +0100
[PATCH 2/7] x86/fpu: Simplify fpu->fpregs_active use Ingo Molnar <mingo@kernel.org> - 2017-01-26 12:30 +0100
Re: [PATCH 2/7] x86/fpu: Simplify fpu->fpregs_active use Andy Lutomirski <luto@amacapital.net> - 2017-01-26 17:40 +0100
[PATCH 1/7] x86/fpu: Simplify the fpu->last_cpu logic and rename it to fpu->fpregs_cached Ingo Molnar <mingo@kernel.org> - 2017-01-26 12:30 +0100
Re: [PATCH 1/7] x86/fpu: Simplify the fpu->last_cpu logic and rename it to fpu->fpregs_cached Rik van Riel <riel@redhat.com> - 2017-01-26 15:30 +0100
Re: [PATCH 1/7] x86/fpu: Simplify the fpu->last_cpu logic and rename it to fpu->fpregs_cached Ingo Molnar <mingo@kernel.org> - 2017-01-26 16:00 +0100
[PATCH] x86/fpu: Unify the naming of the FPU register cache validity flags Ingo Molnar <mingo@kernel.org> - 2017-01-26 16:10 +0100
Re: [PATCH] x86/fpu: Unify the naming of the FPU register cache validity flags Peter Zijlstra <peterz@infradead.org> - 2017-01-26 16:40 +0100
Re: [PATCH 1/7] x86/fpu: Simplify the fpu->last_cpu logic and rename it to fpu->fpregs_cached Rik van Riel <riel@redhat.com> - 2017-01-26 16:00 +0100
Re: [PATCH 1/7] x86/fpu: Simplify the fpu->last_cpu logic and rename it to fpu->fpregs_cached Ingo Molnar <mingo@kernel.org> - 2017-01-26 16:20 +0100
Re: [PATCH 1/7] x86/fpu: Simplify the fpu->last_cpu logic and rename it to fpu->fpregs_cached Andy Lutomirski <luto@amacapital.net> - 2017-01-26 18:00 +0100
csiph-web