Message-ID: <CABPqkBTiC63rKb736YUbAO9Y9Ob0igqRsHwn097AAba7k+EB6Q@mail.gmail.com>
Date:	Sun, 12 Apr 2015 17:37:22 -0700
From:	Stephane Eranian <eranian@...gle.com>
To:	Adrian Hunter <adrian.hunter@...el.com>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	Arnaldo Carvalho de Melo <acme@...hat.com>,
	Peter Zijlstra <peterz@...radead.org>,
	"mingo@...e.hu" <mingo@...e.hu>,
	"ak@...ux.intel.com" <ak@...ux.intel.com>,
	Jiri Olsa <jolsa@...hat.com>,
	Namhyung Kim <namhyung@...nel.org>,
	Rose Belcher <cel@...ibm.com>,
	Sukadev Bhattiprolu <sukadev@...ux.vnet.ibm.com>,
	Sonny Rao <sonnyrao@...omium.org>,
	John Mccutchan <johnmccutchan@...gle.com>,
	David Ahern <dsahern@...il.com>,
	Pawel Moll <pawel.moll@....com>
Subject: Re: [PATCH v6 3/4] perf inject: add jitdump mmap injection support

On Fri, Apr 10, 2015 at 5:51 AM, Adrian Hunter <adrian.hunter@...el.com> wrote:
> On 08/04/15 17:12, Stephane Eranian wrote:
>> On Wed, Apr 8, 2015 at 5:15 AM, Adrian Hunter <adrian.hunter@...el.com> wrote:
>>> On 06/04/15 22:41, Stephane Eranian wrote:
>>>>     > +     if (inject.build_ids) {
>>>>     > +             /*
>>>>     > +              * to make sure the mmap records are ordered correctly
>>>>     > +              * and so that the correct especially due to jitted code
>>>>     > +              * mmaps. We cannot generate the buildid hit list and
>>>>     > +              * inject the jit mmaps at the same time for now.
>>>>     > +              */
>>>>     > +             inject.tool.ordered_events = true;
>>>>     > +             inject.tool.ordering_requires_timestamps = true;
>>>>     > +     }
>>>>     > +
>>>>     > +     if (inject.jit_mode) {
>>>>     > +             inject.tool.mmap2          = perf_event__repipe_mmap2;
>>>>     > +             inject.tool.mmap           = perf_event__repipe_mmap;
>>>>
>>>>     As suggested above, why not make your own tool fns e.g.
>>>>
>>>>                     inject.tool.mmap2          = perf_event__jit_mode_mmap2;
>>>>                     inject.tool.mmap           = perf_event__jit_mode_mmap;
>>>>
>>>>
>>>>     > +             inject.tool.ordered_events = true;
>>>>     > +             inject.tool.ordering_requires_timestamps = true;
>>>>
>>>>     You are taking advantage of a bug in perf-inject, that is the
>>>>     "finished_round" events are not being processed. Really they should be
>>>>     processed and you should inject in time order.
>>>>
>>>> I am not sure I understand.
>>>> Yes, I am trying to have inject reorder the samples instead of perf report.
>>>> You are likely to run inject once, and report many times. Also avoids a
>>>> warning in report about out-of-order events.
>>>
>>> Well forgetting about "finished_round", it seems to me you need to intercept
>>> all the delivered events (which will be in time order) and inject your own
>>> events at the right time.
>>>
>>> At the moment it seems to me you are injecting all your events in one go
>>> when you see the special jitdump mmap. So I would not expect the injected
>>> events to be ordered with respect to other events that come later. But
>>> maybe I misunderstand that?
>>>
>> My understanding is that if I set ordered_events  = true, then events
>> are not delivered immediately to the callbacks. They are queued and
>> sorted and then passed to the callbacks. And yes, there is the finished
>> round mechanism of which I don't quite fully understand the logic in this
>> case.
>>
>>> As I confusingly tried to suggest earlier, one way to see all the
>>> delivered events is to hook the ordered_events "deliver" callback. That
>>> will mean injecting one mmap event at a time.
>>>
>>> Here is just an idea.
>>>
>>> struct perf_inject {
>>>         ...
>>>         ordered_events__deliver_t deliver;
>>> };
>>>
>>> int cmd_inject(int argc, const char **argv, const char *prefix __maybe_unused)
>>> {
>>> ...
>>>         inject.deliver = inject.session->ordered_events.deliver;
>>>         inject.session->ordered_events.deliver = inject_jit_mmap;
>>> ...
>>> }
>>>
>> ok on that.
>>
>>> int inject_jit_mmap(struct ordered_events *oe, struct ordered_event *event)
>>> {
>>>         struct perf_session *session = container_of(oe, struct perf_session, ordered_events);
>>>         struct perf_inject *inject = container_of(session->tool, struct perf_inject, tool);
>>>
>>>         /* Is it time to inject an event */
>>>         if (jit_next_timestamp(inject) < event->timestamp) {
>>>                 /* Yes, so inject it by delivery */
>>>                 perf_session__deliver_synth_event(...);
>>>         }
>>>         inject->deliver(oe, event);
>>> }
>>>
>> That suggests I have buffered all the MMAPs synthesized from the jitdump and
>> I have some sorted queue based on jitdump timestamps.
>
> Yes, but won't the jitdump timestamps already be in order?
>
I looked at implementing your approach and it turns out to be much
more complicated
than what is already there.
I do not inject anything if I don't have to. I am waiting to see a
special MMAP of the
jitdump file to start inject jit mmaps. That could not change in your
scheme. I would
need to track MMAP records and check if they correspond to a jitdump.  Note that
I may have to inject multiple jitdumps in one run of perf inject. In
that case, in the
deliver function, I would have to (1) either have all the jitdump MMAPs prepared
in order for all the jitdumps I have detected or (2) go through all
the jitdump current
head records to see which one matches. In (2) I would have to do this
not just when
I see an MMAP but for any event delivered: for each new event, compare its
timestamp to the timestamps of jitdump current record for ALL jitdump
files. For (1),
I need to build a global list of all jitdump records in for each
jitdump file, i.e, another
RB tree. And then pull them one by one out if I find an event with the
right timestamp.

I think (2) i more reasonable but requires quite some infrastructure
work in the jitdump
code. Most of the pieces are there, the RB tree needs to be added
obviously. I will
experiment with (2).


>>                                                       The test would have to
>> be more sophisticated that this. You'd want to insert at the right
>> time, i.e., you'd
>> have to track the previous and next timestamp in the stream:
>>
>> retry:
>>     j = jit_next_timestamp(inject->jit);
>>    if (prev_event->timestamp <= j && j > next_event->timestamp) {
>
> (prev_event->timestamp <= j) must be always true because otherwise you would
> have injected the jit mmap event already.
>
> So perf is about to deliver an event with timestamp 'event->timestamp', so
> you just need to deliver all your events that have not already been
> delivered but have a timestamp < event->timestamp
>
>
>>         deliver_synth_event(inject->jit);
>>         jit_next(inject->jit);
>>         goto retry;
>>    }
>>
>> All of this is required because the finished round logic is such that perf
>> cannot sort all records at once.
>
> It can sort them all at once, but finished_round is an optimization that
> starts delivering "early" events before having to sort the "later" events.
> The result is still that the delivered events are in sorted order.
>
>>                                  It does sort them by chunks. Thus I need
>> to make sure I inject in the right chunk otherwise it won't work. Am I reading
>> this right?
>
> If you were to queue the events for sorting, yes. What I suggested was
> delivering your events directly at the "right time".
>
>>
>>> You would also need to inject any remaining events right at the end.
>>>
>>> Note the advantage of "delivering" events is that the same approach works whatever the tool.
>>>
>>>
>>
>>
>
--
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