lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Tue, 6 Oct 2015 18:03:06 +0900
From:	Namhyung Kim <namhyung@...nel.org>
To:	Adrian Hunter <adrian.hunter@...el.com>
Cc:	Arnaldo Carvalho de Melo <acme@...nel.org>,
	Ingo Molnar <mingo@...nel.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Jiri Olsa <jolsa@...hat.com>,
	LKML <linux-kernel@...r.kernel.org>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Stephane Eranian <eranian@...gle.com>,
	David Ahern <dsahern@...il.com>,
	Andi Kleen <andi@...stfloor.org>
Subject: Re: [RFC/PATCH 03/38] perf tools: Move auxtrace_mmap field to struct perf_evlist

Hi Adrian,

On Mon, Oct 5, 2015 at 8:29 PM, Adrian Hunter <adrian.hunter@...el.com> wrote:
> On 02/10/15 21:45, Arnaldo Carvalho de Melo wrote:
>> Em Fri, Oct 02, 2015 at 02:18:44PM +0900, Namhyung Kim escreveu:
>>> Since it's gonna share struct mmap with dummy tracking evsel to track
>>> meta events only, let's move auxtrace out of struct perf_mmap.
>> Is this moving around _strictly_ needed?
>
> Also, what if you wanted to capture AUX data and tracking together.

Hmm.. I don't know what's the problem.  It should be orthogonal and
support doing that together IMHO.  Maybe I'm missing something about
the aux data processing and Intel PT.  I'll take a look at it..


>
> In addition, currently Intel PT can have either 1 dummy event for tracking
> plus sched_switch
> or 2 dummy events to allow for system-wide tracking of context switches.
> i.e. there
> are multiple tracking events.

Again, I don't have an idea what's going on this area. I need to look
at the code and think about what I can do.

Thanks for your review!
Namhyung



