[lkml]   [2015]   [Feb]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    Patch in this message
    SubjectRe: [PATCH 4/6] x86,fpu: use disable_task_lazy_fpu_restore helper
    Hash: SHA1

    On 02/03/2015 02:08 PM, Oleg Nesterov wrote:
    > On 02/02, Rik van Riel wrote:
    >> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
    >> On 02/02/2015 02:21 PM, Oleg Nesterov wrote:
    >>> I'll try to read this patch tomorrow. Too late for me.
    >>> I think it is fine, but
    >>> On 02/02, wrote:
    >>>> This also fixes the lazy FPU restore disabling in drop_fpu,
    >>>> which only really works when !use_eager_fpu(). ...
    >>>> --- a/arch/x86/include/asm/fpu-internal.h +++
    >>>> b/arch/x86/include/asm/fpu-internal.h @@ -396,7 +396,7 @@
    >>>> static inline void drop_fpu(struct task_struct *tsk) *
    >>>> Forget coprocessor state.. */ preempt_disable(); -
    >>>> tsk->thread.fpu_counter = 0; +
    >>>> task_disable_lazy_fpu_restore(tsk); __drop_fpu(tsk);
    >>>> clear_used_math();
    >>> perhaps this makes sense anyway, but I am not sure if the
    >>> changelog is right.
    >>> Note that we clear PF_USED_MATH, last_fpu/has_fpu have no
    >>> effect after this.
    >> There are several code paths, including signal handler stack
    >> setup and teardown, where we first clear PF_USED_MATH, but later
    >> on set it again, after setting up a new math state for the task.
    >> We need to ensure we always use that new math state, and never
    >> lazily re-use what is still in the FPU registers.
    > Still can't understand....
    > I can only see only on example of re-setting PF_USED_MATH if
    > eager_fpu, __restore_xstate_sig() does drop_fpu() +
    > math_state_restore(). And this case looks fine in this sense...
    > Although let me repeat that imho math_state_restore() and all
    > current users (including flush_thread() added by "don't abuse FPU
    > in kernel threads") need cleanups in any case. I was going to (try
    > to) do this if/when that series is applied. In particular, in this
    > case math_state_restore() will wrongly return with irqs disabled or
    > I am totally confused.
    > And set_used_math() if copy_from_user() fails doesn't look right,
    > but this is another story.
    > If I missed something, perhaps you can update the changelog to
    > explain how exactly it fixes drop_fpu? Otherwise, imo this patch
    > should not change it. As we already discussed, we can probably
    > cleanup this disable_lazy_fpu logic, but this needs another
    > change/discussion.

    I don't think this changes the behaviour of any code path as the
    kernel currently stands.

    However, having drop_fpu() disable lazy restore in a more explicit
    way may help us going forward, with the "delay FPU state restore to
    kernel -> user space switch" series.

    - --
    All rights reversed
    Version: GnuPG v1

    -----END PGP SIGNATURE-----

     \ /
      Last update: 2015-02-03 23:21    [W:3.660 / U:4.816 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site