On Wed, Jan 25, 2017 at 9:55 PM, John Ferlan <jferlan@redhat.com> wrote:


On 01/18/2017 11:58 PM, Nitesh Konkar wrote:
> This patch series adds support and documentation for
> a generalized hardware cache event called CACHE_L1D
> perf event. This perf event is split into cache_l1dra,
> cache_l1drm, cache_l1dwa, cache_l1dwm, cache_l1dpa and
> cache_l1dpm perf events depending on the config value
> set.
>
> Nitesh Konkar (7):
>   perf: add cache_l1dra perf event support
>   perf: add cache_l1drm perf event support
>   perf: add cache_l1dwa perf event support
>   perf: add cache_l1dwm perf event support
>   perf: add cache_l1dpa perf event support
>   perf: add cache_l1dpm perf event support
>   virsh: Fix manpage typo
>
>  docs/formatdomain.html.in                   | 42 ++++++++++++++++++
>  docs/news.xml                               |  6 ++-
>  docs/schemas/domaincommon.rng               |  6 +++
>  include/libvirt/libvirt-domain.h            | 66 +++++++++++++++++++++++++++++
>  src/libvirt-domain.c                        | 18 ++++++++
>  src/qemu/qemu_driver.c                      |  6 +++
>  src/remote/remote_protocol.x                |  2 +-
>  src/util/virperf.c                          | 34 ++++++++++++++-
>  src/util/virperf.h                          |  6 +++
>  tests/genericxml2xmlindata/generic-perf.xml |  6 +++
>  tools/virsh.pod                             | 26 ++++++++++--
>  11 files changed, 210 insertions(+), 8 deletions(-)
>

Is there any possibility for combining the first 6 patches in some
manner? They're all just some "feature" within the level 1 cache
statistics.  I'm thinking of syntax such as:

virsh perf $dom --enable cache_1d=$LIST where $LIST is a comma separated
list and has the possibility for ra, rm, wa, wm, pa, and pm.

I didn't put much thought to it API wise and whether there's any
other/similar example. It's just something that it would seem would be
useful rather than the repetitiveness of these patches.
I agree. Will work on that.

Also, lots of extra work within the *perl and *go bindings that at this
point to add all the new definitions. I think you should supply the
patches for those rather than relying on myself and/or Daniel to do
that. Just follow the countless previous examples!
Sure. I shall provide it the next time. 

NB: A couple of patches require an extra space between the "*" and
description within include/libvirt/libvirt-domain.h.
Noted. 

I'll handle patch 7 separately...
Sure.
I shall also send a patch series to add perf software events, just like we
have generic hardware events now.  Meanwhile will work on the above
suggestion for hardware cache events.
Thanks,
Nitesh.



John