Re: [libvirt] [PATCH 1/2] perf: Compute cache_l1d config value correctly

2017-01-16 Thread Daniel P. Berrange 
On Sat, Jan 14, 2017 at 01:49:59PM +0530, Nitesh Konkar wrote:
> This patch computes the .attrConfig value for
> cache_l1d correctly and updates the documentation.
> The cache_l1d perf event now is renamed as
> cache_l1dra perf event for measuring read accesses
> for level 1 data cache
> 
> Signed-off-by: Nitesh Konkar 
> ---
>  docs/formatdomain.html.in   | 12 ++--
>  docs/news.xml   |  5 +++--
>  docs/schemas/domaincommon.rng   |  2 +-
>  include/libvirt/libvirt-domain.h| 12 ++--
>  src/libvirt-domain.c|  5 +++--
>  src/qemu/qemu_driver.c  |  2 +-
>  src/util/virperf.c  |  8 +---
>  src/util/virperf.h  |  2 +-
>  tests/genericxml2xmlindata/generic-perf.xml |  2 +-
>  tools/virsh.pod |  6 +++---
>  10 files changed, 30 insertions(+), 26 deletions(-)

I'm not really comfortable with us renaming the public API at this
late stage of the release, as it will have a ripple effect on all
the language bindings too.

IMHO, we need to just stick with this event name now.

> diff --git a/src/util/virperf.c b/src/util/virperf.c
> index 8554723..11e64df 100644
> --- a/src/util/virperf.c
> +++ b/src/util/virperf.c
> @@ -113,9 +113,11 @@ static struct virPerfEventAttr attrs[] = {
>   .attrConfig = 0,
>  # endif
>  },
>   .attrType = PERF_TYPE_HW_CACHE,
> - .attrConfig = PERF_COUNT_HW_CACHE_L1D},
> + .attrConfig = (PERF_COUNT_HW_CACHE_L1D) |
> +   (PERF_COUNT_HW_CACHE_OP_READ << 8) |
> +   (PERF_COUNT_HW_CACHE_RESULT_ACCESS << 16)},
>  };

So this looks like the only part of the patch that we need to
take.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://entangle-photo.org   -o-http://search.cpan.org/~danberr/ :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/2] perf: Compute cache_l1d config value correctly

2017-01-15 Thread Nitesh Konkar 
Hello,

My earlier patch with commit id : ae16c95f1bb5591c27676c5de8d383e5612c3568
(Link:
https://www.redhat.com/archives/libvir-list/2017-January/msg00226.html )
computed the .attrConfig value incorrectly. The right way to  compute
.attrConfig value for a PERF_TYPE_HW_CACHE event is listed here (
https://linux.die.net/man/2/perf_event_open) . I somehow missed this info
at the time of sending the patch and have sent another patch to rectify it.

Link: https://www.redhat.com/archives/libvir-list/2017-January/msg00679.html

I think this patch needs to be in before 3.0.0 is released else a wrong
value shall be displayed on enabling and displaying cache_l1d perf event.

Thanks,
Nitesh Konkar.


On Sat, Jan 14, 2017 at 1:49 PM, Nitesh Konkar <
niteshkonkar.libv...@gmail.com> wrote:

> This patch computes the .attrConfig value for
> cache_l1d correctly and updates the documentation.
> The cache_l1d perf event now is renamed as
> cache_l1dra perf event for measuring read accesses
> for level 1 data cache
>
> Signed-off-by: Nitesh Konkar 
> ---
>  docs/formatdomain.html.in   | 12 ++--
>  docs/news.xml   |  5 +++--
>  docs/schemas/domaincommon.rng   |  2 +-
>  include/libvirt/libvirt-domain.h| 12 ++--
>  src/libvirt-domain.c|  5 +++--
>  src/qemu/qemu_driver.c  |  2 +-
>  src/util/virperf.c  |  8 +---
>  src/util/virperf.h  |  2 +-
>  tests/genericxml2xmlindata/generic-perf.xml |  2 +-
>  tools/virsh.pod |  6 +++---
>  10 files changed, 30 insertions(+), 26 deletions(-)
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 30cb196..a8ee2db 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -1937,7 +1937,7 @@
>event name='stalled_cycles_frontend' enabled='no'/
>event name='stalled_cycles_backend' enabled='no'/
>event name='ref_cpu_cycles' enabled='no'/
> -  event name='cache_l1d' enabled='no'/
> +  event name='cache_l1dra' enabled='no'/
>  /perf
>  ...
>  
> @@ -2013,14 +2013,14 @@
>  
>ref_cpu_cycles
>the count of total cpu cycles not affected by CPU frequency
> scaling
> - by applications running on the platform
> +  by applications running on the platform
>perf.ref_cpu_cycles
>  
>  
> -  cache_l1d
> -  the count of total level 1 data cache by applications running on
> -   the platform
> -  perf.cache_l1d
> +  cache_l1dra
> +  the count of total read accesses for level 1 data cache by
> +  applications running on the platform
> +  perf.cache_l1dra
>  
>
>
> diff --git a/docs/news.xml b/docs/news.xml
> index baafcff..93ab40c 100644
> --- a/docs/news.xml
> +++ b/docs/news.xml
> @@ -82,8 +82,9 @@
>  
>Add support to get the count of branch instructions
>executed, branch misses, bus cycles, stalled frontend
> -  cpu cycles, stalled backend cpu cycles, and ref cpu
> -  cycles by applications running on the platform.
> +  cpu cycles, stalled backend cpu cycles, ref cpu
> +  cycles and cache l1dra by applications running on the
> +  platform.
>  
>
>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index be0a609..a65ad13 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -433,7 +433,7 @@
>stalled_cycles_frontend
>stalled_cycles_backend
>ref_cpu_cycles
> -  cache_l1d
> +  cache_l1dra
>  
>
>
> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-
> domain.h
> index 1e0e74c..3da0e9b 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -2189,15 +2189,15 @@ void 
> virDomainStatsRecordListFree(virDomainStatsRecordPtr
> *stats);
>  # define VIR_PERF_PARAM_REF_CPU_CYCLES "ref_cpu_cycles"
>
>  /**
> - * VIR_PERF_PARAM_CACHE_L1D:
> + * VIR_PERF_PARAM_CACHE_L1DRA:
>   *
> - * Macro for typed parameter name that represents cache_l1d
> + * Macro for typed parameter name that represents cache_l1dra
>   * perf event which can be used to measure the count of total
> - * level 1 data cache by applications running on the platform.
> - * It corresponds to the "perf.cache_l1d" field in the
> - * *Stats APIs.
> + * read accesses for level 1 data cache by applications running
> + * on the platform. It corresponds to the "perf.cache_l1dra"
> + * field in the *Stats APIs.
>   */
> -# define VIR_PERF_PARAM_CACHE_L1D "cache_l1d"
> +# define VIR_PERF_PARAM_CACHE_L1DRA "cache_l1dra"
>
>  int virDomainGetPerfEvents(virDomainPtr dom,
> virTypedParameterPtr *params,
> diff --git

[libvirt] [PATCH 1/2] perf: Compute cache_l1d config value correctly

2017-01-14 Thread Nitesh Konkar 
This patch computes the .attrConfig value for
cache_l1d correctly and updates the documentation.
The cache_l1d perf event now is renamed as
cache_l1dra perf event for measuring read accesses
for level 1 data cache

Signed-off-by: Nitesh Konkar 
---
 docs/formatdomain.html.in   | 12 ++--
 docs/news.xml   |  5 +++--
 docs/schemas/domaincommon.rng   |  2 +-
 include/libvirt/libvirt-domain.h| 12 ++--
 src/libvirt-domain.c|  5 +++--
 src/qemu/qemu_driver.c  |  2 +-
 src/util/virperf.c  |  8 +---
 src/util/virperf.h  |  2 +-
 tests/genericxml2xmlindata/generic-perf.xml |  2 +-
 tools/virsh.pod |  6 +++---
 10 files changed, 30 insertions(+), 26 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 30cb196..a8ee2db 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1937,7 +1937,7 @@
   event name='stalled_cycles_frontend' enabled='no'/
   event name='stalled_cycles_backend' enabled='no'/
   event name='ref_cpu_cycles' enabled='no'/
-  event name='cache_l1d' enabled='no'/
+  event name='cache_l1dra' enabled='no'/
 /perf
 ...
 
@@ -2013,14 +2013,14 @@
 
   ref_cpu_cycles
   the count of total cpu cycles not affected by CPU frequency scaling
- by applications running on the platform
+  by applications running on the platform
   perf.ref_cpu_cycles
 
 
-  cache_l1d
-  the count of total level 1 data cache by applications running on
-   the platform
-  perf.cache_l1d
+  cache_l1dra
+  the count of total read accesses for level 1 data cache by
+  applications running on the platform
+  perf.cache_l1dra
 
   
 
diff --git a/docs/news.xml b/docs/news.xml
index baafcff..93ab40c 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -82,8 +82,9 @@
 
   Add support to get the count of branch instructions
   executed, branch misses, bus cycles, stalled frontend
-  cpu cycles, stalled backend cpu cycles, and ref cpu
-  cycles by applications running on the platform.
+  cpu cycles, stalled backend cpu cycles, ref cpu
+  cycles and cache l1dra by applications running on the
+  platform.
 
   
   
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index be0a609..a65ad13 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -433,7 +433,7 @@
   stalled_cycles_frontend
   stalled_cycles_backend
   ref_cpu_cycles
-  cache_l1d
+  cache_l1dra
 
   
   
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 1e0e74c..3da0e9b 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -2189,15 +2189,15 @@ void 
virDomainStatsRecordListFree(virDomainStatsRecordPtr *stats);
 # define VIR_PERF_PARAM_REF_CPU_CYCLES "ref_cpu_cycles"
 
 /**
- * VIR_PERF_PARAM_CACHE_L1D:
+ * VIR_PERF_PARAM_CACHE_L1DRA:
  *
- * Macro for typed parameter name that represents cache_l1d
+ * Macro for typed parameter name that represents cache_l1dra
  * perf event which can be used to measure the count of total
- * level 1 data cache by applications running on the platform.
- * It corresponds to the "perf.cache_l1d" field in the
- * *Stats APIs.
+ * read accesses for level 1 data cache by applications running
+ * on the platform. It corresponds to the "perf.cache_l1dra"
+ * field in the *Stats APIs.
  */
-# define VIR_PERF_PARAM_CACHE_L1D "cache_l1d"
+# define VIR_PERF_PARAM_CACHE_L1DRA "cache_l1dra"
 
 int virDomainGetPerfEvents(virDomainPtr dom,
virTypedParameterPtr *params,
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 3023f30..fa39069 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -11250,8 +11250,9 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
  * CPU frequency scaling by applications running
  * as unsigned long long. It is produced by the
  * ref_cpu_cycles perf event.
- * "perf.cache_l1d" - The count of total level 1 data cache as unsigned
- *long long. It is produced by cache_l1d perf event.
+ * "perf.cache_l1dra" - The count of total read accesses for level 1 data
+ *  cache as unsigned long long. It is produced by
+ *  cache_l1dra perf event.
  *
  * Note that entire stats groups or individual stat fields may be missing from
  * the output in case they are not supported by the given hypervisor, are not
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 42f9889..7e2ea96