From: will.deacon@arm.com (Will Deacon)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2] ARM: support syscall tracing
Date: Thu, 16 Aug 2012 10:54:31 +0100 [thread overview]
Message-ID: <20120816095431.GE31784@mudshark.cambridge.arm.com> (raw)
In-Reply-To: <502C04CF.2080409@mentor.com>
On Wed, Aug 15, 2012 at 09:21:35PM +0100, Wade Farnsworth wrote:
> Wade Farnsworth wrote:
> > Will Deacon wrote:
> >> On Wed, Aug 15, 2012 at 05:58:44PM +0100, Wade Farnsworth wrote:
> >>> 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.
Yes, that's what I was getting at with the -1 bitmap index. Something like
this should fix it though:
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index 96fc733..bbff120 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -506,13 +506,13 @@ static void perf_syscall_enter(void *ignore, struct pt_regs *regs, long id)
int size;
syscall_nr = syscall_get_nr(current, regs);
- if (!test_bit(syscall_nr, enabled_perf_enter_syscalls))
- return;
-
sys_data = syscall_nr_to_meta(syscall_nr);
if (!sys_data)
return;
+ if (!test_bit(syscall_nr, enabled_perf_enter_syscalls))
+ return;
+
/* get the size after alignment with the u32 buffer size field */
size = sizeof(unsigned long) * sys_data->nb_args + sizeof(*rec);
size = ALIGN(size + sizeof(u32), sizeof(u64));
If you're happy with that, I can post to LKML and see what people say.
Will
next prev parent reply other threads:[~2012-08-16 9:54 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-08-15 15:14 [PATCH v2] ARM: support syscall tracing Wade Farnsworth
2012-08-15 16:21 ` Will Deacon
2012-08-15 16:58 ` Wade Farnsworth
2012-08-15 17:27 ` Will Deacon
2012-08-15 19:35 ` Wade Farnsworth
2012-08-15 20:21 ` Wade Farnsworth
2012-08-16 9:54 ` Will Deacon [this message]
2012-08-16 14:33 ` Wade Farnsworth
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20120816095431.GE31784@mudshark.cambridge.arm.com \
--to=will.deacon@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.