2011-02-07 15:01:05

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 0/9] perf/core improvements and fixes

Hi Ingo,

Please consider pulling from:

git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux-2.6 perf/core

Regards,

- Arnaldo

Borislav Petkov (1):
perf annotate: Fix build error

Denis Kirjanov (1):
perf top: Use pid_t for target_{pid|tid}

Franck Bui-Huu (1):
perf probe: Rewrite find_lazy_match_lines() by using getline(3)

Kyle McMartin (1):
perf tool: Fix gcc 4.6.0 issues

Masami Hiramatsu (4):
tracing/kprobes: Cleanup strict_strtol() using code
tracing/kprobes: Support longer (>128 bytes) command
tracing/kprobes: Add bitfield type
perf probe: Add bitfield member support

Michael Witten (1):
perf tools: Makefile: Use $(QUIET_GEN) for perf.so

Documentation/trace/kprobetrace.txt | 16 ++-
kernel/trace/trace_kprobe.c | 111 +++++++++++++-
tools/perf/Makefile | 2 +-
tools/perf/bench/sched-pipe.c | 2 +-
tools/perf/builtin-sched.c | 12 +-
tools/perf/builtin-top.c | 2 +-
tools/perf/util/annotate.c | 3 -
tools/perf/util/annotate.h | 2 +-
tools/perf/util/header.c | 2 +-
tools/perf/util/probe-finder.c | 165 +++++++++++--------
.../util/scripting-engines/trace-event-python.c | 3 +-
tools/perf/util/symbol.c | 4 +-
tools/perf/util/top.h | 4 +-
tools/perf/util/trace-event-parse.c | 2 +-
tools/perf/util/ui/browsers/map.c | 2 +-
15 files changed, 231 insertions(+), 101 deletions(-)


2011-02-07 15:00:21

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 1/9] perf tools: Makefile: Use $(QUIET_GEN) for perf.so

From: Michael Witten <[email protected]>

So that we get this:

CC /home/acme/git/build/perf/bench/mem-memcpy-x86-64-asm.o
GEN perf-archive
* GEN /home/acme/git/build/perf/python/perf.so
CC /home/acme/git/build/perf/builtin-annotate.o

Instead of silently building the python binding.

LKML-Reference: <[email protected]>
Signed-off-by: Michael Witten <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/Makefile | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index be3eb1d..94f73ab 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -326,7 +326,7 @@ grep-libs = $(filter -l%,$(1))
strip-libs = $(filter-out -l%,$(1))

$(OUTPUT)python/perf.so: $(PYRF_OBJS)
- @python util/setup.py --quiet build_ext --build-lib='$(OUTPUT)python' \
+ $(QUIET_GEN)python util/setup.py --quiet build_ext --build-lib='$(OUTPUT)python' \
--build-temp='$(OUTPUT)python/temp'
#
# No Perl scripts right now:
--
1.6.2.5

2011-02-07 15:00:33

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 6/9] tracing/kprobes: Add bitfield type

From: Masami Hiramatsu <[email protected]>

Add bitfield type for tracing arguments on kprobe-tracer. The syntax of
a bitfield type is:

b<bit-size>@<bit-offset>/<container-size>

e.g.

Accessing 2 bits-width field with 4 bits-offset in 32 bits-width data at
4 bytes offseted from the address pointed by AX register:

+4(%ax):b2@4/32

Since the width of container data depends on the arch, so I just added
the container-size at the end.

Cc: [email protected]
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: Steven Rostedt <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Masami Hiramatsu <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
Documentation/trace/kprobetrace.txt | 16 +++++-
kernel/trace/trace_kprobe.c | 104 ++++++++++++++++++++++++++++++++++-
2 files changed, 118 insertions(+), 2 deletions(-)

diff --git a/Documentation/trace/kprobetrace.txt b/Documentation/trace/kprobetrace.txt
index 5f77d94..6d27ab8 100644
--- a/Documentation/trace/kprobetrace.txt
+++ b/Documentation/trace/kprobetrace.txt
@@ -42,11 +42,25 @@ Synopsis of kprobe_events
+|-offs(FETCHARG) : Fetch memory at FETCHARG +|- offs address.(**)
NAME=FETCHARG : Set NAME as the argument name of FETCHARG.
FETCHARG:TYPE : Set TYPE as the type of FETCHARG. Currently, basic types
- (u8/u16/u32/u64/s8/s16/s32/s64) and string are supported.
+ (u8/u16/u32/u64/s8/s16/s32/s64), "string" and bitfield
+ are supported.

(*) only for return probe.
(**) this is useful for fetching a field of data structures.

