Re: [PATCHv2 2/2] tracing,mm - add kernel pagefault tracepoint for x86 & x86_64

[Steven Rostedt <rostedt@goodmis.org>]

On Mon, 2010-11-15 at 14:43 +0100, Frederic Weisbecker wrote:


> dotraplinkage void __kprobes
> do_page_fault(struct pt_regs *regs, unsigned long error_code)
> {
> 	unsigned long address;
> 
> 	/* Get the faulting address: */
> 	address = read_cr2();
> 
> 	trace_mm_pagefault_start(address, error_code);
> 	__do_page_fault(regs, address, error_code);
> 	trace_mm_pagefault_end(address);
> }
> 
> 
> Would you be ok with that?
> 
> Last thing I worry about is that error_code that is very arch dependent.
> If someone writes a script that depends on the x86 code, it won't work
> elsewhere while it's fairly possible to have a generic tracepoint there.
> 
> So perhaps we rather need a generic enum field instead of the error_code,
> to express the most interesting specific fault attributes. Than can
> probably be added later though, once someone really needs it.
> 
> Hm?

Perhaps we should have:

	trace_arch_mm_pagefault_start(address, error_code);
	__do_page_fault(regs, address, error_code);
	trace_mm_pagefault_end(address);


Then we have a arch/x86/kernel/trace.c that can map trace_arch_...
events to generic events. This file will hold the
trace_mm_pagefault_start(), which is called by
trace_arch_mm_pagefault_start(). Have a hook that when the
trace_mm_pagefault_start() is enabled, we also enable
trace_arch_mm_pagefault_start() that calls this tracepoint. The
trace_mm_pagefault_start() will then translate the
trace_arch_mm_pagfault_start() into the generic trace_mm_pagefault_start
event with the generic error_code that all archs will use.

Reason being, we don't need to do any extra processing in the fast path
when tracing is not enabled.

Also, I'm going to start working on the stable ABI today.

-- Steve