From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S966628AbbBCTJf (ORCPT ); Tue, 3 Feb 2015 14:09:35 -0500 Received: from mx1.redhat.com ([209.132.183.28]:43902 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753118AbbBCTJd (ORCPT ); Tue, 3 Feb 2015 14:09:33 -0500 Date: Tue, 3 Feb 2015 20:08:05 +0100 From: Oleg Nesterov To: Rik van Riel 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 Message-ID: <20150203190805.GA8455@redhat.com> 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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <54CFD35A.8050602@redhat.com> User-Agent: Mutt/1.5.18 (2008-05-17) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. Oleg.