On Wed, 2018-09-12 at 16:40 -0300, Breno Leitao wrote:
> Since the transaction will be doomed with treckpt., the TEXASR[FS]
> should be set, to reflect that the transaction is a failure. This patch
> ensures it before recheckpointing, and remove changes from other places
> that were calling recheckpoint.
TEXASR[FS] should be set by the reclaim. I don't know why you'd need to set this
explicitly in process.c. The only case is when the user supplies a bad signal
context, but we should check that in the signals code, not process.c
Hence I think this patch is wrong.
Also, according to the architecture, TEXASR[FS] HAS TO BE SET on trecheckpoint
otherwise you'll get a TM Bad Thing. You should say that rather than suggesting
it's because the transaction is doomed. It's illegal to not do it. That's why we
have this check in arch/powerpc/kernel/tm.S.
/* Do final sanity check on TEXASR to make sure FS is set. Do this
* here before we load up the userspace r1 so any bugs we hit will get
* a call chain */
mfspr r5, SPRN_TEXASR
srdi r5, r5, 16
li r6, (TEXASR_FS)@h
and r6, r6, r5
1: tdeqi r6, 0
EMIT_BUG_ENTRY 1b,__FILE__,__LINE__,0
Mikey
> Signed-off-by: Breno Leitao <lei...@debian.org>
> ---
> arch/powerpc/kernel/process.c | 6 ++++++
> arch/powerpc/kernel/signal_32.c | 2 --
> arch/powerpc/kernel/signal_64.c | 2 --
> 3 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index 5cace1b744b1..77725b2e4dc1 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -937,6 +937,12 @@ void tm_recheckpoint(struct thread_struct *thread)
> local_irq_save(flags);
> hard_irq_disable();
>
> + /*
> + * Make sure the failure summary is set, since the transaction will be
> + * doomed.
> + */
> + thread->tm_texasr |= TEXASR_FS;
> +
> /* The TM SPRs are restored here, so that TEXASR.FS can be set
> * before the trecheckpoint and no explosion occurs.
> */
> diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
> index 4a1b17409bf3..96956d50538e 100644
> --- a/arch/powerpc/kernel/signal_32.c
> +++ b/arch/powerpc/kernel/signal_32.c
> @@ -851,8 +851,6 @@ static long restore_tm_user_regs(struct pt_regs *regs,
> /* Pull in the MSR TM bits from the user context */
> regs->msr = (regs->msr & ~MSR_TS_MASK) | (msr_hi & MSR_TS_MASK);
>
> - /* Make sure the transaction is marked as failed */
> - current->thread.tm_texasr |= TEXASR_FS;
> /* Make sure restore_tm_state will be called */
> set_thread_flag(TIF_RESTORE_TM);
>
> diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
> index 32402aa23a5e..c84501711b14 100644
> --- a/arch/powerpc/kernel/signal_64.c
> +++ b/arch/powerpc/kernel/signal_64.c
> @@ -569,8 +569,6 @@ static long restore_tm_sigcontexts(struct task_struct
> *tsk,
> }
> }
> #endif
> - /* Make sure the transaction is marked as failed */
> - tsk->thread.tm_texasr |= TEXASR_FS;
> /* Guarantee that restore_tm_state() will be called */
> set_thread_flag(TIF_RESTORE_TM);
>
