Patchwork [3.11.y.z,extended,stable] Patch "powerpc/tm: Disable IRQ in tm_recheckpoint" has been added to staging queue

login
register
mail settings
Submitter Luis Henriques
Date May 14, 2014, 2:58 p.m.
Message ID <1400079481-16928-1-git-send-email-luis.henriques@canonical.com>
Download mbox | patch
Permalink /patch/348843/
State New
Headers show

Comments

Luis Henriques - May 14, 2014, 2:58 p.m.
This is a note to let you know that I have just added a patch titled

    powerpc/tm: Disable IRQ in tm_recheckpoint

to the linux-3.11.y-queue branch of the 3.11.y.z extended stable tree 
which can be found at:

 http://kernel.ubuntu.com/git?p=ubuntu/linux.git;a=shortlog;h=refs/heads/linux-3.11.y-queue

If you, or anyone else, feels it should not be added to this tree, please 
reply to this email.

For more information about the 3.11.y.z tree, see
https://wiki.ubuntu.com/Kernel/Dev/ExtendedStable

Thanks.
-Luis

------

From 909e546ae8486038c36bcd4507828693f1ac361e Mon Sep 17 00:00:00 2001
From: Michael Neuling <mikey@neuling.org>
Date: Fri, 4 Apr 2014 20:19:48 +1100
Subject: powerpc/tm: Disable IRQ in tm_recheckpoint

commit e6b8fd028b584ffca7a7255b8971f254932c9fce upstream.

We can't take an IRQ when we're about to do a trechkpt as our GPR state is set
to user GPR values.

We've hit this when running some IBM Java stress tests in the lab resulting in
the following dump:

  cpu 0x3f: Vector: 700 (Program Check) at [c000000007eb3d40]
      pc: c000000000050074: restore_gprs+0xc0/0x148
      lr: 00000000b52a8184
      sp: ac57d360
     msr: 8000000100201030
    current = 0xc00000002c500000
    paca    = 0xc000000007dbfc00     softe: 0     irq_happened: 0x00
      pid   = 34535, comm = Pooled Thread #
  R00 = 00000000b52a8184   R16 = 00000000b3e48fda
  R01 = 00000000ac57d360   R17 = 00000000ade79bd8
  R02 = 00000000ac586930   R18 = 000000000fac9bcc
  R03 = 00000000ade60000   R19 = 00000000ac57f930
  R04 = 00000000f6624918   R20 = 00000000ade79be8
  R05 = 00000000f663f238   R21 = 00000000ac218a54
  R06 = 0000000000000002   R22 = 000000000f956280
  R07 = 0000000000000008   R23 = 000000000000007e
  R08 = 000000000000000a   R24 = 000000000000000c
  R09 = 00000000b6e69160   R25 = 00000000b424cf00
  R10 = 0000000000000181   R26 = 00000000f66256d4
  R11 = 000000000f365ec0   R27 = 00000000b6fdcdd0
  R12 = 00000000f66400f0   R28 = 0000000000000001
  R13 = 00000000ada71900   R29 = 00000000ade5a300
  R14 = 00000000ac2185a8   R30 = 00000000f663f238
  R15 = 0000000000000004   R31 = 00000000f6624918
  pc  = c000000000050074 restore_gprs+0xc0/0x148
  cfar= c00000000004fe28 dont_restore_vec+0x1c/0x1a4
  lr  = 00000000b52a8184
  msr = 8000000100201030   cr  = 24804888
  ctr = 0000000000000000   xer = 0000000000000000   trap =  700

This moves tm_recheckpoint to a C function and moves the tm_restore_sprs into
that function.  It then adds IRQ disabling over the trechkpt critical section.
It also sets the TEXASR FS in the signals code to ensure this is never set now
that we explictly write the TM sprs in tm_recheckpoint.