+Types
+-----
+Several types are supported for fetch-args. Kprobe tracer will access memory
+by given type. Prefix 's' and 'u' means those types are signed and unsigned
+respectively. Traced arguments are shown in decimal (signed) or hex (unsigned).
+String type is a special type, which fetches a "null-terminated" string from
+kernel space. This means it will fail and store NULL if the string container
+has been paged out.
+Bitfield is another special type, which takes 3 parameters, bit-width, bit-
+offset, and container-size (usually 32). The syntax is;
+
+ b<bit-width>@<bit-offset>/<container-size>
+

Per-Probe Event Filtering
-------------------------
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index c6ed886..ccdc542 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -353,6 +353,43 @@ static __kprobes void free_deref_fetch_param(struct deref_fetch_param *data)
kfree(data);
}

+/* Bitfield fetch function */
+struct bitfield_fetch_param {
+ struct fetch_param orig;
+ unsigned char hi_shift;
+ unsigned char low_shift;
+};
+
+#define DEFINE_FETCH_bitfield(type) \
+static __kprobes void FETCH_FUNC_NAME(bitfield, type)(struct pt_regs *regs,\
+ void *data, void *dest) \
+{ \
+ struct bitfield_fetch_param *bprm = data; \
+ type buf = 0; \
+ call_fetch(&bprm->orig, regs, &buf); \
+ if (buf) { \
+ buf <<= bprm->hi_shift; \
+ buf >>= bprm->low_shift; \
+ } \
+ *(type *)dest = buf; \
+}
+DEFINE_BASIC_FETCH_FUNCS(bitfield)
+#define fetch_bitfield_string NULL
+#define fetch_bitfield_string_size NULL
+
+static __kprobes void
+free_bitfield_fetch_param(struct bitfield_fetch_param *data)
+{
+ /*
+ * Don't check the bitfield itself, because this must be the
+ * last fetch function.
+ */
+ if (CHECK_FETCH_FUNCS(deref, data->orig.fn))
+ free_deref_fetch_param(data->orig.data);
+ else if (CHECK_FETCH_FUNCS(symbol, data->orig.fn))
+ free_symbol_cache(data->orig.data);
+ kfree(data);
+}
/* Default (unsigned long) fetch type */
#define __DEFAULT_FETCH_TYPE(t) u##t
#define _DEFAULT_FETCH_TYPE(t) __DEFAULT_FETCH_TYPE(t)
@@ -367,6 +404,7 @@ enum {
FETCH_MTD_memory,
FETCH_MTD_symbol,
FETCH_MTD_deref,
+ FETCH_MTD_bitfield,
FETCH_MTD_END,
};

@@ -387,6 +425,7 @@ ASSIGN_FETCH_FUNC(retval, ftype), \
ASSIGN_FETCH_FUNC(memory, ftype), \
ASSIGN_FETCH_FUNC(symbol, ftype), \
ASSIGN_FETCH_FUNC(deref, ftype), \
+ASSIGN_FETCH_FUNC(bitfield, ftype), \
} \
}

@@ -430,9 +469,33 @@ static const struct fetch_type *find_fetch_type(const char *type)
if (!type)
type = DEFAULT_FETCH_TYPE_STR;

+ /* Special case: bitfield */
+ if (*type == 'b') {
+ unsigned long bs;
+ type = strchr(type, '/');
+ if (!type)
+ goto fail;
+ type++;
+ if (strict_strtoul(type, 0, &bs))
+ goto fail;
+ switch (bs) {
+ case 8:
+ return find_fetch_type("u8");
+ case 16:
+ return find_fetch_type("u16");
+ case 32:
+ return find_fetch_type("u32");
+ case 64:
+ return find_fetch_type("u64");
+ default:
+ goto fail;
+ }
+ }
+
for (i = 0; i < ARRAY_SIZE(fetch_type_table); i++)
if (strcmp(type, fetch_type_table[i].name) == 0)
return &fetch_type_table[i];
+fail:
return NULL;
}

@@ -586,7 +649,9 @@ error:

