Messages in this thread Patch in this message | | | Date | Tue, 7 Jul 2020 18:05:24 +0200 | From | Jiri Olsa <> | Subject | Re: [PATCH v9 11/15] perf stat: implement control commands handling |
| |
On Tue, Jul 07, 2020 at 05:55:14PM +0300, Alexey Budankov wrote:
SNIP
> process_evlist() now looks suboptimal since record mode code directly calls evlist__ctlfd_process() > and then handles returned command specifically to the mode. So in v10 I replaced process_evlist() > call with direct evlist__ctlfd_process() call and then handling the returned command by printing > command msg tag and counter values in the required order. Like this: > > + clock_gettime(CLOCK_MONOTONIC, &time_start); > + if (!(evlist__poll(evsel_list, time_to_sleep) > 0)) { /* poll timeout or EINTR */ > + if (timeout) > + break; > + else > + stop = handle_interval(interval, times); > + time_to_sleep = sleep_time; > + } else { /* fd revent */ > + if (evlist__ctlfd_process(evsel_list, &cmd) > 0) { > + if (interval) { > + switch (cmd) { > + case EVLIST_CTL_CMD_ENABLE: > + pr_info(EVLIST_ENABLED_MSG); > + process_interval(); > + break; > + case EVLIST_CTL_CMD_DISABLE: > + process_interval(); > + pr_info(EVLIST_DISABLED_MSG); > + break; > + case EVLIST_CTL_CMD_ACK: > + case EVLIST_CTL_CMD_UNSUPPORTED: > + default: > + break; > + } > + } > + } > + clock_gettime(CLOCK_MONOTONIC, &time_stop); > + compute_tts(&time_start, &time_stop, &time_to_sleep); > + }
hum, why not just get the bool from process_evlist like below?
jirka
--- diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c index 5021f7286422..32dd3de93f35 100644 --- a/tools/perf/builtin-stat.c +++ b/tools/perf/builtin-stat.c @@ -485,20 +485,20 @@ static bool handle_interval(unsigned int interval, int *times) return false; } -static bool process_evlist(struct evlist *evlist, unsigned int interval, int *times) +static bool process_evlist(struct evlist *evlist) { - bool stop = false; enum evlist_ctl_cmd cmd = EVLIST_CTL_CMD_UNSUPPORTED; + bool display = false; if (evlist__ctlfd_process(evlist, &cmd) > 0) { switch (cmd) { case EVLIST_CTL_CMD_ENABLE: pr_info(EVLIST_ENABLED_MSG); - stop = handle_interval(interval, times); + display = true; break; case EVLIST_CTL_CMD_DISABLE: - stop = handle_interval(interval, times); pr_info(EVLIST_DISABLED_MSG); + display = true; break; case EVLIST_CTL_CMD_ACK: case EVLIST_CTL_CMD_UNSUPPORTED: @@ -507,7 +507,7 @@ static bool process_evlist(struct evlist *evlist, unsigned int interval, int *ti } } - return stop; + return display; } static void enable_counters(void) @@ -618,7 +618,8 @@ static int dispatch_events(bool forks, int timeout, int interval, int *times) stop = handle_interval(interval, times); time_to_sleep = sleep_time; } else { /* fd revent */ - stop = process_evlist(evsel_list, interval, times); + if (process_evlist(evsel_list)) + stop = handle_interval(interval, times); clock_gettime(CLOCK_MONOTONIC, &time_stop); diff_timespec(&time_diff, &time_stop, &time_start); time_to_sleep -= time_diff.tv_sec * MSEC_PER_SEC +
| |