powerpc: Update DSCR on all CPUs when writing sysfs dscr_default

PATCH New Anton Blanchard 1 participant 4 messages Sep 4, 2012 PowerPC

Share & tools

Anton Blanchard

author

2012-09-04 02:47 UTC #0 lore raw

PATCH 1/4 powerpc: Update DSCR on all CPUs when writing sysfs dscr_default

Writing to dscr_default in sysfs doesn't actually change the DSCR - we rely on a context switch on each CPU to do the work. There is no guarantee we will get a context switch in a reasonable amount of time so fire off an IPI to force an immediate change.

This issue was found with the following test case:

http://ozlabs.org/~anton/junkcode/dscr_explicit_test.c

Signed-off-by Anton Blanchard <[email protected]>

Cc <[email protected]> # 3.0+

---

arch/powerpc/kernel/sysfs.c +8 -0

Index: b/arch/powerpc/kernel/sysfs.c
===================================================================
@@ -194,6 +194,12 @@ static ssize_t show_dscr_default(struct
 	return sprintf(buf, "%lx\n", dscr_default);
 }
 
+static void update_dscr(void *dummy)
+{
+	if (!current->thread.dscr_inherit)
+		mtspr(SPRN_DSCR, dscr_default);
+}
+
 static ssize_t __used store_dscr_default(struct device *dev,
 		struct device_attribute *attr, const char *buf,
 		size_t count)

@@ -206,6 +212,8 @@ static ssize_t __used store_dscr_default
 		return -EINVAL;
 	dscr_default = val;
 
+	on_each_cpu(update_dscr, NULL, 1);
+
 	return count;
 }

Anton Blanchard

author

2012-09-04 02:48 UTC #1 lore raw

PATCH 2/4 powerpc: Keep thread.dscr and thread.dscr_inherit in sync

When we update the DSCR either via emulation of mtspr(DSCR) or via a change to dscr_default in sysfs we don't update thread.dscr. We will eventually update it at context switch time but there is a period where thread.dscr is incorrect.

If we fork at this point we will copy the old value of thread.dscr into the child. To avoid this, always keep thread.dscr in sync with reality.

This issue was found with the following testcase:

http://ozlabs.org/~anton/junkcode/dscr_inherit_test.c

Signed-off-by Anton Blanchard <[email protected]>

Cc <[email protected]> # 3.0+

---

arch/powerpc/kernel/traps.c +2 -1

Index: b/arch/powerpc/kernel/traps.c
===================================================================
@@ -972,8 +972,9 @@ static int emulate_instruction(struct pt
 			cpu_has_feature(CPU_FTR_DSCR)) {
 		PPC_WARN_EMULATED(mtdscr, regs);
 		rd = (instword >> 21) & 0x1f;
-		mtspr(SPRN_DSCR, regs->gpr[rd]);
+		current->thread.dscr = regs->gpr[rd];
 		current->thread.dscr_inherit = 1;
+		mtspr(SPRN_DSCR, current->thread.dscr);
 		return 0;
 	}
 #endif

arch/powerpc/kernel/sysfs.c +3 -1

