Skip to contentSkip to conversation

Skip unnecessary pte makeup when clearing it

PATCH New bill4carson at gmail.com 3 participants 8 messages Jan 30, 2012 – Feb 3, 2012 Arm (32-bit)

lore
Share & tools
lore

bill4carson at gmail.com

author
#0 lore raw

PATCH v3 1/1 Skip unnecessary pte makeup when clearing it

If we are only about to clear a hardware pte entry, then pte makeup code is unnecessary for cpu_v7_set_pte_ext and armv6_set_pte_ext, so just skip it.

Signeed-off-by Bill Carson <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Uwe Kleine-K?nig <[email protected]>
Cc arm-mail-list <[email protected]>

Changes for V3:
- Fix double tab issue pointed out by Uwe Kleine-K?nig

Changes for V2:
- Use "1f" instead of "set_pte" as label
- Build/bootup test with thumb mode
- checkpatch script shows:
total: 0 errors, 0 warnings, 44 lines checked

./0001-Skip-unnecessary-pte-makeup-when-clearing-it.patch has no obvious style problems and is ready for submission.

 arch/arm/mm/proc-macros.S    |   10 +++++-----
 arch/arm/mm/proc-v7-2level.S |   10 +++++-----
 2 files changed, 10 insertions(+), 10 deletions(-)
author
#1 lore raw

PATCH 1/1 Skip unnecessary pte makeup when clearing it.

From: Bill Carson <[email protected]>

If we are only about to clear a hardware pte entry, then pte makeup code is unnecessary for cpu_v7_set_pte_ext and armv6_set_pte_ext, so just skip it.

Signed-off-by Bill Carson <[email protected]>
---
 arch/arm/mm/proc-macros.S    |   10 +++++-----
 arch/arm/mm/proc-v7-2level.S |   10 +++++-----
 2 files changed, 10 insertions(+), 10 deletions(-)
arch/arm/mm/proc-macros.S +5 -5
index 2d8ff3a..907b524 100644
@@ -138,6 +138,10 @@
 	.macro	armv6_set_pte_ext pfx
 	str	r1, [r0], #2048			@ linux version
 
+	tst	r1, #L_PTE_YOUNG
+	tstne	r1, #L_PTE_PRESENT
+	moveq	r3, #0
+	beq	1f
 	bic	r3, r1, #0x000003fc
 	bic	r3, r3, #PTE_TYPE_MASK
 	orr	r3, r3, r2

@@ -163,11 +167,7 @@
 	orrne	r3, r3, #PTE_EXT_XN
 
 	orr	r3, r3, r2
-
-	tst	r1, #L_PTE_YOUNG
-	tstne	r1, #L_PTE_PRESENT
-	moveq	r3, #0
-
+1:
 	str	r3, [r0]
 	mcr	p15, 0, r0, c7, c10, 1		@ flush_pte
 	.endm
arch/arm/mm/proc-v7-2level.S +5 -5
index 3a4b3e7..0ff5338 100644
@@ -76,6 +76,10 @@ ENTRY(cpu_v7_set_pte_ext)
 #ifdef CONFIG_MMU
 	str	r1, [r0]			@ linux version
 
+	tst	r1, #L_PTE_YOUNG
+	tstne	r1, #L_PTE_PRESENT
+	moveq	r3, #0
+	beq	1f
 	bic	r3, r1, #0x000003f0
 	bic	r3, r3, #PTE_TYPE_MASK
 	orr	r3, r3, r2

@@ -98,11 +102,7 @@ ENTRY(cpu_v7_set_pte_ext)
 
 	tst	r1, #L_PTE_XN
 	orrne	r3, r3, #PTE_EXT_XN
