From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751318AbbCTQGs (ORCPT ); Fri, 20 Mar 2015 12:06:48 -0400 Received: from casper.infradead.org ([85.118.1.10]:40617 "EHLO casper.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750823AbbCTQGp (ORCPT ); Fri, 20 Mar 2015 12:06:45 -0400 Date: Fri, 20 Mar 2015 13:06:40 -0300 From: Arnaldo Carvalho de Melo To: David Ahern Cc: linux-kernel@vger.kernel.org, Steven Rostedt , Namhyung Kim , Jiri Olsa Subject: Re: [PATCH] perf trace: Handle legacy syscalls Message-ID: <20150320160640.GL16485@kernel.org> References: <1426790181-19118-1-git-send-email-dsahern@gmail.com> <20150320153226.GI16485@kernel.org> <550C413A.2010301@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <550C413A.2010301@gmail.com> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Em Fri, Mar 20, 2015 at 09:48:10AM -0600, David Ahern escreveu: > On 3/20/15 9:32 AM, Arnaldo Carvalho de Melo wrote: > >But then I don't think we need to do that, i.e. we can just have a > >boolean we set at some point to tell that we need to skip the first > >entry. > > > >I'll try to cook up a patch for that. > > why have a boolean that is checked every time through the loop when its > value will always be the same for a give run? why not just remove the entry > as I suggested? First gut reaction was: hey, we got that from a library, libtraceevent, that could have other users, etc, better not to change it behind its back. I.e. something like this, based on your approach, but not modifying the data structures received from libtraceevent: diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c index 6af6bcec930e..001c6ae9a1b1 100644 --- a/tools/perf/builtin-trace.c +++ b/tools/perf/builtin-trace.c @@ -1135,6 +1135,8 @@ static struct syscall_fmt *syscall_fmt__find(const char *name) struct syscall { struct event_format *tp_format; + int nr_args; + struct format_field *args; const char *name; bool filtered; bool is_exit; @@ -1442,14 +1444,14 @@ static int syscall__set_arg_fmts(struct syscall *sc) struct format_field *field; int idx = 0; - sc->arg_scnprintf = calloc(sc->tp_format->format.nr_fields - 1, sizeof(void *)); + sc->arg_scnprintf = calloc(sc->nr_args, sizeof(void *)); if (sc->arg_scnprintf == NULL) return -1; if (sc->fmt) sc->arg_parm = sc->fmt->arg_parm; - for (field = sc->tp_format->format.fields->next; field; field = field->next) { + for (field = sc->args; field; field = field->next) { if (sc->fmt && sc->fmt->arg_scnprintf[idx]) sc->arg_scnprintf[idx] = sc->fmt->arg_scnprintf[idx]; else if (field->flags & FIELD_IS_POINTER) @@ -1515,6 +1517,14 @@ static int trace__read_syscall_info(struct trace *trace, int id) if (sc->tp_format == NULL) return -1; + sc->args = sc->tp_format->format.fields; + sc->nr_args = sc->tp_format->format.nr_fields; + /* drop nr field - not relevant here; does not exist on older kernels */ + if (sc->args && strcmp(sc->args->name, "nr") == 0) { + sc->args = sc->args->next; + --sc->nr_args; + } + sc->is_exit = !strcmp(name, "exit_group") || !strcmp(name, "exit"); return syscall__set_arg_fmts(sc); @@ -1537,7 +1547,7 @@ static size_t syscall__scnprintf_args(struct syscall *sc, char *bf, size_t size, unsigned char *p; unsigned long val; - if (sc->tp_format != NULL) { + if (sc->args != NULL) { struct format_field *field; u8 bit = 1; struct syscall_arg arg = { @@ -1547,7 +1557,7 @@ static size_t syscall__scnprintf_args(struct syscall *sc, char *bf, size_t size, .thread = thread, }; - for (field = sc->tp_format->format.fields->next; field; + for (field = sc->args; field; field = field->next, ++arg.idx, bit <<= 1) { if (arg.mask & bit) continue;