Discussion:
[libvirt] [PATCHv9 0/2] Introduce x86 Cache Monitoring Technology (CMT
Wang Huaqiang
2018-11-20 13:56:30 UTC
Permalink
These two patches are the remaining part for the CMT enabling series, and most
of the series have been merged.

This series is addressing John's review comments and suggestions, which are
https://www.redhat.com/archives/libvir-list/2018-November/msg00510.html
https://www.redhat.com/archives/libvir-list/2018-November/msg00561.html

Change lists:
Changes in V9:
-. Addressing code review comments form John.
-. Refined the names for new data structure and new functions.
-. Merged qemuDomainGetStatsCpuResMonitorPerTag and
qemuDomainGetStatsCpuResMonitor, and refined new function name
based on the fact that we only support cache monitor now.

Wang Huaqiang (2):
qemu: Report cache occupancy (CMT) with domstats
docs: Updated news.xml for CMT

docs/news.xml | 12 ++++
src/libvirt-domain.c | 12 ++++
src/qemu/qemu_driver.c | 160 ++++++++++++++++++++++++++++++++++++++++++++++++-
tools/virsh.pod | 14 +++++
4 files changed, 197 insertions(+), 1 deletion(-)
--
2.7.4
Wang Huaqiang
2018-11-20 13:56:32 UTC
Permalink
Signed-off-by: Wang Huaqiang <***@intel.com>
---
docs/news.xml | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/docs/news.xml b/docs/news.xml
index 4406aeb..deadb85 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -68,6 +68,18 @@
be viewed via the domain statistics.
</description>
</change>
+ <change>
+ <summary>
+ qemu: Added support for CMT (Cache Monitoring Technology)
+ </summary>
+ <description>
+ Introduced cache monitoring using the <code>monitor</code>
+ element in <code>cachetune</code> for vCPU threads. Added
+ interfaces to get and display the cache utilization statistics
+ through the command 'virsh domstats' via the
+ virConnectGetAllDomainStats API.
+ </description>
+ </change>
</section>
<section title="Improvements">
</section>
--
2.7.4
Wang Huaqiang
2018-11-20 13:56:31 UTC
Permalink
Adding the interface in qemu to report CMT statistic information
through command 'virsh domstats --cpu-total'.

Below is a typical output:

# virsh domstats 1 --cpu-total
Domain: 'ubuntu16.04-base'
...
cpu.cache.monitor.count=2
cpu.cache.monitor.0.name=vcpus_1
cpu.cache.monitor.0.vcpus=1
cpu.cache.monitor.0.bank.count=2
cpu.cache.monitor.0.bank.0.id=0
cpu.cache.monitor.0.bank.0.bytes=4505600
cpu.cache.monitor.0.bank.1.id=1
cpu.cache.monitor.0.bank.1.bytes=5586944
cpu.cache.monitor.1.name=vcpus_4-6
cpu.cache.monitor.1.vcpus=4,5,6
cpu.cache.monitor.1.bank.count=2
cpu.cache.monitor.1.bank.0.id=0
cpu.cache.monitor.1.bank.0.bytes=17571840
cpu.cache.monitor.1.bank.1.id=1
cpu.cache.monitor.1.bank.1.bytes=29106176

Signed-off-by: Wang Huaqiang <***@intel.com>
---
src/libvirt-domain.c | 12 ++++
src/qemu/qemu_driver.c | 160 ++++++++++++++++++++++++++++++++++++++++++++++++-
tools/virsh.pod | 14 +++++
3 files changed, 185 insertions(+), 1 deletion(-)

diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 5b76458..73d602e 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -11415,6 +11415,18 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
* "cpu.user" - user cpu time spent in nanoseconds as unsigned long long.
* "cpu.system" - system cpu time spent in nanoseconds as unsigned long
* long.
+ * "cpu.cache.monitor.count" - the number of cache monitors for this domain
+ * "cpu.cache.monitor.<num>.name" - the name of cache monitor <num>
+ * "cpu.cache.monitor.<num>.vcpus" - vcpu list of cache monitor <num>
+ * "cpu.cache.monitor.<num>.bank.count" - the number of cache banks in
+ * cache monitor <num>
+ * "cpu.cache.monitor.<num>.bank.<index>.id" - host allocated cache id for
+ * bank <index> in cache
+ * monitor <num>
+ * "cpu.cache.monitor.<num>.bank.<index>.bytes" - the number of bytes of
+ * last level cache that the
+ * domain is using on cache
+ * bank <index>
*
* VIR_DOMAIN_STATS_BALLOON:
* Return memory balloon device information.
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7fb9102..d9e216c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19929,6 +19929,158 @@ typedef enum {
#define HAVE_JOB(flags) ((flags) & QEMU_DOMAIN_STATS_HAVE_JOB)


+typedef struct _virQEMUResctrlMonData virQEMUResctrlMonData;
+typedef virQEMUResctrlMonData *virQEMUResctrlMonDataPtr;
+struct _virQEMUResctrlMonData {
+ const char *name;
+ char *vcpus;
+ virResctrlMonitorStatsPtr stats;
+ size_t nstats;
+};
+
+
+static int
+qemuDomainGetResctrlMonData(virDomainObjPtr dom,
+ virQEMUResctrlMonDataPtr resdata)
+{
+ virDomainResctrlDefPtr resctrl = NULL;
+ size_t i = 0;
+ size_t j = 0;
+ size_t k = 0;
+
+ for (i = 0; i < dom->def->nresctrls; i++) {
+ resctrl = dom->def->resctrls[i];
+
+ for (j = 0; j < resctrl->nmonitors; j++) {
+ virDomainResctrlMonDefPtr domresmon = NULL;
+ virResctrlMonitorPtr monitor = NULL;
+
+ domresmon = resctrl->monitors[j];
+ monitor = domresmon->instance;
+
+ if (domresmon->tag != VIR_RESCTRL_MONITOR_TYPE_CACHE)
+ continue;
+
+ /* If virBitmapFormat successfully returns an vcpu string, then
+ * resdata[k].vcpus is assigned with an memory space holding it,
+ * let this newly allocated memory buffer to be freed along with
+ * the free of 'resdata' */
+ if (!(resdata[k].vcpus = virBitmapFormat(domresmon->vcpus)))
+ return -1;
+
+ if (!(resdata[k].name = virResctrlMonitorGetID(monitor))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Could not get monitor ID"));
+ return -1;
+ }
+
+ if (virResctrlMonitorGetCacheOccupancy(monitor,
+ &resdata[k].stats,
+ &resdata[k].nstats) < 0)
+ return -1;
+
+ k++;
+ }
+ }
+
+ return 0;
+}
+
+
+static int
+qemuDomainGetStatsCpuCache(virDomainObjPtr dom,
+ virDomainStatsRecordPtr record,
+ int *maxparams)
+{
+ char param_name[VIR_TYPED_PARAM_FIELD_LENGTH];
+ virQEMUResctrlMonDataPtr resdata = NULL;
+ virDomainResctrlDefPtr resctrl = NULL;
+ virDomainResctrlMonDefPtr domresmon = NULL;
+ unsigned int nresdata = 0;
+ size_t i = 0;
+ size_t j = 0;
+ int ret = -1;
+
+ if (!virDomainObjIsActive(dom))
+ return 0;
+
+ for (i = 0; i < dom->def->nresctrls; i++) {
+ resctrl = dom->def->resctrls[i];
+ for (j = 0; j < resctrl->nmonitors; j++) {
+ domresmon = resctrl->monitors[j];
+ if (domresmon->tag == VIR_RESCTRL_MONITOR_TYPE_CACHE)
+ nresdata++;
+ }
+ }
+
+ if (nresdata == 0)
+ return 0;
+
+ if (VIR_ALLOC_N(resdata, nresdata) < 0)
+ return -1;
+
+ if (qemuDomainGetResctrlMonData(dom, resdata) < 0)
+ goto cleanup;
+
+ snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "cpu.cache.monitor.count");
+ if (virTypedParamsAddUInt(&record->params, &record->nparams,
+ maxparams, param_name, nresdata) < 0)
+ goto cleanup;
+
+ for (i = 0; i < nresdata; i++) {
+ snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "cpu.cache.monitor.%zu.name", i);
+ if (virTypedParamsAddString(&record->params,
+ &record->nparams,
+ maxparams,
+ param_name,
+ resdata[i].name) < 0)
+ goto cleanup;
+
+ snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "cpu.cache.monitor.%zu.vcpus", i);
+ if (virTypedParamsAddString(&record->params, &record->nparams,
+ maxparams, param_name,
+ resdata[i].vcpus) < 0)
+ goto cleanup;
+
+ snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "cpu.cache.monitor.%zu.bank.count", i);
+ if (virTypedParamsAddUInt(&record->params, &record->nparams,
+ maxparams, param_name,
+ resdata[i].nstats) < 0)
+ goto cleanup;
+
+ for (j = 0; j < resdata[i].nstats; j++) {
+ snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "cpu.cache.monitor.%zu.bank.%zu.id", i, j);
+ if (virTypedParamsAddUInt(&record->params, &record->nparams,
+ maxparams, param_name,
+ resdata[i].stats[j].id) < 0)
+ goto cleanup;
+
+ snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "cpu.cache.monitor.%zu.bank.%zu.bytes", i, j);
+ if (virTypedParamsAddUInt(&record->params, &record->nparams,
+ maxparams, param_name,
+ resdata[i].stats[j].val) < 0)
+ goto cleanup;
+ }
+ }
+
+ ret = 0;
+ cleanup:
+ for (i = 0; i < nresdata; i++) {
+ VIR_FREE(resdata[i].vcpus);
+ VIR_FREE(resdata[i].stats);
+ }
+ VIR_FREE(resdata);
+
+ return ret;
+}
+
+
static int
qemuDomainGetStatsCpuCgroup(virDomainObjPtr dom,
virDomainStatsRecordPtr record,
@@ -19976,7 +20128,13 @@ qemuDomainGetStatsCpu(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
int *maxparams,
unsigned int privflags ATTRIBUTE_UNUSED)
{
- return qemuDomainGetStatsCpuCgroup(dom, record, maxparams);
+ if (qemuDomainGetStatsCpuCgroup(dom, record, maxparams) < 0)
+ return -1;
+
+ if (qemuDomainGetStatsCpuCache(dom, record, maxparams) < 0)
+ return -1;
+
+ return 0;
}


diff --git a/tools/virsh.pod b/tools/virsh.pod
index 4876656..86a4996 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1012,6 +1012,20 @@ I<--cpu-total> returns:
"cpu.time" - total cpu time spent for this domain in nanoseconds
"cpu.user" - user cpu time spent in nanoseconds
"cpu.system" - system cpu time spent in nanoseconds
+ "cpu.cache.monitor.count" - the number of cache monitors for this
+ domain
+ "cpu.cache.monitor.<num>.name" - the name of cache monitor <num>
+ "cpu.cache.monitor.<num>.vcpus" - vcpu list of cache monitor <num>
+ "cpu.cache.monitor.<num>.bank.count" - the number of cache banks
+ in cache monitor <num>
+ "cpu.cache.monitor.<num>.bank.<index>.id" - host allocated cache id
+ for bank <index> in
+ cache monitor <num>
+ "cpu.cache.monitor.<num>.bank.<index>.bytes" - the number of bytes
+ of last level cache
+ that the domain is
+ using on cache bank
+ <index>

I<--balloon> returns:
--
2.7.4
John Ferlan
2018-11-23 17:33:45 UTC
Permalink
Post by Wang Huaqiang
Adding the interface in qemu to report CMT statistic information
through command 'virsh domstats --cpu-total'.
# virsh domstats 1 --cpu-total
Domain: 'ubuntu16.04-base'
...
cpu.cache.monitor.count=2
cpu.cache.monitor.0.name=vcpus_1
cpu.cache.monitor.0.vcpus=1
cpu.cache.monitor.0.bank.count=2
cpu.cache.monitor.0.bank.0.id=0
cpu.cache.monitor.0.bank.0.bytes=4505600
cpu.cache.monitor.0.bank.1.id=1
cpu.cache.monitor.0.bank.1.bytes=5586944
cpu.cache.monitor.1.name=vcpus_4-6
cpu.cache.monitor.1.vcpus=4,5,6
cpu.cache.monitor.1.bank.count=2
cpu.cache.monitor.1.bank.0.id=0
cpu.cache.monitor.1.bank.0.bytes=17571840
cpu.cache.monitor.1.bank.1.id=1
cpu.cache.monitor.1.bank.1.bytes=29106176
---
src/libvirt-domain.c | 12 ++++
src/qemu/qemu_driver.c | 160 ++++++++++++++++++++++++++++++++++++++++++++++++-
tools/virsh.pod | 14 +++++
3 files changed, 185 insertions(+), 1 deletion(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 5b76458..73d602e 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -11415,6 +11415,18 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
* "cpu.user" - user cpu time spent in nanoseconds as unsigned long long.
* "cpu.system" - system cpu time spent in nanoseconds as unsigned long
* long.
+ * "cpu.cache.monitor.count" - the number of cache monitors for this domain
+ * "cpu.cache.monitor.<num>.name" - the name of cache monitor <num>
+ * "cpu.cache.monitor.<num>.vcpus" - vcpu list of cache monitor <num>
+ * "cpu.cache.monitor.<num>.bank.count" - the number of cache banks in
+ * cache monitor <num>
+ * "cpu.cache.monitor.<num>.bank.<index>.id" - host allocated cache id for
+ * bank <index> in cache
+ * monitor <num>
+ * "cpu.cache.monitor.<num>.bank.<index>.bytes" - the number of bytes of
+ * last level cache that the
+ * domain is using on cache
+ * bank <index>
*
* Return memory balloon device information.
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7fb9102..d9e216c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19929,6 +19929,158 @@ typedef enum {
#define HAVE_JOB(flags) ((flags) & QEMU_DOMAIN_STATS_HAVE_JOB)
+typedef struct _virQEMUResctrlMonData virQEMUResctrlMonData;
+typedef virQEMUResctrlMonData *virQEMUResctrlMonDataPtr;
+struct _virQEMUResctrlMonData {
+ const char *name;
+ char *vcpus;
+ virResctrlMonitorStatsPtr stats;
+ size_t nstats;
+};
+
+
+static int
+qemuDomainGetResctrlMonData(virDomainObjPtr dom,
+ virQEMUResctrlMonDataPtr resdata)
+{
+ virDomainResctrlDefPtr resctrl = NULL;
+ size_t i = 0;
+ size_t j = 0;
+ size_t k = 0;
+
+ for (i = 0; i < dom->def->nresctrls; i++) {
+ resctrl = dom->def->resctrls[i];
+
+ for (j = 0; j < resctrl->nmonitors; j++) {
+ virDomainResctrlMonDefPtr domresmon = NULL;
+ virResctrlMonitorPtr monitor = NULL;
+
+ domresmon = resctrl->monitors[j];
+ monitor = domresmon->instance;
+
+ if (domresmon->tag != VIR_RESCTRL_MONITOR_TYPE_CACHE)
+ continue;
If you want to make this generic, then you could pass this tag from
qemuDomainGetStatsCpuCache as the rest would seemingly be useful for
VIR_RESCTRL_MONITOR_TYPE_MEMBW eventually, just different results.
Post by Wang Huaqiang
+
+ /* If virBitmapFormat successfully returns an vcpu string, then
+ * resdata[k].vcpus is assigned with an memory space holding it,
+ * let this newly allocated memory buffer to be freed along with
+ * the free of 'resdata' */
+ if (!(resdata[k].vcpus = virBitmapFormat(domresmon->vcpus)))
+ return -1;
+
+ if (!(resdata[k].name = virResctrlMonitorGetID(monitor))) {
Could this ever be NULL? Perhaps we just assign directly and assume
we're good. Alternatively it's a VIR_STRDUP() w/ the corresponding
VIR_FREE(*->name).
Post by Wang Huaqiang
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Could not get monitor ID"));
+ return -1;
+ }
+
+ if (virResctrlMonitorGetCacheOccupancy(monitor,
+ &resdata[k].stats,
+ &resdata[k].nstats) < 0)
+ return -1;
+
+ k++;
+ }
+ }
+
+ return 0;
+}
+
+
+static int
+qemuDomainGetStatsCpuCache(virDomainObjPtr dom,
+ virDomainStatsRecordPtr record,
+ int *maxparams)
+{
+ char param_name[VIR_TYPED_PARAM_FIELD_LENGTH];
+ virQEMUResctrlMonDataPtr resdata = NULL;
+ virDomainResctrlDefPtr resctrl = NULL;
+ virDomainResctrlMonDefPtr domresmon = NULL;
+ unsigned int nresdata = 0;
+ size_t i = 0;
+ size_t j = 0;
+ int ret = -1;
+
+ if (!virDomainObjIsActive(dom))
+ return 0;
+
+ for (i = 0; i < dom->def->nresctrls; i++) {
+ resctrl = dom->def->resctrls[i];
+ for (j = 0; j < resctrl->nmonitors; j++) {
+ domresmon = resctrl->monitors[j];
+ if (domresmon->tag == VIR_RESCTRL_MONITOR_TYPE_CACHE)
+ nresdata++;
+ }
+ }
+
+ if (nresdata == 0)
+ return 0;
+
+ if (VIR_ALLOC_N(resdata, nresdata) < 0)
+ return -1;
Given below - perhaps none of the above really matters if you follow how
virResctrlMonitorGetStats was coded using VIR_APPEND_ELEMENT to append
Post by Wang Huaqiang
+
+ if (qemuDomainGetResctrlMonData(dom, resdata) < 0)
+ goto cleanup;
+
+ snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "cpu.cache.monitor.count");
+ if (virTypedParamsAddUInt(&record->params, &record->nparams,
+ maxparams, param_name, nresdata) < 0)
+ goto cleanup;
+
+ for (i = 0; i < nresdata; i++) {
+ snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "cpu.cache.monitor.%zu.name", i);
+ if (virTypedParamsAddString(&record->params,
+ &record->nparams,
+ maxparams,
+ param_name,
+ resdata[i].name) < 0)
+ goto cleanup;
+
+ snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "cpu.cache.monitor.%zu.vcpus", i);
+ if (virTypedParamsAddString(&record->params, &record->nparams,
+ maxparams, param_name,
+ resdata[i].vcpus) < 0)
+ goto cleanup;
+
+ snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "cpu.cache.monitor.%zu.bank.count", i);
+ if (virTypedParamsAddUInt(&record->params, &record->nparams,
+ maxparams, param_name,
+ resdata[i].nstats) < 0)
+ goto cleanup;
+
+ for (j = 0; j < resdata[i].nstats; j++) {
+ snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "cpu.cache.monitor.%zu.bank.%zu.id", i, j);
+ if (virTypedParamsAddUInt(&record->params, &record->nparams,
+ maxparams, param_name,
+ resdata[i].stats[j].id) < 0)
+ goto cleanup;
+
+ snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "cpu.cache.monitor.%zu.bank.%zu.bytes", i, j);
+ if (virTypedParamsAddUInt(&record->params, &record->nparams,
+ maxparams, param_name,
+ resdata[i].stats[j].val) < 0)
+ goto cleanup;
+ }
+ }
+
+ ret = 0;
+ for (i = 0; i < nresdata; i++) {
+ VIR_FREE(resdata[i].vcpus);
+ VIR_FREE(resdata[i].stats);
+ }
+ VIR_FREE(resdata);
All of this should be replaced by a call to qemuDomainFreeResctrlMonData
which would do the above, but replace the VIR_FREE(resdata[i].stats)
with a call to virResctrlMonitorFreeStats which would essentially:

