Messages in this thread | | | From | Namhyung Kim <> | Date | Wed, 10 Feb 2021 19:02:06 +0900 | Subject | Re: [PATCH v4 1/5] perf stat: Add AGGR_PCIE_PORT mode |
| |
On Mon, Feb 8, 2021 at 8:31 PM Alexander Antonov <alexander.antonov@linux.intel.com> wrote: > > On 2/4/2021 3:07 PM, Namhyung Kim wrote: > > Hello, > > > > On Wed, Feb 3, 2021 at 10:58 PM Alexander Antonov > > <alexander.antonov@linux.intel.com> wrote: > >> Adding AGGR_PCIE_PORT mode to be able to distinguish aggr_mode > >> for root ports in following patches. > > I'm not sure adding the AGGR_PCIE_PORT is the right way. > > In my understanding, the aggr mode is to specify how we aggregate > > counter values of a single event from different cpus. But this seems > > to aggregate counter values from different events. Also the new > > mode is basically the same as AGGR_GLOBAL. > > > > As you will add stat_config.iostat_run to distinguish the iostat > > command, probably we just want to use the global aggr mode > > (and it's the default!) and get rid of the AGGR_PCIE_PORT. > > > > Thoughts? > > > > Thanks, > > Namhyung > Hello Namhyung, > > Actually, you are right. We aggregate counter values from different > events of a > single IIO stack (PCIe root port) to calculate metrics for this IO stack. > But the reason is to prevent using of '-e' and '-M' options in 'iostat' mode > because it can be a reason for the mess in the output that can confuse > users. > > There is an idea to use your suggestion for this part: > > status = iostat_prepare(...); > if (status < 0) > goto out; > if (status == IOSTAT_LIST) > iostat_list(...); > else > ... > > So, we can check if evlist is empty inside iostat_prepare(). If not, print > a warning, for example, "The -e and -M options are not supported. All chosen > events/metrics will be dropped". Then we can free of evlist by using > evlist__delete(), create new one by using evlist__new() and fill the evlist. > > In this case the body of iostat_prepare() function would be: > > iostat_prepare() > { > If (!is_evlist_empty) { > pr_warning(); > evlist__delete(); > evlist__new() > } > > iostat_event_group(); > } > > It will allow to get rid of the AGGR_PCIE_PORT. > What do you think?
LGTM :)
Thanks, Namhyung
| |