From: Arnaldo Carvalho de Melo <[email protected]>
Hi Ingo,
Please pull from:
git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux-2.6 perf/core
Regards,
- Arnaldo
Arnaldo Carvalho de Melo (2):
perf record: Add option to disable collecting build-ids
perf events: Default to using event__process_lost
Davidlohr Bueso (1):
perf tools: Add GCC optimization to memory allocating functions
Ian Munsie (1):
perf symbols: Correct final kernel map guesses
Shawn Bohrer (1):
perf trace: Handle DT_UNKNOWN on filesystems that don't support
d_type
tools/perf/builtin-record.c | 14 ++++++++++--
tools/perf/builtin-trace.c | 33 ++++++++++++++++++++++++-------
tools/perf/util/event.c | 2 +-
tools/perf/util/header.c | 11 ++++++++-
tools/perf/util/header.h | 1 +
tools/perf/util/include/linux/bitops.h | 5 ++++
tools/perf/util/session.c | 2 +-
tools/perf/util/symbol.c | 2 +-
tools/perf/util/util.h | 5 ++-
9 files changed, 57 insertions(+), 18 deletions(-)
From: Ian Munsie <[email protected]>
If a 32bit userspace perf is running on a 64bit kernel, the end of the final
map in the kernel would incorrectly be set to 2^32-1 rather than 2^64-1.
Cc: Ingo Molnar <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ian Munsie <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/event.c | 2 +-
tools/perf/util/symbol.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index dab9e75..7260db7 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -392,7 +392,7 @@ static void event_set_kernel_mmap_len(struct map **maps, event_t *self)
* a zero sized synthesized MMAP event for the kernel.
*/
if (maps[MAP__FUNCTION]->end == 0)
- maps[MAP__FUNCTION]->end = ~0UL;
+ maps[MAP__FUNCTION]->end = ~0ULL;
}
static int event__process_kernel_mmap(event_t *self,
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 0500895..a348906 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -121,7 +121,7 @@ static void __map_groups__fixup_end(struct map_groups *self, enum map_type type)
* We still haven't the actual symbols, so guess the
* last map final address.
*/
- curr->end = ~0UL;
+ curr->end = ~0ULL;
}
static void map_groups__fixup_end(struct map_groups *self)
--
1.6.2.5
From: Arnaldo Carvalho de Melo <[email protected]>
Collecting build-ids for long running sessions may take a long time
because it needs to traverse the whole just collected perf.data stream
of events, marking the DSOs that had hits and then looking for the
.note.gnu.build-id ELF section.
For things like the 'trace' tool that records and right away consumes
the data on systems where its unlikely that the DSOs being monitored
will change while 'trace' runs, it is desirable to remove build id
collection, so add a -B/--no-buildid option to perf record to allow such
use case.
Longer term we'll avoid all this if we, at DSO load time, in the kernel,
take advantage of this slow code path to collect the build-id and stash
it somewhere, so that we can insert it in the PERF_RECORD_MMAP event.
Reported-by: Thomas Gleixner <[email protected]>
Cc: Frederic Weisbecker <[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: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-record.c | 14 +++++++++++---
tools/perf/util/header.c | 11 +++++++++--
tools/perf/util/header.h | 1 +
tools/perf/util/include/linux/bitops.h | 5 +++++
4 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 3d2cb48..024e144 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -61,6 +61,7 @@ static bool inherit_stat = false;
static bool no_samples = false;
static bool sample_address = false;
static bool no_buildid = false;
+static bool no_buildid_cache = false;
static long samples = 0;
static u64 bytes_written = 0;
@@ -437,7 +438,8 @@ static void atexit_header(void)
if (!pipe_output) {
session->header.data_size += bytes_written;
- process_buildids();
+ if (!no_buildid)
+ process_buildids();
perf_header__write(&session->header, output, true);
perf_session__delete(session);
symbol__exit();
@@ -557,6 +559,9 @@ static int __cmd_record(int argc, const char **argv)
return -1;
}
+ if (!no_buildid)
+ perf_header__set_feat(&session->header, HEADER_BUILD_ID);
+
if (!file_new) {
err = perf_header__read(session, output);
if (err < 0)
@@ -831,8 +836,10 @@ const struct option record_options[] = {
"Sample addresses"),
OPT_BOOLEAN('n', "no-samples", &no_samples,
"don't sample"),
- OPT_BOOLEAN('N', "no-buildid-cache", &no_buildid,
+ OPT_BOOLEAN('N', "no-buildid-cache", &no_buildid_cache,
"do not update the buildid cache"),
+ OPT_BOOLEAN('B', "no-buildid", &no_buildid,
+ "do not collect buildids in perf.data"),
OPT_END()
};
@@ -857,7 +864,8 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
}
symbol__init();
- if (no_buildid)
+
+ if (no_buildid_cache || no_buildid)
disable_buildid_cache();
if (!nr_counters) {
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index d7e67b1..f65d7dc 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -152,6 +152,11 @@ void perf_header__set_feat(struct perf_header *self, int feat)
set_bit(feat, self->adds_features);
}
+void perf_header__clear_feat(struct perf_header *self, int feat)
+{
+ clear_bit(feat, self->adds_features);
+}
+
bool perf_header__has_feat(const struct perf_header *self, int feat)
{
return test_bit(feat, self->adds_features);
@@ -431,8 +436,10 @@ static int perf_header__adds_write(struct perf_header *self, int fd)
int idx = 0, err;
session = container_of(self, struct perf_session, header);
- if (perf_session__read_build_ids(session, true))
- perf_header__set_feat(self, HEADER_BUILD_ID);
+
+ if (perf_header__has_feat(self, HEADER_BUILD_ID &&
+ !perf_session__read_build_ids(session, true)))
+ perf_header__clear_feat(self, HEADER_BUILD_ID);
nr_sections = bitmap_weight(self->adds_features, HEADER_FEAT_BITS);
if (!nr_sections)
diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h
index 402ac24..ed550bf 100644
--- a/tools/perf/util/header.h
+++ b/tools/perf/util/header.h
@@ -84,6 +84,7 @@ u64 perf_header__sample_type(struct perf_header *header);
struct perf_event_attr *
perf_header__find_attr(u64 id, struct perf_header *header);
void perf_header__set_feat(struct perf_header *self, int feat);
+void perf_header__clear_feat(struct perf_header *self, int feat);
bool perf_header__has_feat(const struct perf_header *self, int feat);
int perf_header__process_sections(struct perf_header *self, int fd,
diff --git a/tools/perf/util/include/linux/bitops.h b/tools/perf/util/include/linux/bitops.h
index bb4ac2e..8be0b96 100644
--- a/tools/perf/util/include/linux/bitops.h
+++ b/tools/perf/util/include/linux/bitops.h
@@ -13,6 +13,11 @@ static inline void set_bit(int nr, unsigned long *addr)
addr[nr / BITS_PER_LONG] |= 1UL << (nr % BITS_PER_LONG);
}
+static inline void clear_bit(int nr, unsigned long *addr)
+{
+ addr[nr / BITS_PER_LONG] &= ~(1UL << (nr % BITS_PER_LONG));
+}
+
static __always_inline int test_bit(unsigned int nr, const unsigned long *addr)
{
return ((1UL << (nr % BITS_PER_LONG)) &
--
1.6.2.5
From: Shawn Bohrer <[email protected]>
Some filesystems like xfs and reiserfs will return DT_UNKNOWN for the
d_type. Handle this case by calling stat() to determine the type.
Cc: Andreas Schwab <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Shawn Bohrer <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-trace.c | 33 +++++++++++++++++++++++++--------
1 files changed, 25 insertions(+), 8 deletions(-)
diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 86cfe38..4783ed8 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -301,17 +301,34 @@ static int parse_scriptname(const struct option *opt __used,
return 0;
}
-#define for_each_lang(scripts_dir, lang_dirent, lang_next) \
+/* Helper function for filesystems that return a dent->d_type DT_UNKNOWN */
+static int is_directory(const char *base_path, const struct dirent *dent)
+{
+ char path[PATH_MAX];
+ struct stat st;
+
+ sprintf(path, "%s/%s", base_path, dent->d_name);
+ if (stat(path, &st))
+ return 0;
+
+ return S_ISDIR(st.st_mode);
+}
+
+#define for_each_lang(scripts_path, scripts_dir, lang_dirent, lang_next)\
while (!readdir_r(scripts_dir, &lang_dirent, &lang_next) && \
lang_next) \
- if (lang_dirent.d_type == DT_DIR && \
+ if ((lang_dirent.d_type == DT_DIR || \
+ (lang_dirent.d_type == DT_UNKNOWN && \
+ is_directory(scripts_path, &lang_dirent))) && \
(strcmp(lang_dirent.d_name, ".")) && \
(strcmp(lang_dirent.d_name, "..")))
-#define for_each_script(lang_dir, script_dirent, script_next) \
+#define for_each_script(lang_path, lang_dir, script_dirent, script_next)\
while (!readdir_r(lang_dir, &script_dirent, &script_next) && \
script_next) \
- if (script_dirent.d_type != DT_DIR)
+ if (script_dirent.d_type != DT_DIR && \
+ (script_dirent.d_type != DT_UNKNOWN || \
+ !is_directory(lang_path, &script_dirent)))
#define RECORD_SUFFIX "-record"
@@ -466,14 +483,14 @@ static int list_available_scripts(const struct option *opt __used,
if (!scripts_dir)
return -1;
- for_each_lang(scripts_dir, lang_dirent, lang_next) {
+ for_each_lang(scripts_path, scripts_dir, lang_dirent, lang_next) {
snprintf(lang_path, MAXPATHLEN, "%s/%s/bin", scripts_path,
lang_dirent.d_name);
lang_dir = opendir(lang_path);
if (!lang_dir)
continue;
- for_each_script(lang_dir, script_dirent, script_next) {
+ for_each_script(lang_path, lang_dir, script_dirent, script_next) {
script_root = strdup(script_dirent.d_name);
str = ends_with(script_root, REPORT_SUFFIX);
if (str) {
@@ -514,14 +531,14 @@ static char *get_script_path(const char *script_root, const char *suffix)
if (!scripts_dir)
return NULL;
- for_each_lang(scripts_dir, lang_dirent, lang_next) {
+ for_each_lang(scripts_path, scripts_dir, lang_dirent, lang_next) {
snprintf(lang_path, MAXPATHLEN, "%s/%s/bin", scripts_path,
lang_dirent.d_name);
lang_dir = opendir(lang_path);
if (!lang_dir)
continue;
- for_each_script(lang_dir, script_dirent, script_next) {
+ for_each_script(lang_path, lang_dir, script_dirent, script_next) {
__script_root = strdup(script_dirent.d_name);
str = ends_with(__script_root, suffix);
if (str) {
--
1.6.2.5
From: Arnaldo Carvalho de Melo <[email protected]>
Tool developers have to fill in a 'perf_event_ops' method table to
specify how to handle each event, so far the ones that were not
explicitely especified would get a stub that would just discard the
event.
Change that so that tool developers can get the lost event details and
the total number of such events at the end of 'perf report -D' output.
Suggested-by: Thomas Gleixner <[email protected]>
Cc: Frederic Weisbecker <[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: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/session.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index fa9d652..3d56047 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -262,7 +262,7 @@ static void perf_event_ops__fill_defaults(struct perf_event_ops *handler)
if (handler->exit == NULL)
handler->exit = process_event_stub;
if (handler->lost == NULL)
- handler->lost = process_event_stub;
+ handler->lost = event__process_lost;
if (handler->read == NULL)
handler->read = process_event_stub;
if (handler->throttle == NULL)
--
1.6.2.5
From: Davidlohr Bueso <[email protected]>
We can benefit from the alloc_size attribute in xrealloc and zalloc.
Quoting from http://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html:
"The alloc_size attribute is used to tell the compiler that the function return
value points to memory, where the size is given by one or two of the functions
parameters. GCC uses this information to improve the correctness of
__builtin_object_size."
Cc: Ingo Molnar <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
LKML-Reference: <1290777864.2373.2.camel@cowboy>
Signed-off-by: Davidlohr Bueso <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/util.h | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index 7562707..41a5067 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -182,10 +182,11 @@ static inline char *gitstrchrnul(const char *s, int c)
* Wrappers:
*/
extern char *xstrdup(const char *str);
-extern void *xrealloc(void *ptr, size_t size) __attribute__((weak));
+extern void *xrealloc(void *ptr, size_t size) __attribute__((weak, alloc_size(2)));
-static inline void *zalloc(size_t size)
+static inline __attribute__((alloc_size(1)))
+void *zalloc(size_t size)
{
return calloc(1, size);
}
--
1.6.2.5
On Fri, 26 Nov 2010, Arnaldo Carvalho de Melo wrote:
> From: Arnaldo Carvalho de Melo <[email protected]>
>
> Tool developers have to fill in a 'perf_event_ops' method table to
> specify how to handle each event, so far the ones that were not
> explicitely especified would get a stub that would just discard the
> event.
>
> Change that so that tool developers can get the lost event details and
> the total number of such events at the end of 'perf report -D' output.
That should be always displayed if the subcommand does not have it's
own lost event handling. I stared long enough into the wrong place,
just because the stupid thing just was silent about it. And with this
patch it's still silent for the normal use case.
We really want to tell the user when something went wrong. Users do
not run perf report -D when the tool shows fancy events, why should
they? Just because they know that the tool is hiding problems? If
that's the case then the trust into perf tools is about 0.
Darn, being silent about a known problem is the most stupid error
handling ever.
That's what I added at the end of perf_session__process_events()
+ if (self->hists.stats.total_lost)
+ fprintf(stderr, "Lost events. Check IO/CPU overload!\n");
It's hacky, but it does the job and tells me clearly that the trace is
only halfways useful.
There are only two files, which implement their own lost handling:
builtin-inject.c and builtin-script.c
So everything else looks at incomplete data and let's the user wonder
about the resulting crap in the output.
Thanks,
tglx
Em Sat, Nov 27, 2010 at 12:55:24AM +0100, Thomas Gleixner escreveu:
> On Fri, 26 Nov 2010, Arnaldo Carvalho de Melo wrote:
>
> > From: Arnaldo Carvalho de Melo <[email protected]>
> >
> > Tool developers have to fill in a 'perf_event_ops' method table to
> > specify how to handle each event, so far the ones that were not
> > explicitely especified would get a stub that would just discard the
> > event.
> >
> > Change that so that tool developers can get the lost event details and
> > the total number of such events at the end of 'perf report -D' output.
>
> That should be always displayed if the subcommand does not have it's
> own lost event handling. I stared long enough into the wrong place,
> just because the stupid thing just was silent about it. And with this
> patch it's still silent for the normal use case.
Will make it holler if perf_event_ops->lost == event__process_lost and
self->hists.stats.total_lost != 0, as you suggest.
> We really want to tell the user when something went wrong. Users do
> not run perf report -D when the tool shows fancy events, why should
> they? Just because they know that the tool is hiding problems? If
> that's the case then the trust into perf tools is about 0.
>
> Darn, being silent about a known problem is the most stupid error
> handling ever.
>
> That's what I added at the end of perf_session__process_events()
>
> + if (self->hists.stats.total_lost)
> + fprintf(stderr, "Lost events. Check IO/CPU overload!\n");
>
> It's hacky, but it does the job and tells me clearly that the trace is
> only halfways useful.
Ok, will implement it like you suggested, in a followon patch, in both
the --stdio and --tui modes.
- Arnaldo
Em Fri, Nov 26, 2010 at 07:47:19PM -0200, Arnaldo Carvalho de Melo escreveu:
> From: Davidlohr Bueso <[email protected]>
>
> We can benefit from the alloc_size attribute in xrealloc and zalloc.
>
> Quoting from http://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html:
>
> "The alloc_size attribute is used to tell the compiler that the function return
> value points to memory, where the size is given by one or two of the functions
> parameters. GCC uses this information to improve the correctness of
> __builtin_object_size."
Ingo, please don't pull this, it breaks the build with older GCCs...
util/util.h:185: warning: ‘alloc_size’ attribute directive ignored
util/util.h:190: warning: ‘alloc_size’ attribute directive ignored
make: *** [/home/acme/git/build/perf/builtin-annotate.o] Error 1
make: Leaving directory `/home/acme/git/linux-2.6-tip/tools/perf'
[acme@mica linux-2.6-tip]$ gcc -v
Using built-in specs.
Target: x86_64-redhat-linux
Configured with: ../configure --prefix=/usr --mandir=/usr/share/man
--infodir=/usr/share/info --enable-shared --enable-threads=posix
--enable-checking=release --with-system-zlib --enable-__cxa_atexit
--disable-libunwind-exceptions --enable-libgcj-multifile
--enable-languages=c,c++,objc,obj-c++,java,fortran,ada
--enable-java-awt=gtk --disable-dssi --enable-plugin
--with-java-home=/usr/lib/jvm/java-1.4.2-gcj-1.4.2.0/jre
--with-cpu=generic --host=x86_64-redhat-linux
Thread model: posix
gcc version 4.1.2 20071124 (Red Hat 4.1.2-42)
I'll reorg this using compiler.h tricks probably, for now I'll just
remove it from my perf/core branch.
- Arnaldo
On Fri, 2010-11-26 at 22:30 -0200, Arnaldo Carvalho de Melo wrote:
> Em Fri, Nov 26, 2010 at 07:47:19PM -0200, Arnaldo Carvalho de Melo escreveu:
> > From: Davidlohr Bueso <[email protected]>
> >
> > We can benefit from the alloc_size attribute in xrealloc and zalloc.
> >
> > Quoting from http://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html:
> >
> > "The alloc_size attribute is used to tell the compiler that the function return
> > value points to memory, where the size is given by one or two of the functions
> > parameters. GCC uses this information to improve the correctness of
> > __builtin_object_size."
>
> Ingo, please don't pull this, it breaks the build with older GCCs...
>
Sorry about that. This attribute was added for the 4.2 series in early
2007 (http://gcc.gnu.org/ml/gcc-patches/2007-04/msg01649.html)
> util/util.h:185: warning: ‘alloc_size’ attribute directive ignored
> util/util.h:190: warning: ‘alloc_size’ attribute directive ignored
> make: *** [/home/acme/git/build/perf/builtin-annotate.o] Error 1
> make: Leaving directory `/home/acme/git/linux-2.6-tip/tools/perf'
> [acme@mica linux-2.6-tip]$ gcc -v
> Using built-in specs.
> Target: x86_64-redhat-linux
> Configured with: ../configure --prefix=/usr --mandir=/usr/share/man
> --infodir=/usr/share/info --enable-shared --enable-threads=posix
> --enable-checking=release --with-system-zlib --enable-__cxa_atexit
> --disable-libunwind-exceptions --enable-libgcj-multifile
> --enable-languages=c,c++,objc,obj-c++,java,fortran,ada
> --enable-java-awt=gtk --disable-dssi --enable-plugin
> --with-java-home=/usr/lib/jvm/java-1.4.2-gcj-1.4.2.0/jre
> --with-cpu=generic --host=x86_64-redhat-linux
> Thread model: posix
> gcc version 4.1.2 20071124 (Red Hat 4.1.2-42)
>
> I'll reorg this using compiler.h tricks probably, for now I'll just
> remove it from my perf/core branch.
>
> - Arnaldo
>