static void free_probe_arg(struct probe_arg *arg)
{
- if (CHECK_FETCH_FUNCS(deref, arg->fetch.fn))
+ if (CHECK_FETCH_FUNCS(bitfield, arg->fetch.fn))
+ free_bitfield_fetch_param(arg->fetch.data);
+ else if (CHECK_FETCH_FUNCS(deref, arg->fetch.fn))
free_deref_fetch_param(arg->fetch.data);
else if (CHECK_FETCH_FUNCS(symbol, arg->fetch.fn))
free_symbol_cache(arg->fetch.data);
@@ -806,6 +871,41 @@ static int __parse_probe_arg(char *arg, const struct fetch_type *t,
return ret;
}

+#define BYTES_TO_BITS(nb) ((BITS_PER_LONG * (nb)) / sizeof(long))
+
+/* Bitfield type needs to be parsed into a fetch function */
+static int __parse_bitfield_probe_arg(const char *bf,
+ const struct fetch_type *t,
+ struct fetch_param *f)
+{
+ struct bitfield_fetch_param *bprm;
+ unsigned long bw, bo;
+ char *tail;
+
+ if (*bf != 'b')
+ return 0;
+
+ bprm = kzalloc(sizeof(*bprm), GFP_KERNEL);
+ if (!bprm)
+ return -ENOMEM;
+ bprm->orig = *f;
+ f->fn = t->fetch[FETCH_MTD_bitfield];
+ f->data = (void *)bprm;
+
+ bw = simple_strtoul(bf + 1, &tail, 0); /* Use simple one */
+ if (bw == 0 || *tail != '@')
+ return -EINVAL;
+
+ bf = tail + 1;
+ bo = simple_strtoul(bf, &tail, 0);
+ if (tail == bf || *tail != '/')
+ return -EINVAL;
+
+ bprm->hi_shift = BYTES_TO_BITS(t->size) - (bw + bo);
+ bprm->low_shift = bprm->hi_shift + bo;
+ return (BYTES_TO_BITS(t->size) < (bw + bo)) ? -EINVAL : 0;
+}
+
/* String length checking wrapper */
static int parse_probe_arg(char *arg, struct trace_probe *tp,
struct probe_arg *parg, int is_return)
@@ -835,6 +935,8 @@ static int parse_probe_arg(char *arg, struct trace_probe *tp,
parg->offset = tp->size;
tp->size += parg->type->size;
ret = __parse_probe_arg(arg, parg->type, &parg->fetch, is_return);
+ if (ret >= 0)
+ ret = __parse_bitfield_probe_arg(t, parg->type, &parg->fetch);
if (ret >= 0) {
parg->fetch_size.fn = get_fetch_size_function(parg->type,
parg->fetch.fn);
--
1.6.2.5

2011-02-07 15:00:39

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 9/9] perf annotate: Fix build error

From: Borislav Petkov <[email protected]>

A small fix for when NO_NEWT_SUPPORT is defined.

Add a missing "struct" to the function prototype.

Cc: Frederic Weisbecker <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Tom Zanussi <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/annotate.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index b1253aa..bc08b36 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -82,7 +82,7 @@ int symbol__tty_annotate(struct symbol *sym, struct map *map, int evidx,
int max_lines);

#ifdef NO_NEWT_SUPPORT
-static inline int symbol__tui_annotate(symbol *sym __used,
+static inline int symbol__tui_annotate(struct symbol *sym __used,
struct map *map __used, int evidx __used)
{
return 0;
--
1.6.2.5

2011-02-07 15:00:28

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 5/9] tracing/kprobes: Support longer (>128 bytes) command

From: Masami Hiramatsu <[email protected]>

Expand command line buffer of kprobe-tracer to 4096 bytes.

Reported-by: Arnaldo Carvalho de Melo <[email protected]>
Cc: [email protected]
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: Steven Rostedt <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Masami Hiramatsu <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
kernel/trace/trace_kprobe.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 2088893..c6ed886 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -1129,7 +1129,7 @@ static int command_trace_probe(const char *buf)
return ret;
}

-#define WRITE_BUFSIZE 128
+#define WRITE_BUFSIZE 4096

static ssize_t probes_write(struct file *file, const char __user *buffer,
size_t count, loff_t *ppos)
--
1.6.2.5

2011-02-07 15:00:27

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 4/9] tracing/kprobes: Cleanup strict_strtol() using code

From: Masami Hiramatsu <[email protected]>

Since strict_strtol() accepts minus digits started with '-', it doesn't
need to invert after converting.

Cc: [email protected]
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: Steven Rostedt <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Masami Hiramatsu <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
kernel/trace/trace_kprobe.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 2dec9bc..2088893 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -767,16 +767,15 @@ static int __parse_probe_arg(char *arg, const struct fetch_type *t,
}
break;
case '+': /* deref memory */
+ arg++; /* Skip '+', because strict_strtol() rejects it. */
case '-':
tmp = strchr(arg, '(');
if (!tmp)
break;
*tmp = '\0';
- ret = strict_strtol(arg + 1, 0, &offset);
+ ret = strict_strtol(arg, 0, &offset);
if (ret)
break;
- if (arg[0] == '-')
- offset = -offset;
arg = tmp + 1;
tmp = strrchr(arg, ')');
if (tmp) {
--
1.6.2.5

2011-02-07 15:01:31

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 7/9] perf probe: Add bitfield member support

From: Masami Hiramatsu <[email protected]>

Add bitfield member accessing support to probe arguments.

Suggested-by: Arnaldo Carvalho de Melo <[email protected]>
Cc: [email protected]
Cc: Ingo Molnar <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: Steven Rostedt <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Masami Hiramatsu <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/probe-finder.c | 95 ++++++++++++++++++++++++++++-----------
1 files changed, 68 insertions(+), 27 deletions(-)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 46addfb..fd78123 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -33,6 +33,7 @@
#include <ctype.h>
#include <dwarf-regs.h>

+#include <linux/bitops.h>
#include "event.h"
#include "debug.h"
#include "util.h"
@@ -333,13 +334,23 @@ static Dwarf_Die *die_get_real_type(Dwarf_Die *vr_die, Dwarf_Die *die_mem)
return vr_die;
}

