Re: [PATCH] ppc64 ftrace: mark data_access callees "notrace" (pt.1)

[Torsten Duwe]

On Wed, May 20, 2015 at 11:03:25AM +0200, Torsten Duwe wrote:
> On Tue, May 19, 2015 at 01:27:07PM +1000, Michael Ellerman wrote:
> > On Mon, 2015-05-18 at 14:29 +0200, Jiri Kosina wrote:
> > > 
> > > ftrace already handles recursion protection by itself (depending on the 
> > > per-ftrace-ops FTRACE_OPS_FL_RECURSION_SAFE flag).
> > 
> > OK, so I wonder why that's not working for us?
> 
> IIRC a data access fault happens just before that flag is looked at ;-)
> 
> I'm now thinking about a hybrid solution: mark the most critical functions
> "notrace", especially those directly involved with MMU loading, and add
> a per-thread flag to catch the not-so-obvious cases.

I realised the trace_recursion in the "current" task struct is not so far away,
and it should not fault, right? So that part of the solution would look like this
on top of my previous ftrace patch set.
It would impact performance further so I'd stick with the "notrace" on
certain hot functions, like hash_page. What do you think?

	Torsten

diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index 4717859..ae10752 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -72,6 +72,7 @@ int main(void)
 	DEFINE(THREAD, offsetof(struct task_struct, thread));
 	DEFINE(MM, offsetof(struct task_struct, mm));
 	DEFINE(MMCONTEXTID, offsetof(struct mm_struct, context.id));
+	DEFINE(TASK_TRACEREC, offsetof(struct task_struct, trace_recursion));
 #ifdef CONFIG_PPC64
 	DEFINE(AUDITCONTEXT, offsetof(struct task_struct, audit_context));
 	DEFINE(SIGSEGV, SIGSEGV);
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index a4132ef..7842092 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -1202,7 +1202,13 @@ _GLOBAL(ftrace_caller)
 	SAVE_8GPRS(16,r1)
 	SAVE_8GPRS(24,r1)
 
-
+	ld	r3, PACACURRENT(r13)
+	ld	r4, TASK_TRACEREC(r3)
+	andi.	r5, r4, 0x0010 // ( 1 << TRACE_FTRACE_BIT )
+	ori	r4, r4, 0x0010
+	std	r4, TASK_TRACEREC(r3)
+	bne	3f		// ftrace in progress - avoid recursion!
+	
 	LOAD_REG_IMMEDIATE(r3,function_trace_op)
 	ld	r5,0(r3)
 
@@ -1224,9 +1230,14 @@ ftrace_call:
 	bl	ftrace_stub
 	nop
 
+	ld	r3, PACACURRENT(r13)
+	ld	r4, TASK_TRACEREC(r3)
+	andi	r4, r4, 0xffef // ~( 1 << TRACE_FTRACE_BIT )
+	std	r4, TASK_TRACEREC(r3)
+
 	ld	r3, _NIP(r1)
 	mtlr	r3
-
+3:
 	REST_8GPRS(0,r1)
 	REST_8GPRS(8,r1)
 	REST_8GPRS(16,r1)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/