Hi,
On Tue, Dec 03, 2024 at 04:12:33PM +0000, Mark Brown wrote:
On Tue, Dec 03, 2024 at 03:33:18PM +0000, Dave Martin wrote:
On Tue, Dec 03, 2024 at 12:45:57PM +0000, Mark Brown wrote:
- get_cpu_fpsimd_context();
if (current->thread.svcr & SVCR_SM_MASK) {memset(¤t->thread.uw.fpsimd_state.vregs, 0,sizeof(current->thread.uw.fpsimd_state.vregs));Do we need to hold the CPU fpsimd context across this memset?
IIRC, TIF_FOREIGN_FPSTATE can be spontaneously cleared along with dumping of the regs into thread_struct (from current's PoV), but never spontaneously set again. So ... -> [*]
Yes, we could drop the lock here. OTOH this is very simple and easy to understand.
Ack; it works either way.
Since this is a Fixes: patch, it may be better to keep it simple.
/* Ensure any copies on other CPUs aren't reused */fpsimd_flush_task_state(current);(This is very similar to fpsimd_flush_thread(); can they be unified?)
I have a half finished series to replace the whole setup around accessing the state with get/put operations for working on the state which should remove all these functions. The pile of similarly and confusingly named operations we have for working on the state is one of the major sources of issues with this code, even when actively working on the code it's hard to remember exactly which operation does what never mind the rules for which is needed.
Sure, something like that would definitely help.
Cheers ---Dave