Signed-off-by: Michael Neuling <mikey@neuling.org>
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
[ luis: backported to 3.11: used mikey's backport to 3.10, which picks
  the definition of TEXASR_FS from commit
  e4e38121507a27d2ccc4b28d9e7fc4818a12c44c ("KVM: PPC: Book3S HV: Add
  transactional memory support") ]
Signed-off-by: Luis Henriques <luis.henriques@canonical.com>
---
 arch/powerpc/include/asm/reg.h  |  1 +
 arch/powerpc/kernel/process.c   | 34 ++++++++++++++++++++++++++++------
 arch/powerpc/kernel/signal_32.c |  2 ++
 arch/powerpc/kernel/signal_64.c |  2 ++
 arch/powerpc/kernel/tm.S        |  2 +-
 5 files changed, 34 insertions(+), 7 deletions(-)

--
1.9.1
Mauricio Faria de Oliveira - May 21, 2014, 12:34 p.m.
Hi Mikey and Luis,

On 05/14/2014 11:58 AM, Luis Henriques wrote:
> This is a note to let you know that I have just added a patch titled
>
>      powerpc/tm: Disable IRQ in tm_recheckpoint
>
> to the linux-3.11.y-queue branch of the 3.11.y.z extended stable tree
> which can be found at:
 > [...]

This one seems to break a build with ubuntu-trusty.git's master-next.
Reverting it made the build to pass fine.

Maybe there's another commit to pull in?

Thanks.

> diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
> index 7e9dff8..81f929f 100644
> --- a/arch/powerpc/kernel/signal_32.c
> +++ b/arch/powerpc/kernel/signal_32.c
> @@ -863,6 +863,8 @@ static long restore_tm_user_regs(struct pt_regs *regs,
>   	 * 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, msr);
>   	/* Get the top half of the MSR */


$ fakeroot debian/rules binary-generic
[...]
   CC      arch/powerpc/kernel/signal_32.o
/home/test/hdrs/k/arch/powerpc/kernel/signal_32.c: In function 
‘restore_tm_user_regs’:
/home/test/hdrs/k/arch/powerpc/kernel/signal_32.c:885:37: error: 
‘TEXASR_FS’ undeclared (first use in this function)
   current->thread.tm_texasr |= TEXASR_FS;
                                      ^
/home/test/hdrs/k/arch/powerpc/kernel/signal_32.c:885:37: note: each 
undeclared identifier is reported only once for each function it appears in
make[3]: *** [arch/powerpc/kernel/signal_32.o] Error 1
Luis Henriques - May 21, 2014, 12:48 p.m.
Hi Mauricio,

(Adding Kamal to CC)

On Wed, May 21, 2014 at 09:34:51AM -0300, Mauricio Faria de Oliveira wrote:
> Hi Mikey and Luis,
> 
> On 05/14/2014 11:58 AM, Luis Henriques wrote:
> >This is a note to let you know that I have just added a patch titled
> >
> >     powerpc/tm: Disable IRQ in tm_recheckpoint
> >
> >to the linux-3.11.y-queue branch of the 3.11.y.z extended stable tree
> >which can be found at:
> > [...]
> 
> This one seems to break a build with ubuntu-trusty.git's master-next.
> Reverting it made the build to pass fine.
> 
> Maybe there's another commit to pull in?
> 
> Thanks.
>

Thank you for reporting this!

Actually, for the 3.11 stable kernel (that is pulled into Saucy), I've
picked Michael Neuling's backport for 3.10 that includes the
definition of TEXASR_FS from commit e4e38121507a27d2ccc4b28d9e7fc4818a12c44c
(which is what seems to be the cause of this build break.

Kamal, maybe you could pick this for the 3.13 kernel as well?

In the meantime, I'll fix ubuntu-trusty master-next branch.

Cheers,
--
Luís

> >diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
> >index 7e9dff8..81f929f 100644
> >--- a/arch/powerpc/kernel/signal_32.c
> >+++ b/arch/powerpc/kernel/signal_32.c
> >@@ -863,6 +863,8 @@ static long restore_tm_user_regs(struct pt_regs *regs,
> >  	 * 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, msr);
> >  	/* Get the top half of the MSR */
> 
> 
> $ fakeroot debian/rules binary-generic
> [...]
>   CC      arch/powerpc/kernel/signal_32.o
> /home/test/hdrs/k/arch/powerpc/kernel/signal_32.c: In function
> ‘restore_tm_user_regs’:
> /home/test/hdrs/k/arch/powerpc/kernel/signal_32.c:885:37: error:
> ‘TEXASR_FS’ undeclared (first use in this function)
>   current->thread.tm_texasr |= TEXASR_FS;
>                                      ^
> /home/test/hdrs/k/arch/powerpc/kernel/signal_32.c:885:37: note: each
> undeclared identifier is reported only once for each function it
> appears in
> make[3]: *** [arch/powerpc/kernel/signal_32.o] Error 1
> 
> 
> 
> -- 
> Mauricio Faria de Oliveira
> IBM Linux Technology Center
>

Patch

diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index 99222e2..bdb7c45 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -208,6 +208,7 @@ 
 #define SPRN_ACOP	0x1F	/* Available Coprocessor Register */
 #define SPRN_TFIAR	0x81	/* Transaction Failure Inst Addr   */
 #define SPRN_TEXASR	0x82	/* Transaction EXception & Summary */
+#define   TEXASR_FS	__MASK(63-36)	/* Transaction Failure Summary */
 #define SPRN_TEXASRU	0x83	/* ''	   ''	   ''	 Upper 32  */
 #define SPRN_TFHAR	0x80	/* Transaction Failure Handler Addr */
 #define SPRN_CTRLF	0x088
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 273e9ab..db9d554 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -523,6 +523,31 @@  out_and_saveregs:
 	tm_save_sprs(thr);
 }

+extern void __tm_recheckpoint(struct thread_struct *thread,
+			      unsigned long orig_msr);
+
+void tm_recheckpoint(struct thread_struct *thread,
+		     unsigned long orig_msr)
+{
+	unsigned long flags;
+
+	/* We really can't be interrupted here as the TEXASR registers can't
+	 * change and later in the trecheckpoint code, we have a userspace R1.
+	 * So let's hard disable over this region.
+	 */
+	local_irq_save(flags);
+	hard_irq_disable();
+
+	/* The TM SPRs are restored here, so that TEXASR.FS can be set
+	 * before the trecheckpoint and no explosion occurs.
+	 */
+	tm_restore_sprs(thread);
+
+	__tm_recheckpoint(thread, orig_msr);
+
+	local_irq_restore(flags);
+}
+
 static inline void tm_recheckpoint_new_task(struct task_struct *new)
 {
 	unsigned long msr;
@@ -541,13 +566,10 @@  static inline void tm_recheckpoint_new_task(struct task_struct *new)
 	if (!new->thread.regs)
 		return;

-	/* The TM SPRs are restored here, so that TEXASR.FS can be set
-	 * before the trecheckpoint and no explosion occurs.
-	 */
-	tm_restore_sprs(&new->thread);
-
-	if (!MSR_TM_ACTIVE(new->thread.regs->msr))
+	if (!MSR_TM_ACTIVE(new->thread.regs->msr)){
+		tm_restore_sprs(&new->thread);
 		return;
+	}
 	msr = new->thread.tm_orig_msr;
 	/* Recheckpoint to restore original checkpointed register state. */
 	TM_DEBUG("*** tm_recheckpoint of pid %d "
diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index 7e9dff8..81f929f 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -863,6 +863,8 @@  static long restore_tm_user_regs(struct pt_regs *regs,
 	 * 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, msr);
 	/* Get the top half of the MSR */
diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
index 35c20a1..74d9615 100644
--- a/arch/powerpc/kernel/signal_64.c
+++ b/arch/powerpc/kernel/signal_64.c
@@ -513,6 +513,8 @@  static long restore_tm_sigcontexts(struct pt_regs *regs,
 	}
 #endif
 	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, msr);

diff --git a/arch/powerpc/kernel/tm.S b/arch/powerpc/kernel/tm.S
index e3f8da3..df71307 100644
--- a/arch/powerpc/kernel/tm.S
+++ b/arch/powerpc/kernel/tm.S
@@ -306,7 +306,7 @@  dont_backup_fp:
 	 *	Call with IRQs off, stacks get all out of sync for
 	 *	some periods in here!
 	 */
-_GLOBAL(tm_recheckpoint)
+_GLOBAL(__tm_recheckpoint)
 	mfcr	r5
 	mflr	r0
 	std	r5, 8(r1)