[libvirt] conf: Allow > UINT_MAX of cache for NUMA nodes

Lin Yang posted 1 patch 1 year, 5 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20221105002020.71694-1-lin.a.yang@intel.com
src/conf/capabilities.c | 70 +++++++++++++++++++++++++++--------------
src/conf/numa_conf.c    |  2 +-
src/conf/numa_conf.h    |  2 +-
3 files changed, 48 insertions(+), 26 deletions(-)
[libvirt] conf: Allow > UINT_MAX of cache for NUMA nodes
Posted by Lin Yang 1 year, 5 months ago
The high-bandwidth memory (HBM) in cache mode might be greater than
UINT_MAX of cache per NUMA node, so change to unsigned long long.

Signed-off-by: Lin Yang <lin.a.yang@intel.com>
---
 src/conf/capabilities.c | 70 +++++++++++++++++++++++++++--------------
 src/conf/numa_conf.c    |  2 +-
 src/conf/numa_conf.h    |  2 +-
 3 files changed, 48 insertions(+), 26 deletions(-)

diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index e498c77efc..85c06f0d2b 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -1549,10 +1549,10 @@ virCapabilitiesGetNUMAPagesInfo(int node,
 
 
 static int
-virCapabilitiesGetNodeCacheReadFile(const char *prefix,
-                                    const char *dir,
-                                    const char *file,
-                                    unsigned int *value)
+virCapabilitiesGetNodeCacheReadFileUint(const char *prefix,
+                                        const char *dir,
+                                        const char *file,
+                                        unsigned int *value)
 {
     g_autofree char *path = g_build_filename(prefix, dir, file, NULL);
     int rv = virFileReadValueUint(value, "%s", path);
@@ -1570,6 +1570,28 @@ virCapabilitiesGetNodeCacheReadFile(const char *prefix,
 }
 
 
+static int
+virCapabilitiesGetNodeCacheReadFileUllong(const char *prefix,
+                                          const char *dir,
+                                          const char *file,
+                                          unsigned long long *value)
+{
+    g_autofree char *path = g_build_filename(prefix, dir, file, NULL);
+    int rv = virFileReadValueUllong(value, "%s", path);
+
+    if (rv < 0) {
+        if (rv == -2) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("File '%s' does not exist"),
+                           path);
+        }
+        return -1;
+    }
+
+    return 0;
+}
+
+
 static int
 virCapsHostNUMACellCacheComparator(const void *a,
                                    const void *b)
@@ -1612,18 +1634,18 @@ virCapabilitiesGetNodeCache(int node,
             return -1;
         }
 
-        if (virCapabilitiesGetNodeCacheReadFile(path, entry->d_name,
-                                                "size", &cache.size) < 0)
+        if (virCapabilitiesGetNodeCacheReadFileUllong(path, entry->d_name,
+                                                      "size", &cache.size) < 0)
             return -1;
 
         cache.size >>= 10; /* read in bytes but stored in kibibytes */
 
-        if (virCapabilitiesGetNodeCacheReadFile(path, entry->d_name,
-                                                "line_size", &cache.line) < 0)
+        if (virCapabilitiesGetNodeCacheReadFileUint(path, entry->d_name,
+                                                    "line_size", &cache.line) < 0)
             return -1;
 
-        if (virCapabilitiesGetNodeCacheReadFile(path, entry->d_name,
-                                                "indexing", &indexing) < 0)
+        if (virCapabilitiesGetNodeCacheReadFileUint(path, entry->d_name,
+                                                    "indexing", &indexing) < 0)
             return -1;
 
         /* see enum cache_indexing in kernel */
@@ -1638,8 +1660,8 @@ virCapabilitiesGetNodeCache(int node,
                 return -1;
         }
 
-        if (virCapabilitiesGetNodeCacheReadFile(path, entry->d_name,
-                                                "write_policy", &write_policy) < 0)
+        if (virCapabilitiesGetNodeCacheReadFileUint(path, entry->d_name,
+                                                    "write_policy", &write_policy) < 0)
             return -1;
 
         /* see enum cache_write_policy in kernel */
