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

[Ren, Qiaowei]

> -----Original Message-----
> From: Peter Krempa [mailto:pkrempa at redhat.com]
> Sent: Thursday, May 19, 2016 9:46 PM
> To: Ren, Qiaowei <qiaowei.ren at intel.com>
> Cc: libvir-list at 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.ren at 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, &cache) < 0)
> > +    if (virPerfReadEvent(perf, type, &value) < 0)
> >          return -1;
> >
> > -    if (virTypedParamsAddULLong(&record->params,
> > -                                &record->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