Messages in this thread | | | Date | Wed, 29 Apr 2020 11:27:35 +0200 | From | Borislav Petkov <> | Subject | Re: [PATCH v3 05/10] x86/fpu/xstate: Define new functions for clearing fpregs and xstates |
| |
On Sat, Mar 28, 2020 at 09:43:02AM -0700, Yu-cheng Yu wrote: > @@ -318,18 +313,40 @@ static inline void copy_init_fpstate_to_fpregs(void) > * Called by sys_execve(), by the signal handler code and by various > * error paths. > */ > -void fpu__clear(struct fpu *fpu) > +static void fpu__clear(struct fpu *fpu, int clear_user_only)
I said:
"fpu__clear(struct fpu *fpu, bool user_only)" ^^^^^^^^^^^^^^
you made it
..., int clear_user_only)
Why?
If you agree with the review comment, then please do it as suggested. If you don't, then say why you don't.
Why would you do something in-between?
> { > - WARN_ON_FPU(fpu != ¤t->thread.fpu); /* Almost certainly an anomaly */
Why are you moving this into the callers when *both* do it?
> + if (static_cpu_has(X86_FEATURE_FPU)) {
Flip this logic:
if (!static_cpu_has(X86_FEATURE_FPU)) { fpu__drop(fpu); fpu__initialize(fpu); return; }
fpregs_lock(); ...
to save an indentation level and make the important case more readable and locking more prominent.
-- Regards/Gruss, Boris.
https://people.kernel.org/tglx/notes-about-netiquette
| |