perf tools: Fix 'disabled' attribute config for record command

Currently the record command sets all events initially as disabled.

There's non conditional perf_evlist__enable call, that enables all
events before we exec tracee program. That actually screws whole
enable_on_exec logic, because the event is enabled before the traced
program got executed.

What we actually want is:

1) For any type of traced program:
  - all independent events and group leaders are disabled
  - all group members are enabled

   Group members are ruled by group leaders. They need to
   be enabled, because the group scheduling relies on that.

2) For traced programs executed by perf:
   - all independent events and group leaders have
     enable_on_exec set
   - we don't specifically enable or disable any event during
     the record command

   Independent events and group leaders are initially disabled
   and get enabled by exec. Group members are ruled by group
   leaders as stated in 1).

3) For traced programs attached by perf (pid/tid):
   - we specifically enable or disable all events during
     the record command

   When attaching events to already running traced we
   enable/disable events specifically, as there's no
   initial traced exec call.

Fixing appropriate perf_event_attr test case to cover this change.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1352741644-16809-3-git-send-email-jolsa@redhat.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 371702785d6813235277efce039b625ac8ec9c56..268b356391fc25bff4bee38cd232a8a4393ab1d2 100644 (file)
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -701,7 +701,13 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
                }
        }
 
-       perf_evlist__enable(evsel_list);
+       /*
+        * When perf is starting the traced process, all the events
+        * (apart from group members) have enable_on_exec=1 set,
+        * so don't spoil it by prematurely enabling them.
+        */
+       if (!perf_target__none(&opts->target))
+               perf_evlist__enable(evsel_list);
 
        /*
         * Let the child rip
@@ -724,7 +730,12 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
                        waking++;
                }
 
-               if (done)
+               /*
+                * When perf is starting the traced process, at the end events
+                * die with the process and we wait for that. Thus no need to
+                * disable events in this case.
+                */
+               if (done && !perf_target__none(&opts->target))
                        perf_evlist__disable(evsel_list);
        }
 

diff --git a/tools/perf/tests/attr/test-record-group b/tools/perf/tests/attr/test-record-group
index b945f770dc9e384bc238a3662268ae142e9b8882..a6599e9a19d302134359941e92e579c465e80989 100644 (file)
--- a/tools/perf/tests/attr/test-record-group
+++ b/tools/perf/tests/attr/test-record-group
@@ -15,3 +15,4 @@ sample_type=327
 mmap=0
 comm=0
 enable_on_exec=0
+disabled=0

diff --git a/tools/perf/tests/attr/test-record-group1 b/tools/perf/tests/attr/test-record-group1
index 013572f2360506e34ef5364e5d783b10c2275602..5a8359da38af9ed26f4c51ca84026bce2b3f2454 100644 (file)
--- a/tools/perf/tests/attr/test-record-group1
+++ b/tools/perf/tests/attr/test-record-group1
@@ -16,3 +16,4 @@ sample_type=327
 mmap=0
 comm=0
 enable_on_exec=0
+disabled=0

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 6d4a5f6ed75a7c07c0cdcad6e10374e58fa70156..fc4faaa7afb913f06536253d76ff42c6e01e91cb 100644 (file)
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -404,13 +404,40 @@ const char *perf_evsel__name(struct perf_evsel *evsel)
        return evsel->name ?: "unknown";
 }
 
+/*
+ * The enable_on_exec/disabled value strategy:
+ *
+ *  1) For any type of traced program:
+ *    - all independent events and group leaders are disabled
+ *    - all group members are enabled
+ *
+ *     Group members are ruled by group leaders. They need to
+ *     be enabled, because the group scheduling relies on that.
+ *
+ *  2) For traced programs executed by perf:
+ *     - all independent events and group leaders have
+ *       enable_on_exec set
+ *     - we don't specifically enable or disable any event during
+ *       the record command
+ *
+ *     Independent events and group leaders are initially disabled
+ *     and get enabled by exec. Group members are ruled by group
+ *     leaders as stated in 1).
+ *
+ *  3) For traced programs attached by perf (pid/tid):
+ *     - we specifically enable or disable all events during
+ *       the record command
+ *
+ *     When attaching events to already running traced we
+ *     enable/disable events specifically, as there's no
+ *     initial traced exec call.
+ */
 void perf_evsel__config(struct perf_evsel *evsel,
                        struct perf_record_opts *opts)
 {
        struct perf_event_attr *attr = &evsel->attr;
        int track = !evsel->idx; /* only the first counter needs these */
 
-       attr->disabled = 1;
        attr->sample_id_all = opts->sample_id_all_missing ? 0 : 1;
        attr->inherit       = !opts->no_inherit;
        attr->read_format   = PERF_FORMAT_TOTAL_TIME_ENABLED |
@@ -486,6 +513,19 @@ void perf_evsel__config(struct perf_evsel *evsel,
        attr->mmap = track;
        attr->comm = track;
 
+       /*
+        * XXX see the function comment above
+        *
+        * Disabling only independent events or group leaders,
+        * keeping group members enabled.
+        */
+       if (!evsel->leader)
+               attr->disabled = 1;
+
+       /*
+        * Setting enable_on_exec for independent events and
+        * group leaders for traced executed by perf.
+        */
        if (perf_target__none(&opts->target) && (!evsel->leader))
                attr->enable_on_exec = 1;
 }