linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Cyril Bur <cyrilbur@gmail.com>
To: linuxppc-dev@lists.ozlabs.org, wei.guo.simon@gmail.com
Cc: anton@samba.org, mikey@neuling.org
Subject: [PATCH v2 05/20] powerpc: Never giveup a reclaimed thread when enabling kernel {fp, altivec, vsx}
Date: Fri, 12 Aug 2016 09:28:04 +1000	[thread overview]
Message-ID: <20160811232819.11453-6-cyrilbur@gmail.com> (raw)
In-Reply-To: <20160811232819.11453-1-cyrilbur@gmail.com>

After a thread is reclaimed from its active or suspended transactional
state the checkpointed state exists on CPU, this state (along with the
live/transactional state) has been saved in its entirety by the
reclaiming process.

There exists a sequence of events that would cause the kernel to call
one of enable_kernel_fp(), enable_kernel_altivec() or
enable_kernel_vsx() after a thread has been reclaimed. These functions
save away any user state on the CPU so that the kernel can use the
registers. Not only is this saving away unnecessary at this point, it
is actually incorrect. It causes a save of the checkpointed state to
the live structures within the thread struct thus destroying the true
live state for that thread.

Signed-off-by: Cyril Bur <cyrilbur@gmail.com>
---
 arch/powerpc/kernel/process.c | 39 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 36 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 216cf05..0cfbc89 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -198,12 +198,23 @@ EXPORT_SYMBOL_GPL(flush_fp_to_thread);
 
 void enable_kernel_fp(void)
 {
+	unsigned long cpumsr;
+
 	WARN_ON(preemptible());
 
-	msr_check_and_set(MSR_FP);
+	cpumsr = msr_check_and_set(MSR_FP);
 
 	if (current->thread.regs && (current->thread.regs->msr & MSR_FP)) {
 		check_if_tm_restore_required(current);
+		/*
+		 * If a thread has already been reclaimed then the
+		 * checkpointed registers are on the CPU but have definitely
+		 * been saved by the reclaim code. Don't need to and *cannot*
+		 * giveup as this would save  to the 'live' structure not the
+		 * checkpointed structure.
+		 */
+		if(!MSR_TM_ACTIVE(cpumsr) && MSR_TM_ACTIVE(current->thread.regs->msr))
+			return;
 		__giveup_fpu(current);
 	}
 }
@@ -250,12 +261,23 @@ EXPORT_SYMBOL(giveup_altivec);
 
 void enable_kernel_altivec(void)
 {
+	unsigned long cpumsr;
+
 	WARN_ON(preemptible());
 
-	msr_check_and_set(MSR_VEC);
+	cpumsr = msr_check_and_set(MSR_VEC);
 
 	if (current->thread.regs && (current->thread.regs->msr & MSR_VEC)) {
 		check_if_tm_restore_required(current);
+		/*
+		 * If a thread has already been reclaimed then the
+		 * checkpointed registers are on the CPU but have definitely
+		 * been saved by the reclaim code. Don't need to and *cannot*
+		 * giveup as this would save  to the 'live' structure not the
+		 * checkpointed structure.
+		 */
+		if(!MSR_TM_ACTIVE(cpumsr) && MSR_TM_ACTIVE(current->thread.regs->msr))
+			return;
 		__giveup_altivec(current);
 	}
 }
