lkml.org 
[lkml]   [2015]   [Jun]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 08/30] perf tools: Propagate cpu maps through the evlist
    Em Sun, Jun 14, 2015 at 10:19:23AM +0200, Jiri Olsa escreveu:
    > Propagate evlist's cpu_map object through all the evsel objects,
    > while keeping already configured evsel->cpus.
    >
    > It'll be handy to access evsel's cpus directly
    > in following patches.
    >
    > Link: http://lkml.kernel.org/n/tip-myadl53clbkjvzeqolwp95w0@git.kernel.org
    > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
    > ---
    > tools/perf/util/evlist.c | 24 +++++++++++++++++++++++-
    > 1 file changed, 23 insertions(+), 1 deletion(-)
    >
    > diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
    > index f7d80ca58136..c0d3058c1868 100644
    > --- a/tools/perf/util/evlist.c
    > +++ b/tools/perf/util/evlist.c
    > @@ -1085,6 +1085,28 @@ int perf_evlist__mmap(struct perf_evlist *evlist, unsigned int pages,
    > return perf_evlist__mmap_ex(evlist, pages, overwrite, 0, false);
    > }
    >
    > +static int propagate_maps(struct perf_evlist *evlist, struct target *target)

    Please rename this to perf_evlist__set_maps()

    > +{
    > + struct perf_evsel *evsel;

    > + evlist__for_each(evlist, evsel) {
    > + /*
    > + * We already have cpus for evsel (via PMU sysfs) so

    If we already...

    > + * keep it, if there's no target cpu list defined.
    ^ The comma here confused me
    > + */
    > + if (evsel->cpus && target->cpu_list)
    > + cpu_map__put(evsel->cpus);

    If you drop the reference, and then will test it... use a
    cpu_map__zput(&evsel->cpus), so that after the put you don't access
    something you don't hold a reference for anymore...

    But the logic is still confusing! Can you try to rewrite it in a more
    clear way?

    > + if (!evsel->cpus || target->cpu_list)
    > + evsel->cpus = cpu_map__get(evlist->cpus);

    Something like:

    if (evsel->cpus) {
    something
    } else {
    something else
    }
    > +
    > + if (!evsel->cpus)
    > + return -ENOMEM;

    Where was that we tried to allocate memory in this operation?

    > + }
    > +
    > + return 0;
    > +}
    > +
    > int perf_evlist__create_maps(struct perf_evlist *evlist, struct target *target)
    > {
    > evlist->threads = thread_map__new_str(target->pid, target->tid,
    > @@ -1101,7 +1123,7 @@ int perf_evlist__create_maps(struct perf_evlist *evlist, struct target *target)
    > if (evlist->cpus == NULL)
    > goto out_delete_threads;
    >
    > - return 0;
    > + return propagate_maps(evlist, target);
    >
    > out_delete_threads:
    > thread_map__put(evlist->threads);
    > --
    > 1.9.3


    \
     
     \ /
      Last update: 2015-06-15 22:21    [W:3.666 / U:0.040 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site