On Fri, Apr 12, 2019 at 07:19:55PM +0200, Sebastian Andrzej Siewior wrote:
> remove x86_fpu_activate_state.
Ok, zapping it as part of this patch.
> We could also rip all trcepoints out, rethink the situation and add new
> ones based on current code.
Well, since this changes FPU regs handling considerably, I think the
only correct step would be to adjust the tracepoints. BUT(!) we should
not forget we're not supposed to break luserspace.
> - on schedule out, we may need to save registers (depending on
> TIF_NEED_FPU_LOAD) which is new. Before the series we always did.
That makes sense.
> - on schedule in do nothing but set that TIF bit. This is probably
> boring.
Yah.
> - on return to userland we should load the registers but can avoid it if
> we assume that they are valid for the current task
> (fpregs_state_valid())
That is interesting info.
> - in kernel_fpu_begin() we may trash the task's FPU state (by saving its
> registers or by resetting fpu_fpregs_owner_ctx).
Do we care?
You mean, in case you have workloads which might involve a lot of
in-kernel FPU use which would punish task context switches?
> Those might be interesting.
>
> Currently we have:
> "x86/fpu: %p load: %d xfeatures: %llx xcomp_bv: %llx"
>
> and we have to find out what happens based on where that TP was
> recorded. Also I'm not sure if the recorded xfeatures change over time.
> I think they do not…
Good question.
> you mean trace_x86_fpu_activate_state and trace_x86_fpu_regs_activated?
> They were added in d1898b733619b ("x86/fpu: Add tracepoints to dump FPU
> state at key points") and we wouldn't have any otherwise.
I guess it made sense then...
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