if (!stats)
return;

for (i = 0; i < nstats; i++)
VIR_FREE(stats[i]);

VIR_FREE(stats);

This being the opposing action of virResctrlMonitorGetStats.


See and test if the attached patch works for you.

John
Post by Wang Huaqiang
+
+ return ret;
+}
+
+
static int
qemuDomainGetStatsCpuCgroup(virDomainObjPtr dom,
virDomainStatsRecordPtr record,
@@ -19976,7 +20128,13 @@ qemuDomainGetStatsCpu(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
int *maxparams,
unsigned int privflags ATTRIBUTE_UNUSED)
{
- return qemuDomainGetStatsCpuCgroup(dom, record, maxparams);
+ if (qemuDomainGetStatsCpuCgroup(dom, record, maxparams) < 0)
+ return -1;
+
+ if (qemuDomainGetStatsCpuCache(dom, record, maxparams) < 0)
+ return -1;
+
+ return 0;
}
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 4876656..86a4996 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
"cpu.time" - total cpu time spent for this domain in nanoseconds
"cpu.user" - user cpu time spent in nanoseconds
"cpu.system" - system cpu time spent in nanoseconds
+ "cpu.cache.monitor.count" - the number of cache monitors for this
+ domain
+ "cpu.cache.monitor.<num>.name" - the name of cache monitor <num>
+ "cpu.cache.monitor.<num>.vcpus" - vcpu list of cache monitor <num>
+ "cpu.cache.monitor.<num>.bank.count" - the number of cache banks
+ in cache monitor <num>
+ "cpu.cache.monitor.<num>.bank.<index>.id" - host allocated cache id
+ for bank <index> in
+ cache monitor <num>
+ "cpu.cache.monitor.<num>.bank.<index>.bytes" - the number of bytes
+ of last level cache
+ that the domain is
+ using on cache bank
+ <index>
Wang, Huaqiang
2018-11-26 17:04:43 UTC
Permalink
Post by John Ferlan
Post by Wang Huaqiang
Adding the interface in qemu to report CMT statistic information
through command 'virsh domstats --cpu-total'.
# virsh domstats 1 --cpu-total
Domain: 'ubuntu16.04-base'
...
cpu.cache.monitor.count=2
cpu.cache.monitor.0.name=vcpus_1
cpu.cache.monitor.0.vcpus=1
cpu.cache.monitor.0.bank.count=2
cpu.cache.monitor.0.bank.0.id=0
cpu.cache.monitor.0.bank.0.bytes=4505600
cpu.cache.monitor.0.bank.1.id=1
cpu.cache.monitor.0.bank.1.bytes=5586944
cpu.cache.monitor.1.name=vcpus_4-6
cpu.cache.monitor.1.vcpus=4,5,6
cpu.cache.monitor.1.bank.count=2
cpu.cache.monitor.1.bank.0.id=0
cpu.cache.monitor.1.bank.0.bytes=17571840
cpu.cache.monitor.1.bank.1.id=1
cpu.cache.monitor.1.bank.1.bytes=29106176
---
src/libvirt-domain.c | 12 ++++
src/qemu/qemu_driver.c | 160 ++++++++++++++++++++++++++++++++++++++++++++++++-
tools/virsh.pod | 14 +++++
3 files changed, 185 insertions(+), 1 deletion(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 5b76458..73d602e 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -11415,6 +11415,18 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
* "cpu.user" - user cpu time spent in nanoseconds as unsigned long long.
* "cpu.system" - system cpu time spent in nanoseconds as unsigned long
* long.
+ * "cpu.cache.monitor.count" - the number of cache monitors for this domain
+ * "cpu.cache.monitor.<num>.name" - the name of cache monitor <num>
+ * "cpu.cache.monitor.<num>.vcpus" - vcpu list of cache monitor <num>
+ * "cpu.cache.monitor.<num>.bank.count" - the number of cache banks in
+ * cache monitor <num>
+ * "cpu.cache.monitor.<num>.bank.<index>.id" - host allocated cache id for
+ * bank <index> in cache
+ * monitor <num>
+ * "cpu.cache.monitor.<num>.bank.<index>.bytes" - the number of bytes of
+ * last level cache that the
+ * domain is using on cache
+ * bank <index>
*
* Return memory balloon device information.
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7fb9102..d9e216c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19929,6 +19929,158 @@ typedef enum {
#define HAVE_JOB(flags) ((flags) & QEMU_DOMAIN_STATS_HAVE_JOB)
+typedef struct _virQEMUResctrlMonData virQEMUResctrlMonData;
+typedef virQEMUResctrlMonData *virQEMUResctrlMonDataPtr;
+struct _virQEMUResctrlMonData {
+ const char *name;
+ char *vcpus;
+ virResctrlMonitorStatsPtr stats;
+ size_t nstats;
+};
+
+
+static int
+qemuDomainGetResctrlMonData(virDomainObjPtr dom,
+ virQEMUResctrlMonDataPtr resdata)
+{
+ virDomainResctrlDefPtr resctrl = NULL;
+ size_t i = 0;
+ size_t j = 0;
+ size_t k = 0;
+
+ for (i = 0; i < dom->def->nresctrls; i++) {
+ resctrl = dom->def->resctrls[i];
+
+ for (j = 0; j < resctrl->nmonitors; j++) {
+ virDomainResctrlMonDefPtr domresmon = NULL;
+ virResctrlMonitorPtr monitor = NULL;
+
+ domresmon = resctrl->monitors[j];
+ monitor = domresmon->instance;
+
+ if (domresmon->tag != VIR_RESCTRL_MONITOR_TYPE_CACHE)
+ continue;
If you want to make this generic, then you could pass this tag from
qemuDomainGetStatsCpuCache as the rest would seemingly be useful for
VIR_RESCTRL_MONITOR_TYPE_MEMBW eventually, just different results.
Good suggestion. Thanks.
Post by John Ferlan
Post by Wang Huaqiang
+
+ /* If virBitmapFormat successfully returns an vcpu string, then
+ * resdata[k].vcpus is assigned with an memory space holding it,
+ * let this newly allocated memory buffer to be freed along with
+ * the free of 'resdata' */
+ if (!(resdata[k].vcpus = virBitmapFormat(domresmon->vcpus)))
+ return -1;
+
+ if (!(resdata[k].name = virResctrlMonitorGetID(monitor))) {
Could this ever be NULL? Perhaps we just assign directly and assume
we're good. Alternatively it's a VIR_STRDUP() w/ the corresponding
VIR_FREE(*->name).
Resctrl monitor ID will not be NULL at this place.

I am using VIR_STRDUP to make a copy of name and ensure it is freed at
the end

of function in next version.
Post by John Ferlan
Post by Wang Huaqiang
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Could not get monitor ID"));
+ return -1;
+ }
+
+ if (virResctrlMonitorGetCacheOccupancy(monitor,
+ &resdata[k].stats,
+ &resdata[k].nstats) < 0)
+ return -1;
+
+ k++;
+ }
+ }
+
+ return 0;
+}
+
+
+static int
+qemuDomainGetStatsCpuCache(virDomainObjPtr dom,
+ virDomainStatsRecordPtr record,
+ int *maxparams)
+{
+ char param_name[VIR_TYPED_PARAM_FIELD_LENGTH];
+ virQEMUResctrlMonDataPtr resdata = NULL;
+ virDomainResctrlDefPtr resctrl = NULL;
+ virDomainResctrlMonDefPtr domresmon = NULL;
+ unsigned int nresdata = 0;
+ size_t i = 0;
+ size_t j = 0;
+ int ret = -1;
+
+ if (!virDomainObjIsActive(dom))
+ return 0;
+
+ for (i = 0; i < dom->def->nresctrls; i++) {
+ resctrl = dom->def->resctrls[i];
+ for (j = 0; j < resctrl->nmonitors; j++) {
+ domresmon = resctrl->monitors[j];
+ if (domresmon->tag == VIR_RESCTRL_MONITOR_TYPE_CACHE)
+ nresdata++;
+ }
+ }
+
+ if (nresdata == 0)
+ return 0;
+
+ if (VIR_ALLOC_N(resdata, nresdata) < 0)
+ return -1;
Given below - perhaps none of the above really matters if you follow how
virResctrlMonitorGetStats was coded using VIR_APPEND_ELEMENT to append
OK. I'll use VIR_APPEND_ELEMENT to allocate and append new element

for @resdata.  That means the lines of 'if (VIR_ALLOC_N(resdata,
nresdata) < 0)'

will be removed and the @nresdata will not be accumulated before calling

qemuDomainGetResctrlMonData.
Post by John Ferlan
Post by Wang Huaqiang
+
+ if (qemuDomainGetResctrlMonData(dom, resdata) < 0)
+ goto cleanup;
+
+ snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "cpu.cache.monitor.count");
+ if (virTypedParamsAddUInt(&record->params, &record->nparams,
+ maxparams, param_name, nresdata) < 0)
+ goto cleanup;
+
+ for (i = 0; i < nresdata; i++) {
+ snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "cpu.cache.monitor.%zu.name", i);
+ if (virTypedParamsAddString(&record->params,
+ &record->nparams,
+ maxparams,
+ param_name,
+ resdata[i].name) < 0)
+ goto cleanup;
+
+ snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "cpu.cache.monitor.%zu.vcpus", i);
+ if (virTypedParamsAddString(&record->params, &record->nparams,
+ maxparams, param_name,
+ resdata[i].vcpus) < 0)
+ goto cleanup;
+
+ snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "cpu.cache.monitor.%zu.bank.count", i);
+ if (virTypedParamsAddUInt(&record->params, &record->nparams,
+ maxparams, param_name,
+ resdata[i].nstats) < 0)
+ goto cleanup;
+
+ for (j = 0; j < resdata[i].nstats; j++) {
+ snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "cpu.cache.monitor.%zu.bank.%zu.id", i, j);
+ if (virTypedParamsAddUInt(&record->params, &record->nparams,
+ maxparams, param_name,
+ resdata[i].stats[j].id) < 0)
+ goto cleanup;
+
+ snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "cpu.cache.monitor.%zu.bank.%zu.bytes", i, j);
+ if (virTypedParamsAddUInt(&record->params, &record->nparams,
+ maxparams, param_name,
+ resdata[i].stats[j].val) < 0)
+ goto cleanup;
+ }
+ }
+
+ ret = 0;
+ for (i = 0; i < nresdata; i++) {
+ VIR_FREE(resdata[i].vcpus);
+ VIR_FREE(resdata[i].stats);
+ }
+ VIR_FREE(resdata);
All of this should be replaced by a call to qemuDomainFreeResctrlMonData
which would do the above, but replace the VIR_FREE(resdata[i].stats)
if (!stats)
return;
for (i = 0; i < nstats; i++)
VIR_FREE(stats[i]);
VIR_FREE(stats);
This being the opposing action of virResctrlMonitorGetStats.
See and test if the attached patch works for you.
The patch wouldn't work. In virResctrlMonitorGetStats the @stats

is patched as a array of virResctrlMonitorStats not
virResctrlMonitorStatsPtr,

so the line VIR_FREE(stats[i]) does not work. Element of *stats is not
pointer

but a  virResctrlMonitorStats structure.

The VIR_APPEND_ELEMENTis:

             if (VIR_APPEND_ELEMENT(*stats, *nstats, *stat) < 0)

             and @stat is

             virResctrlMonitorStatsPtr stat = NULL;

So it appends the whole element to the end of *@stats.


If you think it is more common to append a pointer not the data

structure, I can change it.

Anyway I'll send a patch to append the pointer (virResctrlMonitorStatsPtr).

You can make the decision to use it or not.
Post by John Ferlan
John
Thanks for review.

Huaqiang
Post by John Ferlan
Post by Wang Huaqiang
+
+ return ret;
+}
+
+
static int
qemuDomainGetStatsCpuCgroup(virDomainObjPtr dom,
virDomainStatsRecordPtr record,
@@ -19976,7 +20128,13 @@ qemuDomainGetStatsCpu(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
int *maxparams,
unsigned int privflags ATTRIBUTE_UNUSED)
{
- return qemuDomainGetStatsCpuCgroup(dom, record, maxparams);
+ if (qemuDomainGetStatsCpuCgroup(dom, record, maxparams) < 0)
+ return -1;
+
+ if (qemuDomainGetStatsCpuCache(dom, record, maxparams) < 0)
+ return -1;
+
+ return 0;
}
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 4876656..86a4996 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
"cpu.time" - total cpu time spent for this domain in nanoseconds
"cpu.user" - user cpu time spent in nanoseconds
"cpu.system" - system cpu time spent in nanoseconds
+ "cpu.cache.monitor.count" - the number of cache monitors for this
+ domain
+ "cpu.cache.monitor.<num>.name" - the name of cache monitor <num>
+ "cpu.cache.monitor.<num>.vcpus" - vcpu list of cache monitor <num>
+ "cpu.cache.monitor.<num>.bank.count" - the number of cache banks
+ in cache monitor <num>
+ "cpu.cache.monitor.<num>.bank.<index>.id" - host allocated cache id
+ for bank <index> in
+ cache monitor <num>
+ "cpu.cache.monitor.<num>.bank.<index>.bytes" - the number of bytes
+ of last level cache
+ that the domain is
+ using on cache bank
+ <index>
Loading...