public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: David Ahern <dsahern@gmail.com>
To: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>
Cc: "Liang, Kan" <kan.liang@intel.com>,
	Andi Kleen <andi@firstfloor.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Huang, Ying" <ying.huang@intel.com>
Subject: Re: [PATCH 1/1] perf,tools: add time out to force stop endless mmap processing
Date: Tue, 16 Jun 2015 09:57:15 -0600	[thread overview]
Message-ID: <5580475B.10704@gmail.com> (raw)
In-Reply-To: <20150616151159.GF10374@kernel.org>

On 6/16/15 9:11 AM, Arnaldo Carvalho de Melo wrote:
> 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.

If you are going to use timeouts then you need a sane upper limit on 
walking /proc altogether as well. i.e, one time limit for individual 
proc files (ie, time limit per task), and one for all of /proc (i.e, 
time limit for all of synthesized_threads). What is a reasonable time 
limit for each? Will it be configurable or hardcoded?

If perf aborts data collection for either a case the user should get a 
warning.

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

And then report side there should be a warning as well (record can be 
done by one person and analysis by another).

David

  parent reply	other threads:[~2015-06-16 15:57 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-10  7:46 [PATCH 1/1] perf,tools: add time out to force stop endless mmap processing kan.liang
2015-06-11 14:06 ` Arnaldo Carvalho de Melo
2015-06-11 14:27   ` Liang, Kan
2015-06-11 15:37     ` Arnaldo Carvalho de Melo
2015-06-11 15:21   ` David Ahern
2015-06-11 18:47     ` Andi Kleen
2015-06-12  0:33       ` David Ahern
2015-06-12 14:42         ` Liang, Kan
2015-06-12 15:41           ` David Ahern
2015-06-12 17:05             ` Liang, Kan
2015-06-12 17:28               ` David Ahern
2015-06-12 18:19                 ` Liang, Kan
2015-06-12 19:29                   ` David Ahern
2015-06-12 19:45                     ` Andi Kleen
2015-06-12 20:39                     ` Liang, Kan
2015-06-12 20:52                       ` David Ahern
2015-06-12 22:41                         ` Liang, Kan
2015-06-13  4:07                           ` David Ahern
2015-06-13 14:59                             ` Liang, Kan
2015-06-13  4:24                       ` David Ahern
2015-06-13 15:06                         ` Liang, Kan
2015-06-16 15:11                         ` Arnaldo Carvalho de Melo
2015-06-16 15:44                           ` Andi Kleen
2015-06-16 15:57                           ` David Ahern [this message]
2015-06-16 16:42                           ` Liang, Kan
2015-06-16 18:08                             ` Arnaldo Carvalho de Melo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5580475B.10704@gmail.com \
    --to=dsahern@gmail.com \
    --cc=andi@firstfloor.org \
    --cc=arnaldo.melo@gmail.com \
    --cc=kan.liang@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ying.huang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox