* [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 [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 [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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®