Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753003AbcD0JOi (ORCPT ); Wed, 27 Apr 2016 05:14:38 -0400 Received: from mga14.intel.com ([192.55.52.115]:19613 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752589AbcD0JOc (ORCPT ); Wed, 27 Apr 2016 05:14:32 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,540,1455004800"; d="scan'208";a="793343376" Subject: Re: [PATCH v6 2/7] perf tools: Derive trigger class from auxtrace_snapshot To: Arnaldo Carvalho de Melo , Wang Nan References: <1461178794-40467-1-git-send-email-wangnan0@huawei.com> <1461178794-40467-3-git-send-email-wangnan0@huawei.com> <20160426135433.GB11033@kernel.org> Cc: jolsa@redhat.com, namhyung@kernel.org, linux-kernel@vger.kernel.org, pi3orama@163.com, He Kuang , Jiri Olsa , Masami Hiramatsu , Zefan Li From: Adrian Hunter Organization: Intel Finland Oy, Registered Address: PL 281, 00181 Helsinki, Business Identity Code: 0357606 - 4, Domiciled in Helsinki Message-ID: <57208211.5060309@intel.com> Date: Wed, 27 Apr 2016 12:10:41 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <20160426135433.GB11033@kernel.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5611 Lines: 171 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 > > 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 >