Messages in this thread | | | Date | Thu, 11 Jan 2018 15:26:17 +0100 | From | Jiri Olsa <> | Subject | Re: [PATCH V3 11/12] perf top: switch default mode to overwrite mode |
| |
On Thu, Dec 21, 2017 at 10:08:53AM -0800, kan.liang@intel.com wrote: > 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. > > Currently, perf top is non overwrite mode. For non overwrite mode, it > tries to read everything in the ringbuffer and doesn't pause the > ringbuffer. Once there are lots of samples delivered persistently, > the processing time could be very long. Also, the latest samples could > be lost when the ringbuffer is full. > > For overwrite mode, it takes a snapshot for the system by pausing the > ringbuffer, which could significantly reducing the processing time. > Also, the overwrite mode always keep the latest samples. > Considering the real time requirement for perf top, the overwrite mode > is more suitable for perf top. > > Actually, perf top was overwrite mode. It is changed to non overwrite > mode since commit 93fc64f14472 ("perf top: Switch to non overwrite > mode"). It's better to change it back to overwrite mode by default. > > For the kernel which doesn't support overwrite mode, it will fall back > to non overwrite mode. > > There would be some records lost in overwrite mode because of pausing > the ringbuffer. It has little impact for the accuracy of the snapshot > and could be tolerant. > The lost events checking is removed. > > For overwrite mode, unconditionally wait 100 ms before each snapshot. It > also reduce the overhead caused by pausing ringbuffer, especially on > light load system. > > Signed-off-by: Kan Liang <kan.liang@intel.com> > --- > tools/perf/builtin-top.c | 44 ++++++++++++++++++++++++------------------ > tools/perf/ui/browsers/hists.c | 12 +++++++++--- > 2 files changed, 34 insertions(+), 22 deletions(-) > > diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c > index 8d19ef7..3c14333 100644 > --- a/tools/perf/builtin-top.c > +++ b/tools/perf/builtin-top.c > @@ -283,16 +283,6 @@ static void perf_top__print_sym_table(struct perf_top *top) > > printf("%-*.*s\n", win_width, win_width, graph_dotted_line); > > - if (hists->stats.nr_lost_warned != > - hists->stats.nr_events[PERF_RECORD_LOST]) { > - hists->stats.nr_lost_warned = > - hists->stats.nr_events[PERF_RECORD_LOST]; > - color_fprintf(stdout, PERF_COLOR_RED, > - "WARNING: LOST %d chunks, Check IO/CPU overload", > - hists->stats.nr_lost_warned); > - ++printed; > - } > -
should this stay for non-overwrite mode?
jirka
| |