linux-kernel - Re: [PATCH 054/208] x86/fpu: Get rid of PF_USED_MATH usage, convert it to fpu->fpstate

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 5 May 2015 17:51:40 -0700
From:	Andy Lutomirski <luto@...capital.net>
To:	Ingo Molnar <mingo@...nel.org>
Cc:	Borislav Petkov <bp@...en8.de>, Fenghua Yu <fenghua.yu@...el.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Dave Hansen <dave.hansen@...ux.intel.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Oleg Nesterov <oleg@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 054/208] x86/fpu: Get rid of PF_USED_MATH usage, convert
 it to fpu->fpstate_active

On May 5, 2015 9:59 PM, "Ingo Molnar" <mingo@...nel.org> wrote:
>
> Introduce a simple fpu->fpstate_active flag in the fpu context data structure
> and use that instead of PF_USED_MATH in task->flags.
>
> Testing for this flag byte should be slightly more efficient than
> testing a bit in a bitmask, but the main advantage is that most
> FPU functions can now be performed on a 'struct fpu' alone, they
> don't need access to 'struct task_struct' anymore.
>
> There's a slight linecount increase, mostly due to the 'fpu' local
> variables and due to extra comments. The local variables will go away
> once we move most of the FPU methods to pure 'struct fpu' parameters.
>
> Reviewed-by: Borislav Petkov <bp@...en8.de>
> Cc: Andy Lutomirski <luto@...capital.net>
> Cc: Dave Hansen <dave.hansen@...ux.intel.com>
> Cc: Fenghua Yu <fenghua.yu@...el.com>
> Cc: H. Peter Anvin <hpa@...or.com>
> Cc: Linus Torvalds <torvalds@...ux-foundation.org>
> Cc: Oleg Nesterov <oleg@...hat.com>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Signed-off-by: Ingo Molnar <mingo@...nel.org>
> ---
>  arch/x86/ia32/ia32_signal.c         |  3 ++-
>  arch/x86/include/asm/fpu-internal.h |  4 ++--
>  arch/x86/include/asm/fpu/types.h    |  6 ++++++
>  arch/x86/include/asm/processor.h    |  6 ++++--
>  arch/x86/kernel/fpu/core.c          | 38 +++++++++++++++++++++++++-------------
>  arch/x86/kernel/fpu/xsave.c         | 11 ++++++-----
>  arch/x86/kernel/signal.c            |  8 +++++---
>  arch/x86/kvm/x86.c                  |  3 ++-
>  arch/x86/math-emu/fpu_entry.c       |  3 ++-
>  9 files changed, 54 insertions(+), 28 deletions(-)
>
> diff --git a/arch/x86/ia32/ia32_signal.c b/arch/x86/ia32/ia32_signal.c
> index bffb2c49ceb6..e1ec6f90d09e 100644
> --- a/arch/x86/ia32/ia32_signal.c
> +++ b/arch/x86/ia32/ia32_signal.c
> @@ -307,6 +307,7 @@ static void __user *get_sigframe(struct ksignal *ksig, struct pt_regs *regs,
>                                  size_t frame_size,
>                                  void __user **fpstate)
>  {
> +       struct fpu *fpu = &current->thread.fpu;
>         unsigned long sp;
>
>         /* Default to using normal stack */
> @@ -321,7 +322,7 @@ static void __user *get_sigframe(struct ksignal *ksig, struct pt_regs *regs,
>                  ksig->ka.sa.sa_restorer)
>                 sp = (unsigned long) ksig->ka.sa.sa_restorer;
>
> -       if (current->flags & PF_USED_MATH) {
> +       if (fpu->fpstate_active) {
>                 unsigned long fx_aligned, math_size;
>
>                 sp = alloc_mathframe(sp, 1, &fx_aligned, &math_size);
> diff --git a/arch/x86/include/asm/fpu-internal.h b/arch/x86/include/asm/fpu-internal.h
> index 2cac49e3b4bd..9311126571ab 100644
> --- a/arch/x86/include/asm/fpu-internal.h
> +++ b/arch/x86/include/asm/fpu-internal.h
> @@ -375,7 +375,7 @@ static inline void drop_fpu(struct task_struct *tsk)
>                 __thread_fpu_end(fpu);
>         }
>
> -       tsk->flags &= ~PF_USED_MATH;
> +       fpu->fpstate_active = 0;
>
>         preempt_enable();
>  }
> @@ -424,7 +424,7 @@ static inline fpu_switch_t switch_fpu_prepare(struct task_struct *old, struct ta
>          * If the task has used the math, pre-load the FPU on xsave processors
>          * or if the past 5 consecutive context-switches used math.
>          */
> -       fpu.preload = (new->flags & PF_USED_MATH) &&
> +       fpu.preload = new_fpu->fpstate_active &&
>                       (use_eager_fpu() || new->thread.fpu.counter > 5);
>
>         if (old_fpu->has_fpu) {
> diff --git a/arch/x86/include/asm/fpu/types.h b/arch/x86/include/asm/fpu/types.h
> index efb520dcf38e..f6317d9aa808 100644
> --- a/arch/x86/include/asm/fpu/types.h
> +++ b/arch/x86/include/asm/fpu/types.h
> @@ -137,6 +137,12 @@ struct fpu {
>          * deal with bursty apps that only use the FPU for a short time:
>          */
>         unsigned char                   counter;
> +       /*
> +        * This flag indicates whether this context is fpstate_active: if the task is
> +        * not running then we can restore from this context, if the task
> +        * is running then we should save into this context.
> +        */
> +       unsigned char                   fpstate_active;

I don't understand.  What does it mean if !fpstate_active?

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/