[RFC PATCH 06/14] powerpc/tm: Do not recheckpoint at sigreturn

Breno Leitao leitao at debian.org
Tue Nov 6 23:40:20 AEDT 2018


Do not recheckpoint at signal code return. Just make sure TIF_RESTORE_TM is
set, which will restore on the exit to userspace by restore_tm_state.

All the FP and VEC lazy restore was already done by tm_reclaim_current(),
where it checked if FP/VEC was set, and filled out the ckfp and ckvr
registers area to the expected value.

The load_fp_state() and load_vr_state() is being called to restore the
clobbered registers after tm_recheckpoint(). This is not necessary anymore,
since restore_tm_state() does it already, so, this is only required to be
done after trecheckpoint(), and the only place calling it is
restore_tm_state().

Signed-off-by: Breno Leitao <leitao at debian.org>
---
 arch/powerpc/kernel/signal_32.c | 38 +++++++++------------------------
 arch/powerpc/kernel/signal_64.c | 30 ++++++++------------------
 2 files changed, 19 insertions(+), 49 deletions(-)

diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index e6474a45cef5..046030d7921e 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -850,28 +850,11 @@ static long restore_tm_user_regs(struct pt_regs *regs,
 		return 1;
 	/* Pull in the MSR TM bits from the user context */
 	regs->msr = (regs->msr & ~MSR_TS_MASK) | (msr_hi & MSR_TS_MASK);
-	/* Now, recheckpoint.  This loads up all of the checkpointed (older)
-	 * registers, including FP and V[S]Rs.  After recheckpointing, the
-	 * transactional versions should be loaded.
-	 */
-	tm_enable();
+
 	/* Make sure the transaction is marked as failed */
 	current->thread.tm_texasr |= TEXASR_FS;
-	/* This loads the checkpointed FP/VEC state, if used */
-	tm_recheckpoint(&current->thread);
-
-	/* This loads the speculative FP/VEC state, if used */
-	msr_check_and_set(msr & (MSR_FP | MSR_VEC));
-	if (msr & MSR_FP) {
-		load_fp_state(&current->thread.fp_state);
-		regs->msr |= (MSR_FP | current->thread.fpexc_mode);
-	}
-#ifdef CONFIG_ALTIVEC
-	if (msr & MSR_VEC) {
-		load_vr_state(&current->thread.vr_state);
-		regs->msr |= MSR_VEC;
-	}
-#endif
+	/* Make sure restore_tm_state will be called */
+	set_thread_flag(TIF_RESTORE_TM);
 
 	return 0;
 }
@@ -1156,16 +1139,15 @@ SYSCALL_DEFINE0(rt_sigreturn)
 
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
 	/*
-	 * If there is a transactional state then throw it away.
-	 * The purpose of a sigreturn is to destroy all traces of the
-	 * signal frame, this includes any transactional state created
-	 * within in. We only check for suspended as we can never be
-	 * active in the kernel, we are active, there is nothing better to
-	 * do than go ahead and Bad Thing later.
-	 * The cause is not important as there will never be a
+	 * If the transaction is active at this point, it means that
+	 * TM_KERNEL_ENTRY was not invoked properly and it is a bug.
+	 * If this is the case, print a warning and try to work around,
+	 * calling tm_reclaim_current() to discard the footprint.
+	 *
+	 * The failure cause is not important as there will never be a
 	 * recheckpoint so it's not user visible.
 	 */
-	if (MSR_TM_SUSPENDED(mfmsr()))
+	if (WARN_ON(MSR_TM_SUSPENDED(mfmsr())))
 		tm_reclaim_current(0);
 
 	if (__get_user(tmp, &rt_sf->uc.uc_link))
diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
index 83d51bf586c7..487c3b6aa2e3 100644
--- a/arch/powerpc/kernel/signal_64.c
+++ b/arch/powerpc/kernel/signal_64.c
@@ -569,21 +569,10 @@ static long restore_tm_sigcontexts(struct task_struct *tsk,
 		}
 	}
 #endif
-	tm_enable();
 	/* Make sure the transaction is marked as failed */
 	tsk->thread.tm_texasr |= TEXASR_FS;
-	/* This loads the checkpointed FP/VEC state, if used */
-	tm_recheckpoint(&tsk->thread);
-
-	msr_check_and_set(msr & (MSR_FP | MSR_VEC));
-	if (msr & MSR_FP) {
-		load_fp_state(&tsk->thread.fp_state);
-		regs->msr |= (MSR_FP | tsk->thread.fpexc_mode);
-	}
-	if (msr & MSR_VEC) {
-		load_vr_state(&tsk->thread.vr_state);
-		regs->msr |= MSR_VEC;
-	}
+	/* Guarantee that restore_tm_state() will be called */
+	set_thread_flag(TIF_RESTORE_TM);
 
 	return err;
 }
@@ -717,16 +706,15 @@ SYSCALL_DEFINE0(rt_sigreturn)
 
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
 	/*
-	 * If there is a transactional state then throw it away.
-	 * The purpose of a sigreturn is to destroy all traces of the
-	 * signal frame, this includes any transactional state created
-	 * within in. We only check for suspended as we can never be
-	 * active in the kernel, we are active, there is nothing better to
-	 * do than go ahead and Bad Thing later.
-	 * The cause is not important as there will never be a
+	 * If the transaction is active at this point, it means that
+	 * TM_KERNEL_ENTRY was not invoked properly and it is a bug.
+	 * If this is the case, print a warning and try to work around,
+	 * calling tm_reclaim_current() to discard the footprint.
+	 *
+	 * The failure cause is not important as there will never be a
 	 * recheckpoint so it's not user visible.
 	 */
-	if (MSR_TM_SUSPENDED(mfmsr()))
+	if (WARN_ON(MSR_TM_SUSPENDED(mfmsr())))
 		tm_reclaim_current(0);
 
 	if (__get_user(msr, &uc->uc_mcontext.gp_regs[PT_MSR]))
-- 
2.19.0



More information about the Linuxppc-dev mailing list