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
