Messages in this thread | | | Date | Thu, 7 Feb 2019 15:10:27 +0100 | From | Sebastian Andrzej Siewior <> | Subject | Re: [PATCH 17/22] x86/fpu: Prepare copy_fpstate_to_sigframe() for TIF_NEED_FPU_LOAD |
| |
On 2019-01-30 13:53:51 [+0100], Borislav Petkov wrote: > > I've been asked to add comment above the sequence so it is understood. I > > think the general approach is easy to follow once the concept is > > understood. I don't mind renaming the TIF_ thingy once again (it > > happend once or twice and I think the current one was suggested by Andy > > unless I mixed things up). > > The problem I have with the above is that > > > > if (test_thread_flag(TIF_NEED_FPU_LOAD)) > > do_that() > > > > becomes > > if (!test_thread_flag(TIF_FPU_REGS_VALID)) > > do_that() > > Err, above it becomes > > if (test_thread_flag(TIF_FPU_REGS_VALID)) > copy_fpregs_to_fpstate(fpu);
The (your) above example yes. But the reverse state if (test_thread_flag(TIF_NEED_FPU_LOAD)) do_that()
becomes if (!test_thread_flag(TIF_FPU_REGS_VALID)) do_that()
> without the "!". I.e., CPU's FPU regs are valid and we need to save them. > > Or am I misreading the comment above?
Your example is correct. But in the opposite case, when ! was not there then we have to add it.
> > and you could argue again the other way around. So do we want NEED_LOAD > > or NEED_SAVE flag which is another way of saying REGS_VALID? > > All fine and dandy except NEED_FPU_LOAD is ambiguous to me: we need to > load them where? Into the CPU? Or into the FPU state save area?
if you need to LOAD then task-saved-area into CPU-state. If you need to save it then CPU-state into task-saved-area.
> TIF_FPU_REGS_VALID is clearer in the sense that the CPU's FPU registers > are currently valid for the current task. As there are no other FPU > registers except the CPU's.
hmmm. I think it is just taste / habit.
> > More importantly the logic is changed when the bit is set and this > > requires more thinking than just doing sed on the patch series. > > Sure. > > And I'll get accustomed to the logic whatever the name is - this is just > a "wouldn't it be better" thing.
If it would be just a name thing then I probably wouldn't mind. But swapping the logic might break things so I try to avoid that.
> Thx. >
Sebastian
| |