Messages in this thread | | | From | "Liang, Kan" <> | Subject | RE: [PATCH V3 08/12] perf top: check per event overwrite term | Date | Fri, 12 Jan 2018 16:46:14 +0000 |
| |
> > On Thu, Jan 11, 2018 at 09:29:21PM +0000, Liang, Kan wrote: > > > > On Thu, Dec 21, 2017 at 10:08:50AM -0800, kan.liang@intel.com wrote: > > > > > From: Kan Liang <kan.liang@intel.com> > > > > > > > > > > Per event overwrite term is not forbidden in perf top, which can > > > > > bring problems. Because perf top only support non-overwrite mode. > > > > > > > > > > Check and forbid inconsistent per event overwrite term in the evlist. > > > > > Make it possible to support either non-overwrite or overwrite mode. > > > > > The overwrite mode is forbidden now, which will be removed when > > > > > the overwrite mode is supported later. > > > > > > > > > > Signed-off-by: Kan Liang <kan.liang@intel.com> > > > > > --- > > > > > tools/perf/builtin-top.c | 60 > > > > ++++++++++++++++++++++++++++++++++++++++++++++++ > > > > > 1 file changed, 60 insertions(+) > > > > > > > > > > diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c > > > > > index c6ccda5..4b85e7b 100644 > > > > > --- a/tools/perf/builtin-top.c > > > > > +++ b/tools/perf/builtin-top.c > > > > > @@ -881,6 +881,56 @@ static void perf_top__mmap_read(struct > > > > > perf_top > > > > *top) > > > > > perf_top__mmap_read_idx(top, i); } > > > > > > > > > > +/* > > > > > + * Check per event overwrite term. > > > > > + * perf top supports consistent mode for all events. > > > > > + * Return -1 if the per event terms set but not consistent. > > > > > > > > please list the rules for overwrite in the comment are they just > > > > top specific? > > > > > > Yes, it's only for perf top. > > > It doesn't support that some events are overwrite mode and other > > > events are non-overwrite mode. > > > I will refine the comments. > > > > > > > > > > > SNIP > > > > > > > > > + if (evsel == perf_evlist__first(evlist)) > > > > > + overwrite = set; > > > > > + else > > > > > + return -1; > > > > > + } > > > > > + } > > > > > + > > > > > + if ((overwrite >= 0) && (opts->overwrite != overwrite)) > > > > > + opts->overwrite = overwrite; > > > > > + > > > > > + return 0; > > > > > +} > > > > > + > > > > > static int perf_top__start_counters(struct perf_top *top) { > > > > > char msg[BUFSIZ]; > > > > > @@ -890,6 +940,16 @@ static int perf_top__start_counters(struct > > > > perf_top *top) > > > > > > > > > > perf_evlist__config(evlist, opts, &callchain_param); > > > > > > > > so perf_evlist__config uses opts->overwrite, which you set in your > > > > perf_top_overwrite_check call.. I'd think you need to call it > > > > sooner > > > > > > > > > > User may set per-event mode, which is processed in > > > perf_evlist__config It's possible that the per-event mode is not same as > opts->overwrite. > > > If so, perf_evlist__config actually uses per-event mode. > > > > > > perf_top_overwrite_check will do the check. If the per-event mode is > > > inconsistent as opts->overwrite, updating the opts->overwrite. > > > > > > It has to be called after the perf_evlist__config. > > > > let's see the rules then first, also wrt opts->overwrite, which is > > used later on I assume > > > > The rules for perf_top_overwrite_check is quite simple. > - All events have same per-event mode. > E.g. "cpu/cpu-cycles,no-overwrite/,cpu/instructions,no-overwrite/ > Using the per-event setting to replace the opts->overwrite if they are > different. > - Events have different per-event mode. > E.g. "cpu/cpu-cycles,overwrite/,cpu/instructions,no-overwrite/" > Error out. > - Some of the event set per-event mode, but some not. > E.g. "cpu/cpu-cycles, /,cpu/instructions,no-overwrite/" > Error out. > > There is no --overwrite option support for perf top. > We have a default opts->overwrite. > In perf_evlist__config, attr->write_backward will be set according to > opts->overwrite at first. But if there is per-event mode, the > attr->write_backward will be updated accordingly. > There may be inconsistent between opts->overwrite and > attr->write_backward. > > perf_top_overwrite_check will be called right after perf_evlist__config. > It can fix the inconsistence by updating the opts->overwrite or error out. > The opts->overwrite will be used by fallback and _mmap_read() later. >
Check the code again. Perf_top_overwrite_check() reply on evsel->config_terms to check the terms, which is already parsed before perf_evlist__config(). You are right, we can call it earlier. I will move the codes right before the perf_evlist__config() in V4.
Thanks, Kan
| |