| 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: <20140714203237.GA18133@kernel.org>
Date: Mon, 14 Jul 2014 17:32:37 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Adrian Hunter <adrian.hunter@...el.com>
Cc: Peter Zijlstra <peterz@...radead.org>,
linux-kernel@...r.kernel.org, David Ahern <dsahern@...il.com>,
Frederic Weisbecker <fweisbec@...il.com>,
Jiri Olsa <jolsa@...hat.com>,
Namhyung Kim <namhyung@...il.com>,
Paul Mackerras <paulus@...ba.org>,
Stephane Eranian <eranian@...gle.com>
Subject: Re: [PATCH 05/41] perf tools: Identify which comms are from exec
Em Mon, Jul 14, 2014 at 01:02:29PM +0300, Adrian Hunter escreveu:
> Signed-off-by: Adrian Hunter <adrian.hunter@...el.com>
> ---
> tools/perf/util/comm.c | 7 +++++--
> tools/perf/util/comm.h | 6 ++++--
> tools/perf/util/machine.c | 4 +++-
> tools/perf/util/thread.c | 24 +++++++++++++++++++-----
> tools/perf/util/thread.h | 10 +++++++++-
> 5 files changed, 40 insertions(+), 11 deletions(-)
>
> diff --git a/tools/perf/util/comm.c b/tools/perf/util/comm.c
> index f9e7776..5e1e80e 100644
> --- a/tools/perf/util/comm.c
> +++ b/tools/perf/util/comm.c
> @@ -74,7 +74,7 @@ static struct comm_str *comm_str__findnew(const char *str, struct rb_root *root)
> return new;
> }
>
> -struct comm *comm__new(const char *str, u64 timestamp)
> +struct comm *comm__new(const char *str, u64 timestamp, bool exec)
> {
> struct comm *comm = zalloc(sizeof(*comm));
>
> @@ -82,6 +82,7 @@ struct comm *comm__new(const char *str, u64 timestamp)
> return NULL;
>
> comm->start = timestamp;
> + comm->exec = exec;
>
> comm->comm_str = comm_str__findnew(str, &comm_str_root);
> if (!comm->comm_str) {
> @@ -94,7 +95,7 @@ struct comm *comm__new(const char *str, u64 timestamp)
> return comm;
> }
>
> -int comm__override(struct comm *comm, const char *str, u64 timestamp)
> +int comm__override(struct comm *comm, const char *str, u64 timestamp, bool exec)
> {
> struct comm_str *new, *old = comm->comm_str;
>
> @@ -106,6 +107,8 @@ int comm__override(struct comm *comm, const char *str, u64 timestamp)
> comm_str__put(old);
> comm->comm_str = new;
> comm->start = timestamp;
> + if (exec && !comm->exec)
> + comm->exec = true;
Why do you need the !comm->exec test?
> return 0;
> }
> diff --git a/tools/perf/util/comm.h b/tools/perf/util/comm.h
> index fac5bd5..51c10ab 100644
> --- a/tools/perf/util/comm.h
> +++ b/tools/perf/util/comm.h
> @@ -11,11 +11,13 @@ struct comm {
> struct comm_str *comm_str;
> u64 start;
> struct list_head list;
> + bool exec;
> };
>
> void comm__free(struct comm *comm);
> -struct comm *comm__new(const char *str, u64 timestamp);
> +struct comm *comm__new(const char *str, u64 timestamp, bool exec);
> const char *comm__str(const struct comm *comm);
> -int comm__override(struct comm *comm, const char *str, u64 timestamp);
> +int comm__override(struct comm *comm, const char *str, u64 timestamp,
> + bool exec);
>
> #endif /* __PERF_COMM_H */
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index 0fa93c1..2513204 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -360,11 +360,13 @@ int machine__process_comm_event(struct machine *machine, union perf_event *event
> struct thread *thread = machine__findnew_thread(machine,
> event->comm.pid,
> event->comm.tid);
> + bool exec = event->header.misc & PERF_RECORD_MISC_COMM_EXEC;
>
> if (dump_trace)
> perf_event__fprintf_comm(event, stdout);
>
> - if (thread == NULL || thread__set_comm(thread, event->comm.comm, sample->time)) {
> + if (thread == NULL ||
> + __thread__set_comm(thread, event->comm.comm, sample->time, exec)) {
> dump_printf("problem processing PERF_RECORD_COMM, skipping event.\n");
> return -1;
> }
> diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
> index ca94295..149e417 100644
> --- a/tools/perf/util/thread.c
> +++ b/tools/perf/util/thread.c
> @@ -76,7 +76,7 @@ struct thread *thread__new(pid_t pid, pid_t tid)
> goto err_thread;
>
> snprintf(comm_str, 32, ":%d", tid);
> - comm = comm__new(comm_str, 0);
> + comm = comm__new(comm_str, 0, false);
> free(comm_str);
> if (!comm)
> goto err_thread;
> @@ -113,19 +113,33 @@ struct comm *thread__comm(const struct thread *thread)
> return list_first_entry(&thread->comm_list, struct comm, list);
> }
>
> +struct comm *thread__exec_comm(const struct thread *thread)
> +{
> + struct comm *comm, *last = NULL;
> +
> + list_for_each_entry(comm, &thread->comm_list, list) {
> + if (comm->exec)
> + return comm;
> + last = comm;
> + }
> +
> + return last;
> +}
> +
> /* CHECKME: time should always be 0 if event aren't ordered */
> -int thread__set_comm(struct thread *thread, const char *str, u64 timestamp)
> +int __thread__set_comm(struct thread *thread, const char *str, u64 timestamp,
> + bool exec)
> {
> struct comm *new, *curr = thread__comm(thread);
> int err;
>
> /* Override latest entry if it had no specific time coverage */
> - if (!curr->start) {
> - err = comm__override(curr, str, timestamp);
> + if (!curr->start && !curr->exec) {
> + err = comm__override(curr, str, timestamp, exec);
> if (err)
> return err;
> } else {
> - new = comm__new(str, timestamp);
> + new = comm__new(str, timestamp, exec);
> if (!new)
> return -ENOMEM;
> list_add(&new->list, &thread->comm_list);
> diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
> index 9de0629..b4269af 100644
> --- a/tools/perf/util/thread.h
> +++ b/tools/perf/util/thread.h
> @@ -38,9 +38,17 @@ static inline void thread__exited(struct thread *thread)
> thread->dead = true;
> }
>
> -int thread__set_comm(struct thread *thread, const char *comm, u64 timestamp);
> +int __thread__set_comm(struct thread *thread, const char *comm, u64 timestamp,
> + bool exec);
> +static inline int thread__set_comm(struct thread *thread, const char *comm,
> + u64 timestamp)
> +{
> + return __thread__set_comm(thread, comm, timestamp, false);
> +}
> +
So this is nice, you leave the existing function, setting exec to false,
so that you don't have to change the existing codepaths where it is not
from 'exec', and provide a __ prefixed variant where 'exec' can be set.
Why not to do the same thing for comm__new() ? No uses, i.e. in all
cases you will need to pass 'exec' as a variable?
I thought about doing the same thing for that machine__findnew_thread,
i.e. provide a variant where tid and pid is passed and one where just
the tid is passed, because the pid is unknown.
BTW, I'll update my perf/core branch with the set of patches that I
merged, so that you can have where to look for a consolidated tree with
the things already processed.
- Arnaldo
> int thread__comm_len(struct thread *thread);
> struct comm *thread__comm(const struct thread *thread);
> +struct comm *thread__exec_comm(const struct thread *thread);
> const char *thread__comm_str(const struct thread *thread);
> void thread__insert_map(struct thread *thread, struct map *map);
> int thread__fork(struct thread *thread, struct thread *parent, u64 timestamp);
> --
> 1.8.3.2
--
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