| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <52676DFC.9030402@intel.com>
Date: Wed, 23 Oct 2013 09:34:36 +0300
From: Adrian Hunter <adrian.hunter@...el.com>
To: Jiri Olsa <jolsa@...hat.com>
CC: linux-kernel@...r.kernel.org,
Corey Ashford <cjashfor@...ux.vnet.ibm.com>,
Frederic Weisbecker <fweisbec@...il.com>,
Ingo Molnar <mingo@...e.hu>,
Namhyung Kim <namhyung@...nel.org>,
Paul Mackerras <paulus@...ba.org>,
Peter Zijlstra <a.p.zijlstra@...llo.nl>,
Arnaldo Carvalho de Melo <acme@...hat.com>,
David Ahern <dsahern@...il.com>,
Andi Kleen <andi@...stfloor.org>
Subject: Re: [PATCH 2/3] perf tools: Add perf_data_file__open interface to
data object
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@...hat.com>
> Cc: Corey Ashford <cjashfor@...ux.vnet.ibm.com>
> Cc: Frederic Weisbecker <fweisbec@...il.com>
> Cc: Ingo Molnar <mingo@...e.hu>
> Cc: Namhyung Kim <namhyung@...nel.org>
> Cc: Paul Mackerras <paulus@...ba.org>
> Cc: Peter Zijlstra <a.p.zijlstra@...llo.nl>
> Cc: Arnaldo Carvalho de Melo <acme@...hat.com>
> Cc: David Ahern <dsahern@...il.com>
> Cc: Adrian Hunter <adrian.hunter@...el.com>
> Cc: Andi Kleen <andi@...stfloor.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)
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists