Re: [libvirt] [PATCH v5 0/8] perf: add more perf events support

2016-09-02 Thread Ren, Qiaowei

> -Original Message-
> From: John Ferlan [mailto:jfer...@redhat.com]
> Sent: Tuesday, August 30, 2016 7:40 PM
> To: Ren, Qiaowei <qiaowei@intel.com>; libvir-list@redhat.com
> Subject: Re: [PATCH v5 0/8] perf: add more perf events support
> 
> 
> 
> On 08/29/2016 10:00 PM, Ren, Qiaowei wrote:
> > Hi John,
> >
> > How about this patch series?
> >
> 
> I made adjustments to your series, but I also made tweaks and those needed to
> be OK'd/checked by someone such as you  Go back to the v4 series and read
> my comment. In particular from review of patch 2 and more specifically code
> flow changes w/r/t the initialization which now mostly become patch 7.
> 
> So please go through the series, make sure it "works" as you expect and let me
> know.  Once that's done and 2.2 is released, I can push these. If tweaks need 
> to
> be made, then we can do so.
> 

John, I just checked and did some testing for new series, and it could work. So 
if no more comments, I guess that you can push these. Thanks for your feedback.

- Qiaowei

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


Re: [libvirt] [PATCH v5 0/8] perf: add more perf events support

2016-08-29 Thread Ren, Qiaowei
Hi John,

How about this patch series? 

Thanks,
Qiaowei

> -Original Message-
> From: John Ferlan [mailto:jfer...@redhat.com]
> Sent: Thursday, August 4, 2016 6:31 AM
> To: libvir-list@redhat.com
> Cc: Ren, Qiaowei <qiaowei@intel.com>
> Subject: [PATCH v5 0/8] perf: add more perf events support
> 
> v4: http://www.redhat.com/archives/libvir-list/2016-July/msg00607.html
> 
> Reworked/reworded the series slightly.  The end result is mostly the same as 
> the
> original, but with the suggested tweaks.
> 
> 
> John Ferlan (3):
>   virsh: Add a forward reference to perf command from domstats --perf
>   virsh: Rework the perf event names into a table.
>   util: Move virPerfNew and virPerfFree
> 
> Qiaowei Ren (5):
>   perf: rename qemuDomainGetStatsPerfRdt()
>   perf: Remove the switch from qemuDomainGetStatsPerf
>   util: Add some comment details for virPerfEventType
>   perf: Adjust the perf initialization
>   perf: add more perf events support
> 
>  docs/formatdomain.html.in   |  24 +++
>  docs/schemas/domaincommon.rng   |   4 +
>  include/libvirt/libvirt-domain.h|  39 +
>  src/libvirt-domain.c|   9 ++
>  src/qemu/qemu_driver.c  |  23 ++-
>  src/util/virperf.c  | 230 
> +---
>  src/util/virperf.h  |  14 +-
>  tests/genericxml2xmlindata/generic-perf.xml |   4 +
>  tools/virsh.pod |  40 +++--
>  9 files changed, 275 insertions(+), 112 deletions(-)
> 
> --
> 2.7.4


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


Re: [libvirt] [PATCH v4 0/4] perf: add more perf events support

2016-07-25 Thread Ren, Qiaowei
Hi John and Peter,

Thanks for your reviews, and do you have any other comments about this patchset?

- Qiaowei

> -Original Message-
> From: Ren, Qiaowei
> Sent: Saturday, July 16, 2016 4:15 PM
> To: libvir-list@redhat.com
> Cc: Daniel P. Berrange <berra...@redhat.com>; Peter Krempa
> <pkre...@redhat.com>; John Ferlan <jfer...@redhat.com>; Ren, Qiaowei
> <qiaowei@intel.com>
> Subject: [PATCH v4 0/4] perf: add more perf events support
> 
> With current perf framework, this patchset refactor virPerfEventEnable() for
> general purpose and add more perf events support based on this change,
> including cache misses, cache references, cpu cycles, instrctions, etc..
> 
> Changes since v3:
>   * separate the patch into 4 patches.
>   * update virsh.pod for new perf events.
>   * introduce a static attr table that would be able to convert the
> VIR_PERF_EVENT_* into their respective "attr.type" and "attr.config".
> 
> Qiaowei Ren (4):
>   perf: rename qemuDomainGetStatsPerfRdt()
>   perf: introduce a static attr table and refactor virPerfEventEnable()
> for general purpose
>   perf: add more perf events support
>   perf: add description for new events in virsh.pod
> 
>  docs/formatdomain.html.in   |  24 
>  docs/schemas/domaincommon.rng   |   4 +
>  include/libvirt/libvirt-domain.h|  39 ++
>  src/libvirt-domain.c|   8 ++
>  src/qemu/qemu_driver.c  |  23 ++--
>  src/util/virperf.c  | 176 
> +---
>  src/util/virperf.h  |  11 ++
>  tests/genericxml2xmlindata/generic-perf.xml |   4 +
>  tools/virsh.pod |   4 +
>  9 files changed, 211 insertions(+), 82 deletions(-)
> 
> --
> 1.9.1


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


Re: [libvirt] [PATCH 1/1] perf: add more perf events support

2016-07-16 Thread Ren, Qiaowei

> -Original Message-
> From: John Ferlan [mailto:jfer...@redhat.com]
> Sent: Wednesday, July 13, 2016 4:02 AM
> To: Ren, Qiaowei <qiaowei@intel.com>; libvir-list@redhat.com
> Cc: Peter Krempa <pkre...@redhat.com>
> Subject: Re: [libvirt] [PATCH 1/1] perf: add more perf events support
> 
> 
> 
> On 06/29/2016 08:10 PM, Qiaowei Ren wrote:
> > With current perf framework, this patch adds support to more perf
>^^^ for more perf
> 
> > events, including cache missing, cache peference, cpu cycles,
> 
> A quick google search turns up "cache references" - there's just too many
> peference or peferences references to comment on them all, but they all need
> to be "references"
> 

John, according perf code from linux kernel, 'cache peference' is from linux 
kernel code. Certainly 'perf list' command show the 'references', and I will 
change it to this.

> > instrction, etc..
> 
> instructions
> 
> >
> > Signed-off-by: Qiaowei Ren <qiaowei@intel.com>
> > ---
> >  docs/formatdomain.html.in   | 24 +++
> >  docs/schemas/domaincommon.rng   |  4 ++
> >  include/libvirt/libvirt-domain.h| 39 +
> >  src/libvirt-domain.c|  8 
> >  src/qemu/qemu_driver.c  | 23 +-
> >  src/util/virperf.c  | 65 
> > -
> >  src/util/virperf.h  |  4 ++
> >  tests/genericxml2xmlindata/generic-perf.xml |  4 ++
> >  8 files changed, 158 insertions(+), 13 deletions(-)
> >
> 
> I see no changes for virsh.pod, see commit id '3110363d' for a recent change
> Peter made in this space...
> 
> I think perhaps it may also be worthwhile to "in a separate patch" alter the
> 'domstats --perf' description to simply reference the 'perf'
> description where each of the collect perf.* events can be listed and 
> described.
> 
> Each of the collectible events could have some sort of tabular output - see 
> how
> 'vol-wipe' describes the various supported algorithms.  So much easier to read
> than one long sentence.
> 
Sure. I will add one patch about this.

> 
> > diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> > index f660aa6..7999e43 100644
> > --- a/docs/formatdomain.html.in
> > +++ b/docs/formatdomain.html.in
> > @@ -1839,6 +1839,10 @@
> >  event name='cmt' enabled='yes'/
> >  event name='mbmt' enabled='no'/
> >  event name='mbml' enabled='yes'/
> > +event name='cache_misses' enabled='no'/
> > +event name='cache_peferences' enabled='no'/
> > +event name='instructions' enabled='no'/
> > +event name='cpu_cycles' enabled='no'/
> >/perf
> >...
> >  
> > @@ -1864,6 +1868,26 @@
> >bandwidth of memory traffic for a memory controller
> >perf.mbml
> >  
> > +
> > +  cache_misses
> > +  the amount of cache missing by applications running on the
> > + platform
> 
> is this the count of caches misses?  amount implies perhaps other things.
> 
Yes. I will change it.

> > +  perf.cache_misses
> > +
> > +
> > +  cache_peferences
> > +  the amount of cache hit by applications running on the 
> > platform
> > +  perf.cache_peferences
> 
> similar is this the count of cache hits, right?
> 
Yes.

> > +
> > +
> > +  instructions
> > +  the amount of instructions by applications running on the
> > + platform
> 
> the count of instructions executed...
> 
> > +  perf.instructions
> > +
> > +
> > +  cpu_cycles
> > +  the amount of cycles one instruction needs
> 
> the number/count of cpu cycles
> 
> > +  perf.cpu_cycles
> > +
> >
> >
> >  Devices
> > diff --git a/docs/schemas/domaincommon.rng
> > b/docs/schemas/domaincommon.rng index 563cb3c..e41dc3a 100644
> > --- a/docs/schemas/domaincommon.rng
> > +++ b/docs/schemas/domaincommon.rng
> > @@ -414,6 +414,10 @@
> >cmt
> >mbmt
> >mbml
> > +  cache_misses
> > +  cache_peferences
> > +  instructions
> > +  cpu_cycles
> >  
> >
> >
> > diff --git a/include/libvirt/libvirt-domain.h
> > b/include/libvirt/libvirt-domain.h
> &g

Re: [libvirt] [PATCH v2 1/1] perf: add more perf events support

2016-06-27 Thread Ren, Qiaowei
Hi Peter,

According to your comment, I updated the XML documentation and post new version 
here. Do you have any more comments?

Thanks,
Qiaowei

