* [patch 0/2] x86/fpu: Make fpu_lock() RT compliant
@ 2020-10-27 10:09 Thomas Gleixner
2020-10-27 10:09 ` [patch 1/2] x86/fpu: Simplify fpregs_[un]lock() Thomas Gleixner
2020-10-27 10:09 ` [patch 2/2] x86/fpu: Make kernel FPU protection RT friendly Thomas Gleixner
0 siblings, 2 replies; 6+ messages in thread
From: Thomas Gleixner @ 2020-10-27 10:09 UTC (permalink / raw)
To: LKML; +Cc: x86, Sebastian Andrzej Siewior
FPU serialization uses
preempt_disable(); local_bh_disable();
which is silly because local_bh_disable() implicitly disables
preemption. So this can be reduced to local_bh_disable().
But this does not work on RT kernels because local_bh_disable() only
serializes bottom half related processing via a local lock, but does not
disable preemption.
On RT kernels preempt_disable() is the right choice because on RT bottom
half processing is always in thread context, so preempt_disable()
implicitly protects against bottom half processing there.
Thanks,
tglx
^ permalink raw reply [flat|nested] 6+ messages in thread
* [patch 1/2] x86/fpu: Simplify fpregs_[un]lock()
2020-10-27 10:09 [patch 0/2] x86/fpu: Make fpu_lock() RT compliant Thomas Gleixner
@ 2020-10-27 10:09 ` Thomas Gleixner
2020-11-11 13:36 ` [tip: x86/fpu] " tip-bot2 for Thomas Gleixner
2020-10-27 10:09 ` [patch 2/2] x86/fpu: Make kernel FPU protection RT friendly Thomas Gleixner
1 sibling, 1 reply; 6+ messages in thread
From: Thomas Gleixner @ 2020-10-27 10:09 UTC (permalink / raw)
To: LKML; +Cc: x86, Sebastian Andrzej Siewior
There is no point in disabling preemption and then disabling bottom
halfs.
Just disabling bottom halfs is sufficient as it implicitly disables
preemption on !RT kernels.
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
arch/x86/include/asm/fpu/api.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
--- a/arch/x86/include/asm/fpu/api.h
+++ b/arch/x86/include/asm/fpu/api.h
@@ -29,17 +29,18 @@ extern void fpregs_mark_activate(void);
* A context switch will (and softirq might) save CPU's FPU registers to
* fpu->state and set TIF_NEED_FPU_LOAD leaving CPU's FPU registers in
* a random state.
+ *
+ * local_bh_disable() protects against both preemption and soft interrupts
+ * on !RT kernels.
*/
static inline void fpregs_lock(void)
{
- preempt_disable();
local_bh_disable();
}
static inline void fpregs_unlock(void)
{
local_bh_enable();
- preempt_enable();
}
#ifdef CONFIG_X86_DEBUG_FPU
^ permalink raw reply [flat|nested] 6+ messages in thread
* [patch 2/2] x86/fpu: Make kernel FPU protection RT friendly
2020-10-27 10:09 [patch 0/2] x86/fpu: Make fpu_lock() RT compliant Thomas Gleixner
2020-10-27 10:09 ` [patch 1/2] x86/fpu: Simplify fpregs_[un]lock() Thomas Gleixner
@ 2020-10-27 10:09 ` Thomas Gleixner
2020-10-27 10:42 ` Sebastian Andrzej Siewior
2020-11-11 13:36 ` [tip: x86/fpu] " tip-bot2 for Thomas Gleixner
1 sibling, 2 replies; 6+ messages in thread
From: Thomas Gleixner @ 2020-10-27 10:09 UTC (permalink / raw)
To: LKML; +Cc: x86, Sebastian Andrzej Siewior
Non RT kernels need to protect FPU against preemption and bottom half
processing. This is achieved by disabling bottom halfs via
local_bh_disable() which implictly disables preemption.
On RT kernels this protection mechanism is not sufficient because
local_bh_disable() does not disable preemption. It serializes bottom half
related processing via a CPU local lock.
As bottom halfs are running always in thread context on RT kernels
disabling preemption is the proper choice as it implicitly prevents bottom
half processing.
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
arch/x86/include/asm/fpu/api.h | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
--- a/arch/x86/include/asm/fpu/api.h
+++ b/arch/x86/include/asm/fpu/api.h
@@ -32,15 +32,27 @@ extern void fpregs_mark_activate(void);
*
* local_bh_disable() protects against both preemption and soft interrupts
* on !RT kernels.
+ *
+ * On RT kernels local_bh_disable() is not sufficient because it only
+ * serializes soft interrupt related sections via a local lock, but stays
+ * preemptible. Disabling preemption is the right choice here as bottom
+ * half processing is always in thread context on RT kernels so it
+ * implicitly prevents bottom half processing as well.
*/
static inline void fpregs_lock(void)
{
- local_bh_disable();
+ if (!IS_ENABLED(CONFIG_PREEMPT_RT))
+ local_bh_disable();
+ else
+ preempt_disable();
}
static inline void fpregs_unlock(void)
{
- local_bh_enable();
+ if (!IS_ENABLED(CONFIG_PREEMPT_RT))
+ local_bh_enable();
+ else
+ preempt_enable();
}
#ifdef CONFIG_X86_DEBUG_FPU
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch 2/2] x86/fpu: Make kernel FPU protection RT friendly
2020-10-27 10:09 ` [patch 2/2] x86/fpu: Make kernel FPU protection RT friendly Thomas Gleixner
@ 2020-10-27 10:42 ` Sebastian Andrzej Siewior
2020-11-11 13:36 ` [tip: x86/fpu] " tip-bot2 for Thomas Gleixner
1 sibling, 0 replies; 6+ messages in thread
From: Sebastian Andrzej Siewior @ 2020-10-27 10:42 UTC (permalink / raw)
To: Thomas Gleixner; +Cc: LKML, x86
On 2020-10-27 11:09:51 [+0100], Thomas Gleixner wrote:
> --- a/arch/x86/include/asm/fpu/api.h
> +++ b/arch/x86/include/asm/fpu/api.h
> @@ -32,15 +32,27 @@ extern void fpregs_mark_activate(void);
> *
> * local_bh_disable() protects against both preemption and soft interrupts
> * on !RT kernels.
> + *
> + * On RT kernels local_bh_disable() is not sufficient because it only
> + * serializes soft interrupt related sections via a local lock, but stays
> + * preemptible. Disabling preemption is the right choice here as bottom
> + * half processing is always in thread context on RT kernels so it
> + * implicitly prevents bottom half processing as well.
The important part is that kernel_fpu_begin() also disables preemption
and it may run in softirq. It does not use fpregs_lock() and
fpregs_lock() serializes against kernel_fpu_begin().
> */
> static inline void fpregs_lock(void)
> {
> - local_bh_disable();
> + if (!IS_ENABLED(CONFIG_PREEMPT_RT))
> + local_bh_disable();
> + else
> + preempt_disable();
Could you please swap that to
if (IS_ENABLED(CONFIG_PREEMPT_RT))
preempt_disable();
else
local_bh_disable();
> }
>
Sebastian
^ permalink raw reply [flat|nested] 6+ messages in thread
* [tip: x86/fpu] x86/fpu: Make kernel FPU protection RT friendly
2020-10-27 10:09 ` [patch 2/2] x86/fpu: Make kernel FPU protection RT friendly Thomas Gleixner
2020-10-27 10:42 ` Sebastian Andrzej Siewior
@ 2020-11-11 13:36 ` tip-bot2 for Thomas Gleixner
1 sibling, 0 replies; 6+ messages in thread
From: tip-bot2 for Thomas Gleixner @ 2020-11-11 13:36 UTC (permalink / raw)
To: linux-tip-commits; +Cc: Thomas Gleixner, x86, linux-kernel
The following commit has been merged into the x86/fpu branch of tip:
Commit-ID: cba08c5dc6dc1a906a0b5ddac9a9ac6c9a64f2e8
Gitweb: https://git.kernel.org/tip/cba08c5dc6dc1a906a0b5ddac9a9ac6c9a64f2e8
Author: Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Tue, 27 Oct 2020 11:09:51 +01:00
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Wed, 11 Nov 2020 14:35:16 +01:00
x86/fpu: Make kernel FPU protection RT friendly
Non RT kernels need to protect FPU against preemption and bottom half
processing. This is achieved by disabling bottom halfs via
local_bh_disable() which implictly disables preemption.
On RT kernels this protection mechanism is not sufficient because
local_bh_disable() does not disable preemption. It serializes bottom half
related processing via a CPU local lock.
As bottom halfs are running always in thread context on RT kernels
disabling preemption is the proper choice as it implicitly prevents bottom
half processing.
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lore.kernel.org/r/20201027101349.588965083@linutronix.de
---
arch/x86/include/asm/fpu/api.h | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/arch/x86/include/asm/fpu/api.h b/arch/x86/include/asm/fpu/api.h
index 2c5bef7..a5aba4a 100644
--- a/arch/x86/include/asm/fpu/api.h
+++ b/arch/x86/include/asm/fpu/api.h
@@ -32,15 +32,29 @@ extern void fpregs_mark_activate(void);
*
* local_bh_disable() protects against both preemption and soft interrupts
* on !RT kernels.
+ *
+ * On RT kernels local_bh_disable() is not sufficient because it only
+ * serializes soft interrupt related sections via a local lock, but stays
+ * preemptible. Disabling preemption is the right choice here as bottom
+ * half processing is always in thread context on RT kernels so it
+ * implicitly prevents bottom half processing as well.
+ *
+ * Disabling preemption also serializes against kernel_fpu_begin().
*/
static inline void fpregs_lock(void)
{
- local_bh_disable();
+ if (!IS_ENABLED(CONFIG_PREEMPT_RT))
+ local_bh_disable();
+ else
+ preempt_disable();
}
static inline void fpregs_unlock(void)
{
- local_bh_enable();
+ if (!IS_ENABLED(CONFIG_PREEMPT_RT))
+ local_bh_enable();
+ else
+ preempt_enable();
}
#ifdef CONFIG_X86_DEBUG_FPU
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [tip: x86/fpu] x86/fpu: Simplify fpregs_[un]lock()
2020-10-27 10:09 ` [patch 1/2] x86/fpu: Simplify fpregs_[un]lock() Thomas Gleixner
@ 2020-11-11 13:36 ` tip-bot2 for Thomas Gleixner
0 siblings, 0 replies; 6+ messages in thread
From: tip-bot2 for Thomas Gleixner @ 2020-11-11 13:36 UTC (permalink / raw)
To: linux-tip-commits; +Cc: Thomas Gleixner, x86, linux-kernel
The following commit has been merged into the x86/fpu branch of tip:
Commit-ID: 5f0c71278d6848b4809f83af90f28196e1505ab1
Gitweb: https://git.kernel.org/tip/5f0c71278d6848b4809f83af90f28196e1505ab1
Author: Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Tue, 27 Oct 2020 11:09:50 +01:00
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Wed, 11 Nov 2020 14:35:16 +01:00
x86/fpu: Simplify fpregs_[un]lock()
There is no point in disabling preemption and then disabling bottom
halfs.
Just disabling bottom halfs is sufficient as it implicitly disables
preemption on !RT kernels.
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lore.kernel.org/r/20201027101349.455380473@linutronix.de
---
arch/x86/include/asm/fpu/api.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/x86/include/asm/fpu/api.h b/arch/x86/include/asm/fpu/api.h
index dcd9503..2c5bef7 100644
--- a/arch/x86/include/asm/fpu/api.h
+++ b/arch/x86/include/asm/fpu/api.h
@@ -29,17 +29,18 @@ extern void fpregs_mark_activate(void);
* A context switch will (and softirq might) save CPU's FPU registers to
* fpu->state and set TIF_NEED_FPU_LOAD leaving CPU's FPU registers in
* a random state.
+ *
+ * local_bh_disable() protects against both preemption and soft interrupts
+ * on !RT kernels.
*/
static inline void fpregs_lock(void)
{
- preempt_disable();
local_bh_disable();
}
static inline void fpregs_unlock(void)
{
local_bh_enable();
- preempt_enable();
}
#ifdef CONFIG_X86_DEBUG_FPU
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-11-11 13:36 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-27 10:09 [patch 0/2] x86/fpu: Make fpu_lock() RT compliant Thomas Gleixner
2020-10-27 10:09 ` [patch 1/2] x86/fpu: Simplify fpregs_[un]lock() Thomas Gleixner
2020-11-11 13:36 ` [tip: x86/fpu] " tip-bot2 for Thomas Gleixner
2020-10-27 10:09 ` [patch 2/2] x86/fpu: Make kernel FPU protection RT friendly Thomas Gleixner
2020-10-27 10:42 ` Sebastian Andrzej Siewior
2020-11-11 13:36 ` [tip: x86/fpu] " tip-bot2 for Thomas Gleixner
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.