* [PATCH powerpc] fix unpaired __trace_hcall_entry and __trace_hcall_exit
@ 2011-12-19 2:06 Li Zhong
2012-01-03 0:54 ` Benjamin Herrenschmidt
0 siblings, 1 reply; 3+ messages in thread
From: Li Zhong @ 2011-12-19 2:06 UTC (permalink / raw)
To: LKML; +Cc: Paul E. McKenney, Paul Mackerras, PowerPC email list,
Anton Blanchard
Unpaired calling of __trace_hcall_entry and __trace_hcall_exit could
cause incorrect preempt count. And it might happen as the global
variable hcall_tracepoint_refcount is checked separately before calling
them.
I don't know much about the powerpc arch. But the idea here is to store
the hcall_tracepoint_refcount locally, so __trace_hcall_entry and
__trace_hcall_exit will be called or not called in pair by checking the
same value.
Reported-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Signed-off-by: Li Zhong <zhong@linux.vnet.ibm.com>
Tested-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
arch/powerpc/platforms/pseries/hvCall.S | 20 +++++++++++---------
1 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/hvCall.S b/arch/powerpc/platforms/pseries/hvCall.S
index fd05fde..1240bd2 100644
--- a/arch/powerpc/platforms/pseries/hvCall.S
+++ b/arch/powerpc/platforms/pseries/hvCall.S
@@ -14,6 +14,7 @@
#include <asm/ptrace.h>
#define STK_PARM(i) (48 + ((i)-3)*8)
+#define REG_SIZE (2*8)
#ifdef CONFIG_TRACEPOINTS
@@ -32,11 +33,12 @@ hcall_tracepoint_refcount:
* unconditional cpu feature.
*/
#define HCALL_INST_PRECALL(FIRST_REG) \
+ std r31,-8(r1); \
BEGIN_FTR_SECTION; \
b 1f; \
END_FTR_SECTION(0, 1); \
- ld r12,hcall_tracepoint_refcount@toc(r2); \
- cmpdi r12,0; \
+ ld r31,hcall_tracepoint_refcount@toc(r2); \
+ cmpdi r31,0; \
beq+ 1f; \
mflr r0; \
std r3,STK_PARM(r3)(r1); \
@@ -49,9 +51,9 @@ END_FTR_SECTION(0, 1); \
std r10,STK_PARM(r10)(r1); \
std r0,16(r1); \
addi r4,r1,STK_PARM(FIRST_REG); \
- stdu r1,-STACK_FRAME_OVERHEAD(r1); \
+ stdu r1,-STACK_FRAME_OVERHEAD-REG_SIZE(r1); \
bl .__trace_hcall_entry; \
- addi r1,r1,STACK_FRAME_OVERHEAD; \
+ addi r1,r1,STACK_FRAME_OVERHEAD+REG_SIZE; \
ld r0,16(r1); \
ld r3,STK_PARM(r3)(r1); \
ld r4,STK_PARM(r4)(r1); \
@@ -74,8 +76,7 @@ END_FTR_SECTION(0, 1); \
BEGIN_FTR_SECTION; \
b 1f; \
END_FTR_SECTION(0, 1); \
- ld r12,hcall_tracepoint_refcount@toc(r2); \
- cmpdi r12,0; \
+ cmpdi r31,0; \
beq+ 1f; \
mflr r0; \
ld r6,STK_PARM(r3)(r1); \
@@ -83,13 +84,14 @@ END_FTR_SECTION(0, 1); \
mr r4,r3; \
mr r3,r6; \
std r0,16(r1); \
- stdu r1,-STACK_FRAME_OVERHEAD(r1); \
+ stdu r1,-STACK_FRAME_OVERHEAD-REG_SIZE(r1); \
bl .__trace_hcall_exit; \
- addi r1,r1,STACK_FRAME_OVERHEAD; \
+ addi r1,r1,STACK_FRAME_OVERHEAD+REG_SIZE; \
ld r0,16(r1); \
ld r3,STK_PARM(r3)(r1); \
mtlr r0; \
-1:
+1: \
+ ld r31,-8(r1);
#define HCALL_INST_POSTCALL_NORETS \
li r5,0; \
--
1.7.5.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH powerpc] fix unpaired __trace_hcall_entry and __trace_hcall_exit
2011-12-19 2:06 [PATCH powerpc] fix unpaired __trace_hcall_entry and __trace_hcall_exit Li Zhong
@ 2012-01-03 0:54 ` Benjamin Herrenschmidt
2012-01-08 3:54 ` [PATCH v2 " Li Zhong
0 siblings, 1 reply; 3+ messages in thread
From: Benjamin Herrenschmidt @ 2012-01-03 0:54 UTC (permalink / raw)
To: Li Zhong
Cc: PowerPC email list, Paul E. McKenney, Paul Mackerras, LKML,
Anton Blanchard
On Mon, 2011-12-19 at 10:06 +0800, Li Zhong wrote:
> Unpaired calling of __trace_hcall_entry and __trace_hcall_exit could
> cause incorrect preempt count. And it might happen as the global
> variable hcall_tracepoint_refcount is checked separately before calling
> them.
>
> I don't know much about the powerpc arch. But the idea here is to store
> the hcall_tracepoint_refcount locally, so __trace_hcall_entry and
> __trace_hcall_exit will be called or not called in pair by checking the
> same value.
Can you re-use an existing spot in the stack frame rather than adding
this entry ? There's plenty of unused spots :-) For example the link
editor doubleword is never going to be used in that function, we could
hijack it safely. It would make the patch (and the code) simpler.
Cheers,
Ben.
> Reported-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Signed-off-by: Li Zhong <zhong@linux.vnet.ibm.com>
> Tested-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> ---
> arch/powerpc/platforms/pseries/hvCall.S | 20 +++++++++++---------
> 1 files changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/hvCall.S b/arch/powerpc/platforms/pseries/hvCall.S
> index fd05fde..1240bd2 100644
> --- a/arch/powerpc/platforms/pseries/hvCall.S
> +++ b/arch/powerpc/platforms/pseries/hvCall.S
> @@ -14,6 +14,7 @@
> #include <asm/ptrace.h>
>
> #define STK_PARM(i) (48 + ((i)-3)*8)
> +#define REG_SIZE (2*8)
>
> #ifdef CONFIG_TRACEPOINTS
>
> @@ -32,11 +33,12 @@ hcall_tracepoint_refcount:
> * unconditional cpu feature.
> */
> #define HCALL_INST_PRECALL(FIRST_REG) \
> + std r31,-8(r1); \
> BEGIN_FTR_SECTION; \
> b 1f; \
> END_FTR_SECTION(0, 1); \
> - ld r12,hcall_tracepoint_refcount@toc(r2); \
> - cmpdi r12,0; \
> + ld r31,hcall_tracepoint_refcount@toc(r2); \
> + cmpdi r31,0; \
> beq+ 1f; \
> mflr r0; \
> std r3,STK_PARM(r3)(r1); \
> @@ -49,9 +51,9 @@ END_FTR_SECTION(0, 1); \
> std r10,STK_PARM(r10)(r1); \
> std r0,16(r1); \
> addi r4,r1,STK_PARM(FIRST_REG); \
> - stdu r1,-STACK_FRAME_OVERHEAD(r1); \
> + stdu r1,-STACK_FRAME_OVERHEAD-REG_SIZE(r1); \
> bl .__trace_hcall_entry; \
> - addi r1,r1,STACK_FRAME_OVERHEAD; \
> + addi r1,r1,STACK_FRAME_OVERHEAD+REG_SIZE; \
> ld r0,16(r1); \
> ld r3,STK_PARM(r3)(r1); \
> ld r4,STK_PARM(r4)(r1); \
> @@ -74,8 +76,7 @@ END_FTR_SECTION(0, 1); \
> BEGIN_FTR_SECTION; \
> b 1f; \
> END_FTR_SECTION(0, 1); \
> - ld r12,hcall_tracepoint_refcount@toc(r2); \
> - cmpdi r12,0; \
> + cmpdi r31,0; \
> beq+ 1f; \
> mflr r0; \
> ld r6,STK_PARM(r3)(r1); \
> @@ -83,13 +84,14 @@ END_FTR_SECTION(0, 1); \
> mr r4,r3; \
> mr r3,r6; \
> std r0,16(r1); \
> - stdu r1,-STACK_FRAME_OVERHEAD(r1); \
> + stdu r1,-STACK_FRAME_OVERHEAD-REG_SIZE(r1); \
> bl .__trace_hcall_exit; \
> - addi r1,r1,STACK_FRAME_OVERHEAD; \
> + addi r1,r1,STACK_FRAME_OVERHEAD+REG_SIZE; \
> ld r0,16(r1); \
> ld r3,STK_PARM(r3)(r1); \
> mtlr r0; \
> -1:
> +1: \
> + ld r31,-8(r1);
>
> #define HCALL_INST_POSTCALL_NORETS \
> li r5,0; \
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2 powerpc] fix unpaired __trace_hcall_entry and __trace_hcall_exit
2012-01-03 0:54 ` Benjamin Herrenschmidt
@ 2012-01-08 3:54 ` Li Zhong
0 siblings, 0 replies; 3+ messages in thread
From: Li Zhong @ 2012-01-08 3:54 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: PowerPC email list, Paul E. McKenney, Paul Mackerras, LKML,
Anton Blanchard
On Tue, 2012-01-03 at 11:54 +1100, Benjamin Herrenschmidt wrote:
> On Mon, 2011-12-19 at 10:06 +0800, Li Zhong wrote:
> > Unpaired calling of __trace_hcall_entry and __trace_hcall_exit could
> > cause incorrect preempt count. And it might happen as the global
> > variable hcall_tracepoint_refcount is checked separately before calling
> > them.
> >
> > I don't know much about the powerpc arch. But the idea here is to store
> > the hcall_tracepoint_refcount locally, so __trace_hcall_entry and
> > __trace_hcall_exit will be called or not called in pair by checking the
> > same value.
>
> Can you re-use an existing spot in the stack frame rather than adding
> this entry ? There's plenty of unused spots :-) For example the link
> editor doubleword is never going to be used in that function, we could
> hijack it safely. It would make the patch (and the code) simpler.
>
> Cheers,
> Ben.
Hi Ben,
Thank you for the suggestion. Below is the updated version, much less
code changes :) Would you please help review again to see whether my
understanding is correct?
Thanks,
Zhong
Reported-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Signed-off-by: Li Zhong <zhong@linux.vnet.ibm.com>
Tested-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
arch/powerpc/platforms/pseries/hvCall.S | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/hvCall.S
b/arch/powerpc/platforms/pseries/hvCall.S
index fd05fde..3ce73d0 100644
--- a/arch/powerpc/platforms/pseries/hvCall.S
+++ b/arch/powerpc/platforms/pseries/hvCall.S
@@ -36,6 +36,7 @@ BEGIN_FTR_SECTION; \
b 1f; \
END_FTR_SECTION(0, 1); \
ld r12,hcall_tracepoint_refcount@toc(r2); \
+ std r12,32(r1); \
cmpdi r12,0; \
beq+ 1f; \
mflr r0; \
@@ -74,7 +75,7 @@ END_FTR_SECTION(0, 1); \
BEGIN_FTR_SECTION; \
b 1f; \
END_FTR_SECTION(0, 1); \
- ld r12,hcall_tracepoint_refcount@toc(r2); \
+ ld r12,32(r1); \
cmpdi r12,0; \
beq+ 1f; \
mflr r0; \
--
1.7.5.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-01-08 3:55 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-12-19 2:06 [PATCH powerpc] fix unpaired __trace_hcall_entry and __trace_hcall_exit Li Zhong
2012-01-03 0:54 ` Benjamin Herrenschmidt
2012-01-08 3:54 ` [PATCH v2 " Li Zhong
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).