@@ -1793,26 +1815,26 @@ virCapabilitiesHostNUMAInitInterconnectsNode(GArray *interconnects,
     if (!virFileExists(path))
         return 0;
 
-    if (virCapabilitiesGetNodeCacheReadFile(path, "initiators",
-                                            "read_bandwidth",
-                                            &read_bandwidth) < 0)
+    if (virCapabilitiesGetNodeCacheReadFileUint(path, "initiators",
+                                                "read_bandwidth",
+                                                &read_bandwidth) < 0)
         return -1;
-    if (virCapabilitiesGetNodeCacheReadFile(path, "initiators",
-                                            "write_bandwidth",
-                                            &write_bandwidth) < 0)
+    if (virCapabilitiesGetNodeCacheReadFileUint(path, "initiators",
+                                                "write_bandwidth",
+                                                &write_bandwidth) < 0)
         return -1;
 
     /* Bandwidths are read in MiB but stored in KiB */
     read_bandwidth <<= 10;
     write_bandwidth <<= 10;
 
-    if (virCapabilitiesGetNodeCacheReadFile(path, "initiators",
-                                            "read_latency",
-                                            &read_latency) < 0)
+    if (virCapabilitiesGetNodeCacheReadFileUint(path, "initiators",
+                                                "read_latency",
+                                                &read_latency) < 0)
         return -1;
-    if (virCapabilitiesGetNodeCacheReadFile(path, "initiators",
-                                            "write_latency",
-                                            &write_latency) < 0)
+    if (virCapabilitiesGetNodeCacheReadFileUint(path, "initiators",
+                                                "write_latency",
+                                                &write_latency) < 0)
         return -1;
 
     initPath = g_strdup_printf("%s/initiators", path);
diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index 688aa7b409..b55bb3ffcb 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -1765,7 +1765,7 @@ virNumaCacheFormat(virBuffer *buf,
         }
 
         virBufferAsprintf(&childBuf,
-                          "<size value='%u' unit='KiB'/>\n",
+                          "<size value='%llu' unit='KiB'/>\n",
                           cache->size);
 
         if (cache->line) {
diff --git a/src/conf/numa_conf.h b/src/conf/numa_conf.h
index 1d1e816870..bbb928abb2 100644
--- a/src/conf/numa_conf.h
+++ b/src/conf/numa_conf.h
@@ -263,7 +263,7 @@ void virNumaDistanceFormat(virBuffer *buf,
 typedef struct _virNumaCache virNumaCache;
 struct _virNumaCache {
     unsigned int level; /* cache level */
-    unsigned int size;  /* cache size */
+    unsigned long long size;  /* cache size */
     unsigned int line;  /* line size, !!! in bytes !!! */
     virNumaCacheAssociativity associativity; /* cache associativity */
     virNumaCachePolicy policy; /* cache policy */
-- 
2.25.1
Re: [libvirt] conf: Allow > UINT_MAX of cache for NUMA nodes
Posted by Michal Prívozník 1 year, 5 months ago
On 11/5/22 01:20, Lin Yang wrote:
> The high-bandwidth memory (HBM) in cache mode might be greater than
> UINT_MAX of cache per NUMA node, so change to unsigned long long.
> 
> Signed-off-by: Lin Yang <lin.a.yang@intel.com>
> ---
>  src/conf/capabilities.c | 70 +++++++++++++++++++++++++++--------------
>  src/conf/numa_conf.c    |  2 +-
>  src/conf/numa_conf.h    |  2 +-
>  3 files changed, 48 insertions(+), 26 deletions(-)
> 
> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
> index e498c77efc..85c06f0d2b 100644
> --- a/src/conf/capabilities.c
> +++ b/src/conf/capabilities.c
> @@ -1549,10 +1549,10 @@ virCapabilitiesGetNUMAPagesInfo(int node,
>  
>  
>  static int
> -virCapabilitiesGetNodeCacheReadFile(const char *prefix,
> -                                    const char *dir,
> -                                    const char *file,
> -                                    unsigned int *value)
> +virCapabilitiesGetNodeCacheReadFileUint(const char *prefix,
> +                                        const char *dir,
> +                                        const char *file,
> +                                        unsigned int *value)
>  {
>      g_autofree char *path = g_build_filename(prefix, dir, file, NULL);
>      int rv = virFileReadValueUint(value, "%s", path);
> @@ -1570,6 +1570,28 @@ virCapabilitiesGetNodeCacheReadFile(const char *prefix,
>  }
>  
>  
> +static int
> +virCapabilitiesGetNodeCacheReadFileUllong(const char *prefix,
> +                                          const char *dir,
> +                                          const char *file,
> +                                          unsigned long long *value)
> +{
> +    g_autofree char *path = g_build_filename(prefix, dir, file, NULL);
> +    int rv = virFileReadValueUllong(value, "%s", path);
> +
> +    if (rv < 0) {
> +        if (rv == -2) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("File '%s' does not exist"),
> +                           path);
> +        }
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +
>  static int
>  virCapsHostNUMACellCacheComparator(const void *a,
>                                     const void *b)
> @@ -1612,18 +1634,18 @@ virCapabilitiesGetNodeCache(int node,
>              return -1;
>          }
>  
> -        if (virCapabilitiesGetNodeCacheReadFile(path, entry->d_name,
> -                                                "size", &cache.size) < 0)
> +        if (virCapabilitiesGetNodeCacheReadFileUllong(path, entry->d_name,
> +                                                      "size", &cache.size) < 0)

Ah, correct. Even kernel formats this as %llu (from drivers/base/node.c):

CACHE_ATTR(size, "%llu")
CACHE_ATTR(line_size, "%u")
CACHE_ATTR(indexing, "%u")
CACHE_ATTR(write_policy, "%u")

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

and pushed.

Michal