| 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: <570F4BAE.7090808@huawei.com>
Date: Thu, 14 Apr 2016 15:50:06 +0800
From: "Wangnan (F)" <wangnan0@...wei.com>
To: Adrian Hunter <adrian.hunter@...el.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>
CC: <linux-kernel@...r.kernel.org>, <pi3orama@....com>,
He Kuang <hekuang@...wei.com>,
Arnaldo Carvalho de Melo <acme@...hat.com>,
"Masami Hiramatsu" <masami.hiramatsu.pt@...achi.com>,
Namhyung Kim <namhyung@...nel.org>,
Zefan Li <lizefan@...wei.com>
Subject: Re: [PATCH 03/10] perf record: Turns auxtrace_snapshot_enable into 3
states
On 2016/4/14 15:15, Adrian Hunter wrote:
> 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?
This patch doesn't prevent taking snapshot when receiving SIGUSR2.
If both --snapshot and --switch-outupt is provided, when SIGUSR2 received,
perf takes auxtrace snapshot and other perf buffer together.
Thank you.
> For that we would need continuous tracking information like MMAPS etc, but
> isn't that needed anyway?
>
>
>> Thanks,
>>
>> - Arnaldo
>>
>>> Signed-off-by: Wang Nan <wangnan0@...wei.com>
>>> Signed-off-by: He Kuang <hekuang@...wei.com>
>>> Acked-by: Jiri Olsa <jolsa@...nel.org>
>>> Cc: Arnaldo Carvalho de Melo <acme@...hat.com>
>>> Cc: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
>>> Cc: Namhyung Kim <namhyung@...nel.org>
>>> Cc: Zefan Li <lizefan@...wei.com>
>>> Cc: pi3orama@....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
Powered by blists - more mailing lists