lkml.org 
[lkml]   [2020]   [Apr]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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 != &current->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

\
 
 \ /
  Last update: 2020-04-29 11:29    [W:0.234 / U:0.092 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site