> -Original Message-
> From: Ren, Qiaowei
> Sent: Tuesday, June 21, 2016 3:41 PM
> To: libvir-list@redhat.com
> Cc: Daniel P. Berrange <berra...@redhat.com>; Peter Krempa
> <pkre...@redhat.com>; Ren, Qiaowei <qiaowei@intel.com>
> Subject: [PATCH v2 1/1] perf: add more perf events support
> 
> With current perf framework, this patch adds support to more perf events,
> including cache missing, cache peference, cpu cycles, instrction, etc..
> 
> Signed-off-by: Qiaowei Ren <qiaowei@intel.com>
> ---
>  docs/formatdomain.html.in| 24 +++
>  docs/schemas/domaincommon.rng|  4 +++
>  include/libvirt/libvirt-domain.h | 39 
>  src/libvirt-domain.c |  8 +
>  src/qemu/qemu_driver.c   | 23 +++---
>  src/util/virperf.c   | 65 
> +++-
>  src/util/virperf.h   |  4 +++
>  7 files changed, 154 insertions(+), 13 deletions(-)
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index
> 7d34363..a4064ae 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -1839,6 +1839,10 @@
>  event name='cmt' enabled='yes'/
>  event name='mbmt' enabled='no'/
>  event name='mbml' enabled='yes'/
> +event name='cache_misses' enabled='no'/
> +event name='cache_peferences' enabled='no'/
> +event name='instructions' enabled='no'/
> +event name='cpu_cycles' enabled='no'/
>/perf
>...
>  
> @@ -1864,6 +1868,26 @@
>bandwidth of memory traffic for a memory controller
>perf.mbml
>  
> +
> +  cache_misses
> +  the amount of cache missing by applications running on the
> platform
> +  perf.cache_misses
> +
> +
> +  cache_peferences
> +  the amount of cache hit by applications running on the 
> platform
> +  perf.cache_peferences
> +
> +
> +  instructions
> +  the amount of instructions by applications running on the 
> platform
> +  perf.instructions
> +
> +
> +  cpu_cycles
> +  the amount of cycles one instruction needs
> +  perf.cpu_cycles
> +
>
> 
>  Devices
> diff --git a/docs/schemas/domaincommon.rng
> b/docs/schemas/domaincommon.rng index 162c2e0..01db999 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -414,6 +414,10 @@
>cmt
>mbmt
>mbml
> +  cache_misses
> +  cache_peferences
> +  instructions
> +  cpu_cycles
>  
>
>
> diff --git a/include/libvirt/libvirt-domain.h 
> b/include/libvirt/libvirt-domain.h
> index cba4fa5..99c4c48 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -1928,6 +1928,45 @@ void
> virDomainStatsRecordListFree(virDomainStatsRecordPtr *stats);
>   */
>  # define VIR_PERF_PARAM_MBML "mbml"
> 
> +/**
> + * VIR_PERF_PARAM_CACHE_MISSES:
> + *
> + * Macro for typed parameter name that represents cache_misses perf
> + * event which can be used to measure the amount of cache missing by
> + * applications running on the platform. It corresponds to the
> + * "perf.cache_misses" field in the *Stats APIs.
> + */
> +# define VIR_PERF_PARAM_CACHE_MISSES "cache_misses"
> +
> +/**
> + * VIR_PERF_PARAM_CACHE_REFERENCES:
> + *
> + * Macro for typed parameter name that represents cache_peferences
> + * perf event which can be used to measure the amount of cache hit
> + * by applications running on the platform. It corresponds to the
> + * "perf.cache_peferences" field in the *Stats APIs.
> + */
> +# define VIR_PERF_PARAM_CACHE_REFERENCES "cache_peferences"
> +
> +/**
> + * VIR_PERF_PARAM_INSTRUCTIONS:
> + *
> + * Macro for typed parameter name that represents instructions perf
> + * event which can be used to measure the amount of instructions
> + * by applications running on the platform. It corresponds to the
> + * "perf.instructions" field in the *Stats APIs.
> + */
> +# define VIR_PERF_PARAM_INSTRUCTIONS "instructions"
> +
> +/**
> + * VIR_PERF_PARAM_CPU_CYCLES:
> + *
> + * Macro for typed parameter name that represents cpu_cycles perf event
> + * which can be used to measure how many cycles one instruction needs.
> + * It corresponds to the "perf.cpu_cycles&quo

Re: [libvirt] [PATCH 1/1] cpu_map.xml: add cmt/mbm feature to x86

2016-06-23 Thread Ren, Qiaowei

> -Original Message-
> From: Daniel P. Berrange [mailto:berra...@redhat.com]
> Sent: Thursday, June 23, 2016 5:18 PM
> To: Ren, Qiaowei <qiaowei@intel.com>; libvir-list@redhat.com; Peter
> Krempa <pkre...@redhat.com>
> Subject: Re: [libvirt] [PATCH 1/1] cpu_map.xml: add cmt/mbm feature to x86
> 
> On Fri, Jun 17, 2016 at 11:17:47AM +0200, Jiri Denemark wrote:
> > On Fri, Jun 17, 2016 at 09:25:14 +0200, Jiri Denemark wrote:
> > > On Fri, Jun 17, 2016 at 09:23:56 +0800, Qiaowei Ren wrote:
> > > > Some Intel processor families (e.g. the Intel Xeon processor E5 v3
> > > > family) introduced some PQos (Platform Qos) features, including
> > > > CMT (Cache Monitoring echnology) and MBM (Memory Bandwidth
> > > > Monitoring), to monitor or control shared resource. This patch add
> > > > them into x86 part of cpu_map.xml to be used for applications
> > > > (like OpenStack) based on libvirt to get cpu capabilities.
> > > >
> > > > Signed-off-by: Qiaowei Ren <qiaowei@intel.com>
> > > > ---
> > > >  src/cpu/cpu_map.xml | 11 +++
> > > >  1 file changed, 11 insertions(+)
> > > >
> > > > diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml index
> > > > 08aded2..2e2cb4f 100644
> > > > --- a/src/cpu/cpu_map.xml
> > > > +++ b/src/cpu/cpu_map.xml
> > > > @@ -320,6 +320,9 @@
> > > >  
> > > >
> > > >  
> > > > +
> > > > +  
> > > > +
> > > >  
> > > >
> > > >  
> > >
> > > This hunk won't apply since about a week ago. Please, use current
> > > git when sending patches.
> > >
> > > > @@ -353,6 +356,14 @@
> > > >
> > > >  
> > > >
> > > > +
> > > > + 
> > > > +  
> > > > +
> > > > + 
> > > > +  
> > > > +
> > > > +
> > >
> > > And keep the list of features sorted by CPUID level, i.e., these
> > > features should go after 0x0d and before 0x8000.
> >
> > Oh and I completely forgot the most important thing: it makes little
> > sense to add CPUID features that QEMU does not support. It will only
> > allow users to see the features in host CPU capabilities. So if the
> > purpose of these patches is to be able to advertise whether the
> > appropriate perf events are supported on current host, CPU features
> > are not the right way of doing that. I think domain capabilities XML
> > would be the right place to advertise what events are supported.
> 
> Nova schedules guests based on the CPU features that the host has, so we 
> really
> do want this to be exposed in the general host capabilities XML description of
> the host CPU. We don't care about running guests with this features - we just
> want to see the host report for them.
> 
> 

Yes, Nova need host report for these features, and I will submit new version 
for this patch based on latest code according to Jiri's previous comment.

Thanks,
Qiaowei

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


Re: [libvirt] [PATCH 1/1] perf: add more perf events support

2016-06-21 Thread Ren, Qiaowei

> -Original Message-
> From: Peter Krempa [mailto:pkre...@redhat.com]
> Sent: Tuesday, June 21, 2016 3:03 PM
> To: Ren, Qiaowei <qiaowei@intel.com>
> Cc: libvir-list@redhat.com
> Subject: Re: [libvirt] [PATCH 1/1] perf: add more perf events support
> 
> On Tue, Jun 21, 2016 at 01:43:30 +, Ren, Qiaowei wrote:
> >
> > > -Original Message-
> > > From: libvir-list-boun...@redhat.com
> > > [mailto:libvir-list-boun...@redhat.com]
> > > On Behalf Of Ren, Qiaowei
> > > Sent: Sunday, June 12, 2016 10:14 AM
> > > To: Peter Krempa <pkre...@redhat.com>
> > > Cc: libvir-list@redhat.com
> > > Subject: Re: [libvirt] [PATCH 1/1] perf: add more perf events
> > > support
> 
> [...]
> 
> > Peter, do you have any other comments about this patch?
> 
> Please see my recent patch:
> https://www.redhat.com/archives/libvir-list/2016-June/msg00995.html
> 
> where I've added some documentation of the XML and fixed the schema and
> added a test that actually is used with the parser since the parser was 
> broken.
> You'll need to add the relevant bits to the files added by those commits.
> 
> Additionally I'd be glad if you could improve the documentation in the XML.
> 

Ok. I will update the XML documentation in next version.

Thanks,
Qiaowei

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


Re: [libvirt] [PATCH 1/1] perf: add more perf events support

2016-06-20 Thread Ren, Qiaowei

> -Original Message-
> From: libvir-list-boun...@redhat.com [mailto:libvir-list-boun...@redhat.com]
> On Behalf Of Ren, Qiaowei
> Sent: Sunday, June 12, 2016 10:14 AM
> To: Peter Krempa <pkre...@redhat.com>
> Cc: libvir-list@redhat.com
> Subject: Re: [libvirt] [PATCH 1/1] perf: add more perf events support
> 
> > > diff --git a/include/libvirt/libvirt-domain.h
> > > b/include/libvirt/libvirt-domain.h
> > > index cba4fa5..99c4c48 100644
> > > --- a/include/libvirt/libvirt-domain.h
> > > +++ b/include/libvirt/libvirt-domain.h
> > > @@ -1928,6 +1928,45 @@ void
> > virDomainStatsRecordListFree(virDomainStatsRecordPtr *stats);
> > >   */
> > >  # define VIR_PERF_PARAM_MBML "mbml"
> >
> > [...]
> >
> > > +/**
> > > + * VIR_PERF_PARAM_INSTRUCTIONS:
> > > + *
> > > + * Macro for typed parameter name that represents instructions perf
> > > + * event which can be used to measure the amount of instructions
> > > + * by applications running on the platform. It corresponds to the
> > > + * "perf.instructions" field in the *Stats APIs.
> >
> > I'm not sure if I understand the implications and usability of this stat 
> > parameter.
> > Could you elaborate on how this can be used?
> >
> > > + */
> > > +# define VIR_PERF_PARAM_INSTRUCTIONS "instructions"
> > > +
> > > +/**
> > > + * VIR_PERF_PARAM_CPU_CYCLES:
> > > + *
> > > + * Macro for typed parameter name that represents cpu_cycles perf
> > > +event
> > > + * which can be used to measure how many cycles one instruction needs.
> > > + * It corresponds to the "perf.cpu_cycles" field in the *Stats APIs.
> >
> > And same for this. I don't really see how this can be used.
> >
> 
> Peter, thanks for your feedback!
> 
> Instructions and cycles can be used to gain IPC (Instructions Per Clock, =
> Instructions/Cycles), and a low IPC ratio indicates the code of the process 
> makes
> poor use of the CPU.
> 

Peter, do you have any other comments about this patch?

Thanks,
Qiaowei

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


Re: [libvirt] [PATCH 1/1] perf: add more perf events support

2016-06-11 Thread Ren, Qiaowei

> -Original Message-
> From: Peter Krempa [mailto:pkre...@redhat.com]
> Sent: Thursday, June 9, 2016 7:50 PM
> To: Ren, Qiaowei <qiaowei@intel.com>
> Cc: libvir-list@redhat.com
> Subject: Re: [libvirt] [PATCH 1/1] perf: add more perf events support
> 
> On Tue, May 31, 2016 at 10:48:15 +0800, Qiaowei Ren wrote:
> > With current perf framework, this patch adds support to more perf
> > events, including cache missing, cache peference, cpu cycles,
> > instrction, etc..
> >
> > Signed-off-by: Qiaowei Ren <qiaowei@intel.com>
> > ---
> >  include/libvirt/libvirt-domain.h | 39 
> >  src/libvirt-domain.c |  8 +
> >  src/qemu/qemu_driver.c   | 23 +++---
> >  src/util/virperf.c   | 65 
> > +++-
> >  src/util/virperf.h   |  4 +++
> >  5 files changed, 126 insertions(+), 13 deletions(-)
> >
> > diff --git a/include/libvirt/libvirt-domain.h
> > b/include/libvirt/libvirt-domain.h
> > index cba4fa5..99c4c48 100644
> > --- a/include/libvirt/libvirt-domain.h
> > +++ b/include/libvirt/libvirt-domain.h
> > @@ -1928,6 +1928,45 @@ void
> virDomainStatsRecordListFree(virDomainStatsRecordPtr *stats);
> >   */
> >  # define VIR_PERF_PARAM_MBML "mbml"
> 
> [...]
> 
> > +/**
> > + * VIR_PERF_PARAM_INSTRUCTIONS:
> > + *
> > + * Macro for typed parameter name that represents instructions perf
> > + * event which can be used to measure the amount of instructions
> > + * by applications running on the platform. It corresponds to the
> > + * "perf.instructions" field in the *Stats APIs.
> 
> I'm not sure if I understand the implications and usability of this stat 
> parameter.
> Could you elaborate on how this can be used?
> 
> > + */
> > +# define VIR_PERF_PARAM_INSTRUCTIONS "instructions"
> > +
> > +/**
> > + * VIR_PERF_PARAM_CPU_CYCLES:
> > + *
> > + * Macro for typed parameter name that represents cpu_cycles perf
> > +event
> > + * which can be used to measure how many cycles one instruction needs.
> > + * It corresponds to the "perf.cpu_cycles" field in the *Stats APIs.
> 
> And same for this. I don't really see how this can be used.
> 

Peter, thanks for your feedback!

Instructions and cycles can be used to gain IPC (Instructions Per Clock, = 
Instructions/Cycles), and a low IPC ratio indicates the code of the process 
makes poor use of the CPU.

Thanks,
Qiaowei

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


Re: [libvirt] [PATCH 1/1] perf: add more perf events support

2016-06-06 Thread Ren, Qiaowei
Hi Peter,

Do you have any comments on this patch?

Thanks,
Qiaowei

> -Original Message-
> From: Ren, Qiaowei
> Sent: Tuesday, May 31, 2016 10:48 AM
> To: libvir-list@redhat.com
> Cc: Daniel P. Berrange <berra...@redhat.com>; Peter Krempa
> <pkre...@redhat.com>; Ren, Qiaowei <qiaowei@intel.com>
> Subject: [PATCH 1/1] perf: add more perf events support
> 
> With current perf framework, this patch adds support to more perf events,
> including cache missing, cache peference, cpu cycles, instrction, etc..
> 
> Signed-off-by: Qiaowei Ren <qiaowei@intel.com>
> ---
>  include/libvirt/libvirt-domain.h | 39 
>  src/libvirt-domain.c |  8 +
>  src/qemu/qemu_driver.c   | 23 +++---
>  src/util/virperf.c   | 65 
> +++-
>  src/util/virperf.h   |  4 +++
>  5 files changed, 126 insertions(+), 13 deletions(-)
> 
> diff --git a/include/libvirt/libvirt-domain.h 
> b/include/libvirt/libvirt-domain.h
> index cba4fa5..99c4c48 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -1928,6 +1928,45 @@ void
> virDomainStatsRecordListFree(virDomainStatsRecordPtr *stats);
>   */
>  # define VIR_PERF_PARAM_MBML "mbml"
> 
> +/**
> + * VIR_PERF_PARAM_CACHE_MISSES:
> + *
> + * Macro for typed parameter name that represents cache_misses perf
> + * event which can be used to measure the amount of cache missing by
> + * applications running on the platform. It corresponds to the
> + * "perf.cache_misses" field in the *Stats APIs.
> + */
> +# define VIR_PERF_PARAM_CACHE_MISSES "cache_misses"
> +
> +/**
> + * VIR_PERF_PARAM_CACHE_REFERENCES:
> + *
> + * Macro for typed parameter name that represents cache_peferences
> + * perf event which can be used to measure the amount of cache hit
> + * by applications running on the platform. It corresponds to the
> + * "perf.cache_peferences" field in the *Stats APIs.
> + */
> +# define VIR_PERF_PARAM_CACHE_REFERENCES "cache_peferences"
> +
> +/**
> + * VIR_PERF_PARAM_INSTRUCTIONS:
> + *
> + * Macro for typed parameter name that represents instructions perf
> + * event which can be used to measure the amount of instructions
> + * by applications running on the platform. It corresponds to the
> + * "perf.instructions" field in the *Stats APIs.
> + */
> +# define VIR_PERF_PARAM_INSTRUCTIONS "instructions"
> +
> +/**
> + * VIR_PERF_PARAM_CPU_CYCLES:
> + *
> + * Macro for typed parameter name that represents cpu_cycles perf event
> + * which can be used to measure how many cycles one instruction needs.
> + * It corresponds to the "perf.cpu_cycles" field in the *Stats APIs.
> + */
> +# define VIR_PERF_PARAM_CPU_CYCLES "cpu_cycles"
> +
>  int virDomainGetPerfEvents(virDomainPtr dom,
> virTypedParameterPtr *params,
> int *nparams, diff --git a/src/libvirt-domain.c 
> b/src/libvirt-
> domain.c index 73ae369..ef71b31 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -11452,6 +11452,14 @@ virConnectGetDomainCapabilities(virConnectPtr
> conn,
>   * "perf.mbml" - the amount of data (bytes/s) sent through the memory
> controller
>   *   on the socket as unsigned long long. It is produced by mbml
>   *   perf event.
> + * "perf.cache_misses" - the amount of cache missing as unsigned long long.
> + * It is produced by cache_misses perf event.
> + * "perf.cache_peferences" - the amount of cache hit as unsigned long long.
> + * It is produced by cache_peferences perf event.
> + * "perf.instructions" - the amount of instructions as unsigned long long.
> + * It is produced by instructions perf event.
> + * "perf.cpu_cycles" - the amount of cycles one instruction needs as
> + unsigned
> + * long long. It is produced by cpu_cycles 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
> 10d3e3d..d465ec5 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -9756,6 +9756,10 @@ qemuDomainSetPerfEvents(virDomainPtr dom,
> VIR_PERF_PARAM_CMT, VIR_TYPED_PARAM_BOOLEAN,
> VIR_PERF_PARAM_MBMT, VIR_TYPED_PARAM_BOOLEAN,
> VIR_PERF_PARAM_MBML, VIR_TYPED_PARAM_BOOLEAN,
> +

Re: [libvirt] [PATCH v3 1/1] perf: add support to perf event for MBM

2016-05-19 Thread Ren, Qiaowei

> -Original Message-
> From: Peter Krempa [mailto:pkre...@redhat.com]
> Sent: Thursday, May 19, 2016 9:46 PM
> To: Ren, Qiaowei <qiaowei@intel.com>
> Cc: libvir-list@redhat.com
> Subject: Re: [libvirt] [PATCH v3 1/1] perf: add support to perf event for MBM
> 
> On Fri, May 13, 2016 at 12:26:07 +0800, Qiaowei Ren wrote:
> > Some Intel processor families (e.g. the Intel Xeon processor E5 v3
> > family) introduced some RDT (Resource Director Technology) features to
> > monitor or control shared resource. Among these features, MBM (Memory
> > Bandwidth Monitoring), which is build on the CMT (Cache Monitoring
> > Technology) infrastructure, provides OS/VMM a way to monitor bandwidth
> > from one level of cache to another.
> >
> > With current perf framework, this patch adds support to perf event for
> > MBM.
> >
> > Signed-off-by: Qiaowei Ren <qiaowei@intel.com>
> > ---
> >  include/libvirt/libvirt-domain.h | 26 -
> >  src/libvirt-domain.c | 12 
> >  src/qemu/qemu_driver.c   | 41 +++---
> >  src/util/virperf.c   | 63 
> > 
> >  src/util/virperf.h   |  2 ++
> >  5 files changed, 108 insertions(+), 36 deletions(-)
> 
> [...]
> 
> > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index
> > c4c4968..670f620 100644
> > --- a/src/qemu/qemu_driver.c
> > +++ b/src/qemu/qemu_driver.c
> 
> [...]
> 
> > @@ -19494,24 +19496,38 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr
> > driver,
> >
> >  #undef QEMU_ADD_COUNT_PARAM
> >
> > +#define QEMU_ADD_PERF_PARAM_ULL(record, maxparams, name, value) \
> do
> > +{ \
> > +char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; \
> > +snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, \
> > + "perf.%s", name); \
> > +if (virTypedParamsAddULLong(&(record)->params, \
> > +&(record)->nparams, \
> > +maxparams, \
> > +param_name, \
> > +value) < 0) \
> > +goto cleanup; \
> > +} while (0)
> 
> This macro is used once so it's not really necessary.
> 
> > +
> >  static int
> > -qemuDomainGetStatsPerfCmt(virPerfPtr perf,
> > +qemuDomainGetStatsPerfRdt(virPerfPtr perf,
> > +  virPerfEventType type,
> >virDomainStatsRecordPtr record,
> >int *maxparams)  {
> > -uint64_t cache = 0;
> > +uint64_t value = 0;
> >
> > -if (virPerfReadEvent(perf, VIR_PERF_EVENT_CMT, ) < 0)
> > +if (virPerfReadEvent(perf, type, ) < 0)
> >  return -1;
> >
> > -if (virTypedParamsAddULLong(>params,
> > ->nparams,
> > -maxparams,
> > -"perf.cache",
> > -cache) < 0)
> > -return -1;
> > +QEMU_ADD_PERF_PARAM_ULL(record, maxparams,
> > +virPerfEventTypeToString(type),
> > +value);
> 
> Otherwise looks good. Thanks for tweaking the documentation.
> 
> I'll push this with the macro dropped in a while.
> 

Peter, thanks very much!

Thanks,
Qiaowei

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


Re: [libvirt] [PATCH 1/1] perf: add support to perf event for MBM

2016-05-12 Thread Ren, Qiaowei

> -Original Message-
> From: Peter Krempa [mailto:pkre...@redhat.com]
> Sent: Thursday, May 12, 2016 4:31 PM
> To: Ren, Qiaowei <qiaowei@intel.com>
> Cc: libvir-list@redhat.com; berra...@redhat.com
> Subject: Re: [libvirt] [PATCH 1/1] perf: add support to perf event for MBM
> 
> On Thu, May 12, 2016 at 14:55:16 +0800, Qiaowei Ren wrote:
> > Some Intel processor families (e.g. the Intel Xeon processor E5 v3
> > family) introduced some RDT (Resource Director Technology) features to
> > monitor or control shared resource. Among these features, MBM (Memory
> > Bandwidth Monitoring), which is build on the CMT (Cache Monitoring
> > Technology) infrastructure, provides OS/VMM a way to monitor bandwidth
> > from one level of cache to another.
> >
> > With current perf framework, this patch adds support to perf event for
> > MBM.
> >
> > Signed-off-by: Qiaowei Ren <qiaowei@intel.com>
> > ---
> >  include/libvirt/libvirt-domain.h | 18 
> >  src/qemu/qemu_driver.c   | 35 +++
> >  src/util/virperf.c   | 60 
> > 
> >  src/util/virperf.h   |  2 ++
> >  4 files changed, 85 insertions(+), 30 deletions(-)
> >
> > diff --git a/include/libvirt/libvirt-domain.h
> > b/include/libvirt/libvirt-domain.h
> > index 160f20f..e4594f0 100644
> > --- a/include/libvirt/libvirt-domain.h
> > +++ b/include/libvirt/libvirt-domain.h
> > @@ -1904,6 +1904,24 @@ void
> virDomainStatsRecordListFree(virDomainStatsRecordPtr *stats);
> >   */
> >  # define VIR_PERF_PARAM_CMT "cmt"
> >
> > +/**
> > + * VIR_PERF_PARAM_MBMT:
> > + *
> > + * Macro for typed parameter name that represents MBMT perf event
> > + * which can be used to monitor total system bandwidth (bytes/s)
> > + * from one level of cache to another.
> > + */
> > +# define VIR_PERF_PARAM_MBMT "mbmt"
> > +
> > +/**
> > + * VIR_PERF_PARAM_MBML:
> > + *
> > + * Macro for typed parameter name that represents MBML perf event
> > + * which can be used to monitor the amount of data (bytes/s) sent
> > + * through the memory controller on the socket.
> > + */
> > +# define VIR_PERF_PARAM_MBML "mbml"
> 
> 
> So I was a little bit mistaken what this is documenting. So these document the
> arguments for virDomainSetPerfEvents where the perf stuff is enabled.
> 
> I was actually thinking this is documenting the values that are returned via 
> the
> virConnectGetAllDomainStats/virDomainListGetStats APIs. I've just noticed that
> the perf events are not documented at all there.
> 
> This should be fixed also for the CMT event. This part of the documentation
> should then state which fields in the *Stats APIs the perf events correspond 
> to.
> 
> In the documentation for the fields returned bulk Stats API the individual 
> fields
> should be marked by which event they are produced.
> 

Sure. The documentation will be added.

> > +
> >  int virDomainGetPerfEvents(virDomainPtr dom,
> > virTypedParameterPtr *params,
> > int *nparams, diff --git
> > a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index
> > c4c4968..a3bd7ec 100644
> > --- a/src/qemu/qemu_driver.c
> > +++ b/src/qemu/qemu_driver.c
> > @@ -10051,6 +10051,8 @@ qemuDomainSetPerfEvents(virDomainPtr dom,
> >
> >  if (virTypedParamsValidate(params, nparams,
> > VIR_PERF_PARAM_CMT,
> > VIR_TYPED_PARAM_BOOLEAN,
> > +   VIR_PERF_PARAM_MBMT, 
> > VIR_TYPED_PARAM_BOOLEAN,
> > +   VIR_PERF_PARAM_MBML,
> > + VIR_TYPED_PARAM_BOOLEAN,
> > NULL) < 0)
> >  return -1;
> >
> > @@ -19495,20 +19497,38 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr
> > driver,  #undef QEMU_ADD_COUNT_PARAM
> >
> >  static int
> > -qemuDomainGetStatsPerfCmt(virPerfPtr perf,
> > +qemuDomainGetStatsPerfRdt(virPerfPtr perf,
> > +  virPerfEventType type,
> >virDomainStatsRecordPtr record,
> >int *maxparams)  {
> > -uint64_t cache = 0;
> > +uint64_t value = 0;
> >
> > -if (virPerfReadEvent(perf, VIR_PERF_EVENT_CMT, ) < 0)
> > +if (virPerfReadEvent(perf, type, ) < 0)
> >  return -1;
> >
> > -if (virTypedParamsAddULLong(>params,
> > +if (type == VIR_PERF

Re: [libvirt] [PATCH 1/1] python: add python binding for Perf API

2016-03-31 Thread Ren, Qiaowei

> -Original Message-
> From: Michal Privoznik [mailto:mpriv...@redhat.com]
> Sent: Thursday, March 31, 2016 10:04 PM
> To: Ren, Qiaowei; libvir-list@redhat.com
> Subject: Re: [libvirt] [PATCH 1/1] python: add python binding for Perf API
> 
> On 31.03.2016 08:16, Michal Privoznik wrote:
> > On 30.03.2016 04:13, Qiaowei Ren wrote:
> >> This patch adds the python binding for virDomainSetPerfEvents and
> >> virDomainSetPerfEvents API.
> >>
> >> Signed-off-by: Qiaowei Ren <qiaowei@intel.com>
> >> ---
> >>  generator.py |  2 ++
> >>  libvirt-override-api.xml | 11 ++
> >>  libvirt-override.c   | 93
> 
> >>  3 files changed, 106 insertions(+)
> >>
> 
> I had to do couple of more tweaks than originally intended. But this is now
> pushed.
> 
Thanks, Michal!

- Qiaowei



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


Re: [libvirt] [PATCH v4 0/8] Add perf and Intel CMT feature support

2016-03-29 Thread Ren, Qiaowei

> -Original Message-
> From: Daniel P. Berrange [mailto:berra...@redhat.com]
> Sent: Tuesday, March 29, 2016 8:14 PM
> To: Ren, Qiaowei
> Cc: libvir-list@redhat.com; Jiri Denemark
> Subject: Re: [PATCH v4 0/8] Add perf and Intel CMT feature support
> 
> On Mon, Mar 28, 2016 at 09:30:25PM +0800, Qiaowei Ren wrote:
> > The series mainly adds Intel CMT feature support into libvirt. CMT is
> > new introduced PQos (Platform Qos) feature to monitor the usage of
> > cache by applications running on the platform.
> >
> > Currently CMT patches has been merged into Linux kernel mainline.
> > The CMT implementation in Linux kernel is based on perf mechanism and
> > there is no support for perf in libvirt, and so this series firstly
> > add perf support into libvirt, including two public API and a set of
> > util interfaces. And based on these APIs and interfaces, thie series
> > implements CMT perf event support.
> >
> > Current state:
> >
> > * 1/8 - perf: add new public APIs for perf event
> > - ACKed by Daniel
> >
> > * 2/8 - perf: implement the remote protocol for perf event
> > - ACKed by Daniel
> >
> > * 3/8 - perf: implement a set of util functions for perf event
> > - ACKed by Daniel
> >
> > * 4/8 - qemu_driver: add support to perf event
> > - restart perf in qemuProcessReconnect and qemuProcessAttach
> >   in patch 6/8
> >
> > * 5/8 - perf: add new xml element
> > - ACKed by Daniel
> >
> > * 6/8 - perf: reenable perf events when libvirtd restart
> > - add qemuDomainPerfRestart() into qemuProcessReconnect() and
> >   qemuProcessAttach()
> >
> > * 7/8 - virsh: implement new command to support perf
> > - ACKed by Daniel
> >
> > * 8/8 - virsh: extend domstats command
> > - ACKed by Daniel
> 
> ACK to all patches. I had some comments inline - in future just make sure you 
> run
> 'make check && make syntax-check' against each patch before submitting. Since
> the fixes were easy I've made them myself and pushed to GIT master.
> 
> 
Ok! Got it! Sorry, I just rebased it into latest code and compiled it and 
ignored this step. Thanks very much for your nice help. ^-^

Thanks,
Qiaowei

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


Re: [libvirt] [PATCH 6/8] perf: reenable perf events when libvirtd restart

2016-01-28 Thread Ren, Qiaowei


> -Original Message-
> From: Daniel P. Berrange [mailto:berra...@redhat.com]
> Sent: Thursday, January 28, 2016 11:21 PM
> To: Ren, Qiaowei
> Cc: libvir-list@redhat.com; Jiri Denemark
> Subject: Re: [PATCH 6/8] perf: reenable perf events when libvirtd restart
> 
> On Thu, Dec 10, 2015 at 08:34:36PM +0800, Qiaowei Ren wrote:
> > When libvirtd daemon restart, this patch will reenable those perf
> > events previously enabled.
> >
> > Signed-off-by: Qiaowei Ren <qiaowei@intel.com>
> > ---
> >  src/qemu/qemu_driver.c | 36 
> >  1 file changed, 36 insertions(+)
> >
> > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index
> > 7d95364..09607ca 100644
> > --- a/src/qemu/qemu_driver.c
> > +++ b/src/qemu/qemu_driver.c
> > @@ -164,6 +164,9 @@ static int qemuDomainGetMaxVcpus(virDomainPtr
> > dom);  static int qemuDomainManagedSaveLoad(virDomainObjPtr vm,
> >   void *opaque);
> >
> > +static int qemuDomainPerfRestart(virDomainObjPtr vm,
> > + void *data);
> > +
> >  static int qemuOpenFile(virQEMUDriverPtr driver,
> >  virDomainObjPtr vm,
> >  const char *path, int oflags, @@ -939,6
> > +942,10 @@ qemuStateInitialize(bool privileged,
> >  qemuDomainManagedSaveLoad,
> >  qemu_driver);
> >
> > +virDomainObjListForEach(qemu_driver->domains,
> > +qemuDomainPerfRestart,
> > +NULL);
> > +
> >  qemuProcessReconnectAll(conn, qemu_driver);
> >
> >  qemu_driver->workerPool = virThreadPoolNew(0, 1, 0,
> > qemuProcessEventHandler, qemu_driver); @@ -3460,6 +3467,35 @@
> qemuDomainManagedSaveLoad(virDomainObjPtr vm,
> >  return ret;
> >  }
> >
> > +static int
> > +qemuDomainPerfRestart(virDomainObjPtr vm,
> > +  void *data ATTRIBUTE_UNUSED) {
> > +size_t i;
> > +virDomainDefPtr def = vm->def;
> > +qemuDomainObjPrivatePtr priv = vm->privateData;
> > +
> > +virPerfFree(priv->perf);
> > +
> > +priv->perf = virPerfNew();
> > +if (!priv->perf)
> > +return -1;
> > +
> > +for (i = 0; i < VIR_PERF_EVENT_LAST; i++) {
> > +if (def->perf->events[i] &&
> > +def->perf->events[i] == VIR_TRISTATE_BOOL_YES) {
> > +if (virPerfEventEnable(priv->perf, i, vm->pid))
> > +goto cleanup;
> > +}
> > +}
> > +
> > +return 0;
> > +
> > + cleanup:
> > +virPerfFree(priv->perf);
> > +return -1;
> > +}
> > +
> 
> Oh, I suggested we add this in qemuProcessReconnect in a previous patch
> 

Yes. I will do this in  qemuProcessReconnect and qemuProcessAttach according to 
the comments in patch 4/8, but because this patch have to depend on the new XML 
element (patch 5/8), and so I have to put these code into the patch and could 
not merge it into the patch 4/8. ^-^

Thanks,
Qiaowei

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


Re: [libvirt] [PATCH 0/8] Add perf and Intel CMT feature support

2016-01-12 Thread Ren, Qiaowei
Hi, Daniel and Jiri,

Do you have more comments about this new version? I guess that it should fix 
those comments on previous version. ^-^

Thanks,
Qiaowei


> -Original Message-
> From: Ren, Qiaowei
> Sent: Thursday, December 10, 2015 8:35 PM
> To: libvir-list@redhat.com
> Cc: Daniel P. Berrange; Jiri Denemark; Ren, Qiaowei
> Subject: [PATCH 0/8] Add perf and Intel CMT feature support
> 
> The series mainly adds Intel CMT feature support into libvirt. CMT is new
> introduced PQos (Platform Qos) feature to monitor the usage of cache by
> applications running on the platform.
> 
> Currently CMT patches has been merged into Linux kernel mainline.
> The CMT implementation in Linux kernel is based on perf mechanism and there is
> no support for perf in libvirt, and so this series firstly add perf support 
> into libvirt,
> including two public API and a set of util interfaces. And based on these 
> APIs and
> interfaces, thie series implements CMT perf event support.
> 
> TODO:
> 1. add support for new APIs into libvirt-python library.
> 
> Changes since v1:
>   * change perf APIs implementation to match new style of the API.
>   * add new xml element
>   * reenable all perf events previously enabled when libvirtd daemon
> restart.
>   * redesign perf util functions.
> 
> Changes since v2:
>   * add an example XML file to the test suite.
>   * add virPerfReadEvent().
>   * change 'perf' xml element to new style.
>   * change 'perf' command to new stype.
> 
> Qiaowei Ren (8):
>   perf: add new public APIs for perf event
>   perf: implement the remote protocol for perf event
>   perf: implement a set of util functions for perf event
>   qemu_driver: add support to perf event
>   perf: add new xml element
>   perf: reenable perf events when libvirtd restart
>   virsh: implement new command to support perf
>   virsh: extend domstats command
> 
>  daemon/remote.c   |  47 
>  docs/schemas/domaincommon.rng |  27 +++
>  include/libvirt/libvirt-domain.h  |  19 ++
>  include/libvirt/virterror.h   |   1 +
>  src/Makefile.am   |   1 +
>  src/conf/domain_conf.c| 111 ++
>  src/conf/domain_conf.h|  10 +
>  src/driver-hypervisor.h   |  12 +
>  src/libvirt-domain.c  |  93 
>  src/libvirt_private.syms  |  12 +
>  src/libvirt_public.syms   |   6 +
>  src/qemu/qemu_domain.h|   3 +
>  src/qemu/qemu_driver.c| 195 +
>  src/qemu/qemu_process.c   |  10 +
>  src/remote/remote_driver.c|  39 
>  src/remote/remote_protocol.x  |  30 ++-
>  src/remote_protocol-structs   |  18 ++
>  src/util/virerror.c   |   1 +
>  src/util/virperf.c| 303 
> ++
>  src/util/virperf.h|  63 ++
>  tests/domainschemadata/domain-perf-simple.xml |  20 ++
>  tools/virsh-domain-monitor.c  |   7 +
>  tools/virsh-domain.c  | 128 +++
>  tools/virsh.pod   |  27 ++-
>  24 files changed, 1180 insertions(+), 3 deletions(-)  create mode 100644
> src/util/virperf.c  create mode 100644 src/util/virperf.h  create mode 100644
> tests/domainschemadata/domain-perf-simple.xml
> 
> --
> 1.9.1


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


Re: [libvirt] [PATCH v2 7/8] virsh: implement new command to support perf

2015-12-13 Thread Ren, Qiaowei


> -Original Message-
> From: Daniel P. Berrange [mailto:berra...@redhat.com]
> Sent: Tuesday, December 8, 2015 7:07 PM
> To: Ren, Qiaowei
> Cc: libvir-list@redhat.com; Jiri Denemark
> Subject: Re: [PATCH v2 7/8] virsh: implement new command to support perf
> 
> On Mon, Dec 07, 2015 at 03:53:58PM +0800, Qiaowei Ren wrote:
> > This patch add new perf command to enable/disable perf event for a
> > guest domain.
> >
> > Signed-off-by: Qiaowei Ren <qiaowei@intel.com>
> > ---
> >  tools/virsh-domain.c | 125
> +++
> >  tools/virsh.pod  |  18 
> >  2 files changed, 143 insertions(+)
> >
> > diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index
> > b7e7606..a47ef9f 100644
> > --- a/tools/virsh-domain.c
> > +++ b/tools/virsh-domain.c
> > @@ -,6 +,125 @@ cmdMemtune(vshControl *ctl, const vshCmd
> *cmd)
> > }
> >
> >  /*
> > + * "perf" command
> > + */
> > +static const vshCmdInfo info_perf[] = {
> > +{.name = "help",
> > +.data = N_("Get or set perf event")
> > +},
> > +{.name = "desc",
> > +.data = N_("Get or set the current perf events for a guest"
> > +   " domain.\n"
> > +   "To get the perf events list use following command: 
> > \n\n"
> > +   "virsh # perf ")
> > +},
> > +{.name = NULL}
> > +};
> > +
> > +static const vshCmdOptDef opts_perf[] = {
> > +{.name = "domain",
> > + .type = VSH_OT_DATA,
> > + .flags = VSH_OFLAG_REQ,
> > + .help = N_("domain name, id or uuid")
> > +},
> > +{.name = "cmt",
> > + .type = VSH_OT_INT,
> > + .help = N_("CMT event, as integer (default 0, disable)")
> > +},
> 
> Imagine we add many more perf events - we don't want to be adding args to
> virsh for each one. Instead of --cmt, we should have '--event cmt'
> 
> 

Ok. I guess that this command will be the following style:

virsh perf  --enable  --disable <>

For instance: virsh perf guest01 --enable cmt

Daniel, do you think this is OK?

Thanks,
Qiaowei

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


Re: [libvirt] [PATCH v2 3/8] perf: implement a set of util functions for perf event

2015-12-08 Thread Ren, Qiaowei

> -Original Message-
> From: Daniel P. Berrange [mailto:berra...@redhat.com]
> Sent: Tuesday, December 8, 2015 6:54 PM
> To: Ren, Qiaowei
> Cc: libvir-list@redhat.com; Jiri Denemark
> Subject: Re: [PATCH v2 3/8] perf: implement a set of util functions for perf 
> event
> 
> On Mon, Dec 07, 2015 at 03:53:54PM +0800, Qiaowei Ren wrote:
> > This patch implement a set of interfaces for perf event. Based on
> > these interfaces, we can implement internal driver API for perf, and
> > get the results of perf conuter you care about.
> >
> > Signed-off-by: Qiaowei Ren <qiaowei@intel.com>
> > ---
> >  include/libvirt/virterror.h |   1 +
> >  src/Makefile.am |   1 +
> >  src/libvirt_private.syms|  12 ++
> >  src/util/virerror.c |   1 +
> >  src/util/virperf.c  | 298
> 
> >  src/util/virperf.h  |  61 +
> >  6 files changed, 374 insertions(+)
> >  create mode 100644 src/util/virperf.c  create mode 100644
> > src/util/virperf.h
> >
> 
> > +int
> > +virPerfEventDisable(virPerfPtr perf,
> > +virPerfEventType type) {
> > +virPerfEventPtr event = virPerfGetEvent(perf, type);
> > +if (event == NULL)
> > +return -1;
> > +
> > +if (ioctl(event->fd, PERF_EVENT_IOC_DISABLE) < 0) {
> > +virReportSystemError(errno,
> > + _("Unable to disable perf event type=%d"),
> > + event->type);
> > +return -1;
> > +}
> 
> Since we're closing the file handle next, is there any benefit in doing this 
> ioctl() ?
> 

I guess that VIR_FORCE_CLOSE() will just close the file handle, but the perf 
event have to be disabled before this. In fact it is from the example in man 
page of perf_event_open().

Thanks,
Qiaowei

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


Re: [libvirt] [PATCH 6/8] qemu_driver: add support to perf event

2015-11-30 Thread Ren, Qiaowei

> -Original Message-
> From: Jiri Denemark [mailto:jdene...@redhat.com]
> Sent: Monday, November 30, 2015 7:09 PM
> To: Ren, Qiaowei
> Cc: libvir-list@redhat.com
> Subject: Re: [libvirt] [PATCH 6/8] qemu_driver: add support to perf event
> 
> On Mon, Nov 30, 2015 at 08:17:05 +, Ren, Qiaowei wrote:
> >
> > > -Original Message-
> > > From: Jiri Denemark [mailto:jdene...@redhat.com]
> > > Sent: Tuesday, November 24, 2015 9:01 PM
> > > To: Ren, Qiaowei
> > > Cc: libvir-list@redhat.com
> > > Subject: Re: [libvirt] [PATCH 6/8] qemu_driver: add support to perf
> > > event
> > >
> > > On Tue, Nov 17, 2015 at 16:00:46 +0800, Qiaowei Ren wrote:
> > > > This patch implement the internal driver API for perf event into
> > > > qemu driver.
> > > >
> > > > In addition, this patch extend virDomainListGetStats API to get
> > > > the statistics for perf event. To do so, we add a
> 'VIR_DOMAIN_STATS_PERF'
> > > > enum to causes reporting of all previously enabled perf events.
> > > >
> > > > Signed-off-by: Qiaowei Ren <qiaowei@intel.com>
> > > > ---
> > > >  include/libvirt/libvirt-domain.h |   1 +
> > > >  src/qemu/qemu_domain.h   |   3 +
> > > >  src/qemu/qemu_driver.c   | 181
> > > +++
> > > >  #define QEMU_SCHED_MIN_PERIOD  1000LL
> > > >  #define QEMU_SCHED_MAX_PERIOD   100LL
> > > >  #define QEMU_SCHED_MIN_QUOTA   1000LL
> > > > @@ -10244,6 +10247,106 @@
> > > qemuDomainGetNumaParameters(virDomainPtr
> > > > dom,  }
> > > >
> > > >  static int
> > > > +qemuDomainSetPerfEvents(virDomainPtr dom,
> > > > +virTypedParameterPtr params,
> > > > +int nparams) {
> > > > +size_t i;
> > > > +virDomainObjPtr vm = NULL;
> > > > +qemuDomainObjPrivatePtr priv;
> > > > +int ret = -1;
> > > > +bool enabled;
> > > > +
> > > > +if (virTypedParamsValidate(params, nparams,
> > > > +   VIR_DOMAIN_PERF_CMT,
> > > > +   VIR_TYPED_PARAM_BOOLEAN,
> > > > +   NULL) < 0)
> > > > +return -1;
> > >
> > > Use virTypedParamsCheck and define the data for it in virperf.h so
> > > that you don't need to change the code here if new event type is added.
> > >
> >
> > Jirka, I checked the difference between virTypedParamsValidate and
> > virTypedParamsCheck, and found that virTypedParamsCheck just check
> > whether the param is valid through param name, but
> > virTypedParamsValidate will check whether the param type is right.
> 
> Oh, sorry for the confusion then. I actually wanted the code to be something 
> like
> 
> if (virTypedParamsValidate(params, nparams, VIR_PERF_PARAMS) < 0)
> return -1;
> 
> and VIR_PERF_PARAMS would be a macro defined in virparams.h:
> 
> # define VIR_PERF_PARAMS \
> VIR_DOMAIN_PERF_CMT, VIR_TYPED_PARAM_BOOLEAN, \
> NULL
> 
> Similar to QEMU_MIGRATION_PARAMETERS and few others.
> 

Got it! Thanks, Jirka.

Qiaowei


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


Re: [libvirt] [PATCH 6/8] qemu_driver: add support to perf event

2015-11-30 Thread Ren, Qiaowei

> -Original Message-
> From: Jiri Denemark [mailto:jdene...@redhat.com]
> Sent: Tuesday, November 24, 2015 9:01 PM
> To: Ren, Qiaowei
> Cc: libvir-list@redhat.com
> Subject: Re: [libvirt] [PATCH 6/8] qemu_driver: add support to perf event
> 
> On Tue, Nov 17, 2015 at 16:00:46 +0800, Qiaowei Ren wrote:
> > This patch implement the internal driver API for perf event into qemu
> > driver.
> >
> > In addition, this patch extend virDomainListGetStats API to get the
> > statistics for perf event. To do so, we add a 'VIR_DOMAIN_STATS_PERF'
> > enum to causes reporting of all previously enabled perf events.
> >
> > Signed-off-by: Qiaowei Ren <qiaowei@intel.com>
> > ---
> >  include/libvirt/libvirt-domain.h |   1 +
> >  src/qemu/qemu_domain.h   |   3 +
> >  src/qemu/qemu_driver.c   | 181
> +++
> >  #define QEMU_SCHED_MIN_PERIOD  1000LL
> >  #define QEMU_SCHED_MAX_PERIOD   100LL
> >  #define QEMU_SCHED_MIN_QUOTA   1000LL
> > @@ -10244,6 +10247,106 @@
> qemuDomainGetNumaParameters(virDomainPtr
> > dom,  }
> >
> >  static int
> > +qemuDomainSetPerfEvents(virDomainPtr dom,
> > +virTypedParameterPtr params,
> > +int nparams)
> > +{
> > +size_t i;
> > +virDomainObjPtr vm = NULL;
> > +qemuDomainObjPrivatePtr priv;
> > +int ret = -1;
> > +bool enabled;
> > +
> > +if (virTypedParamsValidate(params, nparams,
> > +   VIR_DOMAIN_PERF_CMT,
> > +   VIR_TYPED_PARAM_BOOLEAN,
> > +   NULL) < 0)
> > +return -1;
> 
> Use virTypedParamsCheck and define the data for it in virperf.h so that you
> don't need to change the code here if new event type is added.
> 

Jirka, I checked the difference between virTypedParamsValidate and 
virTypedParamsCheck, and found that virTypedParamsCheck just check whether the 
param is valid through param name, but virTypedParamsValidate will check 
whether the param type is right.

In addition, virTypedParamsCheck is just used in three functions in 
libvirt-domain.c, and all driver APIs use virTypedParamsValidate to validate 
the parms like this patch. What do you think about it? 

Thanks,
Qiaowei

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


Re: [libvirt] [PATCH 0/8] Add perf and Intel CMT feature support

2015-11-25 Thread Ren, Qiaowei

> -Original Message-
> From: Daniel P. Berrange [mailto:berra...@redhat.com]
> Sent: Tuesday, November 24, 2015 9:29 PM
> To: Ren, Qiaowei; libvir-list@redhat.com
> Subject: Re: [libvirt] [PATCH 0/8] Add perf and Intel CMT feature support
> 
> On Tue, Nov 24, 2015 at 02:24:22PM +0100, Jiri Denemark wrote:
> > On Tue, Nov 17, 2015 at 16:00:40 +0800, Qiaowei Ren wrote:
> > > The series mainly adds Intel CMT feature support into libvirt. CMT
> > > is new introduced PQos (Platform Qos) feature to monitor the usage
> > > of cache by applications running on the platform.
> > >
> > > Currently CMT patches has been merged into Linux kernel mainline.
> > > The CMT implementation in Linux kernel is based on perf mechanism
> > > and there is no support for perf in libvirt, and so this series
> > > firstly add perf support into libvirt, including two public API and
> > > a set of util interfaces. And based on these APIs and interfaces,
> > > thie series implements CMT perf event support.
> > >
> > > TODO:
> > > 1. This series relys on keeping an open file descriptor for the guest.
> > > We should add one patch to call sys_perf_event_open again iff
> > > libvirtd is restarted.
> >
> > Yes, we should reenable perf events when reconnecting to running
> > domains. Will we need to remember what events were enabled (in domain
> > status XML) or is it something we can read back from the kernel?
> 
> We need to record it somewhere. I guess this raises the question of whether we
> should hide it in status XML, or persist the list of desired perf events in 
> the real
> domain XML, so we can have the option of also having them enabled
> immediately at startup, without requiring separate API call.
> 

I checked the kernel perf interface and could not find the way to read back 
whether events were enabled from the kernel. So we have to record it somewhere 
according to Daniel.

If we just persist the list of desired perf events in the real domain XML, 
users can have the option of having them enabled at startup. But it is also 
possible that users enable more events  at runtime through API call for perf 
event. So should we record it in both real domain XML and status XML?

Thanks,
Qiaowei

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


Re: [libvirt] [PATCH 4/8] perf: implement the remote protocol for perf event

2015-11-24 Thread Ren, Qiaowei

> -Original Message-
> From: Jiri Denemark [mailto:jdene...@redhat.com]
> Sent: Tuesday, November 24, 2015 5:43 PM
> To: Ren, Qiaowei
> Cc: libvir-list@redhat.com
> Subject: Re: [libvirt] [PATCH 4/8] perf: implement the remote protocol for 
> perf
> event
> 
> On Tue, Nov 17, 2015 at 16:00:44 +0800, Qiaowei Ren wrote:
> > Add remote support for perf event.
> >
> > Signed-off-by: Qiaowei Ren <qiaowei@intel.com>
> > ---
> >  daemon/remote.c  | 60
> 
> >  src/remote/remote_driver.c   | 49
> 
> >  src/remote/remote_protocol.x | 32 ++-
> > src/remote_protocol-structs  | 20 +++
> >  4 files changed, 160 insertions(+), 1 deletion(-)
> 
> This will need to be changed to match the new style of the API. But, since you
> are adding a new API, would you mind creating a patch for libvirt-python to 
> add
> this new API to python bindings?
> 

Sure. The patch for libvirt-python should be also sent this maillist with this 
series, right?

Thanks,
Qiaowei 


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


Re: [libvirt] [PATCH 1/8] perf: add new public APIs for perf event

2015-11-23 Thread Ren, Qiaowei

> -Original Message-
> From: Jiri Denemark [mailto:jdene...@redhat.com]
> Sent: Monday, November 23, 2015 11:26 PM
> To: Ren, Qiaowei
> Cc: libvir-list@redhat.com
> Subject: Re: [libvirt] [PATCH 1/8] perf: add new public APIs for perf event
> 
> On Tue, Nov 17, 2015 at 16:00:41 +0800, Qiaowei Ren wrote:
> > API agreed on in
> > https://www.redhat.com/archives/libvir-list/2015-October/msg00872.html
> >
> > * include/libvirt/libvirt-domain.h (virDomainGetPerfEvents,
> > virDomainSetPerfEvents): New declarations.
> > * src/libvirt_public.syms: Export new symbols.
> >
> > Signed-off-by: Qiaowei Ren <qiaowei@intel.com>
> > ---
> >  include/libvirt/libvirt-domain.h | 18 ++
> >  src/libvirt_public.syms  |  6 ++
> >  2 files changed, 24 insertions(+)
> >
> > diff --git a/include/libvirt/libvirt-domain.h
> > b/include/libvirt/libvirt-domain.h
> > index a1ea6a5..69965e6 100644
> > --- a/include/libvirt/libvirt-domain.h
> > +++ b/include/libvirt/libvirt-domain.h
> > @@ -1802,6 +1802,24 @@ int virDomainListGetStats(virDomainPtr *doms,
> > void virDomainStatsRecordListFree(virDomainStatsRecordPtr *stats);
> >
> >  /*
> > + * Perf Event API
> > + */
> > +
> > +/**
> > + * VIR_DOMAIN_PERF_CMT:
> > + *
> > + * Macro for typed parameter name that represents CMT perf event.
> > + */
> > +# define VIR_DOMAIN_PERF_CMT "cmt"
> > +
> > +int virDomainGetPerfEvents(virDomainPtr dom,
> > +   virTypedParameterPtr params,
> > +   int * nparams);
> 
> We don't put a space between * and the name.
> 
> > +int virDomainSetPerfEvents(virDomainPtr dom,
> > +   virTypedParameterPtr params,
> > +   int nparams);
> > +
> > +/*
> >   * BlockJob API
> >   */
> >
> > diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index
> > dd94191..03206e7 100644
> > --- a/src/libvirt_public.syms
> > +++ b/src/libvirt_public.syms
> > @@ -725,4 +725,10 @@ LIBVIRT_1.2.19 {
> >  virDomainRename;
> >  } LIBVIRT_1.2.17;
> >
> > +LIBVIRT_1.2.23 {
> > +global:
> > +virDomainGetPerfEvents;
> > +virDomainSetPerfEvents;
> > +} LIBVIRT_1.2.19;
> > +
> >  #  define new API here using predicted next version number 
> 
> This patch does not compile, you should squash patches 1, 2, and 3 together,
> since 'make all check syntax-check' should pass after each patch in the 
> series.
> 

Ok. This is just from the sample about "Implementing a new API in Libvirt" 
(https://libvirt.org/api_extension.html ). ^-^

I will make these 3 patches together in next version.

Thanks,
Qiaowei


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


Re: [libvirt] [PATCH 3/8] perf: implement the public APIs for perf event

2015-11-23 Thread Ren, Qiaowei

> -Original Message-
> From: Jiri Denemark [mailto:jdene...@redhat.com]
> Sent: Tuesday, November 24, 2015 12:25 AM
> To: Ren, Qiaowei
> Cc: libvir-list@redhat.com
> Subject: Re: [libvirt] [PATCH 3/8] perf: implement the public APIs for perf 
> event
> 
> On Tue, Nov 17, 2015 at 16:00:43 +0800, Qiaowei Ren wrote:
> > * src/libvirt-domain.c: Implement virDomainGetPerfEvents and
> > virDomainSetPerfEvents.
> >
> > Signed-off-by: Qiaowei Ren <qiaowei@intel.com>
> > ---
> >  src/libvirt-domain.c | 106
> > +++
> >  1 file changed, 106 insertions(+)
> >
> > diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index
> > de7eb04..e767606 100644
> > --- a/src/libvirt-domain.c
> > +++ b/src/libvirt-domain.c
> > @@ -9572,6 +9572,112 @@ virDomainOpenChannel(virDomainPtr dom,
> >
> >
> >  /**
> > + * virDomainGetPerfEvents:
> > + * @domain: pointer to domain object
> > + * @params: pointer to perf events parameter object
> > + *  (return value, allocated by the caller)
> > + * @nparams: pointer to number of perf event parameters
> > + *
> > + * Get all perf events setting. On input, @nparams gives the size of
> > + the
> > + * @params array; on output, @nparams gives how many slots were
> > + filled
> > + * with parameter information, which might be less but will not
> > + exceed
> > + * the input value.
> > + *
> > + * As a special case, calling with @params as NULL and @nparams as 0
> > + on
> > + * input will cause @nparams on output to contain the number of
> > + parameters
> > + * supported by the hypervisor. The caller should then allocate
> > + @params
> > + * array, i.e. (sizeof(@virTypedParameter) * @nparams) bytes and call
> > + the
> > + * API again.
> > + *
> > + * See virDomainGetMemoryParameters() for an equivalent usage example.
> 
> You took a wrong API to copy. Requiring the caller to run the API with @params
> == NULL to get the number of parameters and then pass the allocated @params
> to get the values is the original design which we do not use for new APIs. The
> API should just allocate @params, fill it with all the values it knows about, 
> and
> return the count in @nparams. In other words, no caller allocated arrays, 
> please.
> You can look at, e.g., virDomainGetJobStats to see how it works.
> 

Thanks, Jirka. I will update this patch according to new API design.

Thanks,
Qiaowei

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


Re: [libvirt] [PATCH 0/8] Add perf and Intel CMT feature support

2015-11-18 Thread Ren, Qiaowei

> -Original Message-
> From: Daniel P. Berrange [mailto:berra...@redhat.com]
> Sent: Tuesday, November 17, 2015 6:47 PM
> To: Ren, Qiaowei
> Cc: libvir-list@redhat.com; Jiri Denemark
> Subject: Re: [PATCH 0/8] Add perf and Intel CMT feature support
> 
> On Tue, Nov 17, 2015 at 04:00:40PM +0800, Qiaowei Ren wrote:
> > The series mainly adds Intel CMT feature support into libvirt. CMT is
> > new introduced PQos (Platform Qos) feature to monitor the usage of
> > cache by applications running on the platform.
> >
> > Currently CMT patches has been merged into Linux kernel mainline.
> > The CMT implementation in Linux kernel is based on perf mechanism and
> > there is no support for perf in libvirt, and so this series firstly
> > add perf support into libvirt, including two public API and a set of
> > util interfaces. And based on these APIs and interfaces, thie series
> > implements CMT perf event support.
> >
> > TODO:
> > 1. This series relys on keeping an open file descriptor for the guest.
> > We should add one patch to call sys_perf_event_open again iff libvirtd
> > is restarted.
> 
> I've not done a detailed patch review, but overall I think this design is 
> pretty
> good now, so thanks for taking time to re-write this.
> 

Daniel, thanks for your feedback. ^-^

Thanks,
Qiaowei

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


Re: [libvirt] [PATCH 2/3] Qemu: add CMT support

2015-10-31 Thread Ren, Qiaowei

> -Original Message-
> From: Jiri Denemark [mailto:jdene...@redhat.com]
> Sent: Thursday, October 29, 2015 5:52 PM
> To: Ren, Qiaowei
> Cc: libvir-list@redhat.com; Feng, Shaohe
> Subject: Re: [libvirt] [PATCH 2/3] Qemu: add CMT support
> 
> On Thu, Oct 29, 2015 at 14:02:29 +0800, Qiaowei Ren wrote:
> > One RFC in
> > https://www.redhat.com/archives/libvir-list/2015-June/msg01509.html
> >
> > CMT (Cache Monitoring Technology) can be used to measure the usage of
> > cache by VM running on the host. This patch will extend the bulk stats
> > API (virDomainListGetStats) to add this field. Applications based on
> > libvirt can use this API to achieve cache usage of VM. Because CMT
> > implementation in Linux kernel is based on perf mechanism, this patch
> > will enable perf event for CMT when VM is created and disable it when
> > VM is destroyed.
> >
> > Signed-off-by: Qiaowei Ren <qiaowei@intel.com>
> > ---
> >  include/libvirt/libvirt-domain.h |  1 +
> >  src/qemu/qemu_domain.h   |  3 ++
> >  src/qemu/qemu_driver.c   | 48 ++
> >  src/qemu/qemu_process.c  | 86
> 
> >  4 files changed, 138 insertions(+)
> >
> > diff --git a/include/libvirt/libvirt-domain.h
> > b/include/libvirt/libvirt-domain.h
> > index e8202cf..fb5e1f4 100644
> > --- a/include/libvirt/libvirt-domain.h
> > +++ b/include/libvirt/libvirt-domain.h
> > @@ -1764,6 +1764,7 @@ typedef enum {
> >  VIR_DOMAIN_STATS_VCPU = (1 << 3), /* return domain virtual CPU info */
> >  VIR_DOMAIN_STATS_INTERFACE = (1 << 4), /* return domain interfaces
> info */
> >  VIR_DOMAIN_STATS_BLOCK = (1 << 5), /* return domain block info */
> > +VIR_DOMAIN_STATS_CACHE = (1 << 6), /* return domain block info */
> 
> This flag is not documented anywhere and the comment is completely wrong.
> 
> >  } virDomainStatsTypes;
> >
> >  typedef enum {
> > diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index
> > 54e1e7b..31bce33 100644
> > --- a/src/qemu/qemu_domain.h
> > +++ b/src/qemu/qemu_domain.h
> > @@ -196,6 +196,9 @@ struct _qemuDomainObjPrivate {
> >
> >  bool hookRun;  /* true if there was a hook run over this domain
> > */
> >
> > +int cmt_fd;  /* perf handler for CMT */
> 
> So you declare cmt_fd as int...
> 
> > +
> > +
> 
> Why two empty lines?
> 
> >  /* Bitmaps below hold data from the auto NUMA feature */
> >  virBitmapPtr autoNodeset;
> >  virBitmapPtr autoCpuset;
> > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index
> > 4cfae03..8c678c9 100644
> > --- a/src/qemu/qemu_driver.c
> > +++ b/src/qemu/qemu_driver.c
> > @@ -19320,6 +19320,53 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr
> > driver,
> >
> >  #undef QEMU_ADD_COUNT_PARAM
> >
> > +static int
> > +qemuDomainGetStatsCache(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
> > +virDomainObjPtr dom,
> > +virDomainStatsRecordPtr record,
> > +int *maxparams,
> > +unsigned int privflags ATTRIBUTE_UNUSED) {
> > +qemuDomainObjPrivatePtr priv = dom->privateData;
> > +FILE *fd;
> > +unsigned long long cache = 0;
> > +int scaling_factor = 0;
> > +
> > +if (priv->cmt_fd <= 0)
> 
> Shouldn't this only check for cmt_fd < 0?
> 
> > +return -1;
> > +
> > +if (read(priv->cmt_fd, , sizeof(uint64_t)) < 0) {
> 
> Shouldn't cache be declared as uint64_t then?
> 
> > +virReportSystemError(errno, "%s",
> > + _("Unable to read cache data"));
> > +return -1;
> > +}
> > +
> > +fd = fopen("/sys/devices/intel_cqm/events/llc_occupancy.scale", "r");
> > +if (!fd) {
> > +virReportSystemError(errno, "%s",
> > + _("Unable to open CMT scale file"));
> > +return -1;
> > +}
> > +if (fscanf(fd, "%d", _factor) != 1) {
> > +virReportSystemError(errno, "%s",
> > + _("Unable to read CMT scale file"));
> > +VIR_FORCE_FCLOSE(fd);
> > +return -1;
> > +}
> > +VIR_FORCE_FCLOSE(fd);
> > +
> > +cache *= scaling_factor;
> > +
> > +if (virTypedParamsA

Re: [libvirt] [PATCH 2/3] Qemu: add CMT support

2015-10-30 Thread Ren, Qiaowei

> -Original Message-
> From: Daniel P. Berrange [mailto:berra...@redhat.com]
> Sent: Thursday, October 29, 2015 10:56 PM
> To: Ren, Qiaowei
> Cc: libvir-list@redhat.com; Feng, Shaohe
> Subject: Re: [PATCH 2/3] Qemu: add CMT support
> 
> On Thu, Oct 29, 2015 at 02:02:29PM +0800, Qiaowei Ren wrote:
> > One RFC in
> > https://www.redhat.com/archives/libvir-list/2015-June/msg01509.html
> >
> > CMT (Cache Monitoring Technology) can be used to measure the usage of
> > cache by VM running on the host. This patch will extend the bulk stats
> > API (virDomainListGetStats) to add this field. Applications based on
> > libvirt can use this API to achieve cache usage of VM. Because CMT
> > implementation in Linux kernel is based on perf mechanism, this patch
> > will enable perf event for CMT when VM is created and disable it when
> > VM is destroyed.
> >
> > Signed-off-by: Qiaowei Ren <qiaowei@intel.com>
> 
> Thanks for re-sending this patchset, it has reminded me of the concerns /
> questions I had around this previously.
> 
> Just ignoring the code for a minute, IIUC the design is
> 
>  - Open a file handle to the kernel perf system for each running VM
>  - Associate that perf event file handle with the QEMU VM PID
>  - Enable recording of the CMT perf event on that file handle
>  - Report the CMT event values in the virDomainGetStats() API
>call when VIR_DOMAIN_STATS_CACHE is requested
> 
> My two primary concerns are
> 
>  1. Do we want to have a perf event FD open for every running
> VM all the time.
>  2. Is the virDomainGetStats() integration the right API approach
> 
> For item 1, my concern is that the CMT event is only ever going to be consumed
> by OpenStack, and even then, only OpenStack installs which have the schedular
> plugin that cares about the CMT event data. It feels undesirable to have this 
> perf
> system enabled for all libvirt VMs, when perhaps < 1 % of libvirt users 
> actually
> want this data. It feels like we need some mechanism to decide when this event
> is enabled
> 
> For item 2, my concern is first when virDomainGetStats is the right API. I 
> think it
> probably *is* the right API, since I can't think of a better way.
> 
> Should we however, be having a very special case VIR_DOMAIN_STATS_CACHE
> group, or should we have something more generic.
> 
> For example, if I run 'perf event' I see
> 
> List of pre-defined events (to be used in -e):
> 
>   branch-instructions OR branches[Hardware event]
>   branch-misses  [Hardware event]
>   bus-cycles [Hardware event]
>   cache-misses   [Hardware event]
>   cache-references   [Hardware event]
>   cpu-cycles OR cycles   [Hardware event]
>   instructions   [Hardware event]
>   ref-cycles [Hardware event]
>   stalled-cycles-frontend OR idle-cycles-frontend[Hardware event]
> 
>   alignment-faults   [Software event]
>   context-switches OR cs [Software event]
>   cpu-clock  [Software event]
>   cpu-migrations OR migrations   [Software event]
>   dummy  [Software event]
>   emulation-faults   [Software event]
>   major-faults   [Software event]
>   minor-faults   [Software event]
>   ...any many many more...
> 
> 
> Does it make sense to extend the virDomainStats API to *only* deal with
> reporting of 1 specific perf event that you care about right now. It feels 
> like it
> might be better if we did something more general purpose.
> 
> eg what if something wants to get 'major-faults' data in future ?
> So we add a VIR_DOMAIN_STATS_MAJOR_FAULT enum item, etc.
> 
> Combining these two concerns, I think we might need 2 things
> 
>  - A new API to turn on/off collection of specific perf events
> 
> This could be something like
> 
>virDomainGetPerfEvents(virDOmainPtr dom,
>   virTypedParameter params);
> 
> This would fill virTypedParameters with one entry for each perf event, using 
> the
> VIR_TYPED_PARAM_BOOLEAN type. A 'true'
> value would indicate that event is enabled for the VM. A corresponding
> 
>virDomainSetPerfEvents(virDOmainPtr dom,
>   virTypedParameter params);
> 
> w

Re: [libvirt] [PATCH 1/3] cpu_map.xml: add cmt feature to x86

2015-10-29 Thread Ren, Qiaowei

> -Original Message-
> From: Peter Krempa [mailto:pkre...@redhat.com]
> Sent: Thursday, October 29, 2015 4:43 PM
> To: Ren, Qiaowei
> Cc: libvir-list@redhat.com; Feng, Shaohe
> Subject: Re: [libvirt] [PATCH 1/3] cpu_map.xml: add cmt feature to x86
> 
> On Thu, Oct 29, 2015 at 14:02:28 +0800, Qiaowei Ren wrote:
> > Some Intel processor families (e.g. the Intel Xeon processor E5 v3
> > family) introduced CMT (Cache Monitoring Technology) to measure the
> > usage of cache by applications running on the platform. This patch add
> > it into x86 part of cpu_map.xml.
> >
> > Signed-off-by: Qiaowei Ren <qiaowei@intel.com>
> > ---
> >  .gnulib | 2 +-
> >  src/cpu/cpu_map.xml | 3 +++
> >  2 files changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/.gnulib b/.gnulib
> > index f39477d..106a386 16
> > --- a/.gnulib
> > +++ b/.gnulib
> > @@ -1 +1 @@
> > -Subproject commit f39477dba778e99392948dd3dd19ec0d46aee932
> > +Subproject commit 106a3866d01f9dd57ab4f10dbeb0d5a8db73a9f7
> 
> This hunk should not be here. Gnulib versions are changed separately.
> Also I doubt that it's necessary.
> 
Yes. This should be not necessary here.

Thanks,
Qiaowei


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


Re: [libvirt] [PATCH 2/3] Qemu: add CMT support

2015-09-25 Thread Ren, Qiaowei

> -Original Message-
> From: libvir-list-boun...@redhat.com [mailto:libvir-list-boun...@redhat.com]
> On Behalf Of Ren, Qiaowei
> Sent: Monday, August 10, 2015 9:06 AM
> To: 'Daniel P. Berrange'
> Cc: 'libvir-list@redhat.com'
> Subject: Re: [libvirt] [PATCH 2/3] Qemu: add CMT support
> 
> 
> > -Original Message-
> > From: Ren, Qiaowei
> > Sent: Tuesday, July 21, 2015 4:00 PM
> > To: Daniel P. Berrange
> > Cc: libvir-list@redhat.com
> > Subject: RE: [libvirt] [PATCH 2/3] Qemu: add CMT support
> >
> > On Jul 20, 2015 22:34, Daniel P. Berrange wrote:
> > > On Mon, Jul 20, 2015 at 01:50:54PM +, Ren, Qiaowei wrote:
> > >>
> > >> Daniel P. Berrange wrote on Jul 20, 2015 17:32:
> > >>> On Sun, Jul 05, 2015 at 07:43:43PM +0800, Qiaowei Ren wrote:
> > >>>> One RFC in
> > >>>> https://www.redhat.com/archives/libvir-list/2015-June/msg01509.ht
> > >>>> m
> > >>>> l
> > >>>>
> > >>>> CMT (Cache Monitoring Technology) can be used to measure the
> > >>>> usage of cache by VM running on the host. This patch will extend
> > >>>> the bulk stats API (virDomainListGetStats) to add this field.
> > >>>> Applications based on libvirt can use this API to achieve cache
> > >>>> usage of VM. Because CMT implementation in Linux kernel is based
> > >>>> on perf mechanism, this patch will enable perf event for CMT when
> > >>>> VM is created and disable it when VM is destroyed.
> > >>>>
> > >>>> Signed-off-by: Qiaowei Ren <qiaowei@intel.com>
> > >>>>
> > >>>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> > >>>> index
> > >>>> 4cfae03..8c678c9 100644 --- a/src/qemu/qemu_driver.c +++
> > >>>> b/src/qemu/qemu_driver.c @@ -19320,6 +19320,53 @@
> > >>>> qemuDomainGetStatsBlock(virQEMUDriverPtr driver,
> > >>>>
> > >>>>  #undef QEMU_ADD_COUNT_PARAM
> > >>>> +static int +qemuDomainGetStatsCache(virQEMUDriverPtr driver
> > >>>> ATTRIBUTE_UNUSED, +virDomainObjPtr dom, +
> > >>>>virDomainStatsRecordPtr record, +
> > >>>>   int *maxparams, +unsigned int privflags
> > >>>> ATTRIBUTE_UNUSED)
> > >>>
> > >>> So this is a method that is used to collect per-domain information
> > >>>
> > >>>> +{
> > >>>> +qemuDomainObjPrivatePtr priv = dom->privateData;
> > >>>> +FILE *fd;
> > >>>> +unsigned long long cache = 0;
> > >>>> +int scaling_factor = 0;
> > >>>> +
> > >>>> +if (priv->cmt_fd <= 0)
> > >>>> +return -1;
> > >>>> +
> > >>>> +if (read(priv->cmt_fd, , sizeof(uint64_t)) < 0) {
> > >>>> +virReportSystemError(errno, "%s",
> > >>>> + _("Unable to read cache data"));
> > >>>> +return -1;
> > >>>> +}
> > >>>> +
> > >>>> +fd = fopen("/sys/devices/intel_cqm/events/llc_occupancy.scale", 
> > >>>> "r");
> > >>>> +if (!fd) {
> > >>>> +virReportSystemError(errno, "%s",
> > >>>> + _("Unable to open CMT scale file"));
> > >>>> +return -1;
> > >>>> +}
> > >>>> +if (fscanf(fd, "%d", _factor) != 1) {
> > >>>> +virReportSystemError(errno, "%s",
> > >>>> + _("Unable to read CMT scale file"));
> > >>>> +VIR_FORCE_FCLOSE(fd);
> > >>>> +return -1;
> > >>>> +}
> > >>>> +VIR_FORCE_FCLOSE(fd);
> > >>>
> > >>> But this data you are reading is global to the entire host.
> > >>>
> > >>
> > >> In fact this data is only for per-domain.  When the perf syscall is
> > >> called to enable perf event for domain, the pid of that domain is
> > >> passed.
> > >
> > > Ah, I see - you rely on the open file

Re: [libvirt] [PATCH 2/3] Qemu: add CMT support

2015-08-09 Thread Ren, Qiaowei

 -Original Message-
 From: Ren, Qiaowei
 Sent: Tuesday, July 21, 2015 4:00 PM
 To: Daniel P. Berrange
 Cc: libvir-list@redhat.com
 Subject: RE: [libvirt] [PATCH 2/3] Qemu: add CMT support
 
 On Jul 20, 2015 22:34, Daniel P. Berrange wrote:
  On Mon, Jul 20, 2015 at 01:50:54PM +, Ren, Qiaowei wrote:
 
  Daniel P. Berrange wrote on Jul 20, 2015 17:32:
  On Sun, Jul 05, 2015 at 07:43:43PM +0800, Qiaowei Ren wrote:
  One RFC in
  https://www.redhat.com/archives/libvir-list/2015-June/msg01509.htm
  l
 
  CMT (Cache Monitoring Technology) can be used to measure the usage
  of cache by VM running on the host. This patch will extend the bulk
  stats API (virDomainListGetStats) to add this field.
  Applications based on libvirt can use this API to achieve cache
  usage of VM. Because CMT implementation in Linux kernel is based on
  perf mechanism, this patch will enable perf event for CMT when VM
  is created and disable it when VM is destroyed.
 
  Signed-off-by: Qiaowei Ren qiaowei@intel.com
 
  diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index
  4cfae03..8c678c9 100644 --- a/src/qemu/qemu_driver.c +++
  b/src/qemu/qemu_driver.c @@ -19320,6 +19320,53 @@
  qemuDomainGetStatsBlock(virQEMUDriverPtr driver,
 
   #undef QEMU_ADD_COUNT_PARAM
  +static int +qemuDomainGetStatsCache(virQEMUDriverPtr driver
  ATTRIBUTE_UNUSED, +virDomainObjPtr dom, +
 virDomainStatsRecordPtr record, +
int *maxparams, +unsigned int privflags
  ATTRIBUTE_UNUSED)
 
  So this is a method that is used to collect per-domain information
 
  +{
  +qemuDomainObjPrivatePtr priv = dom-privateData;
  +FILE *fd;
  +unsigned long long cache = 0;
  +int scaling_factor = 0;
  +
  +if (priv-cmt_fd = 0)
  +return -1;
  +
  +if (read(priv-cmt_fd, cache, sizeof(uint64_t))  0) {
  +virReportSystemError(errno, %s,
  + _(Unable to read cache data));
  +return -1;
  +}
  +
  +fd = fopen(/sys/devices/intel_cqm/events/llc_occupancy.scale, 
  r);
  +if (!fd) {
  +virReportSystemError(errno, %s,
  + _(Unable to open CMT scale file));
  +return -1;
  +}
  +if (fscanf(fd, %d, scaling_factor) != 1) {
  +virReportSystemError(errno, %s,
  + _(Unable to read CMT scale file));
  +VIR_FORCE_FCLOSE(fd);
  +return -1;
  +}
  +VIR_FORCE_FCLOSE(fd);
 
  But this data you are reading is global to the entire host.
 
 
  In fact this data is only for per-domain.  When the perf syscall is
  called to enable perf event for domain, the pid of that domain is
  passed.
 
  Ah, I see - you rely on the open file descriptor to be associated with the 
  VM pid.
 
 
  -5122,6 +5199,15 @@ void qemuProcessStop(virQEMUDriverPtr driver,
   virPortAllocatorRelease(driver-migrationPorts, priv-nbdPort);
   priv-nbdPort = 0;
  +/* Disable CMT */
  +if (priv-cmt_fd  0) {
 
  You can't rely on keeping an open file descriptor for the guest
  because libvirtd may be restarted.
 
 
  Sorry, I don't really get the meaning of this. You mean that when
  libvirtd is restarted, those resource which the domain opened should
  be closed, right?
 
  No, when libvirtd is restarted, the domains must all continuing
  running without loss of state. You open the FD when starting the
  guest, then libvirtd is restarted, now someone wants to query the perf
  data. The perf FD will not be open anymore because libvirtd was
  restarted. At least you'd need to re-open the file descriptor when
  libvirtd starts up again, for any running guest. I'm not really
  convinced we want to keep the perf file descriptors open for all the
  domains for the entire time they are running. Should really only open them
 when we actually want to read the collected data.
 
 
 Got it! Should open/disable them when read the data.
 
 
  +if (ioctl(priv-cmt_fd, PERF_EVENT_IOC_DISABLE)  0) {
  +virReportSystemError(errno, %s,
  + _(Unable to disable perf event for 
  CMT));
  +}
  +VIR_FORCE_CLOSE(priv-cmt_fd);
  +}
  +
   if (priv-agent) {
   qemuAgentClose(priv-agent);
   priv-agent = NULL;
 
 
  Conceptually I think this approach to implementation is flawed.
  While you are turning on/off the perf events for each QEMU process,
  the data collection does not distinguish data from each QEMU process
  - the data reported is host wide. So this really doesn't make much sense
 IMHO.
 
 
  As mentioned above, the data reported is only for domain.
 
  I'm also wondering whether this is really going to be sufficiently
  useful on its own. CPUs have countless other performance counters
  that I would imagine apps/admins will want to read in order to
  analyse QEMU performance, beyond this new CMT feature. The domain
  stats API won't really

Re: [libvirt] [PATCH 2/3] Qemu: add CMT support

2015-07-21 Thread Ren, Qiaowei
On Jul 20, 2015 22:34, Daniel P. Berrange wrote:
 On Mon, Jul 20, 2015 at 01:50:54PM +, Ren, Qiaowei wrote:
 
 Daniel P. Berrange wrote on Jul 20, 2015 17:32:
 On Sun, Jul 05, 2015 at 07:43:43PM +0800, Qiaowei Ren wrote:
 One RFC in
 https://www.redhat.com/archives/libvir-list/2015-June/msg01509.htm
 l
 
 CMT (Cache Monitoring Technology) can be used to measure the usage
 of cache by VM running on the host. This patch will extend the
 bulk stats API (virDomainListGetStats) to add this field.
 Applications based on libvirt can use this API to achieve cache
 usage of VM. Because CMT implementation in Linux kernel is based
 on perf mechanism, this patch will enable perf event for CMT when
 VM is created and disable it when VM is destroyed.
 
 Signed-off-by: Qiaowei Ren qiaowei@intel.com
 
 diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index
 4cfae03..8c678c9 100644 --- a/src/qemu/qemu_driver.c +++
 b/src/qemu/qemu_driver.c @@ -19320,6 +19320,53 @@
 qemuDomainGetStatsBlock(virQEMUDriverPtr driver,
 
  #undef QEMU_ADD_COUNT_PARAM
 +static int +qemuDomainGetStatsCache(virQEMUDriverPtr driver
 ATTRIBUTE_UNUSED, +virDomainObjPtr dom, +
virDomainStatsRecordPtr record, + 
   int *maxparams, +unsigned int privflags
 ATTRIBUTE_UNUSED)
 
 So this is a method that is used to collect per-domain information
 
 +{
 +qemuDomainObjPrivatePtr priv = dom-privateData;
 +FILE *fd;
 +unsigned long long cache = 0;
 +int scaling_factor = 0;
 +
 +if (priv-cmt_fd = 0)
 +return -1;
 +
 +if (read(priv-cmt_fd, cache, sizeof(uint64_t))  0) {
 +virReportSystemError(errno, %s,
 + _(Unable to read cache data));
 +return -1;
 +}
 +
 +fd = fopen(/sys/devices/intel_cqm/events/llc_occupancy.scale, r);
 +if (!fd) {
 +virReportSystemError(errno, %s,
 + _(Unable to open CMT scale file));
 +return -1;
 +}
 +if (fscanf(fd, %d, scaling_factor) != 1) {
 +virReportSystemError(errno, %s,
 + _(Unable to read CMT scale file));
 +VIR_FORCE_FCLOSE(fd);
 +return -1;
 +}
 +VIR_FORCE_FCLOSE(fd);
 
 But this data you are reading is global to the entire host.
 
 
 In fact this data is only for per-domain.  When the perf syscall is
 called to enable perf event for domain, the pid of that domain is
 passed.
 
 Ah, I see - you rely on the open file descriptor to be associated with the VM 
 pid.
 
 
 -5122,6 +5199,15 @@ void qemuProcessStop(virQEMUDriverPtr driver,
  virPortAllocatorRelease(driver-migrationPorts, priv-nbdPort);
  priv-nbdPort = 0;
 +/* Disable CMT */
 +if (priv-cmt_fd  0) {
 
 You can't rely on keeping an open file descriptor for the guest
 because libvirtd may be restarted.
 
 
 Sorry, I don't really get the meaning of this. You mean that when
 libvirtd is restarted, those resource which the domain opened should
 be closed, right?
 
 No, when libvirtd is restarted, the domains must all continuing running 
 without
 loss of state. You open the FD when starting the guest, then libvirtd is 
 restarted,
 now someone wants to query the perf data. The perf FD will not be open
 anymore because libvirtd was restarted. At least you'd need to re-open the 
 file
 descriptor when libvirtd starts up again, for any running guest. I'm not 
 really
 convinced we want to keep the perf file descriptors open for all the domains 
 for
 the entire time they are running. Should really only open them when we 
 actually
 want to read the collected data.
 

Got it! Should open/disable them when read the data.

 
 +if (ioctl(priv-cmt_fd, PERF_EVENT_IOC_DISABLE)  0) {
 +virReportSystemError(errno, %s,
 + _(Unable to disable perf event for 
 CMT));
 +}
 +VIR_FORCE_CLOSE(priv-cmt_fd);
 +}
 +
  if (priv-agent) {
  qemuAgentClose(priv-agent);
  priv-agent = NULL;
 
 
 Conceptually I think this approach to implementation is flawed. While
 you are turning on/off the perf events for each QEMU process, the data
 collection does not distinguish data from each QEMU process - the data
 reported is host wide. So this really doesn't make much sense IMHO.
 
 
 As mentioned above, the data reported is only for domain.
 
 I'm also wondering whether this is really going to be sufficiently
 useful on its own. CPUs have countless other performance counters that
 I would imagine apps/admins will want to read in order to analyse QEMU
 performance, beyond this new CMT feature. The domain stats API won't
 really scale up to dealing with arbitrary perf event counter reporting
 so I'm not much of a fan of just special casing CMT in this way.
 
 IOW, if we want to support host performance analysis in libvirt,
 then we probably want to design an set of APIs specifically

Re: [libvirt] [PATCH 2/3] Qemu: add CMT support

2015-07-20 Thread Ren, Qiaowei

 -Original Message-
 From: Daniel P. Berrange [mailto:berra...@redhat.com]
 Sent: Monday, July 20, 2015 5:32 PM
 To: Ren, Qiaowei
 Cc: libvir-list@redhat.com
 Subject: Re: [libvirt] [PATCH 2/3] Qemu: add CMT support
 
 On Sun, Jul 05, 2015 at 07:43:43PM +0800, Qiaowei Ren wrote:
  One RFC in
  https://www.redhat.com/archives/libvir-list/2015-June/msg01509.html
 
  CMT (Cache Monitoring Technology) can be used to measure the usage of
  cache by VM running on the host. This patch will extend the bulk stats
  API (virDomainListGetStats) to add this field. Applications based on
  libvirt can use this API to achieve cache usage of VM. Because CMT
  implementation in Linux kernel is based on perf mechanism, this patch
  will enable perf event for CMT when VM is created and disable it when
  VM is destroyed.
 
  Signed-off-by: Qiaowei Ren qiaowei@intel.com
 
  diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index
  4cfae03..8c678c9 100644
  --- a/src/qemu/qemu_driver.c
  +++ b/src/qemu/qemu_driver.c
  @@ -19320,6 +19320,53 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr
  driver,
 
   #undef QEMU_ADD_COUNT_PARAM
 
  +static int
  +qemuDomainGetStatsCache(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
  +virDomainObjPtr dom,
  +virDomainStatsRecordPtr record,
  +int *maxparams,
  +unsigned int privflags ATTRIBUTE_UNUSED)
 
 So this is a method that is used to collect per-domain information
 
  +{
  +qemuDomainObjPrivatePtr priv = dom-privateData;
  +FILE *fd;
  +unsigned long long cache = 0;
  +int scaling_factor = 0;
  +
  +if (priv-cmt_fd = 0)
  +return -1;
  +
  +if (read(priv-cmt_fd, cache, sizeof(uint64_t))  0) {
  +virReportSystemError(errno, %s,
  + _(Unable to read cache data));
  +return -1;
  +}
  +
  +fd = fopen(/sys/devices/intel_cqm/events/llc_occupancy.scale, r);
  +if (!fd) {
  +virReportSystemError(errno, %s,
  + _(Unable to open CMT scale file));
  +return -1;
  +}
  +if (fscanf(fd, %d, scaling_factor) != 1) {
  +virReportSystemError(errno, %s,
  + _(Unable to read CMT scale file));
  +VIR_FORCE_FCLOSE(fd);
  +return -1;
  +}
  +VIR_FORCE_FCLOSE(fd);
 
 But this data you are reading is global to the entire host.
 

In fact this data is only for per-domain.  When the perf syscall is called to 
enable perf event for domain, the pid of that domain is passed.

  +
  +cache *= scaling_factor;
  +
  +if (virTypedParamsAddULLong(record-params,
  +record-nparams,
  +maxparams,
  +cache.current,
  +cache)  0)
  +return -1;
  +
  +return 0;
  +}
  +
 
 
  diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index
  ba84182..00b889d 100644
  --- a/src/qemu/qemu_process.c
  +++ b/src/qemu/qemu_process.c
 
  +/*
  + * Enable CMT(Cache Monitoring Technology) to measure the usage of
  + * cache by VM running on the node.
  + *
  + * Because the hypervisor implement CMT support basedon perf
  +mechanism,
  + * we should enable perf event for CMT. The function 'sys_erf_event_open'
  + * is perf syscall wrapper.
  + */
  +#ifdef __linux__
  +static long sys_perf_event_open(struct perf_event_attr *hw_event,
  +pid_t pid, int cpu, int group_fd,
  +unsigned long flags) {
  +return syscall(__NR_perf_event_open, hw_event, pid, cpu,
  +group_fd, flags); } static int
  +qemuCmtEnable(virDomainObjPtr vm) {
  +qemuDomainObjPrivatePtr priv = vm-privateData;
  +struct perf_event_attr cmt_attr;
  +int event_type;
  +FILE *fp;
  +
  +fp = fopen(/sys/devices/intel_cqm/type, r);
  +if (!fp) {
  +virReportSystemError(errno, %s,
  + _(CMT is not available on this host));
  +return -1;
  +}
  +if (fscanf(fp, %d, event_type) != 1) {
  +virReportSystemError(errno, %s,
  + _(Unable to read event type file.));
  +VIR_FORCE_FCLOSE(fp);
  +return -1;
  +}
  +VIR_FORCE_FCLOSE(fp);
  +
  +memset(cmt_attr, 0, sizeof(struct perf_event_attr));
  +cmt_attr.size = sizeof(struct perf_event_attr);
  +cmt_attr.type = event_type;
  +cmt_attr.config = 1;
  +cmt_attr.inherit = 1;
  +cmt_attr.disabled = 1;
  +cmt_attr.enable_on_exec = 0;
  +
  +priv-cmt_fd = sys_perf_event_open(cmt_attr, vm-pid, -1, -1, 0);
  +if (priv-cmt_fd  0) {
  +virReportSystemError(errno,
  + _(Unable to open perf type=%d for pid=%d),
  + event_type, vm-pid);
  +return -1

Re: [libvirt] [PATCH 2/3] Qemu: add CMT support

2015-07-13 Thread Ren, Qiaowei
On Jul 7, 2015 15:51, Ren, Qiaowei wrote:
 
 
 On Jul 6, 2015 14:49, Prerna wrote:
 
 On Sun, Jul 5, 2015 at 5:13 PM, Qiaowei Ren qiaowei@intel.com
 mailto:qiaowei@intel.com  wrote:
 
 
  One RFC in
  https://www.redhat.com/archives/libvir-list/2015-June/msg01509.html
 
  CMT (Cache Monitoring Technology) can be used to measure the
  usage of cache by VM running on the host. This patch will
  extend the bulk stats API (virDomainListGetStats) to add this
  field. Applications based on libvirt can use this API to achieve
  cache usage of VM. Because CMT implementation in Linux kernel
  is based on perf mechanism, this patch will enable perf event
  for CMT when VM is created and disable it when VM is destroyed.
 
 
 
 
 Hi Ren,
 
 One query wrt this implementation. I see you make a perf ioctl to
 gather CMT stats each time the stats API is invoked.
 
 If the CMT stats are exposed by a hardware counter, then this implies
 logging on a per-cpu (or per-socket ???) basis.
 
 This also implies that the value read will vary as the CPU (or socket)
 on which it is being called changes.
 
 
 Now, with this background, if we need real-world stats on a VM, we need
 this perf ioctl executed on all CPUs/ sockets on which the VM ran.
 Also, once done, we will need to aggregate results from each of these
 sources.
 
 
 In this implementation, I am missing this -- there seems no control
 over which physical CPU the libvirt worker thread will run and collect
 the perf data from. Data collected from this implementation might not
 accurately model the system state.
 
 I _think_ libvirt currently has no way of directing a worker thread to
 collect stats from a given CPU -- if we do, I would be happy to learn
 about it :)
 
 
 Prerna, thanks for your reply. I checked the CMT implementation in
 kernel, and noticed that the series implement new -count() of pmu
 driver which can aggregate the results from each cpu if perf type is
 PERF_TYPE_INTEL_CQM . The following is the link for the patch:
 https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?i
 d=bfe1fc d2688f557a6b6a88f59ea7619228728bd7
 
 So I guess that this patch just need to set right perf type and cpu=-1. Do 
 you
 think this is ok?
 

Peter, according to your feedback about my RFC, I updated our implementation 
and submitted this patch series. Could you help review them?

Thanks,
Qiaowei


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

Re: [libvirt] [PATCH 2/3] Qemu: add CMT support

2015-07-12 Thread Ren, Qiaowei
On Jul 13, 2015 09:58, Prerna wrote:
 Hi Ren,
 
 Thank you for clarifying. I really do not have any more comments on the patch.
 
 

Ok. Thanks for your reply. So do you know how to get more feedback from libvirt 
maintainer, or how to contact with them? Now I could not open libvirt.org and 
don't know anything but this maillist. ^-^

Thanks,
Qiaowei

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

Re: [libvirt] [PATCH 2/3] Qemu: add CMT support

2015-07-12 Thread Ren, Qiaowei
On Jul 7, 2015 15:51, Ren, Qiaowei wrote:
 
 
 On Jul 6, 2015 14:49, Prerna wrote:
 
 On Sun, Jul 5, 2015 at 5:13 PM, Qiaowei Ren qiaowei@intel.com
 mailto:qiaowei@intel.com  wrote:
 
 
  One RFC in
  https://www.redhat.com/archives/libvir-list/2015-June/msg01509.html
 
  CMT (Cache Monitoring Technology) can be used to measure the
  usage of cache by VM running on the host. This patch will
  extend the bulk stats API (virDomainListGetStats) to add this
  field. Applications based on libvirt can use this API to achieve
  cache usage of VM. Because CMT implementation in Linux kernel
  is based on perf mechanism, this patch will enable perf event
  for CMT when VM is created and disable it when VM is destroyed.
 
 
 
 
 Hi Ren,
 
 One query wrt this implementation. I see you make a perf ioctl to
 gather CMT stats each time the stats API is invoked.
 
 If the CMT stats are exposed by a hardware counter, then this implies
 logging on a per-cpu (or per-socket ???) basis.
 
 This also implies that the value read will vary as the CPU (or socket)
 on which it is being called changes.
 
 
 Now, with this background, if we need real-world stats on a VM, we need
 this perf ioctl executed on all CPUs/ sockets on which the VM ran.
 Also, once done, we will need to aggregate results from each of these
 sources.
 
 
 In this implementation, I am missing this -- there seems no control
 over which physical CPU the libvirt worker thread will run and collect
 the perf data from. Data collected from this implementation might not
 accurately model the system state.
 
 I _think_ libvirt currently has no way of directing a worker thread to
 collect stats from a given CPU -- if we do, I would be happy to learn
 about it :)
 
 
 Prerna, thanks for your reply. I checked the CMT implementation in
 kernel, and noticed that the series implement new -count() of pmu
 driver which can aggregate the results from each cpu if perf type is
 PERF_TYPE_INTEL_CQM . The following is the link for the patch:
 https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?i
 d=bfe1fc d2688f557a6b6a88f59ea7619228728bd7
 
 So I guess that this patch just need to set right perf type and cpu=-1. Do 
 you
 think this is ok?
 
 Thanks,
 Qiaowei
 


Could anyone help review this patch series? I would be glad to get more 
comments. ^-^

Thanks,
Qiaowei

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

Re: [libvirt] [PATCH 2/3] Qemu: add CMT support

2015-07-09 Thread Ren, Qiaowei
On Jul 7, 2015 15:51, Ren, Qiaowei wrote:
 
 
 On Jul 6, 2015 14:49, Prerna wrote:
 
 On Sun, Jul 5, 2015 at 5:13 PM, Qiaowei Ren qiaowei@intel.com
 mailto:qiaowei@intel.com  wrote:
 
 
  One RFC in
  https://www.redhat.com/archives/libvir-list/2015-June/msg01509.html
 
  CMT (Cache Monitoring Technology) can be used to measure the
  usage of cache by VM running on the host. This patch will
  extend the bulk stats API (virDomainListGetStats) to add this
  field. Applications based on libvirt can use this API to achieve
  cache usage of VM. Because CMT implementation in Linux kernel
  is based on perf mechanism, this patch will enable perf event
  for CMT when VM is created and disable it when VM is destroyed.
 
 
 
 
 Hi Ren,
 
 One query wrt this implementation. I see you make a perf ioctl to
 gather CMT stats each time the stats API is invoked.
 
 If the CMT stats are exposed by a hardware counter, then this implies
 logging on a per-cpu (or per-socket ???) basis.
 
 This also implies that the value read will vary as the CPU (or socket)
 on which it is being called changes.
 
 
 Now, with this background, if we need real-world stats on a VM, we need
 this perf ioctl executed on all CPUs/ sockets on which the VM ran.
 Also, once done, we will need to aggregate results from each of these
 sources.
 
 
 In this implementation, I am missing this -- there seems no control
 over which physical CPU the libvirt worker thread will run and collect
 the perf data from. Data collected from this implementation might not
 accurately model the system state.
 
 I _think_ libvirt currently has no way of directing a worker thread to
 collect stats from a given CPU -- if we do, I would be happy to learn
 about it :)
 
 
 Prerna, thanks for your reply. I checked the CMT implementation in
 kernel, and noticed that the series implement new -count() of pmu
 driver which can aggregate the results from each cpu if perf type is
 PERF_TYPE_INTEL_CQM . The following is the link for the patch:
 https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?i
 d=bfe1fc d2688f557a6b6a88f59ea7619228728bd7
 
 So I guess that this patch just need to set right perf type and cpu=-1. Do 
 you
 think this is ok?
 

Hi Prerna,

Do you have more comments on this patch series? I would be glad to update my 
implementation. ^-^

Qiaowei


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

Re: [libvirt] [PATCH 2/3] Qemu: add CMT support

2015-07-07 Thread Ren, Qiaowei


On Jul 6, 2015 14:49, Prerna wrote:

 On Sun, Jul 5, 2015 at 5:13 PM, Qiaowei Ren qiaowei@intel.com
 mailto:qiaowei@intel.com  wrote:
 
 
   One RFC in
   https://www.redhat.com/archives/libvir-list/2015-June/msg01509.html
 
   CMT (Cache Monitoring Technology) can be used to measure the
   usage of cache by VM running on the host. This patch will
   extend the bulk stats API (virDomainListGetStats) to add this
   field. Applications based on libvirt can use this API to achieve
   cache usage of VM. Because CMT implementation in Linux kernel
   is based on perf mechanism, this patch will enable perf event
   for CMT when VM is created and disable it when VM is destroyed.
 
 
 
 
 Hi Ren,
 
 One query wrt this implementation. I see you make a perf ioctl to gather CMT
 stats each time the stats API is invoked.
 
 If the CMT stats are exposed by a hardware counter, then this implies logging 
 on
 a per-cpu (or per-socket ???) basis.
 
 This also implies that the value read will vary as the CPU (or socket) on 
 which it is
 being called changes.
 
 
 Now, with this background, if we need real-world stats on a VM, we need this
 perf ioctl executed on all CPUs/ sockets on which the VM ran. Also, once done,
 we will need to aggregate results from each of these sources.
 
 
 In this implementation, I am missing this -- there seems no control over
 which physical CPU the libvirt worker thread will run and collect the
 perf data from. Data collected from this implementation might not
 accurately model the system state.
 
 I _think_ libvirt currently has no way of directing a worker thread to 
 collect stats
 from a given CPU -- if we do, I would be happy to learn about it :)
 

Prerna, thanks for your reply. I checked the CMT implementation in kernel, and 
noticed that the series implement new -count() of pmu driver which can  
aggregate the results from each cpu if perf type is PERF_TYPE_INTEL_CQM . The 
following is the link for the patch: 
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=bfe1fcd2688f557a6b6a88f59ea7619228728bd7
 

So I guess that this patch just need to set right perf type and cpu=-1. Do 
you think this is ok?

Thanks,
Qiaowei


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

Re: [libvirt] RFC: add Intel CMT feature support

2015-06-29 Thread Ren, Qiaowei


 -Original Message-
 From: Peter Krempa [mailto:pkre...@redhat.com]
 Sent: Monday, June 29, 2015 5:44 PM
 To: Ren, Qiaowei
 Cc: libvir-list@redhat.com
 Subject: Re: [libvirt] RFC: add Intel CMT feature support
 
 On Sat, Jun 27, 2015 at 03:45:21 +, Ren, Qiaowei wrote:
  Hi All,
 
  Some Intel processor families (e.g. the Intel Xeon processor E5 v3 family)
 introduced some PQos (Platform Qos) features to monitor or control shared
 resource.
 
- CMT (Cache Monitoring Technology): measure the usage of cache by
 applications running on the platform.
- CAT (Cache Allocation Technology): enable an OS or Hypervisor/VMM to
 specify the amount of cache space into which an application can fill.
- MBM (Memory Bandwidth Monitoring): build on the CMT infrastructure to
 allow monitoring of bandwidth from one level of the cache hierarchy to the 
 next
 - in this case focusing on the L3 cache, which is typically backed directly by
 system memory. As a result of this implementation, memory bandwidth can be
 monitored.
 
  For more information, see Intel 64 and IA-32 Architectures Software
 Developer's Manual.
 
  Among these PQos features, currently CMT patches has been merged into
 Linux kernel mainline. So this patch series proposed in this mail will focus 
 on
 CMT feature support in libvirt.
 
  At the library API layer, I plan on adding cache related field into 
  virDomainInfo
 and virNodeInfo:
 
  Add cache member into virNodeInfo to get total size of cache in one node.
  struct virNodeInfo {
  ...
  unsigned int cores;   /* number of cores per socket, total number of
   processors in case of unusual NUMA topology*/
  unsigned int threads; /* number of threads per core, 1 in case of
   unusual numa topology */
  +  unsigned int cache;   /* cache size in bytes */
 
 We don't allow changing/adding members to public structures. You probably will
 be better of by adding this info to the capabilities XML.
 

Ok. So I have to modify capabilities XML and struct virCaps to add total cache 
size related field.

  };
 
  Add cacheOcc member into virDomainInfo to get cache occupancy for one
 VM.
  struct virDomainInfo {
  ...
  unsigned short nrVirtCpu;   /* the number of virtual CPUs for the 
  domain */
  unsigned long long cpuTime; /* the CPU time used in nanoseconds */
  +  unsigned long long cacheOcc; /* the cache occupancy in Bytes */
 
 Neither this structure can be modified.
 
  };
 
  With this change, those applications like OpenStack based on libvirt can 
  collect
 cache usage for metering and monitoring (e.g. any VM using more than X% of
 cache).
 
 You can use the bulk stats API (virDomainListGetStats) to add this field 
 since that
 uses the typed parameter approach which can be extended.
 

Got it. I will try to extend virDomainListGetStats API.

 
  Because CMT implementation in Linux kernel is based on perf mechanism and
 there is no nice and public API library for perf, I have to wrapper system 
 call for
 perf in libvirt to use CMT feature, and enable/disable perf event for CMT when
 VM is created/destroyed.
 
  Any thoughts on this plan before I start submitting code patches? Do you 
  think
 whether I need add several new APIs and XML element for CMT feature (or more
 PQos features)?
 

Thanks,
Qiaowei

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


[libvirt] RFC: add Intel CMT feature support

2015-06-26 Thread Ren, Qiaowei
Hi All,

Some Intel processor families (e.g. the Intel Xeon processor E5 v3 family) 
introduced some PQos (Platform Qos) features to monitor or control shared 
resource.

  - CMT (Cache Monitoring Technology): measure the usage of cache by 
applications running on the platform.
  - CAT (Cache Allocation Technology): enable an OS or Hypervisor/VMM to 
specify the amount of cache space into which an application can fill.
  - MBM (Memory Bandwidth Monitoring): build on the CMT infrastructure to allow 
monitoring of bandwidth from one level of the cache hierarchy to the next - in 
this case focusing on the L3 cache, which is typically backed directly by 
system memory. As a result of this implementation, memory bandwidth can be 
monitored.

For more information, see Intel 64 and IA-32 Architectures Software 
Developer's Manual.

Among these PQos features, currently CMT patches has been merged into Linux 
kernel mainline. So this patch series proposed in this mail will focus on CMT 
feature support in libvirt.

At the library API layer, I plan on adding cache related field into 
virDomainInfo and virNodeInfo:

Add cache member into virNodeInfo to get total size of cache in one node.
struct virNodeInfo {
...
unsigned int cores;   /* number of cores per socket, total number of
 processors in case of unusual NUMA topology*/
unsigned int threads; /* number of threads per core, 1 in case of
 unusual numa topology */
+  unsigned int cache;   /* cache size in bytes */
};

Add cacheOcc member into virDomainInfo to get cache occupancy for one VM.
struct virDomainInfo {
...
unsigned short nrVirtCpu;   /* the number of virtual CPUs for the domain */
unsigned long long cpuTime; /* the CPU time used in nanoseconds */
+  unsigned long long cacheOcc; /* the cache occupancy in Bytes */
};

With this change, those applications like OpenStack based on libvirt can 
collect cache usage for metering and monitoring (e.g. any VM using more than X% 
of cache).

Because CMT implementation in Linux kernel is based on perf mechanism and there 
is no nice and public API library for perf, I have to wrapper system call for 
perf in libvirt to use CMT feature, and enable/disable perf event for CMT when 
VM is created/destroyed.

Any thoughts on this plan before I start submitting code patches? Do you think 
whether I need add several new APIs and XML element for CMT feature (or more 
PQos features)?

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