Re: [PATCH] arm64/sve: Delay freeing memory in fpsimd_flush_thread()

From: Dave Martin <Dave.Martin@arm.com>
To: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: linux-arm-kernel@lists.infradead.org,
	Mark Brown <broonie@kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [PATCH] arm64/sve: Delay freeing memory in fpsimd_flush_thread()
Date: Thu, 29 Jul 2021 14:58:20 +0100	[thread overview]
Message-ID: <20210729135819.GM1724@arm.com> (raw)
In-Reply-To: <20210729105215.2222338-2-bigeasy@linutronix.de>

On Thu, Jul 29, 2021 at 12:52:14PM +0200, Sebastian Andrzej Siewior wrote:
> fpsimd_flush_thread() invokes kfree() via sve_free() within a preempt disabled
> section which is not working on -RT.
> 
> Delay freeing of memory until preemption is enabled again.

I'll leave this for Mark Brown to comment on, but it looks reasonable to
me.

Cheers
---Dave

> 
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>  arch/arm64/kernel/fpsimd.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
> index e57b23f952846..e098f6c67b1de 100644
> --- a/arch/arm64/kernel/fpsimd.c
> +++ b/arch/arm64/kernel/fpsimd.c
> @@ -226,6 +226,16 @@ static void sve_free(struct task_struct *task)
>  	__sve_free(task);
>  }
>  
> +static void *sve_free_atomic(struct task_struct *task)
> +{
> +	void *sve_state = task->thread.sve_state;
> +
> +	WARN_ON(test_tsk_thread_flag(task, TIF_SVE));
> +
> +	task->thread.sve_state = NULL;
> +	return sve_state;
> +}
> +
>  /*
>   * TIF_SVE controls whether a task can use SVE without trapping while
>   * in userspace, and also the way a task's FPSIMD/SVE state is stored
> @@ -1033,6 +1043,7 @@ void fpsimd_thread_switch(struct task_struct *next)
>  void fpsimd_flush_thread(void)
>  {
>  	int vl, supported_vl;
> +	void *mem = NULL;
>  
>  	if (!system_supports_fpsimd())
>  		return;
> @@ -1045,7 +1056,7 @@ void fpsimd_flush_thread(void)
>  
>  	if (system_supports_sve()) {
>  		clear_thread_flag(TIF_SVE);
> -		sve_free(current);
> +		mem = sve_free_atomic(current);
>  
>  		/*
>  		 * Reset the task vector length as required.
> @@ -1079,6 +1090,7 @@ void fpsimd_flush_thread(void)
>  	}
>  
>  	put_cpu_fpsimd_context();
> +	kfree(mem);
>  }
>  
>  /*
> -- 
> 2.32.0
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel