lkml.org 
[lkml]   [2012]   [Mar]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH/RFC] perf tools: Handle old kernels when opening perf event
    Date
    Changing default value of perf_guest back to false caused problems on old
    kernels and its fix bc76efe64533 ("perf tools: Handle kernels that don't
    support attr.exclude_{guest,host}") worked well for perf record/top.

    But I've just realized that using specific events on perf stat makes same
    kind of troubles too. It's because the parse_events calls event_attr_init
    for all events so that it would have exclude_guest set.

    Instead of fixing perf stat, I thought that changing perf_evsel__open()
    is more appropriate. Please take a look and give comments - especially
    on ->time_not_needed handling in builtin-record.c (I guess the original
    code had a bug) and checking ->sample_id_all_missing inside of
    perf_evsel__config (I believe checking it before perf_evsel__open is
    meaningless since it will always have the same value - so I dropped it).

    Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
    ---
    tools/perf/builtin-record.c | 32 ++++-----------------
    tools/perf/builtin-stat.c | 5 ++-
    tools/perf/builtin-test.c | 21 +++++++++++--
    tools/perf/builtin-top.c | 25 +++-------------
    tools/perf/perf.h | 8 ++++-
    tools/perf/util/evlist.c | 5 ++-
    tools/perf/util/evlist.h | 3 +-
    tools/perf/util/evsel.c | 65 +++++++++++++++++++++++++++++++++++-------
    tools/perf/util/evsel.h | 9 ++++--
    tools/perf/util/python.c | 11 ++++++-
    10 files changed, 111 insertions(+), 73 deletions(-)

    diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
    index 75d230fef202..b926082012b8 100644
    --- a/tools/perf/builtin-record.c
    +++ b/tools/perf/builtin-record.c
    @@ -181,9 +181,11 @@ static void perf_record__open(struct perf_record *rec)
    struct perf_evlist *evlist = rec->evlist;
    struct perf_session *session = rec->session;
    struct perf_record_opts *opts = &rec->opts;
    + struct perf_attr_conf attr_conf;

    first = list_entry(evlist->entries.next, struct perf_evsel, node);

    + memset(&attr_conf, 0, sizeof(attr_conf));
    perf_evlist__config_attrs(evlist, opts);

    list_for_each_entry(pos, &evlist->entries, node) {
    @@ -201,18 +203,14 @@ static void perf_record__open(struct perf_record *rec)
    * We need to move counter creation to perf_session, support
    * different sample_types, etc.
    */
    - bool time_needed = attr->sample_type & PERF_SAMPLE_TIME;
    + attr_conf.time_not_needed = !opts->sample_time &&
    + !opts->raw_samples;

    if (opts->group && pos != first)
    group_fd = first->fd;
    -fallback_missing_features:
    - if (opts->exclude_guest_missing)
    - attr->exclude_guest = attr->exclude_host = 0;
    -retry_sample_id:
    - attr->sample_id_all = opts->sample_id_all_missing ? 0 : 1;
    try_again:
    if (perf_evsel__open(pos, evlist->cpus, evlist->threads,
    - opts->group, group_fd) < 0) {
    + opts->group, group_fd, &attr_conf) < 0) {
    int err = errno;

    if (err == EPERM || err == EACCES) {
    @@ -221,23 +219,6 @@ try_again:
    } else if (err == ENODEV && opts->cpu_list) {
    die("No such device - did you specify"
    " an out-of-range profile CPU?\n");
    - } else if (err == EINVAL) {
    - if (!opts->exclude_guest_missing &&
    - (attr->exclude_guest || attr->exclude_host)) {
    - pr_debug("Old kernel, cannot exclude "
    - "guest or host samples.\n");
    - opts->exclude_guest_missing = true;
    - goto fallback_missing_features;
    - } else if (!opts->sample_id_all_missing) {
    - /*
    - * Old kernel, no attr->sample_id_type_all field
    - */
    - opts->sample_id_all_missing = true;
    - if (!opts->sample_time && !opts->raw_samples && !time_needed)
    - attr->sample_type &= ~PERF_SAMPLE_TIME;
    -
    - goto retry_sample_id;
    - }
    }

    /*
    @@ -245,8 +226,7 @@ try_again:
    * based cpu-clock-tick sw counter, which
    * is always available even if no PMU support:
    */
    - if (attr->type == PERF_TYPE_HARDWARE
    - && attr->config == PERF_COUNT_HW_CPU_CYCLES) {
    + if (perf_evsel__match(pos, HARDWARE, HW_CPU_CYCLES)) {

    if (verbose)
    ui__warning("The cycles event is not supported, "
    diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
    index ea40e4e8b227..5e3cf43177f5 100644
    --- a/tools/perf/builtin-stat.c
    +++ b/tools/perf/builtin-stat.c
    @@ -174,6 +174,7 @@ static struct perf_event_attr very_very_detailed_attrs[] = {


    struct perf_evlist *evsel_list;
    +struct perf_attr_conf attr_conf;

    static bool system_wide = false;
    static int run_idx = 0;
    @@ -295,14 +296,14 @@ static int create_perf_stat_counter(struct perf_evsel *evsel,

    if (system_wide)
    return perf_evsel__open_per_cpu(evsel, evsel_list->cpus,
    - group, group_fd);
    + group, group_fd, &attr_conf);
    if (!target_pid && !target_tid) {
    attr->disabled = 1;
    attr->enable_on_exec = 1;
    }

    return perf_evsel__open_per_thread(evsel, evsel_list->threads,
    - group, group_fd);
    + group, group_fd, &attr_conf);
    }

    /*
    diff --git a/tools/perf/builtin-test.c b/tools/perf/builtin-test.c
    index 3e087ce8daa6..6492bcfb0698 100644
    --- a/tools/perf/builtin-test.c
    +++ b/tools/perf/builtin-test.c
    @@ -270,6 +270,7 @@ static int test__open_syscall_event(void)
    struct thread_map *threads;
    struct perf_evsel *evsel;
    struct perf_event_attr attr;
    + struct perf_attr_conf attr_conf;
    unsigned int nr_open_calls = 111, i;
    int id = trace_event__id("sys_enter_open");

    @@ -293,7 +294,9 @@ static int test__open_syscall_event(void)
    goto out_thread_map_delete;
    }

    - if (perf_evsel__open_per_thread(evsel, threads, false, NULL) < 0) {
    + memset(&attr_conf, 0, sizeof(attr_conf));
    + if (perf_evsel__open_per_thread(evsel, threads, false, NULL,
    + &attr_conf) < 0) {
    pr_debug("failed to open counter: %s, "
    "tweak /proc/sys/kernel/perf_event_paranoid?\n",
    strerror(errno));
    @@ -335,6 +338,7 @@ static int test__open_syscall_event_on_all_cpus(void)
    struct cpu_map *cpus;
    struct perf_evsel *evsel;
    struct perf_event_attr attr;
    + struct perf_attr_conf attr_conf;
    unsigned int nr_open_calls = 111, i;
    cpu_set_t cpu_set;
    int id = trace_event__id("sys_enter_open");
    @@ -368,7 +372,9 @@ static int test__open_syscall_event_on_all_cpus(void)
    goto out_thread_map_delete;
    }

    - if (perf_evsel__open(evsel, cpus, threads, false, NULL) < 0) {
    + memset(&attr_conf, 0, sizeof(attr_conf));
    + if (perf_evsel__open(evsel, cpus, threads, false, NULL,
    + &attr_conf) < 0) {
    pr_debug("failed to open counter: %s, "
    "tweak /proc/sys/kernel/perf_event_paranoid?\n",
    strerror(errno));
    @@ -467,6 +473,7 @@ static int test__basic_mmap(void)
    .sample_type = PERF_SAMPLE_ID,
    .watermark = 0,
    };
    + struct perf_attr_conf attr_conf;
    cpu_set_t cpu_set;
    const char *syscall_names[] = { "getsid", "getppid", "getpgrp",
    "getpgid", };
    @@ -523,6 +530,8 @@ static int test__basic_mmap(void)
    attr.wakeup_events = 1;
    attr.sample_period = 1;

    + memset(&attr_conf, 0, sizeof(attr_conf));
    +
    for (i = 0; i < nsyscalls; ++i) {
    attr.config = ids[i];
    evsels[i] = perf_evsel__new(&attr, i);
    @@ -533,7 +542,8 @@ static int test__basic_mmap(void)

    perf_evlist__add(evlist, evsels[i]);

    - if (perf_evsel__open(evsels[i], cpus, threads, false, NULL) < 0) {
    + if (perf_evsel__open(evsels[i], cpus, threads, false, NULL,
    + &attr_conf) < 0) {
    pr_debug("failed to open counter: %s, "
    "tweak /proc/sys/kernel/perf_event_paranoid?\n",
    strerror(errno));
    @@ -1019,6 +1029,7 @@ static int test__PERF_RECORD(void)
    struct perf_evlist *evlist = perf_evlist__new(NULL, NULL);
    struct perf_evsel *evsel;
    struct perf_sample sample;
    + struct perf_attr_conf attr_conf;
    const char *cmd = "sleep";
    const char *argv[] = { cmd, "1", NULL, };
    char *bname;
    @@ -1097,11 +1108,13 @@ static int test__PERF_RECORD(void)
    goto out_free_cpu_mask;
    }

    + memset(&attr_conf, 0, sizeof(attr_conf));
    +
    /*
    * Call sys_perf_event_open on all the fds on all the evsels,
    * grouping them if asked to.
    */
    - err = perf_evlist__open(evlist, opts.group);
    + err = perf_evlist__open(evlist, opts.group, &attr_conf);
    if (err < 0) {
    pr_debug("perf_evlist__open: %s\n", strerror(errno));
    goto out_delete_evlist;
    diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
    index e3c63aef8efc..b3fd28a573dd 100644
    --- a/tools/perf/builtin-top.c
    +++ b/tools/perf/builtin-top.c
    @@ -843,6 +843,9 @@ static void perf_top__start_counters(struct perf_top *top)
    {
    struct perf_evsel *counter, *first;
    struct perf_evlist *evlist = top->evlist;
    + struct perf_attr_conf attr_conf;
    +
    + memset(&attr_conf, 0, sizeof(attr_conf));

    first = list_entry(evlist->entries.next, struct perf_evsel, node);

    @@ -872,34 +875,16 @@ static void perf_top__start_counters(struct perf_top *top)
    attr->mmap = 1;
    attr->comm = 1;
    attr->inherit = top->inherit;
    -fallback_missing_features:
    - if (top->exclude_guest_missing)
    - attr->exclude_guest = attr->exclude_host = 0;
    -retry_sample_id:
    - attr->sample_id_all = top->sample_id_all_missing ? 0 : 1;
    +
    try_again:
    if (perf_evsel__open(counter, top->evlist->cpus,
    top->evlist->threads, top->group,
    - group_fd) < 0) {
    + group_fd, &attr_conf) < 0) {
    int err = errno;

    if (err == EPERM || err == EACCES) {
    ui__error_paranoid();
    goto out_err;
    - } else if (err == EINVAL) {
    - if (!top->exclude_guest_missing &&
    - (attr->exclude_guest || attr->exclude_host)) {
    - pr_debug("Old kernel, cannot exclude "
    - "guest or host samples.\n");
    - top->exclude_guest_missing = true;
    - goto fallback_missing_features;
    - } else if (!top->sample_id_all_missing) {
    - /*
    - * Old kernel, no attr->sample_id_type_all field
    - */
    - top->sample_id_all_missing = true;
    - goto retry_sample_id;
    - }
    }
    /*
    * If it's cycles then fall back to hrtimer
    diff --git a/tools/perf/perf.h b/tools/perf/perf.h
    index f0227e93665d..60c3fcab384a 100644
    --- a/tools/perf/perf.h
    +++ b/tools/perf/perf.h
    @@ -198,8 +198,6 @@ struct perf_record_opts {
    bool raw_samples;
    bool sample_address;
    bool sample_time;
    - bool sample_id_all_missing;
    - bool exclude_guest_missing;
    bool system_wide;
    bool period;
    unsigned int freq;
    @@ -210,4 +208,10 @@ struct perf_record_opts {
    const char *cpu_list;
    };

    +struct perf_attr_conf {
    + bool sample_id_all_missing;
    + bool exclude_guest_missing;
    + bool time_not_needed;
    +};
    +
    #endif
    diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
    index 159263d17c2d..eee83a0e6231 100644
    --- a/tools/perf/util/evlist.c
    +++ b/tools/perf/util/evlist.c
    @@ -738,7 +738,8 @@ void perf_evlist__set_selected(struct perf_evlist *evlist,
    evlist->selected = evsel;
    }

    -int perf_evlist__open(struct perf_evlist *evlist, bool group)
    +int perf_evlist__open(struct perf_evlist *evlist, bool group,
    + struct perf_attr_conf *attr_conf)
    {
    struct perf_evsel *evsel, *first;
    int err, ncpus, nthreads;
    @@ -752,7 +753,7 @@ int perf_evlist__open(struct perf_evlist *evlist, bool group)
    group_fd = first->fd;

    err = perf_evsel__open(evsel, evlist->cpus, evlist->threads,
    - group, group_fd);
    + group, group_fd, attr_conf);
    if (err < 0)
    goto out_err;
    }
    diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
    index 21f1c9e57f13..84676747d945 100644
    --- a/tools/perf/util/evlist.h
    +++ b/tools/perf/util/evlist.h
    @@ -78,7 +78,8 @@ struct perf_evsel *perf_evlist__id2evsel(struct perf_evlist *evlist, u64 id);

    union perf_event *perf_evlist__mmap_read(struct perf_evlist *self, int idx);

    -int perf_evlist__open(struct perf_evlist *evlist, bool group);
    +int perf_evlist__open(struct perf_evlist *evlist, bool group,
    + struct perf_attr_conf *attr_conf);

    void perf_evlist__config_attrs(struct perf_evlist *evlist,
    struct perf_record_opts *opts);
    diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
    index 302d49a9f985..b8154e873e6d 100644
    --- a/tools/perf/util/evsel.c
    +++ b/tools/perf/util/evsel.c
    @@ -68,7 +68,6 @@ 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->sample_id_all = opts->sample_id_all_missing ? 0 : 1;
    attr->inherit = !opts->no_inherit;
    attr->read_format = PERF_FORMAT_TOTAL_TIME_ENABLED |
    PERF_FORMAT_TOTAL_TIME_RUNNING |
    @@ -111,9 +110,8 @@ void perf_evsel__config(struct perf_evsel *evsel, struct perf_record_opts *opts)
    if (opts->period)
    attr->sample_type |= PERF_SAMPLE_PERIOD;

    - if (!opts->sample_id_all_missing &&
    - (opts->sample_time || opts->system_wide ||
    - !opts->no_inherit || opts->cpu_list))
    + if (opts->sample_time || opts->system_wide ||
    + !opts->no_inherit || opts->cpu_list)
    attr->sample_type |= PERF_SAMPLE_TIME;

    if (opts->raw_samples) {
    @@ -287,7 +285,7 @@ int __perf_evsel__read(struct perf_evsel *evsel,
    return 0;
    }

    -static int __perf_evsel__open(struct perf_evsel *evsel, struct cpu_map *cpus,
    +static int ___perf_evsel__open(struct perf_evsel *evsel, struct cpu_map *cpus,
    struct thread_map *threads, bool group,
    struct xyarray *group_fds)
    {
    @@ -339,6 +337,47 @@ out_close:
    return err;
    }

    +static int __perf_evsel__open(struct perf_evsel *evsel, struct cpu_map *cpus,
    + struct thread_map *threads, bool group,
    + struct xyarray *group_fds,
    + struct perf_attr_conf *attr_conf)
    +{
    + int ret;
    + struct perf_event_attr *attr = &evsel->attr;
    +
    +retry:
    + if (attr_conf->exclude_guest_missing)
    + attr->exclude_guest = attr->exclude_host = 0;
    +
    + attr->sample_id_all = attr_conf->sample_id_all_missing ? 0 : 1;
    +
    + ret = ___perf_evsel__open(evsel, cpus, threads, group, group_fds);
    + if (ret == -EINVAL) {
    + /*
    + * The order of checking is important since it reflects when
    + * the new bit was included to kernel. The recent feature bit
    + * should be checked earlier so that it cannot disable the
    + * unintended feature(s).
    + */
    + if (!attr_conf->exclude_guest_missing &&
    + (attr->exclude_guest || attr->exclude_host)) {
    + pr_debug("Old kernel, cannot exclude "
    + "guest or host samples.\n");
    + attr_conf->exclude_guest_missing = true;
    + goto retry;
    + } else if (!attr_conf->sample_id_all_missing) {
    + pr_debug("Old kernel, cannot set sample_id_all.\n");
    +
    + attr_conf->sample_id_all_missing = true;
    + if (attr_conf->time_not_needed)
    + attr->sample_type &= ~PERF_SAMPLE_TIME;
    + goto retry;
    + }
    + }
    +
    + return ret;
    +}
    +
    void perf_evsel__close(struct perf_evsel *evsel, int ncpus, int nthreads)
    {
    if (evsel->fd == NULL)
    @@ -367,7 +406,8 @@ static struct {

    int perf_evsel__open(struct perf_evsel *evsel, struct cpu_map *cpus,
    struct thread_map *threads, bool group,
    - struct xyarray *group_fd)
    + struct xyarray *group_fd,
    + struct perf_attr_conf *attr_conf)
    {
    if (cpus == NULL) {
    /* Work around old compiler warnings about strict aliasing */
    @@ -377,23 +417,26 @@ int perf_evsel__open(struct perf_evsel *evsel, struct cpu_map *cpus,
    if (threads == NULL)
    threads = &empty_thread_map.map;

    - return __perf_evsel__open(evsel, cpus, threads, group, group_fd);
    + return __perf_evsel__open(evsel, cpus, threads, group, group_fd,
    + attr_conf);
    }

    int perf_evsel__open_per_cpu(struct perf_evsel *evsel,
    struct cpu_map *cpus, bool group,
    - struct xyarray *group_fd)
    + struct xyarray *group_fd,
    + struct perf_attr_conf *attr_conf)
    {
    return __perf_evsel__open(evsel, cpus, &empty_thread_map.map, group,
    - group_fd);
    + group_fd, attr_conf);
    }

    int perf_evsel__open_per_thread(struct perf_evsel *evsel,
    struct thread_map *threads, bool group,
    - struct xyarray *group_fd)
    + struct xyarray *group_fd,
    + struct perf_attr_conf *attr_conf)
    {
    return __perf_evsel__open(evsel, &empty_cpu_map.map, threads, group,
    - group_fd);
    + group_fd, attr_conf);
    }

    static int perf_event__parse_id_sample(const union perf_event *event, u64 type,
    diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
    index 326b8e4d5035..2c72e805e7ed 100644
    --- a/tools/perf/util/evsel.h
    +++ b/tools/perf/util/evsel.h
    @@ -91,13 +91,16 @@ void perf_evsel__close_fd(struct perf_evsel *evsel, int ncpus, int nthreads);

    int perf_evsel__open_per_cpu(struct perf_evsel *evsel,
    struct cpu_map *cpus, bool group,
    - struct xyarray *group_fds);
    + struct xyarray *group_fds,
    + struct perf_attr_conf *attr_conf);
    int perf_evsel__open_per_thread(struct perf_evsel *evsel,
    struct thread_map *threads, bool group,
    - struct xyarray *group_fds);
    + struct xyarray *group_fds,
    + struct perf_attr_conf *attr_conf);
    int perf_evsel__open(struct perf_evsel *evsel, struct cpu_map *cpus,
    struct thread_map *threads, bool group,
    - struct xyarray *group_fds);
    + struct xyarray *group_fds,
    + struct perf_attr_conf *attr_conf);
    void perf_evsel__close(struct perf_evsel *evsel, int ncpus, int nthreads);

    #define perf_evsel__match(evsel, t, c) \
    diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
    index e03b58a48424..b9fd7ec4ce18 100644
    --- a/tools/perf/util/python.c
    +++ b/tools/perf/util/python.c
    @@ -611,6 +611,7 @@ static PyObject *pyrf_evsel__open(struct pyrf_evsel *pevsel,
    PyObject *pcpus = NULL, *pthreads = NULL;
    int group = 0, inherit = 0;
    static char *kwlist[] = { "cpus", "threads", "group", "inherit", NULL };
    + struct perf_attr_conf attr_conf;

    if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|OOii", kwlist,
    &pcpus, &pthreads, &group, &inherit))
    @@ -623,11 +624,15 @@ static PyObject *pyrf_evsel__open(struct pyrf_evsel *pevsel,
    cpus = ((struct pyrf_cpu_map *)pcpus)->cpus;

    evsel->attr.inherit = inherit;
    +
    + memset(&attr_conf, 0, sizeof(attr_conf));
    +
    /*
    * This will group just the fds for this single evsel, to group
    * multiple events, use evlist.open().
    */
    - if (perf_evsel__open(evsel, cpus, threads, group, NULL) < 0) {
    + if (perf_evsel__open(evsel, cpus, threads, group, NULL,
    + &attr_conf) < 0) {
    PyErr_SetFromErrno(PyExc_OSError);
    return NULL;
    }
    @@ -824,11 +829,13 @@ static PyObject *pyrf_evlist__open(struct pyrf_evlist *pevlist,
    struct perf_evlist *evlist = &pevlist->evlist;
    int group = 0;
    static char *kwlist[] = { "group", NULL };
    + struct perf_attr_conf attr_conf;

    if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|OOii", kwlist, &group))
    return NULL;

    - if (perf_evlist__open(evlist, group) < 0) {
    + memset(&attr_conf, 0, sizeof(attr_conf));
    + if (perf_evlist__open(evlist, group, &attr_conf) < 0) {
    PyErr_SetFromErrno(PyExc_OSError);
    return NULL;
    }
    --
    1.7.9


    \
     
     \ /
      Last update: 2012-03-08 08:31    [W:0.046 / U:0.836 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site