Messages in this thread | | | Subject | Re: [PATCH v4 1/5] perf stat: Add AGGR_PCIE_PORT mode | From | Alexander Antonov <> | Date | Mon, 8 Feb 2021 14:30:21 +0300 |
| |
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?
Thank you, Alexander
| |