Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752173AbcDZNyl (ORCPT ); Tue, 26 Apr 2016 09:54:41 -0400 Received: from mail.kernel.org ([198.145.29.136]:40435 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750843AbcDZNyk (ORCPT ); Tue, 26 Apr 2016 09:54:40 -0400 Date: Tue, 26 Apr 2016 10:54:33 -0300 From: Arnaldo Carvalho de Melo To: Wang Nan Cc: jolsa@redhat.com, namhyung@kernel.org, linux-kernel@vger.kernel.org, pi3orama@163.com, Adrian Hunter , He Kuang , Jiri Olsa , Masami Hiramatsu , Zefan Li Subject: Re: [PATCH v6 2/7] perf tools: Derive trigger class from auxtrace_snapshot Message-ID: <20160426135433.GB11033@kernel.org> References: <1461178794-40467-1-git-send-email-wangnan0@huawei.com> <1461178794-40467-3-git-send-email-wangnan0@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1461178794-40467-3-git-send-email-wangnan0@huawei.com> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5331 Lines: 166 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, Best Regards, - Arnaldo > Signed-off-by: Wang Nan > Cc: Adrian Hunter > Cc: He Kuang > Cc: Jiri Olsa > Cc: Masami Hiramatsu > Cc: Namhyung Kim > Cc: Zefan Li > 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 > @@ -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