Re: [Patch v6 08/22] x86/fpu: Ensure TIF_NEED_FPU_LOAD is set after saving FPU state

[Mi, Dapeng]

On 2/12/2026 3:55 AM, Dave Hansen wrote:
> On 2/11/26 11:39, Chang S. Bae wrote:
>> But the changelog doesn't look clear enough to explain the reason to me.
> Yeah, the changelog could use some improvement.
>
> This also leaves the code rather fragile. Right now, all of the:
>
> 	save_fpregs_to_fpstate(fpu);
> 	set_thread_flag(TIF_NEED_FPU_LOAD);
>
> pairs are open-coded. There's precisely nothing stopping someone from
> coming in tomorrow and reversing the order at these or other call sites.
> There's also zero comments left in the code to tell folks not to do this.
>
> Are there enough of these to have a helper that does:
>
> 	save_fpregs_to_fpstate_before_invalidation(fpu);
>
> which could do the save and set TIF_NEED_FPU_LOAD? (that name is awful
> btw, please don't use it).

Sure. Would introduce an inline function and add comments. Thanks.


>
> This:
>
>>  	/* Swap fpstate */
>>  	if (enter_guest) {
>> -		fpu->__task_fpstate = cur_fps;
>> +		WRITE_ONCE(fpu->__task_fpstate, cur_fps);
>> +		barrier();
>>  		fpu->fpstate = guest_fps;
>>  		guest_fps->in_use = true;
>>  	} else {
>>  		guest_fps->in_use = false;
>>  		fpu->fpstate = fpu->__task_fpstate;
>> -		fpu->__task_fpstate = NULL;
>> +		barrier();
>> +		WRITE_ONCE(fpu->__task_fpstate, NULL);
>>  	}
> also urgently needs comments.
>
> I also can't help but think that there might be a nicer way to do that
> without the barrier(). I _think_ two correctly-ordered WRITE_ONCE()'s
> would make the compiler do the same thing as the barrier().
>
> But I'm not fully understanding what the barrier() is doing anyway, so
> take that with a grain of salt.

IMO, barrier() seems a safer way for this situation. I'm not quite sure if
two WRITE_ONCE()'s would block the reordering from compiler, so I asked
Gemini, and here is its answer.

"

WRITE_ONCE() provides a certain level of protection, but it isn't a full
barrier.

WRITE_ONCE(): Prevents the compiler from optimizing the store away or
splitting it into multiple instructions.

Two WRITE_ONCE() calls: The compiler is generally discouraged from
reordering two volatile-style accesses (which WRITE_ONCE uses), but the C
standard doesn't strictly guarantee it in all cases.

"

Thanks.