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 +

Reply via email to