-static bool die_is_signed_type(Dwarf_Die *tp_die)
+static int die_get_attr_udata(Dwarf_Die *tp_die, unsigned int attr_name,
+ Dwarf_Word *result)
{
Dwarf_Attribute attr;
+
+ if (dwarf_attr(tp_die, attr_name, &attr) == NULL ||
+ dwarf_formudata(&attr, result) != 0)
+ return -ENOENT;
+
+ return 0;
+}
+
+static bool die_is_signed_type(Dwarf_Die *tp_die)
+{
Dwarf_Word ret;

- if (dwarf_attr(tp_die, DW_AT_encoding, &attr) == NULL ||
- dwarf_formudata(&attr, &ret) != 0)
+ if (die_get_attr_udata(tp_die, DW_AT_encoding, &ret))
return false;

return (ret == DW_ATE_signed_char || ret == DW_ATE_signed ||
@@ -348,11 +359,29 @@ static bool die_is_signed_type(Dwarf_Die *tp_die)

static int die_get_byte_size(Dwarf_Die *tp_die)
{
- Dwarf_Attribute attr;
Dwarf_Word ret;

- if (dwarf_attr(tp_die, DW_AT_byte_size, &attr) == NULL ||
- dwarf_formudata(&attr, &ret) != 0)
+ if (die_get_attr_udata(tp_die, DW_AT_byte_size, &ret))
+ return 0;
+
+ return (int)ret;
+}
+
+static int die_get_bit_size(Dwarf_Die *tp_die)
+{
+ Dwarf_Word ret;
+
+ if (die_get_attr_udata(tp_die, DW_AT_bit_size, &ret))
+ return 0;
+
+ return (int)ret;
+}
+
+static int die_get_bit_offset(Dwarf_Die *tp_die)
+{
+ Dwarf_Word ret;
+
+ if (die_get_attr_udata(tp_die, DW_AT_bit_offset, &ret))
return 0;

return (int)ret;
@@ -827,6 +856,8 @@ static_var:
return 0;
}

+#define BYTES_TO_BITS(nb) ((nb) * BITS_PER_LONG / sizeof(long))
+
static int convert_variable_type(Dwarf_Die *vr_die,
struct probe_trace_arg *tvar,
const char *cast)
@@ -843,6 +874,14 @@ static int convert_variable_type(Dwarf_Die *vr_die,
return (tvar->type == NULL) ? -ENOMEM : 0;
}

+ if (die_get_bit_size(vr_die) != 0) {
+ /* This is a bitfield */
+ ret = snprintf(buf, 16, "b%d@%d/%lu", die_get_bit_size(vr_die),
+ die_get_bit_offset(vr_die),
+ BYTES_TO_BITS(die_get_byte_size(vr_die)));
+ goto formatted;
+ }
+
if (die_get_real_type(vr_die, &type) == NULL) {
pr_warning("Failed to get a type information of %s.\n",
dwarf_diename(vr_die));
@@ -887,29 +926,31 @@ static int convert_variable_type(Dwarf_Die *vr_die,
return (tvar->type == NULL) ? -ENOMEM : 0;
}

- ret = die_get_byte_size(&type) * 8;
- if (ret) {
- /* Check the bitwidth */
- if (ret > MAX_BASIC_TYPE_BITS) {
- pr_info("%s exceeds max-bitwidth."
- " Cut down to %d bits.\n",
- dwarf_diename(&type), MAX_BASIC_TYPE_BITS);
- ret = MAX_BASIC_TYPE_BITS;
- }
+ ret = BYTES_TO_BITS(die_get_byte_size(&type));
+ if (!ret)
+ /* No size ... try to use default type */
+ return 0;

- ret = snprintf(buf, 16, "%c%d",
- die_is_signed_type(&type) ? 's' : 'u', ret);
- if (ret < 0 || ret >= 16) {
- if (ret >= 16)
- ret = -E2BIG;
- pr_warning("Failed to convert variable type: %s\n",
- strerror(-ret));
- return ret;
- }
- tvar->type = strdup(buf);
- if (tvar->type == NULL)
- return -ENOMEM;
+ /* Check the bitwidth */
+ if (ret > MAX_BASIC_TYPE_BITS) {
+ pr_info("%s exceeds max-bitwidth. Cut down to %d bits.\n",
+ dwarf_diename(&type), MAX_BASIC_TYPE_BITS);
+ ret = MAX_BASIC_TYPE_BITS;
+ }
+ ret = snprintf(buf, 16, "%c%d",
+ die_is_signed_type(&type) ? 's' : 'u', ret);
+
+formatted:
+ if (ret < 0 || ret >= 16) {
+ if (ret >= 16)
+ ret = -E2BIG;
+ pr_warning("Failed to convert variable type: %s\n",
+ strerror(-ret));
+ return ret;
}
+ tvar->type = strdup(buf);
+ if (tvar->type == NULL)
+ return -ENOMEM;
return 0;
}

--
1.6.2.5

2011-02-07 15:02:03

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 2/9] perf top: Use pid_t for target_{pid|tid}

From: Denis Kirjanov <[email protected]>

Use pid_t data type for target_{pid|tid} vars.

Cc: Ingo Molnar <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Denis Kirjanov <[email protected]>
[ committer note: those variables are now in struct perf_top, fixed ]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/top.h | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/top.h b/tools/perf/util/top.h
index fe44afb..62e3293 100644
--- a/tools/perf/util/top.h
+++ b/tools/perf/util/top.h
@@ -46,8 +46,8 @@ struct perf_top {
u64 exact_samples;
u64 guest_us_samples, guest_kernel_samples;
int print_entries, count_filter, delay_secs;
- int display_weighted, freq, rb_entries;
- int sym_counter, target_pid, target_tid;
+ int display_weighted, freq, rb_entries, sym_counter;
+ pid_t target_pid, target_tid;
bool hide_kernel_symbols, hide_user_symbols, zero;
const char *cpu_list;
struct perf_evsel *sym_evsel;
--
1.6.2.5

2011-02-07 15:00:20

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 3/9] perf probe: Rewrite find_lazy_match_lines() by using getline(3)

From: Franck Bui-Huu <[email protected]>

Acked-by: Masami Hiramatsu <[email protected]>
Cc: Masami Hiramatsu <[email protected]>
Cc: lkml <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Franck Bui-Huu <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/probe-finder.c | 70 +++++++++++++++------------------------
1 files changed, 27 insertions(+), 43 deletions(-)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 69215bf..46addfb 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -1234,51 +1234,38 @@ static int find_probe_point_by_line(struct probe_finder *pf)
static int find_lazy_match_lines(struct list_head *head,
const char *fname, const char *pat)
{
- char *fbuf, *p1, *p2;
- int fd, line, nlines = -1;
- struct stat st;
-
- fd = open(fname, O_RDONLY);
- if (fd < 0) {
- pr_warning("Failed to open %s: %s\n", fname, strerror(-fd));
+ FILE *fp;
+ char *line = NULL;
+ size_t line_len;
+ ssize_t len;
+ int count = 0, linenum = 1;
+
+ fp = fopen(fname, "r");
+ if (!fp) {
+ pr_warning("Failed to open %s: %s\n", fname, strerror(errno));
return -errno;
}

- if (fstat(fd, &st) < 0) {
- pr_warning("Failed to get the size of %s: %s\n",
- fname, strerror(errno));
- nlines = -errno;
- goto out_close;
- }
-
- nlines = -ENOMEM;
- fbuf = malloc(st.st_size + 2);
- if (fbuf == NULL)
- goto out_close;
- if (read(fd, fbuf, st.st_size) < 0) {
- pr_warning("Failed to read %s: %s\n", fname, strerror(errno));
- nlines = -errno;
- goto out_free_fbuf;
- }
- fbuf[st.st_size] = '\n'; /* Dummy line */
- fbuf[st.st_size + 1] = '\0';
- p1 = fbuf;
- line = 1;
- nlines = 0;
- while ((p2 = strchr(p1, '\n')) != NULL) {
- *p2 = '\0';
- if (strlazymatch(p1, pat)) {
- line_list__add_line(head, line);
- nlines++;
+ while ((len = getline(&line, &line_len, fp)) > 0) {
+
+ if (line[len - 1] == '\n')
+ line[len - 1] = '\0';
+
+ if (strlazymatch(line, pat)) {
+ line_list__add_line(head, linenum);
+ count++;
}
- line++;
- p1 = p2 + 1;
+ linenum++;
}
-out_free_fbuf:
- free(fbuf);
-out_close:
- close(fd);
- return nlines;
+
+ if (ferror(fp))
+ count = -errno;
+ free(line);
+ fclose(fp);
+
+ if (count == 0)
+ pr_debug("No matched lines found in %s.\n", fname);
+ return count;
}

static int probe_point_lazy_walker(const char *fname, int lineno,
@@ -1312,10 +1299,7 @@ static int find_probe_point_lazy(Dwarf_Die *sp_die, struct probe_finder *pf)
/* Matching lazy line pattern */
ret = find_lazy_match_lines(&pf->lcache, pf->fname,
pf->pev->point.lazy_line);
- if (ret == 0) {
- pr_debug("No matched lines found in %s.\n", pf->fname);
- return 0;
- } else if (ret < 0)
+ if (ret <= 0)
return ret;
}

--
1.6.2.5

2011-02-07 15:02:41

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 8/9] perf tool: Fix gcc 4.6.0 issues

From: Kyle McMartin <[email protected]>

GCC 4.6.0 in Fedora rawhide turned up some compile errors in tools/perf
due to the -Werror=unused-but-set-variable flag.

I've gone through and annotated some of the assignments that had side
effects (ie: return value from a function) with the __used annotation,
and in some cases, just removed unused code.

In a few cases, we were assigning something useful, but not using it in
later parts of the function.

kyle@dreadnought:~/src% gcc --version
gcc (GCC) 4.6.0 20110122 (Red Hat 4.6.0-0.3)

Cc: Ingo Molnar <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Kyle McMartin <[email protected]>
[ committer note: Fixed up the annotation fixes, as that code moved recently ]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/bench/sched-pipe.c | 2 +-
tools/perf/builtin-sched.c | 12 +++---------
tools/perf/builtin-top.c | 2 +-
tools/perf/util/annotate.c | 3 ---
tools/perf/util/header.c | 2 +-
.../util/scripting-engines/trace-event-python.c | 3 +--
tools/perf/util/symbol.c | 4 ++--
tools/perf/util/trace-event-parse.c | 2 +-
tools/perf/util/ui/browsers/map.c | 2 +-
9 files changed, 11 insertions(+), 21 deletions(-)

diff --git a/tools/perf/bench/sched-pipe.c b/tools/perf/bench/sched-pipe.c
index d9ab3ce..0c7454f 100644
--- a/tools/perf/bench/sched-pipe.c
+++ b/tools/perf/bench/sched-pipe.c
@@ -55,7 +55,7 @@ int bench_sched_pipe(int argc, const char **argv,
* discarding returned value of read(), write()
* causes error in building environment for perf
*/
- int ret, wait_stat;
+ int __used ret, wait_stat;
pid_t pid, retpid;

argc = parse_options(argc, argv, options,
diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index ae26211..a32f411 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -369,11 +369,6 @@ static void
process_sched_event(struct task_desc *this_task __used, struct sched_atom *atom)
{
int ret = 0;
- u64 now;
- long long delta;
-
- now = get_nsecs();
- delta = start_time + atom->timestamp - now;

switch (atom->type) {
case SCHED_EVENT_RUN:
@@ -562,7 +557,7 @@ static void wait_for_tasks(void)

static void run_one_test(void)
{
- u64 T0, T1, delta, avg_delta, fluct, std_dev;
+ u64 T0, T1, delta, avg_delta, fluct;

T0 = get_nsecs();
wait_for_tasks();
@@ -578,7 +573,6 @@ static void run_one_test(void)
else
fluct = delta - avg_delta;
sum_fluct += fluct;
- std_dev = sum_fluct / nr_runs / sqrt(nr_runs);
if (!run_avg)
run_avg = delta;
run_avg = (run_avg*9 + delta)/10;
@@ -799,7 +793,7 @@ replay_switch_event(struct trace_switch_event *switch_event,
u64 timestamp,
struct thread *thread __used)
{
- struct task_desc *prev, *next;
+ struct task_desc *prev, __used *next;
u64 timestamp0;
s64 delta;

@@ -1404,7 +1398,7 @@ map_switch_event(struct trace_switch_event *switch_event,
u64 timestamp,
struct thread *thread __used)
{
- struct thread *sched_out, *sched_in;
+ struct thread *sched_out __used, *sched_in;
int new_shortname;
u64 timestamp0;
s64 delta;
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 716118a..b790673 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -865,7 +865,7 @@ static int __cmd_top(void)
{
pthread_t thread;
struct perf_evsel *first;
- int ret;
+ int ret __used;
/*
* FIXME: perf_session__new should allow passing a O_MMAP, so that all this
* mmap reading, etc is encapsulated in it. Use O_WRONLY for now.
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 2973376..1012841 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -236,7 +236,6 @@ int symbol__annotate(struct symbol *sym, struct map *map,
char command[PATH_MAX * 2];
FILE *file;
int err = 0;
- u64 len;
char symfs_filename[PATH_MAX];

if (filename) {
@@ -281,8 +280,6 @@ fallback:
filename, sym->name, map->unmap_ip(map, sym->start),
map->unmap_ip(map, sym->end));

- len = sym->end - sym->start;
-
pr_debug("annotating [%p] %30s : [%p] %30s\n",
dso, dso->long_name, sym, sym->name);

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index c0de5ec..72c124d 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -1145,7 +1145,7 @@ int perf_event__synthesize_tracing_data(int fd, struct perf_evlist *evlist,
{
union perf_event ev;
ssize_t size = 0, aligned_size = 0, padding;
- int err = 0;
+ int err __used = 0;

memset(&ev, 0, sizeof(ev));

diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
index c6d9933..2040b85 100644
--- a/tools/perf/util/scripting-engines/trace-event-python.c
+++ b/tools/perf/util/scripting-engines/trace-event-python.c
@@ -248,8 +248,7 @@ static void python_process_event(int cpu, void *data,
context = PyCObject_FromVoidPtr(scripting_context, NULL);

PyTuple_SetItem(t, n++, PyString_FromString(handler_name));
- PyTuple_SetItem(t, n++,
- PyCObject_FromVoidPtr(scripting_context, NULL));
+ PyTuple_SetItem(t, n++, context);

if (handler) {
PyTuple_SetItem(t, n++, PyInt_FromLong(cpu));
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 7821d0e..3e193f8 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1525,8 +1525,8 @@ int dso__load(struct dso *self, struct map *map, symbol_filter_t filter)
symbol_conf.symfs, self->long_name);
break;
case DSO__ORIG_GUEST_KMODULE:
- if (map->groups && map->groups->machine)
- root_dir = map->groups->machine->root_dir;
+ if (map->groups && machine)
+ root_dir = machine->root_dir;
else
root_dir = "";
snprintf(name, size, "%s%s%s", symbol_conf.symfs,
diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
index 73a0222..d8e622d 100644
--- a/tools/perf/util/trace-event-parse.c
+++ b/tools/perf/util/trace-event-parse.c
@@ -153,7 +153,7 @@ void parse_proc_kallsyms(char *file, unsigned int size __unused)
char *next = NULL;
char *addr_str;
char ch;
- int ret;
+ int ret __used;
int i;

line = strtok_r(file, "\n", &next);
diff --git a/tools/perf/util/ui/browsers/map.c b/tools/perf/util/ui/browsers/map.c
index e515836..8462bff 100644
--- a/tools/perf/util/ui/browsers/map.c
+++ b/tools/perf/util/ui/browsers/map.c
@@ -41,7 +41,7 @@ static int ui_entry__read(const char *title, char *bf, size_t size, int width)
out_free_form:
newtPopWindow();
newtFormDestroy(form);
- return 0;
+ return err;
}

struct map_browser {
--
1.6.2.5

2011-02-07 15:07:33

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 0/9] perf/core improvements and fixes


* Arnaldo Carvalho de Melo <[email protected]> wrote:

> Hi Ingo,
>
> Please consider pulling from:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux-2.6 perf/core
>
> Regards,
>
> - Arnaldo
>
> Borislav Petkov (1):
> perf annotate: Fix build error
>
> Denis Kirjanov (1):
> perf top: Use pid_t for target_{pid|tid}
>
> Franck Bui-Huu (1):
> perf probe: Rewrite find_lazy_match_lines() by using getline(3)
>
> Kyle McMartin (1):
> perf tool: Fix gcc 4.6.0 issues
>
> Masami Hiramatsu (4):
> tracing/kprobes: Cleanup strict_strtol() using code
> tracing/kprobes: Support longer (>128 bytes) command
> tracing/kprobes: Add bitfield type
> perf probe: Add bitfield member support
>
> Michael Witten (1):
> perf tools: Makefile: Use $(QUIET_GEN) for perf.so

hm, the perf probe bits do not build on 32-bit:

util/probe-finder.c: In function ‘convert_variable_type’:
util/probe-finder.c:881: error: format ‘%lu’ expects type ‘long unsigned int’, but argument 6 has type ‘unsigned int’
util/probe-finder.c:881: error: format ‘%lu’ expects type ‘long unsigned int’, but argument 6 has type ‘unsigned int’
make: *** [util/probe-finder.o] Error 1
make: *** Waiting for unfinished jobs....

Thanks,

Ingo

2011-02-07 16:29:47

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 0/9] perf/core improvements and fixes

Em Mon, Feb 07, 2011 at 04:06:40PM +0100, Ingo Molnar escreveu:
> * Arnaldo Carvalho de Melo <[email protected]> wrote:
> > Masami Hiramatsu (4):
> > perf probe: Add bitfield member support
>
> hm, the perf probe bits do not build on 32-bit:
>
> util/probe-finder.c: In function ‘convert_variable_type’:
> util/probe-finder.c:881: error: format ‘%lu’ expects type ‘long unsigned int’, but argument 6 has type ‘unsigned int’
> util/probe-finder.c:881: error: format ‘%lu’ expects type ‘long unsigned int’, but argument 6 has type ‘unsigned int’
> make: *** [util/probe-finder.o] Error 1
> make: *** Waiting for unfinished jobs....

I already reworked this and force pushed, see:

http://git.kernel.org/?p=linux/kernel/git/acme/linux-2.6.git;a=commitdiff;h=2b4f8985a2c2309852d20d16d3589eba15b64c4e

[ committer note: Fixed up '%lu' use for return of die_get_byte_size (int) ]

Thanks,

- Arnaldo

2011-02-07 17:37:24

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 0/9] perf/core improvements and fixes

Em Mon, Feb 07, 2011 at 02:29:16PM -0200, Arnaldo Carvalho de Melo escreveu:
> Em Mon, Feb 07, 2011 at 04:06:40PM +0100, Ingo Molnar escreveu:
> > * Arnaldo Carvalho de Melo <[email protected]> wrote:
> > > Masami Hiramatsu (4):
> > > perf probe: Add bitfield member support
> >
> > hm, the perf probe bits do not build on 32-bit:
> >
> > util/probe-finder.c: In function ‘convert_variable_type’:
> > util/probe-finder.c:881: error: format ‘%lu’ expects type ‘long unsigned int’, but argument 6 has type ‘unsigned int’
> > util/probe-finder.c:881: error: format ‘%lu’ expects type ‘long unsigned int’, but argument 6 has type ‘unsigned int’
> > make: *** [util/probe-finder.o] Error 1
> > make: *** Waiting for unfinished jobs....
>
> I already reworked this and force pushed, see:
>
> http://git.kernel.org/?p=linux/kernel/git/acme/linux-2.6.git;a=commitdiff;h=2b4f8985a2c2309852d20d16d3589eba15b64c4e
>
> [ committer note: Fixed up '%lu' use for return of die_get_byte_size (int) ]

Argh, I used '%d', which is ok on 32-bit, but then, breaks 64-bit, i.e.
the right format is '%zd' since the expression is '... / sizeof(foo)',
test built on both 64 and 32 bit, this time around it seems final.

Just give it some time to go from master to git.k.o.

- Arnaldo

Subject: Re: [PATCH 0/9] perf/core improvements and fixes

(2011/02/08 2:36), Arnaldo Carvalho de Melo wrote:
> Em Mon, Feb 07, 2011 at 02:29:16PM -0200, Arnaldo Carvalho de Melo escreveu:
>> Em Mon, Feb 07, 2011 at 04:06:40PM +0100, Ingo Molnar escreveu:
>>> * Arnaldo Carvalho de Melo <[email protected]> wrote:
>>>> Masami Hiramatsu (4):
>>>> perf probe: Add bitfield member support
>>>
>>> hm, the perf probe bits do not build on 32-bit:
>>>
>>> util/probe-finder.c: In function ‘convert_variable_type’:
>>> util/probe-finder.c:881: error: format ‘%lu’ expects type ‘long unsigned int’, but argument 6 has type ‘unsigned int’
>>> util/probe-finder.c:881: error: format ‘%lu’ expects type ‘long unsigned int’, but argument 6 has type ‘unsigned int’
>>> make: *** [util/probe-finder.o] Error 1
>>> make: *** Waiting for unfinished jobs....
>>
>> I already reworked this and force pushed, see:
>>
>> http://git.kernel.org/?p=linux/kernel/git/acme/linux-2.6.git;a=commitdiff;h=2b4f8985a2c2309852d20d16d3589eba15b64c4e
>>
>> [ committer note: Fixed up '%lu' use for return of die_get_byte_size (int) ]
>
> Argh, I used '%d', which is ok on 32-bit, but then, breaks 64-bit, i.e.
> the right format is '%zd' since the expression is '... / sizeof(foo)',
> test built on both 64 and 32 bit, this time around it seems final.
>
> Just give it some time to go from master to git.k.o.

Oops, thanks Arnaldo and Ingo!
BTW, is there any good way to build 32bit perf on x86-64?

--
Masami HIRAMATSU
2nd Dept. Linux Technology Center
Hitachi, Ltd., Systems Development Laboratory
E-mail: [email protected]