Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1739267 > unrolled thread

[PATCH RFC V3 5/5] perf top: switch back to overwrite mode

Started bykan.liang@intel.com
First post2017-09-25 22:30 +0200
Last post2017-09-25 22:40 +0200
Articles 2 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [PATCH RFC V3 5/5] perf top: switch back to overwrite mode kan.liang@intel.com - 2017-09-25 22:30 +0200
    Re: [PATCH RFC V3 5/5] perf top: switch back to overwrite mode Andi Kleen <ak@linux.intel.com> - 2017-09-25 22:40 +0200

#1739267 — [PATCH RFC V3 5/5] perf top: switch back to overwrite mode

Fromkan.liang@intel.com
Date2017-09-25 22:30 +0200
Subject[PATCH RFC V3 5/5] perf top: switch back to overwrite mode
Message-ID<utIaS-2VS-15@gated-at.bofh.it>
From: Kan Liang <kan.liang@intel.com>

perf_top__mmap_read has severe performance issue in
Knights Landing/Mill, when monitoring in heavy load system. It costs
several minutes to finish, which is unacceptable.

perf top was overwrite mode. But it is changed to non overwrite mode
since commit 93fc64f14472 ("perf top: Switch to non overwrite mode").
For non overwrite mode, it tries to read everything in the ring buffer
and does not check the messup. Once there are lots of samples delivered
shortly, the processing time could be very long.
Knights Landing/Mill as a manycore processor contains a large number of
small cores. Because of the huge core number, it will generated lots of
samples in a heavy load system. Also, since the huge sample#, the mmap
writer probably bite the tail and mess up the samples.

Switching to overwrite mode, which dropping the unsure mmap entries,
significantly speeds up the whole progress.
Considering the real time requirement for perf top, it should switch
back to overwrite mode.

Only warning once if the messup is detected.
Providing some hints to users.

Signed-off-by: Kan Liang <kan.liang@intel.com>
---
 tools/perf/builtin-top.c | 2 +-
 tools/perf/util/evlist.c | 5 ++++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 477a869..4b4af34 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -902,7 +902,7 @@ static int perf_top__start_counters(struct perf_top *top)
 		}
 	}
 
-	if (perf_evlist__mmap(evlist, opts->mmap_pages, false) < 0) {
+	if (perf_evlist__mmap(evlist, opts->mmap_pages, true) < 0) {
 		ui__error("Failed to mmap with %d (%s)\n",
 			    errno, str_error_r(errno, msg, sizeof(msg)));
 		goto out_err;
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 6a0d7ff..ef0b6b1 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -723,7 +723,10 @@ perf_mmap__read(struct perf_mmap *md, bool check_messup, u64 start,
 		 * In either case, truncate and restart at 'end'.
 		 */
 		if (diff > md->mask / 2 || diff < 0) {
-			fprintf(stderr, "WARNING: failed to keep up with mmap data.\n");
+			WARN_ONCE(1, "WARNING: failed to keep up with mmap data.\n"
+				     "Please try increasing the period (-c) or\n"
+				     "decreasing the freq (-F) or\n"
+				     "limiting the number of CPUs");
 
 			/*
 			 * 'end' points to a known good entry, start there.
-- 
2.5.5

[toc] | [next] | [standalone]


#1739274

FromAndi Kleen <ak@linux.intel.com>
Date2017-09-25 22:40 +0200
Message-ID<utIkx-2ZE-11@gated-at.bofh.it>
In reply to#1739267
On Mon, Sep 25, 2017 at 01:23:08PM -0700, kan.liang@intel.com wrote:
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index 6a0d7ff..ef0b6b1 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -723,7 +723,10 @@ perf_mmap__read(struct perf_mmap *md, bool check_messup, u64 start,
>  		 * In either case, truncate and restart at 'end'.
>  		 */
>  		if (diff > md->mask / 2 || diff < 0) {
> -			fprintf(stderr, "WARNING: failed to keep up with mmap data.\n");
> +			WARN_ONCE(1, "WARNING: failed to keep up with mmap data.\n"
> +				     "Please try increasing the period (-c) or\n"
> +				     "decreasing the freq (-F) or\n"
> +				     "limiting the number of CPUs");

should suggest -C too

   "limiting the number of CPUs (-C)");


-Andi

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web