linux-kernel - Re: [PATCH 02/37] perf record: Use a software dummy event to track task/mmap events

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 linux-cve-announce 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]

Message-ID: <54A26A96.4080505@intel.com>
Date:	Tue, 30 Dec 2014 11:04:22 +0200
From:	Adrian Hunter <adrian.hunter@...el.com>
To:	Namhyung Kim <namhyung@...nel.org>
CC:	David Ahern <dsahern@...il.com>,
	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>,
	Stephane Eranian <eranian@...gle.com>,
	Andi Kleen <andi@...stfloor.org>,
	Frederic Weisbecker <fweisbec@...il.com>
Subject: Re: [PATCH 02/37] perf record: Use a software dummy event to track
 task/mmap events

On 30/12/14 07:51, Namhyung Kim wrote:
> Hi Adrian,
> 
> On Mon, Dec 29, 2014 at 02:58:12PM +0200, Adrian Hunter wrote:
>> On 27/12/14 07:28, Namhyung Kim wrote:
>>> Hi David,
>>>
>>> On Sat, Dec 27, 2014 at 1:27 AM, David Ahern <dsahern@...il.com> wrote:
>>>> On 12/24/14 12:14 AM, Namhyung Kim wrote:
>>>>>
>>>>> Prepend a software dummy event into evlist to track task/comm/mmap
>>>>> events separately.  This is a preparation of multi-file/thread support
>>>>> which will come later.
>>>>
>>>>
>>>> Are you are making this the first event because of how perf internals are
>>>> coded -- that the first event tracks tasks events? With the tracking bit in
>>>> evsel you should not need to do that. Is there another reason?
>>>
>>> Yeah, I know the tracking bit can be set to any evsel in the evlist.
>>> But I'd like to keep it at a fixed index so that it can be easily
>>> identified at later stages (like perf report) too.  Ideally, it'd be
>>> great if we have a way to distinguish this auto-added dummy tracking
>>> event from other (user-added) (dummy?) tracking events if any.
>>>
>>>>
>>>> ---8<---
>>>>
>>>>> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
>>>>> index cfbe2b99b9aa..72dff295237e 100644
>>>>> --- a/tools/perf/util/evlist.c
>>>>> +++ b/tools/perf/util/evlist.c
>>>>> @@ -193,6 +193,44 @@ int perf_evlist__add_default(struct perf_evlist
>>>>> *evlist)
>>>>>         return -ENOMEM;
>>>>>   }
>>>>>
>>>>> +int perf_evlist__prepend_dummy(struct perf_evlist *evlist)
>>>>> +{
>>>>> +       struct perf_event_attr attr = {
>>>>> +               .type = PERF_TYPE_SOFTWARE,
>>>>> +               .config = PERF_COUNT_SW_DUMMY,
>>
>> Probably need .exclude_kernel = 1, here
> 
> Ah, right.
> 
>>
>>>>> +       };
>>>>> +       struct perf_evsel *evsel, *pos;
>>>>> +
>>>>> +       event_attr_init(&attr);
>>>>> +
>>>>> +       evsel = perf_evsel__new(&attr);
>>>>> +       if (evsel == NULL)
>>>>> +               goto error;
>>>>> +
>>>>> +       /* use strdup() because free(evsel) assumes name is allocated */
>>>>> +       evsel->name = strdup("dummy");
>>>>> +       if (!evsel->name)
>>>>> +               goto error_free;
>>>>> +
>>>>> +       list_for_each_entry(pos, &evlist->entries, node) {
>>>>> +               pos->idx += 1;
>>>>> +               pos->tracking = false;
>>>>> +       }
>>>>> +
>>>>> +       list_add(&evsel->node, &evlist->entries);
>>>>> +       evsel->idx = 0;
>>>>> +       evsel->tracking = true;
>>>>
>>>>
>>>> perf_evlist__set_tracking_event()?
>>>
>>> I found that after I wrote this, so yes, it can use the function
>>> instead of the oped-code.  But the loop traversal is needed anyway to
>>> fixup the evsel->idx.
>>
>> perf_evlist__set_tracking_event() also ensures there is only one tracking
>> event so it is easy to identify. It is the only event with attr->mmap etc
>> set to 1. Then you can use perf_evlist__add().
> 
> Well, yes, I think we can put the dummy tracking event anywhere in the
> evlist with this, but I still slightly prefer put it at a fixed
> location for a possible code simplification..

Probably you should not be checking for the "tracking event" at all on the
processing side and instead just have perf report ignore all dummy events.
That generalizes things even more.

What is the code simplification?

--
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/