public inbox for linux-arm-kernel@lists.infradead.org 
 help / color / mirror / Atom feed

From: wade_farnsworth@mentor•com (Wade Farnsworth)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v2] ARM: support syscall tracing
Date: Thu, 16 Aug 2012 07:33:45 -0700	[thread overview]
Message-ID: <502D04C9.2050501@mentor.com> (raw)
In-Reply-To: <20120816095431.GE31784@mudshark.cambridge.arm.com>

Will Deacon wrote:
> 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

I think you have to make a corresponding change in perf_syscall_exit, 
but yes, this looks right to me.

Thanks,

Wade

     prev parent reply	other threads:[~2012-08-16 14:33 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
2012-08-16 14:33             ` Wade Farnsworth [this message]

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=502D04C9.2050501@mentor.com \
    --to=wade_farnsworth@mentor$(echo .)com \
    --cc=linux-arm-kernel@lists$(echo .)infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox