Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752996AbcDNHTV (ORCPT ); Thu, 14 Apr 2016 03:19:21 -0400 Received: from mga03.intel.com ([134.134.136.65]:32492 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750974AbcDNHTS (ORCPT ); Thu, 14 Apr 2016 03:19:18 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,484,1455004800"; d="scan'208";a="686053990" Subject: Re: [PATCH 03/10] perf record: Turns auxtrace_snapshot_enable into 3 states To: Arnaldo Carvalho de Melo References: <1460535673-159866-1-git-send-email-wangnan0@huawei.com> <1460535673-159866-4-git-send-email-wangnan0@huawei.com> <20160413155533.GG9056@kernel.org> Cc: Wang Nan , linux-kernel@vger.kernel.org, pi3orama@163.com, He Kuang , Arnaldo Carvalho de Melo , Masami Hiramatsu , Namhyung Kim , 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: <570F4393.6080908@intel.com> Date: Thu, 14 Apr 2016 10:15:31 +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: <20160413155533.GG9056@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: 4791 Lines: 153 On 13/04/16 18:55, Arnaldo Carvalho de Melo wrote: > Em Wed, Apr 13, 2016 at 08:21:06AM +0000, Wang Nan escreveu: >> auxtrace_snapshot_enable has only two states (0/1). Turns it into a >> triple states enum so SIGUSR2 handler can safely do other works without >> triggering auxtrace snapshot. > > Adrian, can you take a look at this? Is it ok with you? Please forgive me if these are stupid questions: First I am wondering why we wouldn't want to snapshot auxtrace data at the same time as the perf buffer? For that we would need continuous tracking information like MMAPS etc, but isn't that needed anyway? > > Thanks, > > - Arnaldo > >> Signed-off-by: Wang Nan >> Signed-off-by: He Kuang >> Acked-by: Jiri Olsa >> Cc: Arnaldo Carvalho de Melo >> Cc: Masami Hiramatsu >> Cc: Namhyung Kim >> Cc: Zefan Li >> Cc: pi3orama@163.com >> --- >> tools/perf/builtin-record.c | 59 +++++++++++++++++++++++++++++++++++++-------- >> 1 file changed, 49 insertions(+), 10 deletions(-) >> >> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c >> index eb6a199..480033f 100644 >> --- a/tools/perf/builtin-record.c >> +++ b/tools/perf/builtin-record.c >> @@ -125,7 +125,43 @@ out: >> static volatile int done; >> static volatile int signr = -1; >> static volatile int child_finished; >> -static volatile int auxtrace_snapshot_enabled; >> + >> +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; >> >> @@ -249,7 +285,7 @@ static void record__read_auxtrace_snapshot(struct record *rec) >> } else { >> auxtrace_snapshot_err = auxtrace_record__snapshot_finish(rec->itr); >> if (!auxtrace_snapshot_err) >> - auxtrace_snapshot_enabled = 1; >> + auxtrace_snapshot_enable(); >> } >> } >> >> @@ -615,10 +651,13 @@ static int __cmd_record(struct record *rec, int argc, const char **argv) >> signal(SIGCHLD, sig_handler); >> signal(SIGINT, sig_handler); >> signal(SIGTERM, sig_handler); >> - if (rec->opts.auxtrace_snapshot_mode) >> + >> + if (rec->opts.auxtrace_snapshot_mode) { >> signal(SIGUSR2, snapshot_sig_handler); >> - else >> + auxtrace_snapshot_on(); >> + } else { >> signal(SIGUSR2, SIG_IGN); >> + } >> >> session = perf_session__new(file, false, tool); >> if (session == NULL) { >> @@ -744,12 +783,12 @@ static int __cmd_record(struct record *rec, int argc, const char **argv) >> perf_evlist__enable(rec->evlist); >> } >> >> - auxtrace_snapshot_enabled = 1; >> + auxtrace_snapshot_enable(); >> for (;;) { >> unsigned long long hits = rec->samples; >> >> if (record__mmap_read_all(rec) < 0) { >> - auxtrace_snapshot_enabled = 0; >> + auxtrace_snapshot_disable(); >> err = -1; >> goto out_child; >> } >> @@ -787,12 +826,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_enabled = 0; >> + auxtrace_snapshot_disable(); >> perf_evlist__disable(rec->evlist); >> disabled = true; >> } >> } >> - auxtrace_snapshot_enabled = 0; >> + auxtrace_snapshot_disable(); >> >> if (forks && workload_exec_errno) { >> char msg[STRERR_BUFSIZE]; >> @@ -1358,9 +1397,9 @@ out_symbol_exit: >> >> static void snapshot_sig_handler(int sig __maybe_unused) >> { >> - if (!auxtrace_snapshot_enabled) >> + if (!auxtrace_snapshot_is_enabled()) >> return; >> - auxtrace_snapshot_enabled = 0; >> + auxtrace_snapshot_disable(); >> auxtrace_snapshot_err = auxtrace_record__snapshot_start(record.itr); >> auxtrace_record__snapshot_started = 1; >> } >> -- >> 1.8.3.4 >