Re: [PATCH 1/1] perf,tools: add time out to force stop endless mmap processing

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Fri, Jun 12, 2015 at 10:24:36PM -0600, David Ahern escreveu:
> coming back to this ...

> On 6/12/15 2:39 PM, Liang, Kan wrote:
> >>>Yes, perf always can read proc file. The problem is that the proc file
> >>>is huge and keep growing faster than proc reader.
> >>>So perf top do loop in perf_event__synthesize_mmap_events until the
> >>>test case exit.

> >>I'm confused. How are you getting the above time to read /proc maps if it
> >>never finishes?

> >I just tried to simplify the issue for perf record. So you may noticed that
> >I only read one thread. There are several threads in the system.
> >Also, I do the perf record test when starting the test case.
> >The proc file is not that big.
> >For perf top, it will monitor whole system. So it never finishes.
> 
> If the proc file is not that big for perf-record why is it a problem for
> perf-top? Both should only be reading the maps file for the thread group
> leader once and after it is processed getting MMAP events for changes. Why
> do you say perf-top can't handle it but perf-record can?

'perf top' does more than 'perf record', so it is conceivable that in
some circumstances 'perf record' can go thru, while top struggles.

That being said this happens when synthesizing PERF_RECORD_ events for
existing threads, i.e. at tool start time, for both top and record, so,
for this specific case, there should be no difference, if the workloads
running in both cases are the same at tool start up phase.

Then, that being said, having a sane upper limit on the time for
processing those events makes the tool more robust and allows it to do
most of its work, just samples for the maps not synthesized will fail to
get resolved to symbols/DSOs.

For those cases we should, during synthesizing, do both what Kan did in
his patch, i.e. emit a log warning with the COMM/PID that we are
truncating /proc/PID/maps parsing, and increment a counter that, just
after we finish synthesizing we should report, in a similar way as we
do in perf_session__warn_about_errors() after processing events,
something like:

        +--------------------------------------------------------+
        | %d map information files for pre-existing threads were |
        | not processed, if there are samples for addresses they |
        | will not be resolved, you may find out which are these |
        | threads by running with -v and redirecting the output  |
        | to a file.                                             |
        +--------------------------------------------------------+

Ideally, as an extra step, we could flip a flag on the 'struct thread'
where these maps got truncated and add some visual cue to the
hist_entry instances (lines in the top UI).

Perhaps we should add a per-thread-proc-map-processing timeout parameter
to the synthesizing routines instead of having that hardcoded, i.e.
allow the tool to specify what is reasonable for it, but that wouldn't
be strictly required for a first patch, emitting the dialog box above
after synthesizing, if truncation happened, is.

Agreed?

- Arnaldo