>
>>
>> - Arnaldo
>>
>>> Cc: Adrian Hunter <adrian.hunter@...el.com>
>>> Signed-off-by: Namhyung Kim <namhyung@...nel.org>
>>> ---
>>>  tools/perf/builtin-record.c |  4 ++--
>>>  tools/perf/util/evlist.c    | 30 +++++++++++++++++++++---------
>>>  tools/perf/util/evlist.h    |  2 +-
>>>  3 files changed, 24 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
>>> index 5e01c070dbf2..0accac6e0812 100644
>>> --- a/tools/perf/builtin-record.c
>>> +++ b/tools/perf/builtin-record.c
>>> @@ -220,7 +220,7 @@ static int record__auxtrace_read_snapshot_all(struct record *rec)
>>>
>>>      for (i = 0; i < rec->evlist->nr_mmaps; i++) {
>>>              struct auxtrace_mmap *mm =
>>> -                            &rec->evlist->mmap[i].auxtrace_mmap;
>>> +                            &rec->evlist->auxtrace_mmap[i];
>>>
>>>              if (!mm->base)
>>>                      continue;
>>> @@ -405,7 +405,7 @@ static int record__mmap_read_all(struct record *rec)
>>>      int rc = 0;
>>>
>>>      for (i = 0; i < rec->evlist->nr_mmaps; i++) {
>>> -            struct auxtrace_mmap *mm = &rec->evlist->mmap[i].auxtrace_mmap;
>>> +            struct auxtrace_mmap *mm = &rec->evlist->auxtrace_mmap[i];
>>>
>>>              if (rec->evlist->mmap[i].base) {
>>>                      if (record__mmap_read(rec, i) != 0) {
>>> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
>>> index e46adcd5b408..042dffc67986 100644
>>> --- a/tools/perf/util/evlist.c
>>> +++ b/tools/perf/util/evlist.c
>>> @@ -810,9 +810,12 @@ union perf_event *perf_evlist__mmap_read(struct perf_evlist *evlist, int idx)
>>>      return event;
>>>  }
>>>
>>> -static bool perf_mmap__empty(struct perf_mmap *md)
>>> +static bool perf_evlist__mmap_empty(struct perf_evlist *evlist, int idx)
>>>  {
>>> -    return perf_mmap__read_head(md) == md->prev && !md->auxtrace_mmap.base;
>>> +    struct perf_mmap *md = &evlist->mmap[idx];
>>> +
>>> +    return perf_mmap__read_head(md) == md->prev &&
>>> +            evlist->auxtrace_mmap[idx].base == NULL;
>>>  }
>>>
>>>  static void perf_evlist__mmap_get(struct perf_evlist *evlist, int idx)
>>> @@ -838,7 +841,7 @@ void perf_evlist__mmap_consume(struct perf_evlist *evlist, int idx)
>>>              perf_mmap__write_tail(md, old);
>>>      }
>>>
>>> -    if (atomic_read(&md->refcnt) == 1 && perf_mmap__empty(md))
>>> +    if (atomic_read(&md->refcnt) == 1 && perf_evlist__mmap_empty(evlist, idx))
>>>              perf_evlist__mmap_put(evlist, idx);
>>>  }
>>>
>>> @@ -879,7 +882,7 @@ static void __perf_evlist__munmap(struct perf_evlist *evlist, int idx)
>>>              evlist->mmap[idx].base = NULL;
>>>              atomic_set(&evlist->mmap[idx].refcnt, 0);
>>>      }
>>> -    auxtrace_mmap__munmap(&evlist->mmap[idx].auxtrace_mmap);
>>> +    auxtrace_mmap__munmap(&evlist->auxtrace_mmap[idx]);
>>>  }
>>>
>>>  void perf_evlist__munmap(struct perf_evlist *evlist)
>>> @@ -901,7 +904,15 @@ static int perf_evlist__alloc_mmap(struct perf_evlist *evlist)
>>>      if (cpu_map__empty(evlist->cpus))
>>>              evlist->nr_mmaps = thread_map__nr(evlist->threads);
>>>      evlist->mmap = zalloc(evlist->nr_mmaps * sizeof(struct perf_mmap));
>>> -    return evlist->mmap != NULL ? 0 : -ENOMEM;
>>> +    if (evlist->mmap == NULL)
>>> +            return -ENOMEM;
>>> +    evlist->auxtrace_mmap = calloc(evlist->nr_mmaps,
>>> +                                   sizeof(struct auxtrace_mmap));
>>> +    if (evlist->auxtrace_mmap == NULL) {
>>> +            zfree(&evlist->mmap);
>>> +            return -ENOMEM;
>>> +    }
>>> +    return 0;
>>>  }
>>>
>>>  struct mmap_params {
>>> @@ -938,10 +949,6 @@ static int __perf_evlist__mmap(struct perf_evlist *evlist, int idx,
>>>              return -1;
>>>      }
>>>
>>> -    if (auxtrace_mmap__mmap(&evlist->mmap[idx].auxtrace_mmap,
>>> -                            &mp->auxtrace_mp, evlist->mmap[idx].base, fd))
>>> -            return -1;
>>> -
>>>      return 0;
>>>  }
>>>
>>> @@ -963,6 +970,11 @@ static int perf_evlist__mmap_per_evsel(struct perf_evlist *evlist, int idx,
>>>                      *output = fd;
>>>                      if (__perf_evlist__mmap(evlist, idx, mp, *output) < 0)
>>>                              return -1;
>>> +
>>> +                    if (auxtrace_mmap__mmap(&evlist->auxtrace_mmap[idx],
>>> +                                            &mp->auxtrace_mp,
>>> +                                            evlist->mmap[idx].base, fd))
>>> +                            return -1;
>>>              } else {
>>>                      if (ioctl(fd, PERF_EVENT_IOC_SET_OUTPUT, *output) != 0)
>>>                              return -1;
>>> diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
>>> index 414e383885f5..51574ce8ac69 100644
>>> --- a/tools/perf/util/evlist.h
>>> +++ b/tools/perf/util/evlist.h
>>> @@ -30,7 +30,6 @@ struct perf_mmap {
>>>      int              mask;
>>>      atomic_t         refcnt;
>>>      u64              prev;
>>> -    struct auxtrace_mmap auxtrace_mmap;
>>>      char             event_copy[PERF_SAMPLE_MAX_SIZE] __attribute__((aligned(8)));
>>>  };
>>>
>>> @@ -53,6 +52,7 @@ struct perf_evlist {
>>>      } workload;
>>>      struct fdarray   pollfd;
>>>      struct perf_mmap *mmap;
>>> +    struct auxtrace_mmap *auxtrace_mmap;
>>>      struct thread_map *threads;
>>>      struct cpu_map    *cpus;
>>>      struct perf_evsel *selected;
>>> --
>>> 2.6.0
>



-- 
Thanks,
Namhyung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists