From mboxrd@z Thu Jan 1 00:00:00 1970 From: Will Deacon Subject: Re: [PATCH v7 02/16] arm64: Use update{,_tsk}_thread_flag() Date: Wed, 9 May 2018 17:17:28 +0100 Message-ID: <20180509161728.GC26671@arm.com> References: <1525882385-29181-1-git-send-email-Dave.Martin@arm.com> <1525882385-29181-3-git-send-email-Dave.Martin@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 287DC4A0CB for ; Wed, 9 May 2018 12:10:36 -0400 (EDT) Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id rOVS2wBGf5fU for ; Wed, 9 May 2018 12:10:13 -0400 (EDT) Received: from foss.arm.com (foss.arm.com [217.140.101.70]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 74AED4A0D3 for ; Wed, 9 May 2018 12:07:03 -0400 (EDT) Content-Disposition: inline In-Reply-To: <1525882385-29181-3-git-send-email-Dave.Martin@arm.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kvmarm-bounces@lists.cs.columbia.edu Sender: kvmarm-bounces@lists.cs.columbia.edu To: Dave Martin Cc: Christoffer Dall , Ard Biesheuvel , Marc Zyngier , Catalin Marinas , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org List-Id: kvmarm@lists.cs.columbia.edu Hi Dave, On Wed, May 09, 2018 at 05:12:51PM +0100, Dave Martin wrote: > This patch uses the new update_thread_flag() helpers to simplify a > couple of if () set; else clear; constructs. > > No functional change. > > Signed-off-by: Dave Martin > Acked-by: Marc Zyngier > Cc: Catalin Marinas > Cc: Will Deacon > --- > arch/arm64/kernel/fpsimd.c | 19 +++++++------------ > 1 file changed, 7 insertions(+), 12 deletions(-) > > diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c > index 87a3536..0c4e7e0 100644 > --- a/arch/arm64/kernel/fpsimd.c > +++ b/arch/arm64/kernel/fpsimd.c > @@ -618,10 +618,8 @@ int sve_set_vector_length(struct task_struct *task, > task->thread.sve_vl = vl; > > out: > - if (flags & PR_SVE_VL_INHERIT) > - set_tsk_thread_flag(task, TIF_SVE_VL_INHERIT); > - else > - clear_tsk_thread_flag(task, TIF_SVE_VL_INHERIT); > + update_tsk_thread_flag(task, TIF_SVE_VL_INHERIT, > + flags & PR_SVE_VL_INHERIT); > > return 0; > } > @@ -902,7 +900,7 @@ void fpsimd_thread_switch(struct task_struct *next) > if (current->mm) > task_fpsimd_save(); > > - if (next->mm) { > + if (next->mm) > /* > * If we are switching to a task whose most recent userland > * FPSIMD state is already in the registers of *this* cpu, > @@ -910,13 +908,10 @@ void fpsimd_thread_switch(struct task_struct *next) > * the TIF_FOREIGN_FPSTATE flag so the state will be loaded > * upon the next return to userland. > */ > - if (__this_cpu_read(fpsimd_last_state.st) == > - &next->thread.uw.fpsimd_state > - && next->thread.fpsimd_cpu == smp_processor_id()) > - clear_tsk_thread_flag(next, TIF_FOREIGN_FPSTATE); > - else > - set_tsk_thread_flag(next, TIF_FOREIGN_FPSTATE); > - } > + update_tsk_thread_flag(next, TIF_FOREIGN_FPSTATE, > + __this_cpu_read(fpsimd_last_state.st) != > + &next->thread.uw.fpsimd_state || > + next->thread.fpsimd_cpu != smp_processor_id()); Given the multi-line comment and this multi-line call, I'd be inclined to leave the curlies in place and then use a local bool for the complex condition. With that: Acked-by: Will Deacon Will From mboxrd@z Thu Jan 1 00:00:00 1970 From: will.deacon@arm.com (Will Deacon) Date: Wed, 9 May 2018 17:17:28 +0100 Subject: [PATCH v7 02/16] arm64: Use update{,_tsk}_thread_flag() In-Reply-To: <1525882385-29181-3-git-send-email-Dave.Martin@arm.com> References: <1525882385-29181-1-git-send-email-Dave.Martin@arm.com> <1525882385-29181-3-git-send-email-Dave.Martin@arm.com> Message-ID: <20180509161728.GC26671@arm.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Dave, On Wed, May 09, 2018 at 05:12:51PM +0100, Dave Martin wrote: > This patch uses the new update_thread_flag() helpers to simplify a > couple of if () set; else clear; constructs. > > No functional change. > > Signed-off-by: Dave Martin > Acked-by: Marc Zyngier > Cc: Catalin Marinas > Cc: Will Deacon > --- > arch/arm64/kernel/fpsimd.c | 19 +++++++------------ > 1 file changed, 7 insertions(+), 12 deletions(-) > > diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c > index 87a3536..0c4e7e0 100644 > --- a/arch/arm64/kernel/fpsimd.c > +++ b/arch/arm64/kernel/fpsimd.c > @@ -618,10 +618,8 @@ int sve_set_vector_length(struct task_struct *task, > task->thread.sve_vl = vl; > > out: > - if (flags & PR_SVE_VL_INHERIT) > - set_tsk_thread_flag(task, TIF_SVE_VL_INHERIT); > - else > - clear_tsk_thread_flag(task, TIF_SVE_VL_INHERIT); > + update_tsk_thread_flag(task, TIF_SVE_VL_INHERIT, > + flags & PR_SVE_VL_INHERIT); > > return 0; > } > @@ -902,7 +900,7 @@ void fpsimd_thread_switch(struct task_struct *next) > if (current->mm) > task_fpsimd_save(); > > - if (next->mm) { > + if (next->mm) > /* > * If we are switching to a task whose most recent userland > * FPSIMD state is already in the registers of *this* cpu, > @@ -910,13 +908,10 @@ void fpsimd_thread_switch(struct task_struct *next) > * the TIF_FOREIGN_FPSTATE flag so the state will be loaded > * upon the next return to userland. > */ > - if (__this_cpu_read(fpsimd_last_state.st) == > - &next->thread.uw.fpsimd_state > - && next->thread.fpsimd_cpu == smp_processor_id()) > - clear_tsk_thread_flag(next, TIF_FOREIGN_FPSTATE); > - else > - set_tsk_thread_flag(next, TIF_FOREIGN_FPSTATE); > - } > + update_tsk_thread_flag(next, TIF_FOREIGN_FPSTATE, > + __this_cpu_read(fpsimd_last_state.st) != > + &next->thread.uw.fpsimd_state || > + next->thread.fpsimd_cpu != smp_processor_id()); Given the multi-line comment and this multi-line call, I'd be inclined to leave the curlies in place and then use a local bool for the complex condition. With that: Acked-by: Will Deacon Will