[PATCH] Loongarch:Prevent cond_resched occurring within kernel-fpu

Tianyang Zhang posted 1 patch 7 months, 1 week ago
arch/loongarch/kernel/kfpu.c | 22 ++++++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)
[PATCH] Loongarch:Prevent cond_resched occurring within kernel-fpu
Posted by Tianyang Zhang 7 months, 1 week ago
When CONFIG_PREEMPT_COUNT is not configured, preempt_disable/preempt_enable
merely acts as a barrier. However, cond_resched can still trigger a
context switch and modify the EUEN, resulting in do_fpu being activated
within kernel-fpu critical section, as demonstrated in the following path:

dcn32_calculate_wm_and_dlg
    DC_FP_START
	dcn32_calculate_wm_and_dlg_fpu
	    dcn32_find_dummy_latency_index_for_fw_based_mclk_switch
		dcn32_internal_validate_bw
		    dcn32_enable_phantom_stream
			dc_create_stream_for_sink
			   kzalloc(GFP_KERNEL)
				__kmem_cache_alloc_node
				    __cond_resched
    DC_FP_END

This patch is similar to d02198550423a0b695e7a24ec77153209ad45b09
(x86/fpu: Improve crypto performance by making kernel-mode FPU reliably
usablein softirqs),to avoids the issue, and extends kernel-fpu application
scenarios to softirq.

Signed-off-by: Tianyang Zhang <zhangtianyang@loongson.cn>
---
 arch/loongarch/kernel/kfpu.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/arch/loongarch/kernel/kfpu.c b/arch/loongarch/kernel/kfpu.c
index ec5b28e570c9..e60bbaca357a 100644
--- a/arch/loongarch/kernel/kfpu.c
+++ b/arch/loongarch/kernel/kfpu.c
@@ -18,11 +18,28 @@ static unsigned int euen_mask = CSR_EUEN_FPEN;
 static DEFINE_PER_CPU(bool, in_kernel_fpu);
 static DEFINE_PER_CPU(unsigned int, euen_current);
 
+static inline void fpu_lock(void)
+{
+	if (!IS_ENABLED(CONFIG_PREEMPT_RT))
+		local_bh_disable();
+	else
+		preempt_disable();
+}
+
+static inline void fpu_unlock(void)
+{
+	if (!IS_ENABLED(CONFIG_PREEMPT_RT))
+		local_bh_enable();
+	else
+		preempt_enable();
+}
+
 void kernel_fpu_begin(void)
 {
 	unsigned int *euen_curr;
 
-	preempt_disable();
+	if (!irqs_disabled())
+		fpu_lock();
 
 	WARN_ON(this_cpu_read(in_kernel_fpu));
 
@@ -73,7 +90,8 @@ void kernel_fpu_end(void)
 
 	this_cpu_write(in_kernel_fpu, false);
 
-	preempt_enable();
+	if (!irqs_disabled())
+		fpu_unlock();
 }
 EXPORT_SYMBOL_GPL(kernel_fpu_end);
 
-- 
2.20.1
Re: [PATCH] Loongarch:Prevent cond_resched occurring within kernel-fpu
Posted by Huacai Chen 7 months, 1 week ago
Hi, Tianyang,

LoongArch is not Loongarch in the title.

On Wed, May 7, 2025 at 6:06 PM Tianyang Zhang <zhangtianyang@loongson.cn> wrote:
>
> When CONFIG_PREEMPT_COUNT is not configured, preempt_disable/preempt_enable
> merely acts as a barrier. However, cond_resched can still trigger a
> context switch and modify the EUEN, resulting in do_fpu being activated
> within kernel-fpu critical section, as demonstrated in the following path:
>
> dcn32_calculate_wm_and_dlg
>     DC_FP_START
>         dcn32_calculate_wm_and_dlg_fpu
>             dcn32_find_dummy_latency_index_for_fw_based_mclk_switch
>                 dcn32_internal_validate_bw
>                     dcn32_enable_phantom_stream
>                         dc_create_stream_for_sink
>                            kzalloc(GFP_KERNEL)
>                                 __kmem_cache_alloc_node
>                                     __cond_resched
>     DC_FP_END
>
> This patch is similar to d02198550423a0b695e7a24ec77153209ad45b09
> (x86/fpu: Improve crypto performance by making kernel-mode FPU reliably
> usablein softirqs),to avoids the issue, and extends kernel-fpu application
> scenarios to softirq.
>

You need to Cc the stable list here.

> Signed-off-by: Tianyang Zhang <zhangtianyang@loongson.cn>
> ---
>  arch/loongarch/kernel/kfpu.c | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/arch/loongarch/kernel/kfpu.c b/arch/loongarch/kernel/kfpu.c
> index ec5b28e570c9..e60bbaca357a 100644
> --- a/arch/loongarch/kernel/kfpu.c
> +++ b/arch/loongarch/kernel/kfpu.c
> @@ -18,11 +18,28 @@ static unsigned int euen_mask = CSR_EUEN_FPEN;
>  static DEFINE_PER_CPU(bool, in_kernel_fpu);
>  static DEFINE_PER_CPU(unsigned int, euen_current);
>
> +static inline void fpu_lock(void)
The x86 naming, fpregs_lock() is better than fpu_lock(), we can keep the same.

To save time, I have fixed them [1], you can fetch it to send V2.
[1] https://github.com/chenhuacai/linux/commit/fa0952bb7667b138a8941bb3e86775adc9f4731f

Huacai

> +{
> +       if (!IS_ENABLED(CONFIG_PREEMPT_RT))
> +               local_bh_disable();
> +       else
> +               preempt_disable();
> +}
> +
> +static inline void fpu_unlock(void)
> +{
> +       if (!IS_ENABLED(CONFIG_PREEMPT_RT))
> +               local_bh_enable();
> +       else
> +               preempt_enable();
> +}
> +
>  void kernel_fpu_begin(void)
>  {
>         unsigned int *euen_curr;
>
> -       preempt_disable();
> +       if (!irqs_disabled())
> +               fpu_lock();
>
>         WARN_ON(this_cpu_read(in_kernel_fpu));
>
> @@ -73,7 +90,8 @@ void kernel_fpu_end(void)
>
>         this_cpu_write(in_kernel_fpu, false);
>
> -       preempt_enable();
> +       if (!irqs_disabled())
> +               fpu_unlock();
>  }
>  EXPORT_SYMBOL_GPL(kernel_fpu_end);
>
> --
> 2.20.1
>