[PATCH 1/2] powerpc/tm: Move tm_enable definition

[Christophe Leroy]

Le 01/10/2018 à 21:47, Breno Leitao a écrit :
> The goal of this patch is to move function tm_enabled() to tm.h in order
> to allow this function to be used by other files as an inline function.
> 
> This patch also removes the double inclusion of tm.h in the traps.c
> source code. One inclusion is inside a CONFIG_PPC64 ifdef block, and
> another one is in the generic part. This double inclusion causes a
> redefinition of tm_enable(), that is why it is being fixed here.
> 
> There is generic code (non CONFIG_PPC64, thus, non
> CONFIG_PPC_TRANSACTIONAL_MEM also) using some TM definitions, which
> explains why tm.h is being imported in the generic code. This is
> not correct, and this code is now surrounded by a
> CONFIG_PPC_TRANSACTIONAL_MEM ifdef block.

You should leave the generic inclusion and remove the second one.

Don't use #ifdef blocks when not necessary, see 
https://www.kernel.org/doc/html/latest/process/coding-style.html#conditional-compilation

> 
> These ifdef inclusion will avoid calling tm_abort_check() completely,
> but it is not a problem since this function is just returning 'false' if
> CONFIG_PPC_TRANSACTIONAL_MEM is not defined.

As tm_abort_check() always returns false, gcc will see it and will optimise-out the check by itself, 
no worry.


> 
> Signed-off-by: Breno Leitao <leitao at debian.org>
> ---
>   arch/powerpc/include/asm/tm.h | 5 +++++
>   arch/powerpc/kernel/process.c | 5 -----
>   arch/powerpc/kernel/traps.c   | 5 ++++-
>   3 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/tm.h b/arch/powerpc/include/asm/tm.h
> index e94f6db5e367..646d45a2aaae 100644
> --- a/arch/powerpc/include/asm/tm.h
> +++ b/arch/powerpc/include/asm/tm.h
> @@ -19,4 +19,9 @@ extern void tm_restore_sprs(struct thread_struct *thread);
>   
>   extern bool tm_suspend_disabled;
>   
> +static inline bool tm_enabled(struct task_struct *tsk)
> +{
> +	return tsk && tsk->thread.regs && (tsk->thread.regs->msr & MSR_TM);
> +}
> +
>   #endif /* __ASSEMBLY__ */
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index 913c5725cdb2..c1ca2451fa3b 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -862,11 +862,6 @@ static inline bool hw_brk_match(struct arch_hw_breakpoint *a,
>   
>   #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
>   
> -static inline bool tm_enabled(struct task_struct *tsk)
> -{
> -	return tsk && tsk->thread.regs && (tsk->thread.regs->msr & MSR_TM);
> -}
> -
>   static void tm_reclaim_thread(struct thread_struct *thr, uint8_t cause)
>   {
>   	/*
> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index c85adb858271..a3d6298b8074 100644
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -64,7 +64,6 @@
>   #include <asm/rio.h>
>   #include <asm/fadump.h>
>   #include <asm/switch_to.h>
> -#include <asm/tm.h>
>   #include <asm/debug.h>
>   #include <asm/asm-prototypes.h>
>   #include <asm/hmi.h>
> @@ -1276,9 +1275,11 @@ static int emulate_instruction(struct pt_regs *regs)
>   
>   	/* Emulate load/store string insn. */
>   	if ((instword & PPC_INST_STRING_GEN_MASK) == PPC_INST_STRING) {
> +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM

This ifdef is not needed, tm_abort_check() returns false when CONFIG_PPC_TRANSACTIONAL_MEM is not 
defined.

>   		if (tm_abort_check(regs,
>   				   TM_CAUSE_EMULATE | TM_CAUSE_PERSISTENT))
>   			return -EINVAL;
> +#endif
>   		PPC_WARN_EMULATED(string, regs);
>   		return emulate_string_inst(regs, instword);
>   	}
> @@ -1508,8 +1509,10 @@ void alignment_exception(struct pt_regs *regs)
>   	if (!arch_irq_disabled_regs(regs))
>   		local_irq_enable();
>   
> +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM

Same here.

>   	if (tm_abort_check(regs, TM_CAUSE_ALIGNMENT | TM_CAUSE_PERSISTENT))
>   		goto bail;
> +#endif
>   
>   	/* we don't implement logging of alignment exceptions */
>   	if (!(current->thread.align_ctl & PR_UNALIGN_SIGBUS))
>