From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753114AbbBCWBn (ORCPT ); Tue, 3 Feb 2015 17:01:43 -0500 Received: from mx1.redhat.com ([209.132.183.28]:54935 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750785AbbBCWBm (ORCPT ); Tue, 3 Feb 2015 17:01:42 -0500 Message-ID: <54D14531.3060502@redhat.com> Date: Tue, 03 Feb 2015 17:01:21 -0500 From: Rik van Riel User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.3.0 MIME-Version: 1.0 To: Oleg Nesterov CC: dave.hansen@linux.intel.com, sbsiddha@gmail.com, luto@amacapital.net, tglx@linutronix.de, mingo@redhat.com, hpa@zytor.com, fenghua.yu@intel.com, x86@kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 4/6] x86,fpu: use disable_task_lazy_fpu_restore helper References: <20150129210723.GA31584@redhat.com> <1422900051-10778-1-git-send-email-riel@redhat.com> <1422900051-10778-5-git-send-email-riel@redhat.com> <20150202192139.GA17624@redhat.com> <54CFD35A.8050602@redhat.com> <20150203190805.GA8455@redhat.com> In-Reply-To: <20150203190805.GA8455@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org -----BEGIN PGP SIGNED MESSAGE----- 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, riel@redhat.com 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 -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJU0UUxAAoJEM553pKExN6DzSMIAIuPNXZ8P5zHAPMLePQhfaZK QaatOplJ5F8GNN3bT2sJHLIPfDJVokMbGFv7ff8/jPsZ03uBm6onFPp/4Qn7dGZ4 FQyFX7OkmR7D4NJ0KZ9J1ghWhfRhmxfAqr/qwrdovUJ/Hfz73FdqGPYpP90qvY/2 0psSaW6ubJen6l9QYBear5juhQE3+akfwc/D1ItpZtZRUHOJTewBiww/9I/kUrDZ mp1j4szVEjaQ9jB5Et4KO4EFaaEFzdj2JXwSV10neBgLrZ5dixYII1kUFQQFxvTd bse2Rjsh0+6ESkUOoXLy1y0IYbjrQqEc+YdrGbnf64XJLIjj1a8U8F3dU5xPdCM= =0+mw -----END PGP SIGNATURE-----