@@ -324,12 +346,23 @@ static void save_vsx(struct task_struct *tsk)
 
 void enable_kernel_vsx(void)
 {
+	unsigned long cpumsr;
+
 	WARN_ON(preemptible());
 
-	msr_check_and_set(MSR_FP|MSR_VEC|MSR_VSX);
+	cpumsr = msr_check_and_set(MSR_FP|MSR_VEC|MSR_VSX);
 
 	if (current->thread.regs && (current->thread.regs->msr & MSR_VSX)) {
 		check_if_tm_restore_required(current);
+		/*
+		 * If a thread has already been reclaimed then the
+		 * checkpointed registers are on the CPU but have definitely
+		 * been saved by the reclaim code. Don't need to and *cannot*
+		 * giveup as this would save  to the 'live' structure not the
+		 * checkpointed structure.
+		 */
+		if(!MSR_TM_ACTIVE(cpumsr) && MSR_TM_ACTIVE(current->thread.regs->msr))
+			return;
 		if (current->thread.regs->msr & MSR_FP)
 			__giveup_fpu(current);
 		if (current->thread.regs->msr & MSR_VEC)
-- 
2.9.2

  parent reply	other threads:[~2016-08-11 23:29 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-11 23:27 [PATCH v2 00/20] Consistent TM structures Cyril Bur
2016-08-11 23:28 ` [PATCH v2 01/20] selftests/powerpc: Compile selftests against headers without AT_HWCAP2 Cyril Bur
2016-08-11 23:28 ` [PATCH v2 02/20] powerpc: Always restore FPU/VEC/VSX if hardware transactional memory in use Cyril Bur
2016-08-12  3:01   ` kbuild test robot
2016-08-11 23:28 ` [PATCH v2 03/20] powerpc: Add check_if_tm_restore_required() to giveup_all() Cyril Bur
2016-08-11 23:28 ` [PATCH v2 04/20] powerpc: Return the new MSR from msr_check_and_set() Cyril Bur
2016-08-11 23:28 ` Cyril Bur [this message]
2016-08-11 23:28 ` [PATCH v2 06/20] selftests/powerpc: Check for VSX preservation across userspace preemption Cyril Bur
2016-08-11 23:28 ` [PATCH v2 07/20] selftests/powerpc: Rework FPU stack placement macros and move to header file Cyril Bur
2016-08-11 23:28 ` [PATCH v2 08/20] selftests/powerpc: Move VMX stack frame macros " Cyril Bur
2016-08-11 23:28 ` [PATCH v2 09/20] selftests/powerpc: Introduce GPR asm helper " Cyril Bur
2016-08-11 23:28 ` [PATCH v2 10/20] selftests/powerpc: Add transactional memory defines Cyril Bur
2016-08-11 23:28 ` [PATCH v2 11/20] selftests/powerpc: Allow tests to extend their kill timeout Cyril Bur
2016-08-11 23:28 ` [PATCH v2 12/20] selftests/powerpc: Add TM tcheck helpers in C Cyril Bur
2016-08-11 23:28 ` [PATCH v2 13/20] selftests/powerpc: Check that signals always get delivered Cyril Bur
2016-08-11 23:28 ` [PATCH v2 14/20] selftests/powerpc: Add checks for transactional GPRs in signal contexts Cyril Bur
2016-08-11 23:28 ` [PATCH v2 15/20] selftests/powerpc: Add checks for transactional FPUs " Cyril Bur
2016-08-11 23:28 ` [PATCH v2 16/20] selftests/powerpc: Add checks for transactional VMXs " Cyril Bur
2016-08-11 23:28 ` [PATCH v2 17/20] selftests/powerpc: Add checks for transactional VSXs " Cyril Bur
2016-08-11 23:28 ` [PATCH v2 18/20] powerpc: tm: Always use fp_state and vr_state to store live registers Cyril Bur
2016-08-14  1:56   ` Simon Guo
2016-08-15  7:25     ` Cyril Bur
2016-08-15  9:42       ` Simon Guo
2016-08-15 23:03         ` Cyril Bur
2016-08-11 23:28 ` [PATCH v2 19/20] powerpc: tm: Rename transct_(*) to ck(\1)_state Cyril Bur
2016-08-11 23:28 ` [PATCH v2 20/20] powerpc: Remove do_load_up_transact_{fpu,altivec} Cyril Bur

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160811232819.11453-6-cyrilbur@gmail.com \
    --to=cyrilbur@gmail.com \
    --cc=anton@samba.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mikey@neuling.org \
    --cc=wei.guo.simon@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).