Messages in this thread | | | Subject | Re: [PATCH v6 2/7] perf tools: Derive trigger class from auxtrace_snapshot | From | Adrian Hunter <> | Date | Wed, 27 Apr 2016 12:10:41 +0300 |
| |
On 26/04/16 16:54, Arnaldo Carvalho de Melo wrote: > Em Wed, Apr 20, 2016 at 06:59:49PM +0000, Wang Nan escreveu: >> auxtrace_snapshot_state matches trigger model. Use trigger to implement >> it. auxtrace_snapshot_state and auxtrace_snapshot_err are absorbed. > > Adrian, this is changing code you wrote, can you please take a look? An > Acked-by would be excellent,
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> > Best Regards, > > - Arnaldo > >> Signed-off-by: Wang Nan <wangnan0@huawei.com> >> Cc: Adrian Hunter <adrian.hunter@intel.com> >> Cc: He Kuang <hekuang@huawei.com> >> Cc: Jiri Olsa <jolsa@kernel.org> >> Cc: Masami Hiramatsu <mhiramat@kernel.org> >> Cc: Namhyung Kim <namhyung@kernel.org> >> Cc: Zefan Li <lizefan@huawei.com> >> Cc: pi3orama@163.com >> --- >> tools/perf/builtin-record.c | 73 +++++++++++++-------------------------------- >> 1 file changed, 20 insertions(+), 53 deletions(-) >> >> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c >> index bd95933..f4710c8 100644 >> --- a/tools/perf/builtin-record.c >> +++ b/tools/perf/builtin-record.c >> @@ -34,6 +34,7 @@ >> #include "util/parse-regs-options.h" >> #include "util/llvm-utils.h" >> #include "util/bpf-loader.h" >> +#include "util/trigger.h" >> #include "asm/bug.h" >> >> #include <unistd.h> >> @@ -127,44 +128,8 @@ static volatile int done; >> static volatile int signr = -1; >> static volatile int child_finished; >> >> -static volatile enum { >> - AUXTRACE_SNAPSHOT_OFF = -1, >> - AUXTRACE_SNAPSHOT_DISABLED = 0, >> - AUXTRACE_SNAPSHOT_ENABLED = 1, >> -} auxtrace_snapshot_state = AUXTRACE_SNAPSHOT_OFF; >> - >> -static inline void >> -auxtrace_snapshot_on(void) >> -{ >> - auxtrace_snapshot_state = AUXTRACE_SNAPSHOT_DISABLED; >> -} >> - >> -static inline void >> -auxtrace_snapshot_enable(void) >> -{ >> - if (auxtrace_snapshot_state == AUXTRACE_SNAPSHOT_OFF) >> - return; >> - auxtrace_snapshot_state = AUXTRACE_SNAPSHOT_ENABLED; >> -} >> - >> -static inline void >> -auxtrace_snapshot_disable(void) >> -{ >> - if (auxtrace_snapshot_state == AUXTRACE_SNAPSHOT_OFF) >> - return; >> - auxtrace_snapshot_state = AUXTRACE_SNAPSHOT_DISABLED; >> -} >> - >> -static inline bool >> -auxtrace_snapshot_is_enabled(void) >> -{ >> - if (auxtrace_snapshot_state == AUXTRACE_SNAPSHOT_OFF) >> - return false; >> - return auxtrace_snapshot_state == AUXTRACE_SNAPSHOT_ENABLED; >> -} >> - >> -static volatile int auxtrace_snapshot_err; >> static volatile int auxtrace_record__snapshot_started; >> +static DEFINE_TRIGGER(auxtrace_snapshot_trigger); >> >> static void sig_handler(int sig) >> { >> @@ -282,11 +247,12 @@ static void record__read_auxtrace_snapshot(struct record *rec) >> { >> pr_debug("Recording AUX area tracing snapshot\n"); >> if (record__auxtrace_read_snapshot_all(rec) < 0) { >> - auxtrace_snapshot_err = -1; >> + trigger_error(&auxtrace_snapshot_trigger); >> } else { >> - auxtrace_snapshot_err = auxtrace_record__snapshot_finish(rec->itr); >> - if (!auxtrace_snapshot_err) >> - auxtrace_snapshot_enable(); >> + if (auxtrace_record__snapshot_finish(rec->itr)) >> + trigger_error(&auxtrace_snapshot_trigger); >> + else >> + trigger_ready(&auxtrace_snapshot_trigger); >> } >> } >> >> @@ -686,7 +652,7 @@ static int __cmd_record(struct record *rec, int argc, const char **argv) >> >> if (rec->opts.auxtrace_snapshot_mode) { >> signal(SIGUSR2, snapshot_sig_handler); >> - auxtrace_snapshot_on(); >> + trigger_on(&auxtrace_snapshot_trigger); >> } else { >> signal(SIGUSR2, SIG_IGN); >> } >> @@ -815,21 +781,21 @@ static int __cmd_record(struct record *rec, int argc, const char **argv) >> perf_evlist__enable(rec->evlist); >> } >> >> - auxtrace_snapshot_enable(); >> + trigger_ready(&auxtrace_snapshot_trigger); >> for (;;) { >> unsigned long long hits = rec->samples; >> >> if (record__mmap_read_all(rec) < 0) { >> - auxtrace_snapshot_disable(); >> + trigger_error(&auxtrace_snapshot_trigger); >> err = -1; >> goto out_child; >> } >> >> if (auxtrace_record__snapshot_started) { >> auxtrace_record__snapshot_started = 0; >> - if (!auxtrace_snapshot_err) >> + if (!trigger_is_error(&auxtrace_snapshot_trigger)) >> record__read_auxtrace_snapshot(rec); >> - if (auxtrace_snapshot_err) { >> + if (trigger_is_error(&auxtrace_snapshot_trigger)) { >> pr_err("AUX area tracing snapshot failed\n"); >> err = -1; >> goto out_child; >> @@ -858,12 +824,12 @@ static int __cmd_record(struct record *rec, int argc, const char **argv) >> * disable events in this case. >> */ >> if (done && !disabled && !target__none(&opts->target)) { >> - auxtrace_snapshot_disable(); >> + trigger_off(&auxtrace_snapshot_trigger); >> perf_evlist__disable(rec->evlist); >> disabled = true; >> } >> } >> - auxtrace_snapshot_disable(); >> + trigger_off(&auxtrace_snapshot_trigger); >> >> if (forks && workload_exec_errno) { >> char msg[STRERR_BUFSIZE]; >> @@ -1445,9 +1411,10 @@ out_symbol_exit: >> >> static void snapshot_sig_handler(int sig __maybe_unused) >> { >> - if (!auxtrace_snapshot_is_enabled()) >> - return; >> - auxtrace_snapshot_disable(); >> - auxtrace_snapshot_err = auxtrace_record__snapshot_start(record.itr); >> - auxtrace_record__snapshot_started = 1; >> + if (trigger_is_ready(&auxtrace_snapshot_trigger)) { >> + trigger_hit(&auxtrace_snapshot_trigger); >> + auxtrace_record__snapshot_started = 1; >> + if (auxtrace_record__snapshot_start(record.itr)) >> + trigger_error(&auxtrace_snapshot_trigger); >> + } >> } >> -- >> 1.8.3.4 >
| |