* [GIT PULL 0/5] perf/core improvements and fixes
@ 2010-11-26 21:47 Arnaldo Carvalho de Melo
2010-11-26 21:47 ` [PATCH 1/5] perf record: Add option to disable collecting build-ids Arnaldo Carvalho de Melo
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-11-26 21:47 UTC (permalink / raw
To: Ingo Molnar
Cc: linux-kernel, Arnaldo Carvalho de Melo, Andreas Schwab,
Arnaldo Carvalho de Melo, Davidlohr Bueso, Frederic Weisbecker,
Ian Munsie, Ingo Molnar, Mike Galbraith, Paul Mackerras,
Peter Zijlstra, Shawn Bohrer, Stephane Eranian, Thomas Gleixner,
Tom Zanussi
From: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
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(-)
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/5] perf record: Add option to disable collecting build-ids
2010-11-26 21:47 [GIT PULL 0/5] perf/core improvements and fixes Arnaldo Carvalho de Melo
@ 2010-11-26 21:47 ` Arnaldo Carvalho de Melo
2010-11-26 21:47 ` [PATCH 2/5] perf events: Default to using event__process_lost Arnaldo Carvalho de Melo
` (3 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-11-26 21:47 UTC (permalink / raw
To: Ingo Molnar
Cc: linux-kernel, Arnaldo Carvalho de Melo, Frederic Weisbecker,
Mike Galbraith, Paul Mackerras, Peter Zijlstra, Stephane Eranian,
Thomas Gleixner, Tom Zanussi
From: Arnaldo Carvalho de Melo <acme@redhat.com>
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 <tglx@linutronix.de>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Tom Zanussi <tzanussi@gmail.com>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-record.c | 14 +++++++++++---
| 11 +++++++++--
| 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) {
--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)
--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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/5] perf events: Default to using event__process_lost
2010-11-26 21:47 [GIT PULL 0/5] perf/core improvements and fixes Arnaldo Carvalho de Melo
2010-11-26 21:47 ` [PATCH 1/5] perf record: Add option to disable collecting build-ids Arnaldo Carvalho de Melo
@ 2010-11-26 21:47 ` Arnaldo Carvalho de Melo
2010-11-26 23:55 ` Thomas Gleixner
2010-11-26 21:47 ` [PATCH 3/5] perf tools: Add GCC optimization to memory allocating functions Arnaldo Carvalho de Melo
` (2 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-11-26 21:47 UTC (permalink / raw
To: Ingo Molnar
Cc: linux-kernel, Arnaldo Carvalho de Melo, Frederic Weisbecker,
Mike Galbraith, Paul Mackerras, Peter Zijlstra, Stephane Eranian,
Thomas Gleixner, Tom Zanussi
From: Arnaldo Carvalho de Melo <acme@redhat.com>
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 <tglx@linutronix.de>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
CC: Thomas Gleixner <tglx@linutronix.de>
Cc: Tom Zanussi <tzanussi@gmail.com>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/5] perf tools: Add GCC optimization to memory allocating functions
2010-11-26 21:47 [GIT PULL 0/5] perf/core improvements and fixes Arnaldo Carvalho de Melo
2010-11-26 21:47 ` [PATCH 1/5] perf record: Add option to disable collecting build-ids Arnaldo Carvalho de Melo
2010-11-26 21:47 ` [PATCH 2/5] perf events: Default to using event__process_lost Arnaldo Carvalho de Melo
@ 2010-11-26 21:47 ` Arnaldo Carvalho de Melo
2010-11-27 0:30 ` Arnaldo Carvalho de Melo
2010-11-26 21:47 ` [PATCH 4/5] perf symbols: Correct final kernel map guesses Arnaldo Carvalho de Melo
2010-11-26 21:47 ` [PATCH 5/5] perf trace: Handle DT_UNKNOWN on filesystems that don't support d_type Arnaldo Carvalho de Melo
4 siblings, 1 reply; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-11-26 21:47 UTC (permalink / raw
To: Ingo Molnar
Cc: linux-kernel, Davidlohr Bueso, Ingo Molnar, Paul Mackerras,
Peter Zijlstra, Arnaldo Carvalho de Melo
From: Davidlohr Bueso <dave@gnu.org>
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 <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
LKML-Reference: <1290777864.2373.2.camel@cowboy>
Signed-off-by: Davidlohr Bueso <dave@gnu.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 4/5] perf symbols: Correct final kernel map guesses
2010-11-26 21:47 [GIT PULL 0/5] perf/core improvements and fixes Arnaldo Carvalho de Melo
` (2 preceding siblings ...)
2010-11-26 21:47 ` [PATCH 3/5] perf tools: Add GCC optimization to memory allocating functions Arnaldo Carvalho de Melo
@ 2010-11-26 21:47 ` Arnaldo Carvalho de Melo
2010-11-26 21:47 ` [PATCH 5/5] perf trace: Handle DT_UNKNOWN on filesystems that don't support d_type Arnaldo Carvalho de Melo
4 siblings, 0 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-11-26 21:47 UTC (permalink / raw
To: Ingo Molnar
Cc: linux-kernel, Ian Munsie, Ingo Molnar, Paul Mackerras,
Peter Zijlstra, Arnaldo Carvalho de Melo
From: Ian Munsie <imunsie@au1.ibm.com>
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 <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
LKML-Reference: <1290658375-10342-1-git-send-email-imunsie@au1.ibm.com>
Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 5/5] perf trace: Handle DT_UNKNOWN on filesystems that don't support d_type
2010-11-26 21:47 [GIT PULL 0/5] perf/core improvements and fixes Arnaldo Carvalho de Melo
` (3 preceding siblings ...)
2010-11-26 21:47 ` [PATCH 4/5] perf symbols: Correct final kernel map guesses Arnaldo Carvalho de Melo
@ 2010-11-26 21:47 ` Arnaldo Carvalho de Melo
4 siblings, 0 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-11-26 21:47 UTC (permalink / raw
To: Ingo Molnar
Cc: linux-kernel, Shawn Bohrer, Andreas Schwab, Ingo Molnar,
Paul Mackerras, Peter Zijlstra, Arnaldo Carvalho de Melo
From: Shawn Bohrer <sbohrer@rgmadvisors.com>
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 <schwab@linux-m68k.org>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
LKML-Reference: <1290355779-3276-1-git-send-email-sbohrer@rgmadvisors.com>
Signed-off-by: Shawn Bohrer <sbohrer@rgmadvisors.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/5] perf events: Default to using event__process_lost
2010-11-26 21:47 ` [PATCH 2/5] perf events: Default to using event__process_lost Arnaldo Carvalho de Melo
@ 2010-11-26 23:55 ` Thomas Gleixner
2010-11-27 0:18 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 10+ messages in thread
From: Thomas Gleixner @ 2010-11-26 23:55 UTC (permalink / raw
To: Arnaldo Carvalho de Melo
Cc: Ingo Molnar, linux-kernel, Arnaldo Carvalho de Melo,
Frederic Weisbecker, Mike Galbraith, Paul Mackerras,
Peter Zijlstra, Stephane Eranian, Tom Zanussi
On Fri, 26 Nov 2010, Arnaldo Carvalho de Melo wrote:
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/5] perf events: Default to using event__process_lost
2010-11-26 23:55 ` Thomas Gleixner
@ 2010-11-27 0:18 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-11-27 0:18 UTC (permalink / raw
To: Thomas Gleixner
Cc: Ingo Molnar, linux-kernel, Frederic Weisbecker, Mike Galbraith,
Paul Mackerras, Peter Zijlstra, Stephane Eranian, Tom Zanussi
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 <acme@redhat.com>
> >
> > 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/5] perf tools: Add GCC optimization to memory allocating functions
2010-11-26 21:47 ` [PATCH 3/5] perf tools: Add GCC optimization to memory allocating functions Arnaldo Carvalho de Melo
@ 2010-11-27 0:30 ` Arnaldo Carvalho de Melo
2010-11-29 12:54 ` Davidlohr Bueso
0 siblings, 1 reply; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-11-27 0:30 UTC (permalink / raw
To: Ingo Molnar; +Cc: linux-kernel, Davidlohr Bueso, Paul Mackerras, Peter Zijlstra
Em Fri, Nov 26, 2010 at 07:47:19PM -0200, Arnaldo Carvalho de Melo escreveu:
> From: Davidlohr Bueso <dave@gnu.org>
>
> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/5] perf tools: Add GCC optimization to memory allocating functions
2010-11-27 0:30 ` Arnaldo Carvalho de Melo
@ 2010-11-29 12:54 ` Davidlohr Bueso
0 siblings, 0 replies; 10+ messages in thread
From: Davidlohr Bueso @ 2010-11-29 12:54 UTC (permalink / raw
To: Arnaldo Carvalho de Melo
Cc: Ingo Molnar, linux-kernel, Paul Mackerras, Peter Zijlstra
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 <dave@gnu.org>
> >
> > 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
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2010-11-29 12:54 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-11-26 21:47 [GIT PULL 0/5] perf/core improvements and fixes Arnaldo Carvalho de Melo
2010-11-26 21:47 ` [PATCH 1/5] perf record: Add option to disable collecting build-ids Arnaldo Carvalho de Melo
2010-11-26 21:47 ` [PATCH 2/5] perf events: Default to using event__process_lost Arnaldo Carvalho de Melo
2010-11-26 23:55 ` Thomas Gleixner
2010-11-27 0:18 ` Arnaldo Carvalho de Melo
2010-11-26 21:47 ` [PATCH 3/5] perf tools: Add GCC optimization to memory allocating functions Arnaldo Carvalho de Melo
2010-11-27 0:30 ` Arnaldo Carvalho de Melo
2010-11-29 12:54 ` Davidlohr Bueso
2010-11-26 21:47 ` [PATCH 4/5] perf symbols: Correct final kernel map guesses Arnaldo Carvalho de Melo
2010-11-26 21:47 ` [PATCH 5/5] perf trace: Handle DT_UNKNOWN on filesystems that don't support d_type Arnaldo Carvalho de Melo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).