lkml.org 
[lkml]   [2020]   [Apr]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v3 05/10] x86/fpu/xstate: Define new functions for clearing fpregs and xstates
From
Date
On Wed, 2020-04-29 at 11:27 +0200, Borislav Petkov wrote:
> 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.

All fixed.

Thanks,
Yu-cheng

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