The underlying resctrl monitoring is actually using 64 bit counters,
not the 32bit one. Correct this by using 64bit data type for reading
hardware value.
To keep the interface consistent, the result of CPU last level cache
that occupied by vcpu processors of specific restrl monitor group is
still reported with a truncated 32bit data type. because, in silicon
world, CPU cache size will never exceed 4GB.
Signed-off-by: Wang Huaqiang <huaqiang.wang@intel.com>
---
src/qemu/qemu_driver.c | 15 +++++++++++++--
src/util/virfile.c | 40 ++++++++++++++++++++++++++++++++++++++++
src/util/virfile.h | 2 ++
src/util/virresctrl.c | 6 +++---
src/util/virresctrl.h | 2 +-
5 files changed, 59 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7a7f388767..ad6f3130ae 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -20740,8 +20740,19 @@ qemuDomainGetStatsCpuCache(virQEMUDriverPtr driver,
"cpu.cache.monitor.%zu.bank.%zu.id", i, j) < 0)
goto cleanup;
- if (virTypedParamListAddUInt(params, resdata[i]->stats[j]->vals[0],
- "cpu.cache.monitor.%zu.bank.%zu.bytes", i, j) < 0)
+ /* 'resdata[i]->stats[j]->vals[0]' keeps the value of how many last
+ * level cache in bank j currently occupied by the vcpus listed in
+ * resource monitor i, in bytes. This value is reported through a
+ * 64 bit hardware counter, so it is better to be arranged with
+ * data type in 64 bit width, but considering the fact that
+ * physical cache on a CPU could never be designed to be bigger
+ * than 4G bytes in size, to keep the 'domstats' interface
+ * historically consistent, it is safe to report the value with a
+ * truncated 'UInt' data type here. */
+ if (virTypedParamListAddUInt(params,
+ (unsigned int)resdata[i]->stats[j]->vals[0],
+ "cpu.cache.monitor.%zu.bank.%zu.bytes",
+ i, j) < 0)
goto cleanup;
}
}
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 4fd865dd83..7bf33682c7 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -4196,6 +4196,46 @@ virFileReadValueUint(unsigned int *value, const char *format, ...)
}
+/**
+ * virFileReadValueUllong:
+ * @value: pointer to unsigned long long to be filled in with the value
+ * @format, ...: file to read from
+ *
+ * Read unsigned int from @format and put it into @value.
+ *
+ * Return -2 for non-existing file, -1 on other errors and 0 if everything went
+ * fine.
+ */
+int
+virFileReadValueUllong(unsigned long long *value, const char *format, ...)
+{
+ g_autofree char *str = NULL;
+ g_autofree char *path = NULL;
+ va_list ap;
+
+ va_start(ap, format);
+ path = g_strdup_vprintf(format, ap);
+ va_end(ap);
+
+ if (!virFileExists(path))
+ return -2;
+
+ if (virFileReadAll(path, INT_BUFSIZE_BOUND(*value), &str) < 0)
+ return -1;
+
+ virStringTrimOptionalNewline(str);
+
+ if (virStrToLong_ullp(str, NULL, 10, value) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Invalid unsigned long long value '%s' in file '%s'"),
+ str, path);
+ return -1;
+ }
+
+ return 0;
+}
+
+
/**
* virFileReadValueScaledInt:
* @value: pointer to unsigned long long int to be filled in with the value
diff --git a/src/util/virfile.h b/src/util/virfile.h
index bcae40ee06..756c9a70b9 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -361,6 +361,8 @@ int virFileReadValueInt(int *value, const char *format, ...)
G_GNUC_PRINTF(2, 3);
int virFileReadValueUint(unsigned int *value, const char *format, ...)
G_GNUC_PRINTF(2, 3);
+int virFileReadValueUllong(unsigned long long *value, const char *format, ...)
+ G_GNUC_PRINTF(2, 3);
int virFileReadValueBitmap(virBitmapPtr *value, const char *format, ...)
G_GNUC_PRINTF(2, 3);
int virFileReadValueScaledInt(unsigned long long *value, const char *format, ...)
diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
index b78fded026..684d2ce398 100644
--- a/src/util/virresctrl.c
+++ b/src/util/virresctrl.c
@@ -2678,7 +2678,7 @@ virResctrlMonitorGetStats(virResctrlMonitorPtr monitor,
int rv = -1;
int ret = -1;
size_t i = 0;
- unsigned int val = 0;
+ unsigned long long val = 0;
DIR *dirp = NULL;
char *datapath = NULL;
char *filepath = NULL;
@@ -2734,8 +2734,8 @@ virResctrlMonitorGetStats(virResctrlMonitorPtr monitor,
goto cleanup;
for (i = 0; resources[i]; i++) {
- rv = virFileReadValueUint(&val, "%s/%s/%s", datapath,
- ent->d_name, resources[i]);
+ rv = virFileReadValueUllong(&val, "%s/%s/%s", datapath,
+ ent->d_name, resources[i]);
if (rv == -2) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("File '%s/%s/%s' does not exist."),
diff --git a/src/util/virresctrl.h b/src/util/virresctrl.h
index 3dd7c96348..11f275acf4 100644
--- a/src/util/virresctrl.h
+++ b/src/util/virresctrl.h
@@ -204,7 +204,7 @@ struct _virResctrlMonitorStats {
char **features;
/* @vals store the statistical record values and @val[0] is the value for
* @features[0], @val[1] for@features[1] ... respectively */
- unsigned int *vals;
+ unsigned long long *vals;
/* The length of @vals array */
size_t nvals;
};
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Jan 02, 2020 at 06:45:04PM +0800, Wang Huaqiang wrote: > The underlying resctrl monitoring is actually using 64 bit counters, > not the 32bit one. Correct this by using 64bit data type for reading > hardware value. > > To keep the interface consistent, the result of CPU last level cache > that occupied by vcpu processors of specific restrl monitor group is > still reported with a truncated 32bit data type. because, in silicon > world, CPU cache size will never exceed 4GB. > > Signed-off-by: Wang Huaqiang <huaqiang.wang@intel.com> > --- > src/qemu/qemu_driver.c | 15 +++++++++++++-- > src/util/virfile.c | 40 ++++++++++++++++++++++++++++++++++++++++ > src/util/virfile.h | 2 ++ > src/util/virresctrl.c | 6 +++--- > src/util/virresctrl.h | 2 +- > 5 files changed, 59 insertions(+), 6 deletions(-) Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> > diff --git a/src/util/virfile.h b/src/util/virfile.h > index bcae40ee06..756c9a70b9 100644 > --- a/src/util/virfile.h > +++ b/src/util/virfile.h > @@ -361,6 +361,8 @@ int virFileReadValueInt(int *value, const char *format, ...) > G_GNUC_PRINTF(2, 3); > int virFileReadValueUint(unsigned int *value, const char *format, ...) > G_GNUC_PRINTF(2, 3); > +int virFileReadValueUllong(unsigned long long *value, const char *format, ...) > + G_GNUC_PRINTF(2, 3); Also needs adding to libvirt_private.syms, which I did when pushing. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Hi Daniel, Thanks for review! Br Huaqiang > -----Original Message----- > From: Daniel P. Berrangé <berrange@redhat.com> > Sent: Monday, January 6, 2020 10:16 PM > To: Wang, Huaqiang <huaqiang.wang@intel.com> > Cc: libvir-list@redhat.com > Subject: Re: [PATCHv2 1/2] util, resctrl: using 64bit interface instead of 32bit > for counters > > On Thu, Jan 02, 2020 at 06:45:04PM +0800, Wang Huaqiang wrote: > > The underlying resctrl monitoring is actually using 64 bit counters, > > not the 32bit one. Correct this by using 64bit data type for reading > > hardware value. > > > > To keep the interface consistent, the result of CPU last level cache > > that occupied by vcpu processors of specific restrl monitor group is > > still reported with a truncated 32bit data type. because, in silicon > > world, CPU cache size will never exceed 4GB. > > > > Signed-off-by: Wang Huaqiang <huaqiang.wang@intel.com> > > --- > > src/qemu/qemu_driver.c | 15 +++++++++++++-- > > src/util/virfile.c | 40 ++++++++++++++++++++++++++++++++++++++++ > > src/util/virfile.h | 2 ++ > > src/util/virresctrl.c | 6 +++--- > > src/util/virresctrl.h | 2 +- > > 5 files changed, 59 insertions(+), 6 deletions(-) > > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> > > > diff --git a/src/util/virfile.h b/src/util/virfile.h index > > bcae40ee06..756c9a70b9 100644 > > --- a/src/util/virfile.h > > +++ b/src/util/virfile.h > > @@ -361,6 +361,8 @@ int virFileReadValueInt(int *value, const char > *format, ...) > > G_GNUC_PRINTF(2, 3); > > int virFileReadValueUint(unsigned int *value, const char *format, ...) > > G_GNUC_PRINTF(2, 3); > > +int virFileReadValueUllong(unsigned long long *value, const char > > +*format, ...) G_GNUC_PRINTF(2, 3); > > Also needs adding to libvirt_private.syms, which I did when pushing. > > > Regards, > Daniel > -- > |: https://berrange.com -o- > https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o- https://fstop138.berrange.com :| > |: https://entangle-photo.org -o- > https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2026 Red Hat, Inc.