* [PATCH 0/3] perf tools: Factor perf data handling
@ 2013-10-07 9:31 Jiri Olsa
2013-10-07 9:31 ` [PATCH 1/3] perf tools: Add data object to handle perf data file Jiri Olsa
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Jiri Olsa @ 2013-10-07 9:31 UTC (permalink / raw)
To: linux-kernel
Cc: Jiri Olsa, Corey Ashford, Frederic Weisbecker, Ingo Molnar,
Namhyung Kim, Paul Mackerras, Peter Zijlstra,
Arnaldo Carvalho de Melo, David Ahern, Adrian Hunter, Andi Kleen
hi,
adding patches to centralize perf.data handling. New object
perf_data_file is added as a handler for perf.data file.
This cleans up perf.data handling and will become handy
for other perf.data file related changes in future, like
multiple file storage or separating storage into multiple
threads.
This patchset is extracted and rebased as generic one
from my earlier 'multiple file support' patchset:
http://marc.info/?t=137803189400001&r=1&w=2
It's reachable here:
git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
perf/file38
thanks,
jirka
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Andi Kleen <andi@firstfloor.org>
---
Jiri Olsa (3):
perf tools: Add data object to handle perf data file
perf tools: Add perf_data_file__open interface to data object
perf tools: Separating data file properties from session
tools/perf/Makefile | 1 +
tools/perf/builtin-annotate.c | 11 ++++++---
tools/perf/builtin-buildid-cache.c | 8 +++++--
tools/perf/builtin-buildid-list.c | 11 ++++++---
tools/perf/builtin-diff.c | 19 +++++++++------
tools/perf/builtin-evlist.c | 7 +++++-
tools/perf/builtin-inject.c | 7 +++++-
tools/perf/builtin-kmem.c | 7 +++++-
tools/perf/builtin-kvm.c | 13 +++++++---
tools/perf/builtin-lock.c | 7 +++++-
tools/perf/builtin-mem.c | 9 +++++--
tools/perf/builtin-record.c | 78 +++++++++++++++++++-----------------------------------------
tools/perf/builtin-report.c | 18 ++++++++++----
tools/perf/builtin-sched.c | 6 ++++-
tools/perf/builtin-script.c | 17 +++++++++----
tools/perf/builtin-timechart.c | 10 ++++++--
tools/perf/builtin-top.c | 7 ++----
tools/perf/builtin-trace.c | 8 ++++---
tools/perf/perf.h | 1 -
tools/perf/util/data.c | 120 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
tools/perf/util/data.h | 48 +++++++++++++++++++++++++++++++++++++
tools/perf/util/header.c | 20 +++++++++-------
tools/perf/util/session.c | 125 +++++++++++++++++++++++++++++++++++-------------------------------------------------------------
tools/perf/util/session.h | 11 ++++-----
24 files changed, 376 insertions(+), 193 deletions(-)
create mode 100644 tools/perf/util/data.c
create mode 100644 tools/perf/util/data.h
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/3] perf tools: Add data object to handle perf data file
2013-10-07 9:31 [PATCH 0/3] perf tools: Factor perf data handling Jiri Olsa
@ 2013-10-07 9:31 ` Jiri Olsa
2013-10-07 9:31 ` [PATCH 2/3] perf tools: Add perf_data_file__open interface to data object Jiri Olsa
2013-10-07 9:31 ` [PATCH 3/3] perf tools: Separating data file properties from session Jiri Olsa
2 siblings, 0 replies; 6+ messages in thread
From: Jiri Olsa @ 2013-10-07 9:31 UTC (permalink / raw)
To: linux-kernel
Cc: Jiri Olsa, Corey Ashford, Frederic Weisbecker, Ingo Molnar,
Namhyung Kim, Paul Mackerras, Peter Zijlstra,
Arnaldo Carvalho de Melo, David Ahern, Adrian Hunter, Andi Kleen
This patch is adding 'struct perf_data_file' object as
a placeholder for all attributes regarding perf.data
file handling. Changing perf_session__new to take it
as an argument.
The rest of the functionality will be added later to keep
this change simple enough, because all the places using
perf_session are changed now.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Andi Kleen <andi@firstfloor.org>
---
tools/perf/builtin-annotate.c | 9 ++++--
tools/perf/builtin-buildid-cache.c | 8 +++--
tools/perf/builtin-buildid-list.c | 9 ++++--
tools/perf/builtin-diff.c | 19 +++++++-----
tools/perf/builtin-evlist.c | 7 ++++-
tools/perf/builtin-inject.c | 7 ++++-
tools/perf/builtin-kmem.c | 7 ++++-
tools/perf/builtin-kvm.c | 13 ++++++--
tools/perf/builtin-lock.c | 7 ++++-
tools/perf/builtin-mem.c | 9 ++++--
tools/perf/builtin-record.c | 61 ++++++++++++++++++++------------------
tools/perf/builtin-report.c | 10 +++++--
tools/perf/builtin-sched.c | 6 +++-
tools/perf/builtin-script.c | 15 ++++++++--
tools/perf/builtin-timechart.c | 10 +++++--
tools/perf/builtin-top.c | 6 +++-
tools/perf/builtin-trace.c | 8 +++--
tools/perf/perf.h | 1 -
tools/perf/util/data.h | 29 ++++++++++++++++++
tools/perf/util/session.c | 12 ++++----
tools/perf/util/session.h | 6 ++--
21 files changed, 186 insertions(+), 73 deletions(-)
create mode 100644 tools/perf/util/data.h
diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index ddde407..13ccea2 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -28,6 +28,7 @@
#include "util/hist.h"
#include "util/session.h"
#include "util/tool.h"
+#include "util/data.h"
#include "arch/common.h"
#include <dlfcn.h>
@@ -203,9 +204,13 @@ static int __cmd_annotate(struct perf_annotate *ann)
struct perf_session *session;
struct perf_evsel *pos;
u64 total_nr_samples;
+ struct perf_data_file file = {
+ .path = input_name,
+ .mode = PERF_DATA_MODE_READ,
+ .force = ann->force,
+ };
- session = perf_session__new(input_name, O_RDONLY,
- ann->force, false, &ann->tool);
+ session = perf_session__new(&file, false, &ann->tool);
if (session == NULL)
return -ENOMEM;
diff --git a/tools/perf/builtin-buildid-cache.c b/tools/perf/builtin-buildid-cache.c
index c96c8fa..581c62a 100644
--- a/tools/perf/builtin-buildid-cache.c
+++ b/tools/perf/builtin-buildid-cache.c
@@ -82,8 +82,12 @@ static bool dso__missing_buildid_cache(struct dso *dso, int parm __maybe_unused)
static int build_id_cache__fprintf_missing(const char *filename, bool force, FILE *fp)
{
- struct perf_session *session = perf_session__new(filename, O_RDONLY,
- force, false, NULL);
+ struct perf_data_file file = {
+ .path = filename,
+ .mode = PERF_DATA_MODE_READ,
+ .force = force,
+ };
+ struct perf_session *session = perf_session__new(&file, false, NULL);
if (session == NULL)
return -1;
diff --git a/tools/perf/builtin-buildid-list.c b/tools/perf/builtin-buildid-list.c
index e74366a..0164c1c 100644
--- a/tools/perf/builtin-buildid-list.c
+++ b/tools/perf/builtin-buildid-list.c
@@ -15,6 +15,7 @@
#include "util/parse-options.h"
#include "util/session.h"
#include "util/symbol.h"
+#include "util/data.h"
static int sysfs__fprintf_build_id(FILE *fp)
{
@@ -52,6 +53,11 @@ static bool dso__skip_buildid(struct dso *dso, int with_hits)
static int perf_session__list_build_ids(bool force, bool with_hits)
{
struct perf_session *session;
+ struct perf_data_file file = {
+ .path = input_name,
+ .mode = PERF_DATA_MODE_READ,
+ .force = force,
+ };
symbol__elf_init();
/*
@@ -60,8 +66,7 @@ static int perf_session__list_build_ids(bool force, bool with_hits)
if (filename__fprintf_build_id(input_name, stdout))
goto out;
- session = perf_session__new(input_name, O_RDONLY, force, false,
- &build_id__mark_dso_hit_ops);
+ session = perf_session__new(&file, false, &build_id__mark_dso_hit_ops);
if (session == NULL)
return -1;
/*
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index f28799e..3fb2d46 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -16,6 +16,7 @@
#include "util/sort.h"
#include "util/symbol.h"
#include "util/util.h"
+#include "util/data.h"
#include <stdlib.h>
#include <math.h>
@@ -42,7 +43,7 @@ struct diff_hpp_fmt {
struct data__file {
struct perf_session *session;
- const char *file;
+ struct perf_data_file file;
int idx;
struct hists *hists;
struct diff_hpp_fmt fmt[PERF_HPP_DIFF__MAX_INDEX];
@@ -599,7 +600,7 @@ static void data__fprintf(void)
data__for_each_file(i, d)
fprintf(stdout, "# [%d] %s %s\n",
- d->idx, d->file,
+ d->idx, d->file.path,
!d->idx ? "(Baseline)" : "");
fprintf(stdout, "#\n");
@@ -661,17 +662,16 @@ static int __cmd_diff(void)
int ret = -EINVAL, i;
data__for_each_file(i, d) {
- d->session = perf_session__new(d->file, O_RDONLY, force,
- false, &tool);
+ d->session = perf_session__new(&d->file, false, &tool);
if (!d->session) {
- pr_err("Failed to open %s\n", d->file);
+ pr_err("Failed to open %s\n", d->file.path);
ret = -ENOMEM;
goto out_delete;
}
ret = perf_session__process_events(d->session, &tool);
if (ret) {
- pr_err("Failed to process %s\n", d->file);
+ pr_err("Failed to process %s\n", d->file.path);
goto out_delete;
}
@@ -1014,7 +1014,12 @@ static int data_init(int argc, const char **argv)
return -ENOMEM;
data__for_each_file(i, d) {
- d->file = use_default ? defaults[i] : argv[i];
+ struct perf_data_file *file = &d->file;
+
+ file->path = use_default ? defaults[i] : argv[i];
+ file->mode = PERF_DATA_MODE_READ,
+ file->force = force,
+
d->idx = i;
}
diff --git a/tools/perf/builtin-evlist.c b/tools/perf/builtin-evlist.c
index 05bd9df..20b0f12 100644
--- a/tools/perf/builtin-evlist.c
+++ b/tools/perf/builtin-evlist.c
@@ -14,13 +14,18 @@
#include "util/parse-events.h"
#include "util/parse-options.h"
#include "util/session.h"
+#include "util/data.h"
static int __cmd_evlist(const char *file_name, struct perf_attr_details *details)
{
struct perf_session *session;
struct perf_evsel *pos;
+ struct perf_data_file file = {
+ .path = file_name,
+ .mode = PERF_DATA_MODE_READ,
+ };
- session = perf_session__new(file_name, O_RDONLY, 0, false, NULL);
+ session = perf_session__new(&file, 0, NULL);
if (session == NULL)
return -ENOMEM;
diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index 423875c..646e220 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -15,6 +15,7 @@
#include "util/tool.h"
#include "util/debug.h"
#include "util/build-id.h"
+#include "util/data.h"
#include "util/parse-options.h"
@@ -347,6 +348,10 @@ static int __cmd_inject(struct perf_inject *inject)
{
struct perf_session *session;
int ret = -EINVAL;
+ struct perf_data_file file = {
+ .path = inject->input_name,
+ .mode = PERF_DATA_MODE_READ,
+ };
signal(SIGINT, sig_handler);
@@ -357,7 +362,7 @@ static int __cmd_inject(struct perf_inject *inject)
inject->tool.tracing_data = perf_event__repipe_tracing_data;
}
- session = perf_session__new(inject->input_name, O_RDONLY, false, true, &inject->tool);
+ session = perf_session__new(&file, true, &inject->tool);
if (session == NULL)
return -ENOMEM;
diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
index 9b5f077..1126382 100644
--- a/tools/perf/builtin-kmem.c
+++ b/tools/perf/builtin-kmem.c
@@ -13,6 +13,7 @@
#include "util/parse-options.h"
#include "util/trace-event.h"
+#include "util/data.h"
#include "util/debug.h"
@@ -486,8 +487,12 @@ static int __cmd_kmem(void)
{ "kmem:kfree", perf_evsel__process_free_event, },
{ "kmem:kmem_cache_free", perf_evsel__process_free_event, },
};
+ struct perf_data_file file = {
+ .path = input_name,
+ .mode = PERF_DATA_MODE_READ,
+ };
- session = perf_session__new(input_name, O_RDONLY, 0, false, &perf_kmem);
+ session = perf_session__new(&file, false, &perf_kmem);
if (session == NULL)
return -ENOMEM;
diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index cfa0b79..188bb29 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -17,6 +17,7 @@
#include "util/tool.h"
#include "util/stat.h"
#include "util/top.h"
+#include "util/data.h"
#include <sys/prctl.h>
#include <sys/timerfd.h>
@@ -1215,10 +1216,13 @@ static int read_events(struct perf_kvm_stat *kvm)
.comm = perf_event__process_comm,
.ordered_samples = true,
};
+ struct perf_data_file file = {
+ .path = input_name,
+ .mode = PERF_DATA_MODE_READ,
+ };
kvm->tool = eops;
- kvm->session = perf_session__new(kvm->file_name, O_RDONLY, 0, false,
- &kvm->tool);
+ kvm->session = perf_session__new(&file, false, &kvm->tool);
if (!kvm->session) {
pr_err("Initializing perf session failed\n");
return -EINVAL;
@@ -1450,6 +1454,9 @@ static int kvm_events_live(struct perf_kvm_stat *kvm,
"perf kvm stat live [<options>]",
NULL
};
+ struct perf_data_file file = {
+ .mode = PERF_DATA_MODE_WRITE,
+ };
/* event handling */
@@ -1514,7 +1521,7 @@ static int kvm_events_live(struct perf_kvm_stat *kvm,
/*
* perf session
*/
- kvm->session = perf_session__new(NULL, O_WRONLY, false, false, &kvm->tool);
+ kvm->session = perf_session__new(&file, false, &kvm->tool);
if (kvm->session == NULL) {
err = -ENOMEM;
goto out;
diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
index 6a9076f..33c7253 100644
--- a/tools/perf/builtin-lock.c
+++ b/tools/perf/builtin-lock.c
@@ -15,6 +15,7 @@
#include "util/debug.h"
#include "util/session.h"
#include "util/tool.h"
+#include "util/data.h"
#include <sys/types.h>
#include <sys/prctl.h>
@@ -853,8 +854,12 @@ static int __cmd_report(bool display_info)
.comm = perf_event__process_comm,
.ordered_samples = true,
};
+ struct perf_data_file file = {
+ .path = input_name,
+ .mode = PERF_DATA_MODE_READ,
+ };
- session = perf_session__new(input_name, O_RDONLY, 0, false, &eops);
+ session = perf_session__new(&file, false, &eops);
if (!session) {
pr_err("Initializing perf session failed\n");
return -ENOMEM;
diff --git a/tools/perf/builtin-mem.c b/tools/perf/builtin-mem.c
index 253133a..31c00f1 100644
--- a/tools/perf/builtin-mem.c
+++ b/tools/perf/builtin-mem.c
@@ -5,6 +5,7 @@
#include "util/trace-event.h"
#include "util/tool.h"
#include "util/session.h"
+#include "util/data.h"
#define MEM_OPERATION_LOAD "load"
#define MEM_OPERATION_STORE "store"
@@ -119,10 +120,14 @@ static int process_sample_event(struct perf_tool *tool,
static int report_raw_events(struct perf_mem *mem)
{
+ struct perf_data_file file = {
+ .path = input_name,
+ .mode = PERF_DATA_MODE_READ,
+ };
int err = -EINVAL;
int ret;
- struct perf_session *session = perf_session__new(input_name, O_RDONLY,
- 0, false, &mem->tool);
+ struct perf_session *session = perf_session__new(&file, false,
+ &mem->tool);
if (session == NULL)
return -ENOMEM;
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index da13840..bf8d0f1 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -24,6 +24,7 @@
#include "util/symbol.h"
#include "util/cpumap.h"
#include "util/thread_map.h"
+#include "util/data.h"
#include <unistd.h>
#include <sched.h>
@@ -65,11 +66,10 @@ struct perf_record {
struct perf_tool tool;
struct perf_record_opts opts;
u64 bytes_written;
- const char *output_name;
+ struct perf_data_file file;
struct perf_evlist *evlist;
struct perf_session *session;
const char *progname;
- int output;
int realtime_prio;
bool no_buildid;
bool no_buildid_cache;
@@ -84,8 +84,10 @@ static void advance_output(struct perf_record *rec, size_t size)
static int write_output(struct perf_record *rec, void *buf, size_t size)
{
+ struct perf_data_file *file = &rec->file;
+
while (size) {
- int ret = write(rec->output, buf, size);
+ int ret = write(file->fd, buf, size);
if (ret < 0) {
pr_err("failed to write\n");
@@ -248,13 +250,15 @@ out:
static int process_buildids(struct perf_record *rec)
{
- u64 size = lseek(rec->output, 0, SEEK_CUR);
+ struct perf_data_file *file = &rec->file;
+ struct perf_session *session = rec->session;
+ u64 size = lseek(file->fd, 0, SEEK_CUR);
if (size == 0)
return 0;
- rec->session->fd = rec->output;
- return __perf_session__process_events(rec->session, rec->post_processing_offset,
+ session->fd = file->fd;
+ return __perf_session__process_events(session, rec->post_processing_offset,
size - rec->post_processing_offset,
size, &build_id__mark_dso_hit_ops);
}
@@ -262,17 +266,18 @@ static int process_buildids(struct perf_record *rec)
static void perf_record__exit(int status, void *arg)
{
struct perf_record *rec = arg;
+ struct perf_data_file *file = &rec->file;
if (status != 0)
return;
- if (!rec->opts.pipe_output) {
+ if (!file->is_pipe) {
rec->session->header.data_size += rec->bytes_written;
if (!rec->no_buildid)
process_buildids(rec);
perf_session__write_header(rec->session, rec->evlist,
- rec->output, true);
+ file->fd, true);
perf_session__delete(rec->session);
perf_evlist__delete(rec->evlist);
symbol__exit();
@@ -342,14 +347,15 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
{
struct stat st;
int flags;
- int err, output, feat;
+ int err, feat;
unsigned long waking = 0;
const bool forks = argc > 0;
struct machine *machine;
struct perf_tool *tool = &rec->tool;
struct perf_record_opts *opts = &rec->opts;
struct perf_evlist *evsel_list = rec->evlist;
- const char *output_name = rec->output_name;
+ struct perf_data_file *file = &rec->file;
+ const char *output_name = file->path;
struct perf_session *session;
bool disabled = false;
@@ -363,13 +369,13 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
if (!output_name) {
if (!fstat(STDOUT_FILENO, &st) && S_ISFIFO(st.st_mode))
- opts->pipe_output = true;
+ file->is_pipe = true;
else
- rec->output_name = output_name = "perf.data";
+ file->path = output_name = "perf.data";
}
if (output_name) {
if (!strcmp(output_name, "-"))
- opts->pipe_output = true;
+ file->is_pipe = true;
else if (!stat(output_name, &st) && st.st_size) {
char oldname[PATH_MAX];
snprintf(oldname, sizeof(oldname), "%s.old",
@@ -381,19 +387,16 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
flags = O_CREAT|O_RDWR|O_TRUNC;
- if (opts->pipe_output)
- output = STDOUT_FILENO;
+ if (file->is_pipe)
+ file->fd = STDOUT_FILENO;
else
- output = open(output_name, flags, S_IRUSR | S_IWUSR);
- if (output < 0) {
+ file->fd = open(output_name, flags, S_IRUSR | S_IWUSR);
+ if (file->fd < 0) {
perror("failed to create output file");
return -1;
}
- rec->output = output;
-
- session = perf_session__new(output_name, O_WRONLY,
- true, false, NULL);
+ session = perf_session__new(file, false, NULL);
if (session == NULL) {
pr_err("Not enough memory for reading perf file header\n");
return -1;
@@ -415,7 +418,7 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
if (forks) {
err = perf_evlist__prepare_workload(evsel_list, &opts->target,
- argv, opts->pipe_output,
+ argv, file->is_pipe,
true);
if (err < 0) {
pr_err("Couldn't run the workload!\n");
@@ -436,13 +439,13 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
*/
on_exit(perf_record__exit, rec);
- if (opts->pipe_output) {
- err = perf_header__write_pipe(output);
+ if (file->is_pipe) {
+ err = perf_header__write_pipe(file->fd);
if (err < 0)
goto out_delete_session;
} else {
err = perf_session__write_header(session, evsel_list,
- output, false);
+ file->fd, false);
if (err < 0)
goto out_delete_session;
}
@@ -455,11 +458,11 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
goto out_delete_session;
}
- rec->post_processing_offset = lseek(output, 0, SEEK_CUR);
+ rec->post_processing_offset = lseek(file->fd, 0, SEEK_CUR);
machine = &session->machines.host;
- if (opts->pipe_output) {
+ if (file->is_pipe) {
err = perf_event__synthesize_attrs(tool, session,
process_synthesized_event);
if (err < 0) {
@@ -476,7 +479,7 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
* return this more properly and also
* propagate errors that now are calling die()
*/
- err = perf_event__synthesize_tracing_data(tool, output, evsel_list,
+ err = perf_event__synthesize_tracing_data(tool, file->fd, evsel_list,
process_synthesized_event);
if (err <= 0) {
pr_err("Couldn't record tracing data.\n");
@@ -842,7 +845,7 @@ const struct option record_options[] = {
OPT_STRING('C', "cpu", &record.opts.target.cpu_list, "cpu",
"list of cpus to monitor"),
OPT_U64('c', "count", &record.opts.user_interval, "event period to sample"),
- OPT_STRING('o', "output", &record.output_name, "file",
+ OPT_STRING('o', "output", &record.file.path, "file",
"output file name"),
OPT_BOOLEAN('i', "no-inherit", &record.opts.no_inherit,
"child tasks do not inherit counters"),
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index c902229..e62f63b 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -33,6 +33,7 @@
#include "util/thread.h"
#include "util/sort.h"
#include "util/hist.h"
+#include "util/data.h"
#include "arch/common.h"
#include <dlfcn.h>
@@ -858,6 +859,9 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
"Don't show entries under that percent", parse_percent_limit),
OPT_END()
};
+ struct perf_data_file file = {
+ .mode = PERF_DATA_MODE_READ,
+ };
perf_config(perf_report_config, &report);
@@ -887,9 +891,11 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
perf_hpp__init();
}
+ file.path = input_name;
+ file.force = report.force;
+
repeat:
- session = perf_session__new(input_name, O_RDONLY,
- report.force, false, &report.tool);
+ session = perf_session__new(&file, false, &report.tool);
if (session == NULL)
return -ENOMEM;
diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index d8c51b2..5a46b10 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -1446,8 +1446,12 @@ static int perf_sched__read_events(struct perf_sched *sched,
{ "sched:sched_migrate_task", process_sched_migrate_task_event, },
};
struct perf_session *session;
+ struct perf_data_file file = {
+ .path = input_name,
+ .mode = PERF_DATA_MODE_READ,
+ };
- session = perf_session__new(input_name, O_RDONLY, 0, false, &sched->tool);
+ session = perf_session__new(&file, false, &sched->tool);
if (session == NULL) {
pr_debug("No Memory for session\n");
return -1;
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 7f31a3d..05da75e 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -15,6 +15,7 @@
#include "util/evlist.h"
#include "util/evsel.h"
#include "util/sort.h"
+#include "util/data.h"
#include <linux/bitmap.h>
static char const *script_name;
@@ -1115,10 +1116,14 @@ int find_scripts(char **scripts_array, char **scripts_path_array)
char scripts_path[MAXPATHLEN], lang_path[MAXPATHLEN];
DIR *scripts_dir, *lang_dir;
struct perf_session *session;
+ struct perf_data_file file = {
+ .path = input_name,
+ .mode = PERF_DATA_MODE_READ,
+ };
char *temp;
int i = 0;
- session = perf_session__new(input_name, O_RDONLY, 0, false, NULL);
+ session = perf_session__new(&file, false, NULL);
if (!session)
return -1;
@@ -1319,12 +1324,17 @@ int cmd_script(int argc, const char **argv, const char *prefix __maybe_unused)
"perf script [<options>] <top-script> [script-args]",
NULL
};
+ struct perf_data_file file = {
+ .mode = PERF_DATA_MODE_READ,
+ };
setup_scripting();
argc = parse_options(argc, argv, options, script_usage,
PARSE_OPT_STOP_AT_NON_OPTION);
+ file.path = input_name;
+
if (argc > 1 && !strncmp(argv[0], "rec", strlen("rec"))) {
rec_script_path = get_script_path(argv[1], RECORD_SUFFIX);
if (!rec_script_path)
@@ -1488,8 +1498,7 @@ int cmd_script(int argc, const char **argv, const char *prefix __maybe_unused)
if (!script_name)
setup_pager();
- session = perf_session__new(input_name, O_RDONLY, 0, false,
- &perf_script);
+ session = perf_session__new(&file, false, &perf_script);
if (session == NULL)
return -ENOMEM;
diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
index c2e0231..e11c61d 100644
--- a/tools/perf/builtin-timechart.c
+++ b/tools/perf/builtin-timechart.c
@@ -36,6 +36,7 @@
#include "util/session.h"
#include "util/svghelper.h"
#include "util/tool.h"
+#include "util/data.h"
#define SUPPORT_OLD_POWER_EVENTS 1
#define PWR_EVENT_EXIT -1
@@ -990,8 +991,13 @@ static int __cmd_timechart(const char *output_name)
{ "power:power_frequency", process_sample_power_frequency },
#endif
};
- struct perf_session *session = perf_session__new(input_name, O_RDONLY,
- 0, false, &perf_timechart);
+ struct perf_data_file file = {
+ .path = input_name,
+ .mode = PERF_DATA_MODE_READ,
+ };
+
+ struct perf_session *session = perf_session__new(&file, false,
+ &perf_timechart);
int ret = -EINVAL;
if (session == NULL)
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index cf4bba0..1aefdfe 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -930,11 +930,15 @@ static int __cmd_top(struct perf_top *top)
struct perf_record_opts *opts = &top->record_opts;
pthread_t thread;
int ret;
+ struct perf_data_file file = {
+ .mode = PERF_DATA_MODE_WRITE,
+ };
+
/*
* 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.
*/
- top->session = perf_session__new(NULL, O_WRONLY, false, false, NULL);
+ top->session = perf_session__new(&file, false, NULL);
if (top->session == NULL)
return -ENOMEM;
diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 1bb8f15..4e3bf72 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -1636,7 +1636,10 @@ static int trace__replay(struct trace *trace)
{ "raw_syscalls:sys_enter", trace__sys_enter, },
{ "raw_syscalls:sys_exit", trace__sys_exit, },
};
-
+ struct perf_data_file file = {
+ .path = input_name,
+ .mode = PERF_DATA_MODE_READ,
+ };
struct perf_session *session;
int err = -1;
@@ -1659,8 +1662,7 @@ static int trace__replay(struct trace *trace)
if (symbol__init() < 0)
return -1;
- session = perf_session__new(input_name, O_RDONLY, 0, false,
- &trace->tool);
+ session = perf_session__new(&file, false, &trace->tool);
if (session == NULL)
return -ENOMEM;
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index cf20187..0914630 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -218,7 +218,6 @@ struct perf_record_opts {
bool no_delay;
bool no_inherit;
bool no_samples;
- bool pipe_output;
bool raw_samples;
bool sample_address;
bool sample_weight;
diff --git a/tools/perf/util/data.h b/tools/perf/util/data.h
new file mode 100644
index 0000000..ffa0186
--- /dev/null
+++ b/tools/perf/util/data.h
@@ -0,0 +1,29 @@
+#ifndef __PERF_DATA_H
+#define __PERF_DATA_H
+
+#include <stdbool.h>
+
+enum perf_data_mode {
+ PERF_DATA_MODE_WRITE,
+ PERF_DATA_MODE_READ,
+};
+
+struct perf_data_file {
+ const char *path;
+ int fd;
+ bool is_pipe;
+ bool force;
+ enum perf_data_mode mode;
+};
+
+static inline bool perf_data_file__is_read(struct perf_data_file *file)
+{
+ return file->mode == PERF_DATA_MODE_READ;
+}
+
+static inline bool perf_data_file__is_write(struct perf_data_file *file)
+{
+ return file->mode == PERF_DATA_MODE_WRITE;
+}
+
+#endif /* __PERF_DATA_H */
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 6c1d444..6e5fad1 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -106,11 +106,11 @@ static void perf_session__destroy_kernel_maps(struct perf_session *self)
machines__destroy_kernel_maps(&self->machines);
}
-struct perf_session *perf_session__new(const char *filename, int mode,
- bool force, bool repipe,
- struct perf_tool *tool)
+struct perf_session *perf_session__new(struct perf_data_file *file,
+ bool repipe, struct perf_tool *tool)
{
struct perf_session *self;
+ const char *filename = file->path;
struct stat st;
size_t len;
@@ -134,11 +134,11 @@ struct perf_session *perf_session__new(const char *filename, int mode,
INIT_LIST_HEAD(&self->ordered_samples.to_free);
machines__init(&self->machines);
- if (mode == O_RDONLY) {
- if (perf_session__open(self, force) < 0)
+ if (perf_data_file__is_read(file)) {
+ if (perf_session__open(self, file->force) < 0)
goto out_delete;
perf_session__set_id_hdr_size(self);
- } else if (mode == O_WRONLY) {
+ } else if (perf_data_file__is_write(file)) {
/*
* In O_RDONLY mode this will be performed when reading the
* kernel MMAP event, in perf_event__process_mmap().
diff --git a/tools/perf/util/session.h b/tools/perf/util/session.h
index 3aa75fb..1e4d813 100644
--- a/tools/perf/util/session.h
+++ b/tools/perf/util/session.h
@@ -7,6 +7,7 @@
#include "machine.h"
#include "symbol.h"
#include "thread.h"
+#include "data.h"
#include <linux/rbtree.h>
#include <linux/perf_event.h>
@@ -49,9 +50,8 @@ struct perf_session {
struct perf_tool;
-struct perf_session *perf_session__new(const char *filename, int mode,
- bool force, bool repipe,
- struct perf_tool *tool);
+struct perf_session *perf_session__new(struct perf_data_file *file,
+ bool repipe, struct perf_tool *tool);
void perf_session__delete(struct perf_session *session);
void perf_event_header__bswap(struct perf_event_header *self);
--
1.7.11.7
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] perf tools: Add perf_data_file__open interface to data object
2013-10-07 9:31 [PATCH 0/3] perf tools: Factor perf data handling Jiri Olsa
2013-10-07 9:31 ` [PATCH 1/3] perf tools: Add data object to handle perf data file Jiri Olsa
@ 2013-10-07 9:31 ` Jiri Olsa
2013-10-07 9:31 ` [PATCH 3/3] perf tools: Separating data file properties from session Jiri Olsa
2 siblings, 0 replies; 6+ messages in thread
From: Jiri Olsa @ 2013-10-07 9:31 UTC (permalink / raw)
To: linux-kernel
Cc: Jiri Olsa, Corey Ashford, Frederic Weisbecker, Ingo Molnar,
Namhyung Kim, Paul Mackerras, Peter Zijlstra,
Arnaldo Carvalho de Melo, David Ahern, Adrian Hunter, Andi Kleen
Adding perf_data_file__open interface to data object
to open the perf.data file for both read and write.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Andi Kleen <andi@firstfloor.org>
---
tools/perf/Makefile | 1 +
tools/perf/builtin-record.c | 34 +------------
tools/perf/builtin-top.c | 9 +---
tools/perf/util/data.c | 120 ++++++++++++++++++++++++++++++++++++++++++++
tools/perf/util/data.h | 4 ++
tools/perf/util/session.c | 95 +++++++++++------------------------
tools/perf/util/session.h | 2 +-
7 files changed, 158 insertions(+), 107 deletions(-)
create mode 100644 tools/perf/util/data.c
diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index b62e12d..1a7ecbd 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -364,6 +364,7 @@ LIB_OBJS += $(OUTPUT)util/vdso.o
LIB_OBJS += $(OUTPUT)util/stat.o
LIB_OBJS += $(OUTPUT)util/record.o
LIB_OBJS += $(OUTPUT)util/srcline.o
+LIB_OBJS += $(OUTPUT)util/data.o
LIB_OBJS += $(OUTPUT)ui/setup.o
LIB_OBJS += $(OUTPUT)ui/helpline.o
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index bf8d0f1..bfe7b93 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -345,8 +345,6 @@ out:
static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
{
- struct stat st;
- int flags;
int err, feat;
unsigned long waking = 0;
const bool forks = argc > 0;
@@ -355,7 +353,6 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
struct perf_record_opts *opts = &rec->opts;
struct perf_evlist *evsel_list = rec->evlist;
struct perf_data_file *file = &rec->file;
- const char *output_name = file->path;
struct perf_session *session;
bool disabled = false;
@@ -367,35 +364,6 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
signal(SIGUSR1, sig_handler);
signal(SIGTERM, sig_handler);
- if (!output_name) {
- if (!fstat(STDOUT_FILENO, &st) && S_ISFIFO(st.st_mode))
- file->is_pipe = true;
- else
- file->path = output_name = "perf.data";
- }
- if (output_name) {
- if (!strcmp(output_name, "-"))
- file->is_pipe = true;
- else if (!stat(output_name, &st) && st.st_size) {
- char oldname[PATH_MAX];
- snprintf(oldname, sizeof(oldname), "%s.old",
- output_name);
- unlink(oldname);
- rename(output_name, oldname);
- }
- }
-
- flags = O_CREAT|O_RDWR|O_TRUNC;
-
- if (file->is_pipe)
- file->fd = STDOUT_FILENO;
- else
- file->fd = open(output_name, flags, S_IRUSR | S_IWUSR);
- if (file->fd < 0) {
- perror("failed to create output file");
- return -1;
- }
-
session = perf_session__new(file, false, NULL);
if (session == NULL) {
pr_err("Not enough memory for reading perf file header\n");
@@ -586,7 +554,7 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
fprintf(stderr,
"[ perf record: Captured and wrote %.3f MB %s (~%" PRIu64 " samples) ]\n",
(double)rec->bytes_written / 1024.0 / 1024.0,
- output_name,
+ file->path,
rec->bytes_written / 24);
return 0;
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 1aefdfe..5ab4015 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -930,15 +930,8 @@ static int __cmd_top(struct perf_top *top)
struct perf_record_opts *opts = &top->record_opts;
pthread_t thread;
int ret;
- struct perf_data_file file = {
- .mode = PERF_DATA_MODE_WRITE,
- };
- /*
- * 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.
- */
- top->session = perf_session__new(&file, false, NULL);
+ top->session = perf_session__new(NULL, false, NULL);
if (top->session == NULL)
return -ENOMEM;
diff --git a/tools/perf/util/data.c b/tools/perf/util/data.c
new file mode 100644
index 0000000..7d09faf
--- /dev/null
+++ b/tools/perf/util/data.c
@@ -0,0 +1,120 @@
+#include <linux/compiler.h>
+#include <linux/kernel.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
+#include <string.h>
+
+#include "data.h"
+#include "util.h"
+
+static bool check_pipe(struct perf_data_file *file)
+{
+ struct stat st;
+ bool is_pipe = false;
+ int fd = perf_data_file__is_read(file) ?
+ STDIN_FILENO : STDOUT_FILENO;
+
+ if (!file->path) {
+ if (!fstat(fd, &st) && S_ISFIFO(st.st_mode))
+ is_pipe = true;
+ } else {
+ if (!strcmp(file->path, "-"))
+ is_pipe = true;
+ }
+
+ if (is_pipe)
+ file->fd = fd;
+
+ return file->is_pipe = is_pipe;
+}
+
+static int check_backup(struct perf_data_file *file)
+{
+ struct stat st;
+
+ if (!stat(file->path, &st) && st.st_size) {
+ /* TODO check errors properly */
+ char oldname[PATH_MAX];
+ snprintf(oldname, sizeof(oldname), "%s.old",
+ file->path);
+ unlink(oldname);
+ rename(file->path, oldname);
+ }
+
+ return 0;
+}
+
+static int open_file_read(struct perf_data_file *file)
+{
+ struct stat st;
+ int fd;
+
+ fd = open(file->path, O_RDONLY);
+ if (fd < 0) {
+ int err = errno;
+
+ pr_err("failed to open %s: %s", file->path, strerror(err));
+ if (err == ENOENT && !strcmp(file->path, "perf.data"))
+ pr_err(" (try 'perf record' first)");
+ pr_err("\n");
+ return -err;
+ }
+
+ if (fstat(fd, &st) < 0)
+ goto out_close;
+
+ if (!file->force && st.st_uid && (st.st_uid != geteuid())) {
+ pr_err("file %s not owned by current user or root\n",
+ file->path);
+ goto out_close;
+ }
+
+ if (!st.st_size) {
+ pr_info("zero-sized file (%s), nothing to do!\n",
+ file->path);
+ goto out_close;
+ }
+
+ file->size = st.st_size;
+ return fd;
+
+ out_close:
+ close(fd);
+ return -1;
+}
+
+static int open_file_write(struct perf_data_file *file)
+{
+ if (check_backup(file))
+ return -1;
+
+ return open(file->path, O_CREAT|O_RDWR|O_TRUNC, S_IRUSR|S_IWUSR);
+}
+
+static int open_file(struct perf_data_file *file)
+{
+ int fd;
+
+ fd = perf_data_file__is_read(file) ?
+ open_file_read(file) : open_file_write(file);
+
+ file->fd = fd;
+ return fd < 0 ? -1 : 0;
+}
+
+int perf_data_file__open(struct perf_data_file *file)
+{
+ if (check_pipe(file))
+ return 0;
+
+ if (!file->path)
+ file->path = "perf.data";
+
+ return open_file(file);
+}
+
+void perf_data_file__close(struct perf_data_file *file)
+{
+ close(file->fd);
+}
diff --git a/tools/perf/util/data.h b/tools/perf/util/data.h
index ffa0186..d6c262e 100644
--- a/tools/perf/util/data.h
+++ b/tools/perf/util/data.h
@@ -13,6 +13,7 @@ struct perf_data_file {
int fd;
bool is_pipe;
bool force;
+ unsigned long size;
enum perf_data_mode mode;
};
@@ -26,4 +27,7 @@ static inline bool perf_data_file__is_write(struct perf_data_file *file)
return file->mode == PERF_DATA_MODE_WRITE;
}
+int perf_data_file__open(struct perf_data_file *file);
+void perf_data_file__close(struct perf_data_file *file);
+
#endif /* __PERF_DATA_H */
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 6e5fad1..8d6ae30 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -16,73 +16,35 @@
#include "perf_regs.h"
#include "vdso.h"
-static int perf_session__open(struct perf_session *self, bool force)
+static int perf_session__open(struct perf_session *self)
{
- struct stat input_stat;
-
- if (!strcmp(self->filename, "-")) {
- self->fd_pipe = true;
- self->fd = STDIN_FILENO;
-
+ if (self->fd_pipe) {
if (perf_session__read_header(self) < 0)
pr_err("incompatible file format (rerun with -v to learn more)");
-
return 0;
}
- self->fd = open(self->filename, O_RDONLY);
- if (self->fd < 0) {
- int err = errno;
-
- pr_err("failed to open %s: %s", self->filename, strerror(err));
- if (err == ENOENT && !strcmp(self->filename, "perf.data"))
- pr_err(" (try 'perf record' first)");
- pr_err("\n");
- return -errno;
- }
-
- if (fstat(self->fd, &input_stat) < 0)
- goto out_close;
-
- if (!force && input_stat.st_uid && (input_stat.st_uid != geteuid())) {
- pr_err("file %s not owned by current user or root\n",
- self->filename);
- goto out_close;
- }
-
- if (!input_stat.st_size) {
- pr_info("zero-sized file (%s), nothing to do!\n",
- self->filename);
- goto out_close;
- }
-
if (perf_session__read_header(self) < 0) {
pr_err("incompatible file format (rerun with -v to learn more)");
- goto out_close;
+ return -1;
}
if (!perf_evlist__valid_sample_type(self->evlist)) {
pr_err("non matching sample_type");
- goto out_close;
+ return -1;
}
if (!perf_evlist__valid_sample_id_all(self->evlist)) {
pr_err("non matching sample_id_all");
- goto out_close;
+ return -1;
}
if (!perf_evlist__valid_read_format(self->evlist)) {
pr_err("non matching read_format");
- goto out_close;
+ return -1;
}
- self->size = input_stat.st_size;
return 0;
-
-out_close:
- close(self->fd);
- self->fd = -1;
- return -1;
}
void perf_session__set_id_hdr_size(struct perf_session *session)
@@ -110,35 +72,35 @@ struct perf_session *perf_session__new(struct perf_data_file *file,
bool repipe, struct perf_tool *tool)
{
struct perf_session *self;
- const char *filename = file->path;
- struct stat st;
- size_t len;
-
- if (!filename || !strlen(filename)) {
- if (!fstat(STDIN_FILENO, &st) && S_ISFIFO(st.st_mode))
- filename = "-";
- else
- filename = "perf.data";
- }
- len = strlen(filename);
- self = zalloc(sizeof(*self) + len);
-
- if (self == NULL)
+ self = zalloc(sizeof(*self));
+ if (!self)
goto out;
- memcpy(self->filename, filename, len);
self->repipe = repipe;
INIT_LIST_HEAD(&self->ordered_samples.samples);
INIT_LIST_HEAD(&self->ordered_samples.sample_cache);
INIT_LIST_HEAD(&self->ordered_samples.to_free);
machines__init(&self->machines);
- if (perf_data_file__is_read(file)) {
- if (perf_session__open(self, file->force) < 0)
+ if (file) {
+ if (perf_data_file__open(file))
goto out_delete;
- perf_session__set_id_hdr_size(self);
- } else if (perf_data_file__is_write(file)) {
+
+ self->fd = file->fd;
+ self->fd_pipe = file->is_pipe;
+ self->filename = file->path;
+ self->size = file->size;
+
+ if (perf_data_file__is_read(file)) {
+ if (perf_session__open(self) < 0)
+ goto out_close;
+
+ perf_session__set_id_hdr_size(self);
+ }
+ }
+
+ if (!file || perf_data_file__is_write(file)) {
/*
* In O_RDONLY mode this will be performed when reading the
* kernel MMAP event, in perf_event__process_mmap().
@@ -153,10 +115,13 @@ struct perf_session *perf_session__new(struct perf_data_file *file,
tool->ordered_samples = false;
}
-out:
return self;
-out_delete:
+
+ out_close:
+ perf_data_file__close(file);
+ out_delete:
perf_session__delete(self);
+ out:
return NULL;
}
diff --git a/tools/perf/util/session.h b/tools/perf/util/session.h
index 1e4d813..c6a21d9 100644
--- a/tools/perf/util/session.h
+++ b/tools/perf/util/session.h
@@ -39,7 +39,7 @@ struct perf_session {
bool fd_pipe;
bool repipe;
struct ordered_samples ordered_samples;
- char filename[1];
+ const char *filename;
};
#define PRINT_IP_OPT_IP (1<<0)
--
1.7.11.7
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] perf tools: Separating data file properties from session
2013-10-07 9:31 [PATCH 0/3] perf tools: Factor perf data handling Jiri Olsa
2013-10-07 9:31 ` [PATCH 1/3] perf tools: Add data object to handle perf data file Jiri Olsa
2013-10-07 9:31 ` [PATCH 2/3] perf tools: Add perf_data_file__open interface to data object Jiri Olsa
@ 2013-10-07 9:31 ` Jiri Olsa
2 siblings, 0 replies; 6+ messages in thread
From: Jiri Olsa @ 2013-10-07 9:31 UTC (permalink / raw)
To: linux-kernel
Cc: Jiri Olsa, Corey Ashford, Frederic Weisbecker, Ingo Molnar,
Namhyung Kim, Paul Mackerras, Peter Zijlstra,
Arnaldo Carvalho de Melo, David Ahern, Adrian Hunter, Andi Kleen
Removing 'fd, fd_pipe, filename, size' from struct perf_session
and replacing them with struct perf_data_file object.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Andi Kleen <andi@firstfloor.org>
---
tools/perf/builtin-annotate.c | 2 +-
tools/perf/builtin-buildid-list.c | 2 +-
tools/perf/builtin-record.c | 1 -
tools/perf/builtin-report.c | 8 +++++---
tools/perf/builtin-script.c | 2 +-
tools/perf/util/data.h | 15 +++++++++++++++
| 20 ++++++++++++--------
tools/perf/util/session.c | 36 +++++++++++++++++++-----------------
tools/perf/util/session.h | 5 +----
9 files changed, 55 insertions(+), 36 deletions(-)
diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 13ccea2..19f5c8c 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -263,7 +263,7 @@ static int __cmd_annotate(struct perf_annotate *ann)
}
if (total_nr_samples == 0) {
- ui__error("The %s file has no samples!\n", session->filename);
+ ui__error("The %s file has no samples!\n", file.path);
goto out_delete;
}
diff --git a/tools/perf/builtin-buildid-list.c b/tools/perf/builtin-buildid-list.c
index 0164c1c..ed3873b 100644
--- a/tools/perf/builtin-buildid-list.c
+++ b/tools/perf/builtin-buildid-list.c
@@ -73,7 +73,7 @@ static int perf_session__list_build_ids(bool force, bool with_hits)
* in pipe-mode, the only way to get the buildids is to parse
* the record stream. Buildids are stored as RECORD_HEADER_BUILD_ID
*/
- if (with_hits || session->fd_pipe)
+ if (with_hits || perf_data_file__is_pipe(&file))
perf_session__process_events(session, &build_id__mark_dso_hit_ops);
perf_session__fprintf_dsos_buildid(session, stdout, dso__skip_buildid, with_hits);
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index bfe7b93..3a843a1 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -257,7 +257,6 @@ static int process_buildids(struct perf_record *rec)
if (size == 0)
return 0;
- session->fd = file->fd;
return __perf_session__process_events(session, rec->post_processing_offset,
size - rec->post_processing_offset,
size, &build_id__mark_dso_hit_ops);
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index e62f63b..adaad0e 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -368,8 +368,9 @@ static int perf_report__setup_sample_type(struct perf_report *rep)
{
struct perf_session *self = rep->session;
u64 sample_type = perf_evlist__combined_sample_type(self->evlist);
+ bool is_pipe = perf_data_file__is_pipe(self->file);
- if (!self->fd_pipe && !(sample_type & PERF_SAMPLE_CALLCHAIN)) {
+ if (!is_pipe && !(sample_type & PERF_SAMPLE_CALLCHAIN)) {
if (sort__has_parent) {
ui__error("Selected --sort parent, but no "
"callchain data. Did you call "
@@ -392,7 +393,7 @@ static int perf_report__setup_sample_type(struct perf_report *rep)
}
if (sort__mode == SORT_MODE__BRANCH) {
- if (!self->fd_pipe &&
+ if (!is_pipe &&
!(sample_type & PERF_SAMPLE_BRANCH_STACK)) {
ui__error("Selected -b but no branch data. "
"Did you call perf record without -b?\n");
@@ -490,6 +491,7 @@ static int __cmd_report(struct perf_report *rep)
struct map *kernel_map;
struct kmap *kernel_kmap;
const char *help = "For a higher level overview, try: perf report --sort comm,dso";
+ struct perf_data_file *file = session->file;
signal(SIGINT, sig_handler);
@@ -571,7 +573,7 @@ static int __cmd_report(struct perf_report *rep)
}
if (nr_samples == 0) {
- ui__error("The %s file has no samples!\n", session->filename);
+ ui__error("The %s file has no samples!\n", file->path);
return 0;
}
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 05da75e..5a6f283 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -1525,7 +1525,7 @@ int cmd_script(int argc, const char **argv, const char *prefix __maybe_unused)
return -1;
}
- input = open(session->filename, O_RDONLY); /* input_name */
+ input = open(file.path, O_RDONLY); /* input_name */
if (input < 0) {
perror("failed to open file");
return -1;
diff --git a/tools/perf/util/data.h b/tools/perf/util/data.h
index d6c262e..8c2df80 100644
--- a/tools/perf/util/data.h
+++ b/tools/perf/util/data.h
@@ -27,6 +27,21 @@ static inline bool perf_data_file__is_write(struct perf_data_file *file)
return file->mode == PERF_DATA_MODE_WRITE;
}
+static inline int perf_data_file__is_pipe(struct perf_data_file *file)
+{
+ return file->is_pipe;
+}
+
+static inline int perf_data_file__fd(struct perf_data_file *file)
+{
+ return file->fd;
+}
+
+static inline unsigned long perf_data_file__size(struct perf_data_file *file)
+{
+ return file->size;
+}
+
int perf_data_file__open(struct perf_data_file *file);
void perf_data_file__close(struct perf_data_file *file);
--git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 26441d0..ce4dfc3 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -22,6 +22,7 @@
#include "vdso.h"
#include "strbuf.h"
#include "build-id.h"
+#include "data.h"
static bool no_buildid_cache = false;
@@ -2174,7 +2175,7 @@ int perf_header__fprintf_info(struct perf_session *session, FILE *fp, bool full)
{
struct header_print_data hd;
struct perf_header *header = &session->header;
- int fd = session->fd;
+ int fd = perf_data_file__fd(session->file);
hd.fp = fp;
hd.full = full;
@@ -2635,7 +2636,8 @@ static int perf_header__read_pipe(struct perf_session *session)
struct perf_header *header = &session->header;
struct perf_pipe_file_header f_header;
- if (perf_file_header__read_pipe(&f_header, header, session->fd,
+ if (perf_file_header__read_pipe(&f_header, header,
+ perf_data_file__fd(session->file),
session->repipe) < 0) {
pr_debug("incompatible file format\n");
return -EINVAL;
@@ -2736,18 +2738,19 @@ static int perf_evlist__prepare_tracepoint_events(struct perf_evlist *evlist,
int perf_session__read_header(struct perf_session *session)
{
+ struct perf_data_file *file = session->file;
struct perf_header *header = &session->header;
struct perf_file_header f_header;
struct perf_file_attr f_attr;
u64 f_id;
int nr_attrs, nr_ids, i, j;
- int fd = session->fd;
+ int fd = perf_data_file__fd(file);
session->evlist = perf_evlist__new();
if (session->evlist == NULL)
return -ENOMEM;
- if (session->fd_pipe)
+ if (perf_data_file__is_pipe(file))
return perf_header__read_pipe(session);
if (perf_file_header__read(&f_header, header, fd) < 0)
@@ -2963,18 +2966,19 @@ int perf_event__process_tracing_data(struct perf_tool *tool __maybe_unused,
struct perf_session *session)
{
ssize_t size_read, padding, size = event->tracing_data.size;
- off_t offset = lseek(session->fd, 0, SEEK_CUR);
+ int fd = perf_data_file__fd(session->file);
+ off_t offset = lseek(fd, 0, SEEK_CUR);
char buf[BUFSIZ];
/* setup for reading amidst mmap */
- lseek(session->fd, offset + sizeof(struct tracing_data_event),
+ lseek(fd, offset + sizeof(struct tracing_data_event),
SEEK_SET);
- size_read = trace_report(session->fd, &session->pevent,
+ size_read = trace_report(fd, &session->pevent,
session->repipe);
padding = PERF_ALIGN(size_read, sizeof(u64)) - size_read;
- if (readn(session->fd, buf, padding) < 0) {
+ if (readn(fd, buf, padding) < 0) {
pr_err("%s: reading input file", __func__);
return -1;
}
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 8d6ae30..3cc1f60 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -18,17 +18,16 @@
static int perf_session__open(struct perf_session *self)
{
- if (self->fd_pipe) {
- if (perf_session__read_header(self) < 0)
- pr_err("incompatible file format (rerun with -v to learn more)");
- return 0;
- }
+ struct perf_data_file *file = self->file;
if (perf_session__read_header(self) < 0) {
pr_err("incompatible file format (rerun with -v to learn more)");
return -1;
}
+ if (perf_data_file__is_pipe(file))
+ return 0;
+
if (!perf_evlist__valid_sample_type(self->evlist)) {
pr_err("non matching sample_type");
return -1;
@@ -87,10 +86,7 @@ struct perf_session *perf_session__new(struct perf_data_file *file,
if (perf_data_file__open(file))
goto out_delete;
- self->fd = file->fd;
- self->fd_pipe = file->is_pipe;
- self->filename = file->path;
- self->size = file->size;
+ self->file = file;
if (perf_data_file__is_read(file)) {
if (perf_session__open(self) < 0)
@@ -158,7 +154,8 @@ void perf_session__delete(struct perf_session *self)
perf_session__delete_threads(self);
perf_session_env__delete(&self->header.env);
machines__exit(&self->machines);
- close(self->fd);
+ if (self->file)
+ perf_data_file__close(self->file);
free(self);
vdso__exit();
}
@@ -1006,6 +1003,7 @@ static int perf_session_deliver_event(struct perf_session *session,
static int perf_session__process_user_event(struct perf_session *session, union perf_event *event,
struct perf_tool *tool, u64 file_offset)
{
+ int fd = perf_data_file__fd(session->file);
int err;
dump_event(session, event, file_offset, NULL);
@@ -1019,7 +1017,7 @@ static int perf_session__process_user_event(struct perf_session *session, union
return err;
case PERF_RECORD_HEADER_TRACING_DATA:
/* setup for reading amidst mmap */
- lseek(session->fd, file_offset, SEEK_SET);
+ lseek(fd, file_offset, SEEK_SET);
return tool->tracing_data(tool, event, session);
case PERF_RECORD_HEADER_BUILD_ID:
return tool->build_id(tool, event, session);
@@ -1146,6 +1144,7 @@ volatile int session_done;
static int __perf_session__process_pipe_events(struct perf_session *self,
struct perf_tool *tool)
{
+ int fd = perf_data_file__fd(self->file);
union perf_event *event;
uint32_t size, cur_size = 0;
void *buf = NULL;
@@ -1164,7 +1163,7 @@ static int __perf_session__process_pipe_events(struct perf_session *self,
return -errno;
more:
event = buf;
- err = readn(self->fd, event, sizeof(struct perf_event_header));
+ err = readn(fd, event, sizeof(struct perf_event_header));
if (err <= 0) {
if (err == 0)
goto done;
@@ -1196,7 +1195,7 @@ more:
p += sizeof(struct perf_event_header);
if (size - sizeof(struct perf_event_header)) {
- err = readn(self->fd, p, size - sizeof(struct perf_event_header));
+ err = readn(fd, p, size - sizeof(struct perf_event_header));
if (err <= 0) {
if (err == 0) {
pr_err("unexpected end of event stream\n");
@@ -1275,6 +1274,7 @@ int __perf_session__process_events(struct perf_session *session,
u64 data_offset, u64 data_size,
u64 file_size, struct perf_tool *tool)
{
+ int fd = perf_data_file__fd(session->file);
u64 head, page_offset, file_offset, file_pos, progress_next;
int err, mmap_prot, mmap_flags, map_idx = 0;
size_t mmap_size;
@@ -1307,7 +1307,7 @@ int __perf_session__process_events(struct perf_session *session,
mmap_flags = MAP_PRIVATE;
}
remap:
- buf = mmap(NULL, mmap_size, mmap_prot, mmap_flags, session->fd,
+ buf = mmap(NULL, mmap_size, mmap_prot, mmap_flags, fd,
file_offset);
if (buf == MAP_FAILED) {
pr_err("failed to mmap file\n");
@@ -1369,16 +1369,17 @@ out_err:
int perf_session__process_events(struct perf_session *self,
struct perf_tool *tool)
{
+ u64 size = perf_data_file__size(self->file);
int err;
if (perf_session__register_idle_thread(self) == NULL)
return -ENOMEM;
- if (!self->fd_pipe)
+ if (!perf_data_file__is_pipe(self->file))
err = __perf_session__process_events(self,
self->header.data_offset,
self->header.data_size,
- self->size, tool);
+ size, tool);
else
err = __perf_session__process_pipe_events(self, tool);
@@ -1602,13 +1603,14 @@ int perf_session__cpu_bitmap(struct perf_session *session,
void perf_session__fprintf_info(struct perf_session *session, FILE *fp,
bool full)
{
+ int fd = perf_data_file__fd(session->file);
struct stat st;
int ret;
if (session == NULL || fp == NULL)
return;
- ret = fstat(session->fd, &st);
+ ret = fstat(fd, &st);
if (ret == -1)
return;
diff --git a/tools/perf/util/session.h b/tools/perf/util/session.h
index c6a21d9..5748198 100644
--- a/tools/perf/util/session.h
+++ b/tools/perf/util/session.h
@@ -30,16 +30,13 @@ struct ordered_samples {
struct perf_session {
struct perf_header header;
- unsigned long size;
struct machines machines;
struct perf_evlist *evlist;
struct pevent *pevent;
struct events_stats stats;
- int fd;
- bool fd_pipe;
bool repipe;
struct ordered_samples ordered_samples;
- const char *filename;
+ struct perf_data_file *file;
};
#define PRINT_IP_OPT_IP (1<<0)
--
1.7.11.7
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] perf tools: Add perf_data_file__open interface to data object
[not found] <1381847254-28809-1-git-send-email-jolsa@redhat.com>
@ 2013-10-15 14:27 ` Jiri Olsa
2013-10-23 6:34 ` Adrian Hunter
0 siblings, 1 reply; 6+ messages in thread
From: Jiri Olsa @ 2013-10-15 14:27 UTC (permalink / raw)
To: linux-kernel
Cc: Jiri Olsa, Corey Ashford, Frederic Weisbecker, Ingo Molnar,
Namhyung Kim, Paul Mackerras, Peter Zijlstra,
Arnaldo Carvalho de Melo, David Ahern, Adrian Hunter, Andi Kleen
Adding perf_data_file__open interface to data object
to open the perf.data file for both read and write.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Andi Kleen <andi@firstfloor.org>
---
tools/perf/Makefile.perf | 1 +
tools/perf/builtin-record.c | 34 +------------
tools/perf/builtin-top.c | 9 +---
tools/perf/util/data.c | 120 ++++++++++++++++++++++++++++++++++++++++++++
tools/perf/util/data.h | 4 ++
tools/perf/util/session.c | 95 +++++++++++------------------------
tools/perf/util/session.h | 2 +-
7 files changed, 158 insertions(+), 107 deletions(-)
create mode 100644 tools/perf/util/data.c
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index c873e03..326a26e 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -365,6 +365,7 @@ LIB_OBJS += $(OUTPUT)util/vdso.o
LIB_OBJS += $(OUTPUT)util/stat.o
LIB_OBJS += $(OUTPUT)util/record.o
LIB_OBJS += $(OUTPUT)util/srcline.o
+LIB_OBJS += $(OUTPUT)util/data.o
LIB_OBJS += $(OUTPUT)ui/setup.o
LIB_OBJS += $(OUTPUT)ui/helpline.o
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 37088f9..d83d54a 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -345,8 +345,6 @@ out:
static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
{
- struct stat st;
- int flags;
int err, feat;
unsigned long waking = 0;
const bool forks = argc > 0;
@@ -355,7 +353,6 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
struct perf_record_opts *opts = &rec->opts;
struct perf_evlist *evsel_list = rec->evlist;
struct perf_data_file *file = &rec->file;
- const char *output_name = file->path;
struct perf_session *session;
bool disabled = false;
@@ -367,35 +364,6 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
signal(SIGUSR1, sig_handler);
signal(SIGTERM, sig_handler);
- if (!output_name) {
- if (!fstat(STDOUT_FILENO, &st) && S_ISFIFO(st.st_mode))
- file->is_pipe = true;
- else
- file->path = output_name = "perf.data";
- }
- if (output_name) {
- if (!strcmp(output_name, "-"))
- file->is_pipe = true;
- else if (!stat(output_name, &st) && st.st_size) {
- char oldname[PATH_MAX];
- snprintf(oldname, sizeof(oldname), "%s.old",
- output_name);
- unlink(oldname);
- rename(output_name, oldname);
- }
- }
-
- flags = O_CREAT|O_RDWR|O_TRUNC;
-
- if (file->is_pipe)
- file->fd = STDOUT_FILENO;
- else
- file->fd = open(output_name, flags, S_IRUSR | S_IWUSR);
- if (file->fd < 0) {
- perror("failed to create output file");
- return -1;
- }
-
session = perf_session__new(file, false, NULL);
if (session == NULL) {
pr_err("Not enough memory for reading perf file header\n");
@@ -586,7 +554,7 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
fprintf(stderr,
"[ perf record: Captured and wrote %.3f MB %s (~%" PRIu64 " samples) ]\n",
(double)rec->bytes_written / 1024.0 / 1024.0,
- output_name,
+ file->path,
rec->bytes_written / 24);
return 0;
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 752bebe..d934f70 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -929,15 +929,8 @@ static int __cmd_top(struct perf_top *top)
struct perf_record_opts *opts = &top->record_opts;
pthread_t thread;
int ret;
- struct perf_data_file file = {
- .mode = PERF_DATA_MODE_WRITE,
- };
- /*
- * 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.
- */
- top->session = perf_session__new(&file, false, NULL);
+ top->session = perf_session__new(NULL, false, NULL);
if (top->session == NULL)
return -ENOMEM;
diff --git a/tools/perf/util/data.c b/tools/perf/util/data.c
new file mode 100644
index 0000000..7d09faf
--- /dev/null
+++ b/tools/perf/util/data.c
@@ -0,0 +1,120 @@
+#include <linux/compiler.h>
+#include <linux/kernel.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
+#include <string.h>
+
+#include "data.h"
+#include "util.h"
+
+static bool check_pipe(struct perf_data_file *file)
+{
+ struct stat st;
+ bool is_pipe = false;
+ int fd = perf_data_file__is_read(file) ?
+ STDIN_FILENO : STDOUT_FILENO;
+
+ if (!file->path) {
+ if (!fstat(fd, &st) && S_ISFIFO(st.st_mode))
+ is_pipe = true;
+ } else {
+ if (!strcmp(file->path, "-"))
+ is_pipe = true;
+ }
+
+ if (is_pipe)
+ file->fd = fd;
+
+ return file->is_pipe = is_pipe;
+}
+
+static int check_backup(struct perf_data_file *file)
+{
+ struct stat st;
+
+ if (!stat(file->path, &st) && st.st_size) {
+ /* TODO check errors properly */
+ char oldname[PATH_MAX];
+ snprintf(oldname, sizeof(oldname), "%s.old",
+ file->path);
+ unlink(oldname);
+ rename(file->path, oldname);
+ }
+
+ return 0;
+}
+
+static int open_file_read(struct perf_data_file *file)
+{
+ struct stat st;
+ int fd;
+
+ fd = open(file->path, O_RDONLY);
+ if (fd < 0) {
+ int err = errno;
+
+ pr_err("failed to open %s: %s", file->path, strerror(err));
+ if (err == ENOENT && !strcmp(file->path, "perf.data"))
+ pr_err(" (try 'perf record' first)");
+ pr_err("\n");
+ return -err;
+ }
+
+ if (fstat(fd, &st) < 0)
+ goto out_close;
+
+ if (!file->force && st.st_uid && (st.st_uid != geteuid())) {
+ pr_err("file %s not owned by current user or root\n",
+ file->path);
+ goto out_close;
+ }
+
+ if (!st.st_size) {
+ pr_info("zero-sized file (%s), nothing to do!\n",
+ file->path);
+ goto out_close;
+ }
+
+ file->size = st.st_size;
+ return fd;
+
+ out_close:
+ close(fd);
+ return -1;
+}
+
+static int open_file_write(struct perf_data_file *file)
+{
+ if (check_backup(file))
+ return -1;
+
+ return open(file->path, O_CREAT|O_RDWR|O_TRUNC, S_IRUSR|S_IWUSR);
+}
+
+static int open_file(struct perf_data_file *file)
+{
+ int fd;
+
+ fd = perf_data_file__is_read(file) ?
+ open_file_read(file) : open_file_write(file);
+
+ file->fd = fd;
+ return fd < 0 ? -1 : 0;
+}
+
+int perf_data_file__open(struct perf_data_file *file)
+{
+ if (check_pipe(file))
+ return 0;
+
+ if (!file->path)
+ file->path = "perf.data";
+
+ return open_file(file);
+}
+
+void perf_data_file__close(struct perf_data_file *file)
+{
+ close(file->fd);
+}
diff --git a/tools/perf/util/data.h b/tools/perf/util/data.h
index ffa0186..d6c262e 100644
--- a/tools/perf/util/data.h
+++ b/tools/perf/util/data.h
@@ -13,6 +13,7 @@ struct perf_data_file {
int fd;
bool is_pipe;
bool force;
+ unsigned long size;
enum perf_data_mode mode;
};
@@ -26,4 +27,7 @@ static inline bool perf_data_file__is_write(struct perf_data_file *file)
return file->mode == PERF_DATA_MODE_WRITE;
}
+int perf_data_file__open(struct perf_data_file *file);
+void perf_data_file__close(struct perf_data_file *file);
+
#endif /* __PERF_DATA_H */
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 8b551a5..061e81f 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -16,73 +16,35 @@
#include "perf_regs.h"
#include "vdso.h"
-static int perf_session__open(struct perf_session *self, bool force)
+static int perf_session__open(struct perf_session *self)
{
- struct stat input_stat;
-
- if (!strcmp(self->filename, "-")) {
- self->fd_pipe = true;
- self->fd = STDIN_FILENO;
-
+ if (self->fd_pipe) {
if (perf_session__read_header(self) < 0)
pr_err("incompatible file format (rerun with -v to learn more)");
-
return 0;
}
- self->fd = open(self->filename, O_RDONLY);
- if (self->fd < 0) {
- int err = errno;
-
- pr_err("failed to open %s: %s", self->filename, strerror(err));
- if (err == ENOENT && !strcmp(self->filename, "perf.data"))
- pr_err(" (try 'perf record' first)");
- pr_err("\n");
- return -errno;
- }
-
- if (fstat(self->fd, &input_stat) < 0)
- goto out_close;
-
- if (!force && input_stat.st_uid && (input_stat.st_uid != geteuid())) {
- pr_err("file %s not owned by current user or root\n",
- self->filename);
- goto out_close;
- }
-
- if (!input_stat.st_size) {
- pr_info("zero-sized file (%s), nothing to do!\n",
- self->filename);
- goto out_close;
- }
-
if (perf_session__read_header(self) < 0) {
pr_err("incompatible file format (rerun with -v to learn more)");
- goto out_close;
+ return -1;
}
if (!perf_evlist__valid_sample_type(self->evlist)) {
pr_err("non matching sample_type");
- goto out_close;
+ return -1;
}
if (!perf_evlist__valid_sample_id_all(self->evlist)) {
pr_err("non matching sample_id_all");
- goto out_close;
+ return -1;
}
if (!perf_evlist__valid_read_format(self->evlist)) {
pr_err("non matching read_format");
- goto out_close;
+ return -1;
}
- self->size = input_stat.st_size;
return 0;
-
-out_close:
- close(self->fd);
- self->fd = -1;
- return -1;
}
void perf_session__set_id_hdr_size(struct perf_session *session)
@@ -110,35 +72,35 @@ struct perf_session *perf_session__new(struct perf_data_file *file,
bool repipe, struct perf_tool *tool)
{
struct perf_session *self;
- const char *filename = file->path;
- struct stat st;
- size_t len;
-
- if (!filename || !strlen(filename)) {
- if (!fstat(STDIN_FILENO, &st) && S_ISFIFO(st.st_mode))
- filename = "-";
- else
- filename = "perf.data";
- }
- len = strlen(filename);
- self = zalloc(sizeof(*self) + len);
-
- if (self == NULL)
+ self = zalloc(sizeof(*self));
+ if (!self)
goto out;
- memcpy(self->filename, filename, len);
self->repipe = repipe;
INIT_LIST_HEAD(&self->ordered_samples.samples);
INIT_LIST_HEAD(&self->ordered_samples.sample_cache);
INIT_LIST_HEAD(&self->ordered_samples.to_free);
machines__init(&self->machines);
- if (perf_data_file__is_read(file)) {
- if (perf_session__open(self, file->force) < 0)
+ if (file) {
+ if (perf_data_file__open(file))
goto out_delete;
- perf_session__set_id_hdr_size(self);
- } else if (perf_data_file__is_write(file)) {
+
+ self->fd = file->fd;
+ self->fd_pipe = file->is_pipe;
+ self->filename = file->path;
+ self->size = file->size;
+
+ if (perf_data_file__is_read(file)) {
+ if (perf_session__open(self) < 0)
+ goto out_close;
+
+ perf_session__set_id_hdr_size(self);
+ }
+ }
+
+ if (!file || perf_data_file__is_write(file)) {
/*
* In O_RDONLY mode this will be performed when reading the
* kernel MMAP event, in perf_event__process_mmap().
@@ -153,10 +115,13 @@ struct perf_session *perf_session__new(struct perf_data_file *file,
tool->ordered_samples = false;
}
-out:
return self;
-out_delete:
+
+ out_close:
+ perf_data_file__close(file);
+ out_delete:
perf_session__delete(self);
+ out:
return NULL;
}
diff --git a/tools/perf/util/session.h b/tools/perf/util/session.h
index f2f6251..e1ca2d0 100644
--- a/tools/perf/util/session.h
+++ b/tools/perf/util/session.h
@@ -39,7 +39,7 @@ struct perf_session {
bool fd_pipe;
bool repipe;
struct ordered_samples ordered_samples;
- char filename[1];
+ const char *filename;
};
#define PRINT_IP_OPT_IP (1<<0)
--
1.7.11.7
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/3] perf tools: Add perf_data_file__open interface to data object
2013-10-15 14:27 ` [PATCH 2/3] perf tools: Add perf_data_file__open interface to data object Jiri Olsa
@ 2013-10-23 6:34 ` Adrian Hunter
0 siblings, 0 replies; 6+ messages in thread
From: Adrian Hunter @ 2013-10-23 6:34 UTC (permalink / raw)
To: Jiri Olsa
Cc: linux-kernel, Corey Ashford, Frederic Weisbecker, Ingo Molnar,
Namhyung Kim, Paul Mackerras, Peter Zijlstra,
Arnaldo Carvalho de Melo, David Ahern, Andi Kleen
On 15/10/13 17:27, Jiri Olsa wrote:
> Adding perf_data_file__open interface to data object
> to open the perf.data file for both read and write.
>
> Signed-off-by: Jiri Olsa <jolsa@redhat.com>
> Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: Andi Kleen <andi@firstfloor.org>
> ---
> tools/perf/Makefile.perf | 1 +
> tools/perf/builtin-record.c | 34 +------------
> tools/perf/builtin-top.c | 9 +---
> tools/perf/util/data.c | 120 ++++++++++++++++++++++++++++++++++++++++++++
> tools/perf/util/data.h | 4 ++
> tools/perf/util/session.c | 95 +++++++++++------------------------
> tools/perf/util/session.h | 2 +-
> 7 files changed, 158 insertions(+), 107 deletions(-)
> create mode 100644 tools/perf/util/data.c
>
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index c873e03..326a26e 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -365,6 +365,7 @@ LIB_OBJS += $(OUTPUT)util/vdso.o
> LIB_OBJS += $(OUTPUT)util/stat.o
> LIB_OBJS += $(OUTPUT)util/record.o
> LIB_OBJS += $(OUTPUT)util/srcline.o
> +LIB_OBJS += $(OUTPUT)util/data.o
Also needs:
LIB_H += util/data.h
>
> LIB_OBJS += $(OUTPUT)ui/setup.o
> LIB_OBJS += $(OUTPUT)ui/helpline.o
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index 37088f9..d83d54a 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -345,8 +345,6 @@ out:
>
> static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
> {
> - struct stat st;
> - int flags;
> int err, feat;
> unsigned long waking = 0;
> const bool forks = argc > 0;
> @@ -355,7 +353,6 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
> struct perf_record_opts *opts = &rec->opts;
> struct perf_evlist *evsel_list = rec->evlist;
> struct perf_data_file *file = &rec->file;
> - const char *output_name = file->path;
> struct perf_session *session;
> bool disabled = false;
>
> @@ -367,35 +364,6 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
> signal(SIGUSR1, sig_handler);
> signal(SIGTERM, sig_handler);
>
> - if (!output_name) {
> - if (!fstat(STDOUT_FILENO, &st) && S_ISFIFO(st.st_mode))
> - file->is_pipe = true;
> - else
> - file->path = output_name = "perf.data";
> - }
> - if (output_name) {
> - if (!strcmp(output_name, "-"))
> - file->is_pipe = true;
> - else if (!stat(output_name, &st) && st.st_size) {
> - char oldname[PATH_MAX];
> - snprintf(oldname, sizeof(oldname), "%s.old",
> - output_name);
> - unlink(oldname);
> - rename(output_name, oldname);
> - }
> - }
> -
> - flags = O_CREAT|O_RDWR|O_TRUNC;
> -
> - if (file->is_pipe)
> - file->fd = STDOUT_FILENO;
> - else
> - file->fd = open(output_name, flags, S_IRUSR | S_IWUSR);
> - if (file->fd < 0) {
> - perror("failed to create output file");
> - return -1;
> - }
> -
> session = perf_session__new(file, false, NULL);
> if (session == NULL) {
> pr_err("Not enough memory for reading perf file header\n");
> @@ -586,7 +554,7 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
> fprintf(stderr,
> "[ perf record: Captured and wrote %.3f MB %s (~%" PRIu64 " samples) ]\n",
> (double)rec->bytes_written / 1024.0 / 1024.0,
> - output_name,
> + file->path,
> rec->bytes_written / 24);
>
> return 0;
> diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> index 752bebe..d934f70 100644
> --- a/tools/perf/builtin-top.c
> +++ b/tools/perf/builtin-top.c
> @@ -929,15 +929,8 @@ static int __cmd_top(struct perf_top *top)
> struct perf_record_opts *opts = &top->record_opts;
> pthread_t thread;
> int ret;
> - struct perf_data_file file = {
> - .mode = PERF_DATA_MODE_WRITE,
> - };
>
> - /*
> - * 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.
> - */
> - top->session = perf_session__new(&file, false, NULL);
> + top->session = perf_session__new(NULL, false, NULL);
> if (top->session == NULL)
> return -ENOMEM;
>
> diff --git a/tools/perf/util/data.c b/tools/perf/util/data.c
> new file mode 100644
> index 0000000..7d09faf
> --- /dev/null
> +++ b/tools/perf/util/data.c
> @@ -0,0 +1,120 @@
> +#include <linux/compiler.h>
> +#include <linux/kernel.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <unistd.h>
> +#include <string.h>
> +
> +#include "data.h"
> +#include "util.h"
> +
> +static bool check_pipe(struct perf_data_file *file)
> +{
> + struct stat st;
> + bool is_pipe = false;
> + int fd = perf_data_file__is_read(file) ?
> + STDIN_FILENO : STDOUT_FILENO;
> +
> + if (!file->path) {
> + if (!fstat(fd, &st) && S_ISFIFO(st.st_mode))
> + is_pipe = true;
> + } else {
> + if (!strcmp(file->path, "-"))
> + is_pipe = true;
> + }
> +
> + if (is_pipe)
> + file->fd = fd;
> +
> + return file->is_pipe = is_pipe;
> +}
> +
> +static int check_backup(struct perf_data_file *file)
> +{
> + struct stat st;
> +
> + if (!stat(file->path, &st) && st.st_size) {
> + /* TODO check errors properly */
> + char oldname[PATH_MAX];
> + snprintf(oldname, sizeof(oldname), "%s.old",
> + file->path);
> + unlink(oldname);
> + rename(file->path, oldname);
> + }
> +
> + return 0;
> +}
> +
> +static int open_file_read(struct perf_data_file *file)
> +{
> + struct stat st;
> + int fd;
> +
> + fd = open(file->path, O_RDONLY);
> + if (fd < 0) {
> + int err = errno;
> +
> + pr_err("failed to open %s: %s", file->path, strerror(err));
> + if (err == ENOENT && !strcmp(file->path, "perf.data"))
> + pr_err(" (try 'perf record' first)");
> + pr_err("\n");
> + return -err;
> + }
> +
> + if (fstat(fd, &st) < 0)
> + goto out_close;
> +
> + if (!file->force && st.st_uid && (st.st_uid != geteuid())) {
> + pr_err("file %s not owned by current user or root\n",
> + file->path);
> + goto out_close;
> + }
> +
> + if (!st.st_size) {
> + pr_info("zero-sized file (%s), nothing to do!\n",
> + file->path);
> + goto out_close;
> + }
> +
> + file->size = st.st_size;
> + return fd;
> +
> + out_close:
> + close(fd);
> + return -1;
> +}
> +
> +static int open_file_write(struct perf_data_file *file)
> +{
> + if (check_backup(file))
> + return -1;
> +
> + return open(file->path, O_CREAT|O_RDWR|O_TRUNC, S_IRUSR|S_IWUSR);
> +}
> +
> +static int open_file(struct perf_data_file *file)
> +{
> + int fd;
> +
> + fd = perf_data_file__is_read(file) ?
> + open_file_read(file) : open_file_write(file);
> +
> + file->fd = fd;
> + return fd < 0 ? -1 : 0;
> +}
> +
> +int perf_data_file__open(struct perf_data_file *file)
> +{
> + if (check_pipe(file))
> + return 0;
> +
> + if (!file->path)
> + file->path = "perf.data";
> +
> + return open_file(file);
> +}
> +
> +void perf_data_file__close(struct perf_data_file *file)
> +{
> + close(file->fd);
> +}
> diff --git a/tools/perf/util/data.h b/tools/perf/util/data.h
> index ffa0186..d6c262e 100644
> --- a/tools/perf/util/data.h
> +++ b/tools/perf/util/data.h
> @@ -13,6 +13,7 @@ struct perf_data_file {
> int fd;
> bool is_pipe;
> bool force;
> + unsigned long size;
> enum perf_data_mode mode;
> };
>
> @@ -26,4 +27,7 @@ static inline bool perf_data_file__is_write(struct perf_data_file *file)
> return file->mode == PERF_DATA_MODE_WRITE;
> }
>
> +int perf_data_file__open(struct perf_data_file *file);
> +void perf_data_file__close(struct perf_data_file *file);
> +
> #endif /* __PERF_DATA_H */
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index 8b551a5..061e81f 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -16,73 +16,35 @@
> #include "perf_regs.h"
> #include "vdso.h"
>
> -static int perf_session__open(struct perf_session *self, bool force)
> +static int perf_session__open(struct perf_session *self)
> {
> - struct stat input_stat;
> -
> - if (!strcmp(self->filename, "-")) {
> - self->fd_pipe = true;
> - self->fd = STDIN_FILENO;
> -
> + if (self->fd_pipe) {
> if (perf_session__read_header(self) < 0)
> pr_err("incompatible file format (rerun with -v to learn more)");
> -
> return 0;
> }
>
> - self->fd = open(self->filename, O_RDONLY);
> - if (self->fd < 0) {
> - int err = errno;
> -
> - pr_err("failed to open %s: %s", self->filename, strerror(err));
> - if (err == ENOENT && !strcmp(self->filename, "perf.data"))
> - pr_err(" (try 'perf record' first)");
> - pr_err("\n");
> - return -errno;
> - }
> -
> - if (fstat(self->fd, &input_stat) < 0)
> - goto out_close;
> -
> - if (!force && input_stat.st_uid && (input_stat.st_uid != geteuid())) {
> - pr_err("file %s not owned by current user or root\n",
> - self->filename);
> - goto out_close;
> - }
> -
> - if (!input_stat.st_size) {
> - pr_info("zero-sized file (%s), nothing to do!\n",
> - self->filename);
> - goto out_close;
> - }
> -
> if (perf_session__read_header(self) < 0) {
> pr_err("incompatible file format (rerun with -v to learn more)");
> - goto out_close;
> + return -1;
> }
>
> if (!perf_evlist__valid_sample_type(self->evlist)) {
> pr_err("non matching sample_type");
> - goto out_close;
> + return -1;
> }
>
> if (!perf_evlist__valid_sample_id_all(self->evlist)) {
> pr_err("non matching sample_id_all");
> - goto out_close;
> + return -1;
> }
>
> if (!perf_evlist__valid_read_format(self->evlist)) {
> pr_err("non matching read_format");
> - goto out_close;
> + return -1;
> }
>
> - self->size = input_stat.st_size;
> return 0;
> -
> -out_close:
> - close(self->fd);
> - self->fd = -1;
> - return -1;
> }
>
> void perf_session__set_id_hdr_size(struct perf_session *session)
> @@ -110,35 +72,35 @@ struct perf_session *perf_session__new(struct perf_data_file *file,
> bool repipe, struct perf_tool *tool)
> {
> struct perf_session *self;
> - const char *filename = file->path;
> - struct stat st;
> - size_t len;
> -
> - if (!filename || !strlen(filename)) {
> - if (!fstat(STDIN_FILENO, &st) && S_ISFIFO(st.st_mode))
> - filename = "-";
> - else
> - filename = "perf.data";
> - }
>
> - len = strlen(filename);
> - self = zalloc(sizeof(*self) + len);
> -
> - if (self == NULL)
> + self = zalloc(sizeof(*self));
> + if (!self)
> goto out;
>
> - memcpy(self->filename, filename, len);
> self->repipe = repipe;
> INIT_LIST_HEAD(&self->ordered_samples.samples);
> INIT_LIST_HEAD(&self->ordered_samples.sample_cache);
> INIT_LIST_HEAD(&self->ordered_samples.to_free);
> machines__init(&self->machines);
>
> - if (perf_data_file__is_read(file)) {
> - if (perf_session__open(self, file->force) < 0)
> + if (file) {
> + if (perf_data_file__open(file))
> goto out_delete;
> - perf_session__set_id_hdr_size(self);
> - } else if (perf_data_file__is_write(file)) {
> +
> + self->fd = file->fd;
> + self->fd_pipe = file->is_pipe;
> + self->filename = file->path;
> + self->size = file->size;
> +
> + if (perf_data_file__is_read(file)) {
> + if (perf_session__open(self) < 0)
> + goto out_close;
> +
> + perf_session__set_id_hdr_size(self);
> + }
> + }
> +
> + if (!file || perf_data_file__is_write(file)) {
> /*
> * In O_RDONLY mode this will be performed when reading the
> * kernel MMAP event, in perf_event__process_mmap().
> @@ -153,10 +115,13 @@ struct perf_session *perf_session__new(struct perf_data_file *file,
> tool->ordered_samples = false;
> }
>
> -out:
> return self;
> -out_delete:
> +
> + out_close:
> + perf_data_file__close(file);
> + out_delete:
> perf_session__delete(self);
> + out:
> return NULL;
> }
>
> diff --git a/tools/perf/util/session.h b/tools/perf/util/session.h
> index f2f6251..e1ca2d0 100644
> --- a/tools/perf/util/session.h
> +++ b/tools/perf/util/session.h
> @@ -39,7 +39,7 @@ struct perf_session {
> bool fd_pipe;
> bool repipe;
> struct ordered_samples ordered_samples;
> - char filename[1];
> + const char *filename;
> };
>
> #define PRINT_IP_OPT_IP (1<<0)
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-10-23 6:35 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-07 9:31 [PATCH 0/3] perf tools: Factor perf data handling Jiri Olsa
2013-10-07 9:31 ` [PATCH 1/3] perf tools: Add data object to handle perf data file Jiri Olsa
2013-10-07 9:31 ` [PATCH 2/3] perf tools: Add perf_data_file__open interface to data object Jiri Olsa
2013-10-07 9:31 ` [PATCH 3/3] perf tools: Separating data file properties from session Jiri Olsa
[not found] <1381847254-28809-1-git-send-email-jolsa@redhat.com>
2013-10-15 14:27 ` [PATCH 2/3] perf tools: Add perf_data_file__open interface to data object Jiri Olsa
2013-10-23 6:34 ` Adrian Hunter
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 NNTP newsgroup(s).