On 14/04/16 10:50, Wangnan (F) wrote:
>
>
> 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.
So it was a stupid question ;-)
>
> If both --snapshot and --switch-outupt is provided, when SIGUSR2 received,
> perf takes auxtrace snapshot and other perf buffer together.
How do you keep from losing tracking information like MMAP events? Are they
is a different buffer?
>
> 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 <wangn...@huawei.com>
>>>> Signed-off-by: He Kuang <heku...@huawei.com>
>>>> Acked-by: Jiri Olsa <jo...@kernel.org>
>>>> Cc: Arnaldo Carvalho de Melo <a...@redhat.com>
>>>> Cc: Masami Hiramatsu <masami.hiramatsu...@hitachi.com>
>>>> Cc: Namhyung Kim <namhy...@kernel.org>
>>>> Cc: Zefan Li <lize...@huawei.com>
>>>> Cc: pi3or...@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
>
>
>
