[PATCH v2] ARM: support syscall tracing

Wade Farnsworth wrote:

Will Deacon wrote:

quoted

On Wed, Aug 15, 2012 at 05:58:44PM +0100, Wade Farnsworth wrote:

quoted

We need to set current_thread_info()->syscall, since it's used in the
call to syscall_get_nr() in perf_syscall_{enter,exit}.

Damn. I think that also means we have a bug, given that the SYSCALL_TRACE
code can set this to -1, which gets used as an index into a bitmap by the
looks of it. Considering that we have to pass the syscall number to
trace_sys_enter anyway, it also seems broken.

I agree. Looking at the other architectures, it seems the analogous
function to ptrace_syscall_trace can return -1 under certain
circumstances, but the original syscall value should be passed onto
trace_sys_enter and returned from syscall_get_nr(). So, I'm thinking
that we should modify our behavior accordingly. What this means for us
is that we never store -1 in the thread_info syscall field, and then
pass that into trace_sys_enter instead of the ptrace_syscall_trace
return value. Do you see any problems with this approach?

Hmm, on closer inspection it looks that perf_syscall_enter is broken. 
ftrace_syscall_enter correctly returns if result of a syscall_get_nr is 
negative.  The perf version omits the check for negative values. 
Furthermore, the value of syscall_get_nr() should be -1 if it's not 
called when not in a syscall.  So storing the -1 in the thread_info 
syscall field is not necessarily wrong.  Still, I think the value we 
pass into trace_sys_enter should be the actual syscall number, i.e. the 
value of scno.

quoted

quoted

What about moving the setting of ->syscall to
syscall_trace_{enter,exit}? That would preserve ptrace_syscall_trace()
for the TIF_SYSCALL_TRACE case only, but ensure that the field is set
the TRACEPOINT case as well. Thoughts?

I'd be tempted to set the thing unconditionally before checking the
thread
flag at the top of ptrace_syscall_trace. This hangs off the slowpath
anyway
and it makes everything a lot more readable.

OK, I'll make that change.

Wade