Index: b/arch/powerpc/kernel/sysfs.c
===================================================================
@@ -196,8 +196,10 @@ static ssize_t show_dscr_default(struct
 
 static void update_dscr(void *dummy)
 {
-	if (!current->thread.dscr_inherit)
+	if (!current->thread.dscr_inherit) {
+		current->thread.dscr = dscr_default;
 		mtspr(SPRN_DSCR, dscr_default);
+	}
 }
 
 static ssize_t __used store_dscr_default(struct device *dev,

Anton Blanchard

author

2012-09-04 02:49 UTC #2 lore raw

PATCH 3/4 powerpc: Fix DSCR inheritance in copy_thread()

In reply to Anton Blanchard

If the default DSCR is non zero we set thread.dscr_inherit in copy_thread() meaning the new thread and all its children will ignore future updates to the default DSCR. This is not intended and is a change in behaviour that a number of our users have hit.

We just need to inherit thread.dscr and thread.dscr_inherit from the parent which ends up being much simpler.

This was found with the following test case:

http://ozlabs.org/~anton/junkcode/dscr_default_test.c

Signed-off-by Anton Blanchard <[email protected]>

Cc <[email protected]> # 3.0+

---

arch/powerpc/kernel/process.c +2 ~~-10~~

Index: b/arch/powerpc/kernel/process.c
===================================================================
@@ -802,16 +802,8 @@ int copy_thread(unsigned long clone_flag
 #endif /* CONFIG_PPC_STD_MMU_64 */
 #ifdef CONFIG_PPC64 
 	if (cpu_has_feature(CPU_FTR_DSCR)) {
-		if (current->thread.dscr_inherit) {
-			p->thread.dscr_inherit = 1;
-			p->thread.dscr = current->thread.dscr;
-		} else if (0 != dscr_default) {
-			p->thread.dscr_inherit = 1;
-			p->thread.dscr = dscr_default;
-		} else {
-			p->thread.dscr_inherit = 0;
-			p->thread.dscr = 0;
-		}
+		p->thread.dscr_inherit = current->thread.dscr_inherit;
+		p->thread.dscr = current->thread.dscr;
 	}
 #endif

Anton Blanchard

author

2012-09-04 02:51 UTC #3 lore raw

PATCH 4/4 powerpc: Restore correct DSCR in context switch

In reply to Anton Blanchard

During a context switch we always restore the per thread DSCR value. If we aren't doing explicit DSCR management
(ie thread.dscr_inherit == 0) and the default DSCR changed while the process has been sleeping we end up with the wrong value.

Check thread.dscr_inherit and select the default DSCR or per thread DSCR as required.

This was found with the following test case, when running with more threads than CPUs (ie forcing context switching):

http://ozlabs.org/~anton/junkcode/dscr_default_test.c

With the four patches applied I can run a combination of all test cases successfully at the same time:

http://ozlabs.org/~anton/junkcode/dscr_default_test.c
http://ozlabs.org/~anton/junkcode/dscr_explicit_test.c
http://ozlabs.org/~anton/junkcode/dscr_inherit_test.c

Signed-off-by Anton Blanchard <[email protected]>

Cc <[email protected]> # 3.0+

---

arch/powerpc/kernel/entry_64.S +17 -6

Index: b/arch/powerpc/kernel/entry_64.S
===================================================================
@@ -370,6 +370,12 @@ _GLOBAL(ret_from_fork)
 	li	r3,0
 	b	syscall_exit
 
+	.section	".toc","aw"
+DSCR_DEFAULT:
+	.tc dscr_default[TC],dscr_default
+
+	.section	".text"
+
 /*
  * This routine switches between two different tasks.  The process
  * state of one is saved on its kernel stack.  Then the state

@@ -509,9 +515,6 @@ END_MMU_FTR_SECTION_IFSET(MMU_FTR_1T_SEG
 	mr	r1,r8		/* start using new stack pointer */
 	std	r7,PACAKSAVE(r13)
 
-	ld	r6,_CCR(r1)
-	mtcrf	0xFF,r6
-
 #ifdef CONFIG_ALTIVEC
 BEGIN_FTR_SECTION
 	ld	r0,THREAD_VRSAVE(r4)

@@ -520,14 +523,22 @@ END_FTR_SECTION_IFSET(CPU_FTR_ALTIVEC)
 #endif /* CONFIG_ALTIVEC */
 #ifdef CONFIG_PPC64
 BEGIN_FTR_SECTION
+	lwz	r6,THREAD_DSCR_INHERIT(r4)
+	ld	r7,DSCR_DEFAULT@toc(2)
 	ld	r0,THREAD_DSCR(r4)
-	cmpd	r0,r25
-	beq	1f
+	cmpwi	r6,0
+	bne	1f
+	ld	r0,0(r7)
+1:	cmpd	r0,r25
+	beq	2f
 	mtspr	SPRN_DSCR,r0
-1:	
+2:
 END_FTR_SECTION_IFSET(CPU_FTR_DSCR)
 #endif
 
+	ld	r6,_CCR(r1)
+	mtcrf	0xFF,r6
+
 	/* r3-r13 are destroyed -- Cort */
 	REST_8GPRS(14, r1)
 	REST_10GPRS(22, r1)

arch/powerpc/kernel/asm-offsets.c +1 -0

Index: b/arch/powerpc/kernel/asm-offsets.c
===================================================================
@@ -76,6 +76,7 @@ int main(void)
 	DEFINE(SIGSEGV, SIGSEGV);
 	DEFINE(NMI_MASK, NMI_MASK);
 	DEFINE(THREAD_DSCR, offsetof(struct thread_struct, dscr));
+	DEFINE(THREAD_DSCR_INHERIT, offsetof(struct thread_struct, dscr_inherit));
 #else
 	DEFINE(THREAD_INFO, offsetof(struct task_struct, stack));
 #endif /* CONFIG_PPC64 */