-
-	tst	r1, #L_PTE_YOUNG
-	tstne	r1, #L_PTE_PRESENT
-	moveq	r3, #0
-
+1:
  ARM(	str	r3, [r0, #2048]! )
  THUMB(	add	r0, r0, #2048 )
  THUMB(	str	r3, [r0] )
Signature · 1 line
1.7.1
#2 lore raw

PATCH 1/1 Skip unnecessary pte makeup when clearing it.

Hello,

Quoted 6 lines · On Mon, Jan 30, 2012 at 04:36:07PM +0800, bill4carson at gmail.com wrote:

From: Bill Carson <[email protected]>

If we are only about to clear a hardware pte entry, then pte makeup code is unnecessary for cpu_v7_set_pte_ext and armv6_set_pte_ext, so just skip it.

Signed-off-by Bill Carson <[email protected]>

I havn't tested and I don't know if the patch brings any advantages like increased speed. But AFAICT it doesn't change the behaviour of armv6_set_pte_ext and cpu_v7_set_pte_ext.

Best regards
Uwe

Signature · 2 lines
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
author
#3 lore raw

PATCH 1/1 Skip unnecessary pte makeup when clearing it.

Quoted 5 lines · Uwe Kleine-K?nig

Hello,

Quoted 2 lines · On Mon, Jan 30, 2012 at 04:36:07PM +0800, bill4carson at gmail.com wrote:

From: Bill Carson<[email protected]>

Quoted 8 lines
Quoted 4 lines

If we are only about to clear a hardware pte entry, then pte makeup code is unnecessary for cpu_v7_set_pte_ext and armv6_set_pte_ext, so just skip it.

Signed-off-by Bill Carson<[email protected]>

I havn't tested and I don't know if the patch brings any advantages like increased speed. But AFAICT it doesn't change the behaviour of armv6_set_pte_ext and cpu_v7_set_pte_ext.

Hi, Uwe

I'm sorry I didn't state the purpose of this patch clearly. As a matter of fact, it does change the behavior of set_pte_ext :)

Without this patch, the code path when:
set a pte: line 147->173, 176->181
clear a pte: line 147->174, 176->181

Point is line 147->173 takes a lot of cpu cycles to figure out the right r3, This is only used when set a pte, when clearing a pte, r3 *ALWAYS* has zero value! which means line 147->173 doesn't need to be executed in such case.

145ENTRY(cpu_v7_set_pte_ext)
146#ifdef CONFIG_MMU
147 str r1, [r0] @ linux version
148
149 bic r3, r1, #0x000003f0
150 bic r3, r3, #PTE_TYPE_MASK
151 orr r3, r3, r2
152 orr r3, r3, #PTE_EXT_AP0 | 2
153
154 tst r1, #1 << 4
155 orrne r3, r3, #PTE_EXT_TEX(1)
156
157 eor r1, r1, #L_PTE_DIRTY
158 tst r1, #L_PTE_RDONLY | L_PTE_DIRTY
159 orrne r3, r3, #PTE_EXT_APX
160
161 tst r1, #L_PTE_USER
162 orrne r3, r3, #PTE_EXT_AP1
163#ifdef CONFIG_CPU_USE_DOMAINS
164 @ allow kernel read/write access to read-only user pages 165 tstne r3, #PTE_EXT_APX
166 bicne r3, r3, #PTE_EXT_APX | PTE_EXT_AP0 167#endif
168
169 tst r1, #L_PTE_XN
170 orrne r3, r3, #PTE_EXT_XN
171
172 tst r1, #L_PTE_YOUNG
173 tstne r1, #L_PTE_PRESENT
174 moveq r3, #0
175
176 ARM( str r3, [r0, #2048]! )
177 THUMB( add r0, r0, #2048 )
178 THUMB( str r3, [r0] )
179 mcr p15, 0, r0, c7, c10, 1 @ flush_pte 180#endif
181 mov pc, lr
182ENDPROC(cpu_v7_set_pte_ext)

With this patch, the code path when:
set a pte: line 147->150, 153->181
clear a pte: line 147->152, 176->181

The code path when setting a pte does not change much at all. But code path of clearing a pte is significantly shorter than before, and performance enhancement is handy here.

145 ENTRY(cpu_v7_set_pte_ext)
146 #ifdef CONFIG_MMU
147 str r1, [r0] @ linux version
148
149 tst r1, #L_PTE_YOUNG
150 tstne r1, #L_PTE_PRESENT
151 moveq r3, #0
152 beq 1f
153 bic r3, r1, #0x000003f0
154 bic r3, r3, #PTE_TYPE_MASK
155 orr r3, r3, r2
156 orr r3, r3, #PTE_EXT_AP0 | 2
157
158 tst r1, #1 << 4
159 orrne r3, r3, #PTE_EXT_TEX(1)
160
161 eor r1, r1, #L_PTE_DIRTY
162 tst r1, #L_PTE_RDONLY | L_PTE_DIRTY
163 orrne r3, r3, #PTE_EXT_APX
164
165 tst r1, #L_PTE_USER
166 orrne r3, r3, #PTE_EXT_AP1
167 #ifdef CONFIG_CPU_USE_DOMAINS
168 @ allow kernel read/write access to read-only user pages 169 tstne r3, #PTE_EXT_APX
170 bicne r3, r3, #PTE_EXT_APX | PTE_EXT_AP0
171 #endif
172
173 tst r1, #L_PTE_XN
174 orrne r3, r3, #PTE_EXT_XN
175
176 1:
177 ARM( str r3, [r0, #2048]! )
178 THUMB( add r0, r0, #2048 )
179 THUMB( str r3, [r0] )
180 mcr p15, 0, r0, c7, c10, 1 @ flush_pte
181 #endif
182 mov pc, lr
ENDPROC(cpu_v7_set_pte_ext)

I hope the above explanation could justify this patch.

Regards
Bill

Quoted 3 lines

Best regards
Uwe

Signature · 4 lines
I am a slow learner
but I will keep trying to fight for my dreams!

--bill
author
#4 lore raw

PATCH 1/1 Skip unnecessary pte makeup when clearing it.

Quoted 27 lines · On 2012?02?03? 15:43, bill4carson wrote:
Quoted 10 lines · Uwe Kleine-K?nig

Hello,

Quoted 7 lines · On Mon, Jan 30, 2012 at 04:36:07PM +0800, bill4carson at gmail.com wrote:

From: Bill Carson<[email protected]>

If we are only about to clear a hardware pte entry, then pte makeup code is
unnecessary for cpu_v7_set_pte_ext and armv6_set_pte_ext, so just skip it.

Quoted 5 lines
Quoted 1 line
Signed-off-by Bill Carson<[email protected]>

I havn't tested and I don't know if the patch brings any advantages like increased speed. But AFAICT it doesn't change the behaviour of armv6_set_pte_ext and cpu_v7_set_pte_ext.

Hi, Uwe

I'm sorry I didn't state the purpose of this patch clearly. As a matter of fact, it does change the behavior of set_pte_ext :)

Without this patch, the code path when:
set a pte: line 147->173, 176->181
clear a pte: line 147->174, 176->181

Quoted 7 lines

Point is line 147->173 takes a lot of cpu cycles to figure out the right r3,
This is only used when set a pte, when clearing a pte, r3 *ALWAYS* has zero
value! which means line 147->173 doesn't need to be executed in such case.

to be precisely 149->170

Quoted 103 lines

145ENTRY(cpu_v7_set_pte_ext)
146#ifdef CONFIG_MMU
147 str r1, [r0] @ linux version
148
149 bic r3, r1, #0x000003f0
150 bic r3, r3, #PTE_TYPE_MASK
151 orr r3, r3, r2
152 orr r3, r3, #PTE_EXT_AP0 | 2
153
154 tst r1, #1 << 4
155 orrne r3, r3, #PTE_EXT_TEX(1)
156
157 eor r1, r1, #L_PTE_DIRTY
158 tst r1, #L_PTE_RDONLY | L_PTE_DIRTY
159 orrne r3, r3, #PTE_EXT_APX
160
161 tst r1, #L_PTE_USER
162 orrne r3, r3, #PTE_EXT_AP1
163#ifdef CONFIG_CPU_USE_DOMAINS
164 @ allow kernel read/write access to read-only user pages 165 tstne r3, #PTE_EXT_APX
166 bicne r3, r3, #PTE_EXT_APX | PTE_EXT_AP0 167#endif
168
169 tst r1, #L_PTE_XN
170 orrne r3, r3, #PTE_EXT_XN
171
172 tst r1, #L_PTE_YOUNG
173 tstne r1, #L_PTE_PRESENT
174 moveq r3, #0
175
176 ARM( str r3, [r0, #2048]! )
177 THUMB( add r0, r0, #2048 )
178 THUMB( str r3, [r0] )
179 mcr p15, 0, r0, c7, c10, 1 @ flush_pte 180#endif
181 mov pc, lr
182ENDPROC(cpu_v7_set_pte_ext)

With this patch, the code path when:
set a pte: line 147->150, 153->181
clear a pte: line 147->152, 176->181

The code path when setting a pte does not change much at all. But code path of clearing a pte is significantly shorter than before, and performance enhancement is handy here.

145 ENTRY(cpu_v7_set_pte_ext)
146 #ifdef CONFIG_MMU
147 str r1, [r0] @ linux version
148
149 tst r1, #L_PTE_YOUNG
150 tstne r1, #L_PTE_PRESENT
151 moveq r3, #0
152 beq 1f
153 bic r3, r1, #0x000003f0
154 bic r3, r3, #PTE_TYPE_MASK
155 orr r3, r3, r2
156 orr r3, r3, #PTE_EXT_AP0 | 2
157
158 tst r1, #1 << 4
159 orrne r3, r3, #PTE_EXT_TEX(1)
160
161 eor r1, r1, #L_PTE_DIRTY
162 tst r1, #L_PTE_RDONLY | L_PTE_DIRTY
163 orrne r3, r3, #PTE_EXT_APX
164
165 tst r1, #L_PTE_USER
166 orrne r3, r3, #PTE_EXT_AP1
167 #ifdef CONFIG_CPU_USE_DOMAINS
168 @ allow kernel read/write access to read-only user pages 169 tstne r3, #PTE_EXT_APX
170 bicne r3, r3, #PTE_EXT_APX | PTE_EXT_AP0
171 #endif
172
173 tst r1, #L_PTE_XN
174 orrne r3, r3, #PTE_EXT_XN
175
176 1:
177 ARM( str r3, [r0, #2048]! )
178 THUMB( add r0, r0, #2048 )
179 THUMB( str r3, [r0] )
180 mcr p15, 0, r0, c7, c10, 1 @ flush_pte
181 #endif
182 mov pc, lr
ENDPROC(cpu_v7_set_pte_ext)

I hope the above explanation could justify this patch.

Regards
Bill

Quoted 3 lines

Best regards
Uwe

Signature · 4 lines
I am a slow learner
but I will keep trying to fight for my dreams!

--bill
#5 lore raw

PATCH 1/1 Skip unnecessary pte makeup when clearing it.

In reply to bill4carson
Quoted 12 lines · On Fri, Feb 03, 2012 at 03:43:58PM +0800, bill4carson wrote:
Quoted 9 lines · Uwe Kleine-K?nig

Hello,

Quoted 6 lines · On Mon, Jan 30, 2012 at 04:36:07PM +0800, bill4carson at gmail.com wrote:

From: Bill Carson<[email protected]>

If we are only about to clear a hardware pte entry, then pte makeup code is unnecessary for cpu_v7_set_pte_ext and armv6_set_pte_ext, so just skip it.

Signed-off-by Bill Carson<[email protected]>
Quoted 8 lines
Quoted 4 lines

I havn't tested and I don't know if the patch brings any advantages like increased speed. But AFAICT it doesn't change the behaviour of armv6_set_pte_ext and cpu_v7_set_pte_ext.

Hi, Uwe

I'm sorry I didn't state the purpose of this patch clearly. As a matter of fact, it does change the behavior of set_pte_ext :)

Depends on what you call behaviour (and it's not the 'u' you dropped that makes a difference :-). I meant that the side effects don't change. It's only that they are accomplished in a different (probably more effective) way.

Best regards
Uwe

Signature · 2 lines
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
author
#6 lore raw

PATCH 1/1 Skip unnecessary pte makeup when clearing it.

Quoted 17 lines · Uwe Kleine-K?nig
Quoted 16 lines · On Fri, Feb 03, 2012 at 03:43:58PM +0800, bill4carson wrote:
Quoted 13 lines · Uwe Kleine-K?nig

Hello,

Quoted 6 lines · On Mon, Jan 30, 2012 at 04:36:07PM +0800, bill4carson at gmail.com wrote:

From: Bill Carson<[email protected]>

If we are only about to clear a hardware pte entry, then pte makeup code is unnecessary for cpu_v7_set_pte_ext and armv6_set_pte_ext, so just skip it.

Signed-off-by Bill Carson<[email protected]>

I havn't tested and I don't know if the patch brings any advantages like increased speed. But AFAICT it doesn't change the behaviour of armv6_set_pte_ext and cpu_v7_set_pte_ext.

Quoted 9 lines
Quoted 4 lines

Hi, Uwe

I'm sorry I didn't state the purpose of this patch clearly. As a matter of fact, it does change the behavior of set_pte_ext :)

Depends on what you call behaviour (and it's not the 'u' you dropped that makes a difference :-). I meant that the side effects don't change. It's only that they are accomplished in a different (probably more effective) way.

Thanks for your explanation, I'm getting what you mean now :)

Yes, from outside point of view, set_pte_ext provides exact function as before, from inside point of view, it will behave faster than before with this little modification. I see no reason why not do so.

Or am I missing something here?

Quoted 3 lines

Best regards
Uwe

Signature · 4 lines
I am a slow learner
but I will keep trying to fight for my dreams!

--bill
#7 lore raw

PATCH 1/1 Skip unnecessary pte makeup when clearing it.

In reply to bill4carson at gmail.com
Quoted 6 lines · On Mon, Jan 30, 2012 at 08:36:07AM +0000, bill4carson at gmail.com wrote:

From: Bill Carson <[email protected]>

If we are only about to clear a hardware pte entry, then pte makeup code is unnecessary for cpu_v7_set_pte_ext and armv6_set_pte_ext, so just skip it.

Signed-off-by Bill Carson <[email protected]>

I'll give similar answer to Uwe - the patch looks fine but I haven't tested or run any benchmarks to tell whether it's worth.

Anyway,

Acked-by Catalin Marinas <[email protected]>