[v4] Expose resource control capabilites on cache bank

[libvirt] [V4 RESEND PATCH] Expose resource control capabilites on cache bank

Posted by taget 6 years, 11 months ago

From: Eli Qiao <liyong.qiao@intel.com>

* This patch amends the cache bank capability as follow:

<cache>
  <bank id='0' level='3' type='unified' size='15360' unit='KiB' cpus='0-5'>
    <control min='768' unit='KiB' scope='both' max_allocation='4'/>
  </bank>
  <bank id='1' level='3' type='unified' size='15360' unit='KiB' cpus='6-11'>
    <control min='768' unit='KiB' scope='both' max_allocation='4'/>
  </bank>
</cache>

For CDP enabled on x86 arch, we will have:
<cache>
  <bank id='0' level='3' type='unified' size='15360' unit='KiB' cpus='0-5'>
    <control min='768' unit='KiB' scope='code' max_allocation='4'/>
    <control min='768' unit='KiB' scope='data' max_allocation='4'/>
  </bank>
...

* Added a new testdata directory `linux-resctrl-cdp` to test CDP enabled
case.

* Along with vircaps2xmltest case updated.

P.S. here the scope is from /sys/fs/resctrl/info/L3{"" "code" "data"}, I
keep it capital upper as I need to use a macro to convert from enum to
string easily.

Signed-off-by: Eli Qiao <liyong.qiao@intel.com>
---
 docs/schemas/capability.rng                        |  20 ++++
 src/conf/capabilities.c                            | 133 ++++++++++++++++++++-
 src/conf/capabilities.h                            |  10 ++
 .../vircaps2xmldata/linux-resctrl-cdp/resctrl/cpus |   1 +
 .../linux-resctrl-cdp/resctrl/info/L3CODE/cbm_mask |   1 +
 .../resctrl/info/L3CODE/min_cbm_bits               |   1 +
 .../resctrl/info/L3CODE/num_closids                |   1 +
 .../linux-resctrl-cdp/resctrl/info/L3DATA/cbm_mask |   1 +
 .../resctrl/info/L3DATA/min_cbm_bits               |   1 +
 .../resctrl/info/L3DATA/num_closids                |   1 +
 .../linux-resctrl-cdp/resctrl/manualres/cpus       |   1 +
 .../linux-resctrl-cdp/resctrl/manualres/schemata   |   2 +
 .../linux-resctrl-cdp/resctrl/manualres/tasks      |   0
 .../linux-resctrl-cdp/resctrl/schemata             |   2 +
 .../linux-resctrl-cdp/resctrl/tasks                |   0
 tests/vircaps2xmldata/linux-resctrl-cdp/system     |   1 +
 .../vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml |  55 +++++++++
 tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml   |   8 +-
 tests/vircaps2xmltest.c                            |   8 ++
 19 files changed, 244 insertions(+), 3 deletions(-)
 create mode 100644 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/cpus
 create mode 100755 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/cbm_mask
 create mode 100755 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/min_cbm_bits
 create mode 100755 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/num_closids
 create mode 100755 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/cbm_mask
 create mode 100755 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/min_cbm_bits
 create mode 100755 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/num_closids
 create mode 100644 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/cpus
 create mode 100644 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/schemata
 create mode 100644 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/tasks
 create mode 100644 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/schemata
 create mode 100644 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/tasks
 create mode 120000 tests/vircaps2xmldata/linux-resctrl-cdp/system
 create mode 100644 tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml

diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng
index 26f0aa2..927fc18 100644
--- a/docs/schemas/capability.rng
+++ b/docs/schemas/capability.rng
@@ -277,6 +277,26 @@
           <attribute name='cpus'>
             <ref name='cpuset'/>
           </attribute>
+          <zeroOrMore>
+            <element name='control'>
+              <attribute name='min'>
+                <ref name='unsignedInt'/>
+              </attribute>
+              <attribute name='unit'>
+                <ref name='unit'/>
+              </attribute>
+              <attribute name='scope'>
+                <choice>
+                  <value>both</value>
+                  <value>code</value>
+                  <value>data</value>
+                </choice>
+              </attribute>
+              <attribute name='max_allocation'>
+                <ref name='unsignedInt'/>
+              </attribute>
+            </element>
+          </zeroOrMore>
         </element>
       </oneOrMore>
     </element>
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index d699b08..c4a1fdf 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -51,6 +51,7 @@
 #define VIR_FROM_THIS VIR_FROM_CAPABILITIES
 
 #define SYSFS_SYSTEM_PATH "/sys/devices/system"
+#define SYSFS_RESCTRL_PATH "/sys/fs/resctrl"
 
 VIR_LOG_INIT("conf.capabilities")
 
@@ -872,6 +873,9 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
                             virCapsHostCacheBankPtr *caches)
 {
     size_t i = 0;
+    size_t j = 0;
+    int indent = virBufferGetIndent(buf, false);
+    virBuffer controlBuf = VIR_BUFFER_INITIALIZER;
 
     if (!ncaches)
         return 0;
@@ -893,13 +897,35 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
          */
         virBufferAsprintf(buf,
                           "<bank id='%u' level='%u' type='%s' "
-                          "size='%llu' unit='%s' cpus='%s'/>\n",
+                          "size='%llu' unit='%s' cpus='%s'",
                           bank->id, bank->level,
                           virCacheTypeToString(bank->type),
                           bank->size >> (kilos * 10),
                           kilos ? "KiB" : "B",
                           cpus_str);
 
+        virBufferAdjustIndent(&controlBuf, indent + 4);
+        for (j = 0; j < bank->ncontrols; j++) {
+            bool min_kilos = !(bank->controls[j]->min % 1024);
+            virBufferAsprintf(&controlBuf,
+                              "<control min='%llu' unit='%s' "
+                              "scope='%s' max_allocation='%u'/>\n",
+                              bank->controls[j]->min >> (min_kilos * 10),
+                              min_kilos ? "KiB" : "B",
+                              virCacheTypeToString(bank->controls[j]->scope),
+                              bank->controls[j]->max_allocation);
+        }
+
+        if (virBufferUse(&controlBuf)) {
+            virBufferAddLit(buf, ">\n");
+            virBufferAddBuffer(buf, &controlBuf);
+            virBufferAddLit(buf, "</bank>\n");
+
+        } else {
+            virBufferAddLit(buf, "/>\n");
+        }
+
+        virBufferFreeAndReset(&controlBuf);
         VIR_FREE(cpus_str);
     }
 
@@ -1519,13 +1545,102 @@ virCapsHostCacheBankEquals(virCapsHostCacheBankPtr a,
 void
 virCapsHostCacheBankFree(virCapsHostCacheBankPtr ptr)
 {
+    size_t i;
+
     if (!ptr)
         return;
 
     virBitmapFree(ptr->cpus);
+    for (i = 0; i < ptr->ncontrols; i++)
+        VIR_FREE(ptr->controls[i]);
+    VIR_FREE(ptr->controls);
     VIR_FREE(ptr);
 }
 
+/* test which TYPE of cache control supported
+ * -1: don't support
+ *  0: cat
+ *  1: cdp
+ */
+static int
+virCapabilitiesGetCacheControlType(virCapsHostCacheBankPtr bank)
+{
+    int ret = -1;
+    char *path = NULL;
+    if (virAsprintf(&path,
+                    SYSFS_RESCTRL_PATH "/info/L%u",
+                    bank->level) < 0)
+        return -1;
+
+    if (virFileExists(path)) {
+        ret = 0;
+    } else {
+        VIR_FREE(path);
+        /* for CDP enabled case, CODE and DATA will show together */
+        if (virAsprintf(&path,
+                        SYSFS_RESCTRL_PATH "/info/L%uCODE",
+                        bank->level) < 0)
+            return -1;
+        if (virFileExists(path))
+            ret = 1;
+    }
+
+    VIR_FREE(path);
+    return ret;
+}
+
+static int
+virCapabilitiesGetCacheControl(virCapsHostCacheBankPtr bank,
+                               virCacheType scope)
+{
+    int ret = -1;
+    char *path = NULL;
+    char *cbm_mask = NULL;
+    char *type_upper = NULL;
+    virCapsHostCacheControlPtr control;
+
+    if (VIR_ALLOC(control) < 0)
+        goto cleanup;
+
+    if ((scope > VIR_CACHE_TYPE_BOTH)
+            && (virStringToUpper(&type_upper, virCacheTypeToString(scope)) < 0))
+        goto cleanup;
+
+    if (virFileReadValueUint(&control->max_allocation,
+                             SYSFS_RESCTRL_PATH "/info/L%u%s/num_closids",
+                             bank->level,
+                             type_upper ? type_upper : "") < 0)
+        goto cleanup;
+
+    if (virFileReadValueString(&cbm_mask,
+                               SYSFS_RESCTRL_PATH
+                               "/info/L%u%s/cbm_mask",
+                               bank->level,
+                               type_upper ? type_upper: "") < 0)
+        goto cleanup;
+
+    virStringTrimOptionalNewline(cbm_mask);
+
+    /* cbm_mask: cache bit mask, it's in hex, eg: fffff */
+    control->min = bank->size / (strlen(cbm_mask) * 4);
+
+    control->scope = scope;
+
+    if (VIR_APPEND_ELEMENT(bank->controls,
+                           bank->ncontrols,
+                           control) < 0)
+        goto cleanup;
+
+    ret = 0;
+
+ cleanup:
+    VIR_FREE(path);
+    VIR_FREE(cbm_mask);
+    VIR_FREE(type_upper);
+    VIR_FREE(control);
+    return ret;
+}
+
 int
 virCapabilitiesInitCaches(virCapsPtr caps)
 {
@@ -1534,6 +1649,7 @@ virCapabilitiesInitCaches(virCapsPtr caps)
     ssize_t pos = -1;
     DIR *dirp = NULL;
     int ret = -1;
+    int typeret;
     char *path = NULL;
     char *type = NULL;
     struct dirent *ent = NULL;
@@ -1607,12 +1723,27 @@ virCapabilitiesInitCaches(virCapsPtr caps)
                                        SYSFS_SYSTEM_PATH, pos, ent->d_name) < 0)
                 goto cleanup;
 
+            typeret = virCapabilitiesGetCacheControlType(bank);
+
+            if (typeret == 0) {
+                if (virCapabilitiesGetCacheControl(bank,
+                                                   VIR_CACHE_TYPE_BOTH) < 0)
+                    goto cleanup;
+            } else if (typeret == 1) {
+                if ((virCapabilitiesGetCacheControl(bank,
+                                                    VIR_CACHE_TYPE_CODE) < 0) ||
+                        (virCapabilitiesGetCacheControl(bank,
+                                                        VIR_CACHE_TYPE_DATA) < 0))
+                    goto cleanup;
+            }
+
             kernel_type = virCacheKernelTypeFromString(type);
             if (kernel_type < 0) {
                 virReportError(VIR_ERR_INTERNAL_ERROR,
                                _("Unknown cache type '%s'"), type);
                 goto cleanup;
             }
+
             bank->type = kernel_type;
             VIR_FREE(type);
 
diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h
index a8cccf7..ee87d59 100644
--- a/src/conf/capabilities.h
+++ b/src/conf/capabilities.h
@@ -148,6 +148,14 @@ typedef enum {
 
 VIR_ENUM_DECL(virCache);
 
+typedef struct _virCapsHostCacheControl virCapsHostCacheControl;
+typedef virCapsHostCacheControl *virCapsHostCacheControlPtr;
+struct _virCapsHostCacheControl {
+    unsigned long long min; /* minimum cache control size in B */
+    virCacheType scope;  /* data, code or both */
+    unsigned int max_allocation; /* max number of supported allocations */
+};
+
 typedef struct _virCapsHostCacheBank virCapsHostCacheBank;
 typedef virCapsHostCacheBank *virCapsHostCacheBankPtr;
 struct _virCapsHostCacheBank {
@@ -156,6 +164,8 @@ struct _virCapsHostCacheBank {
     unsigned long long size; /* B */
     virCacheType type;  /* Data, Instruction or Unified */
     virBitmapPtr cpus;  /* All CPUs that share this bank */
+    size_t ncontrols;
+    virCapsHostCacheControlPtr *controls;
 };
 
 typedef struct _virCapsHost virCapsHost;
diff --git a/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/cpus b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/cpus
new file mode 100644
index 0000000..b3a79aa
--- /dev/null
+++ b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/cpus
@@ -0,0 +1 @@
+ffffff,ffffffff,ffffffff
diff --git a/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/cbm_mask b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/cbm_mask
new file mode 100755
index 0000000..78031da
--- /dev/null
+++ b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/cbm_mask
@@ -0,0 +1 @@
+fffff
diff --git a/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/min_cbm_bits b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/min_cbm_bits
new file mode 100755
index 0000000..d00491f
--- /dev/null
+++ b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/min_cbm_bits
@@ -0,0 +1 @@
+1
diff --git a/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/num_closids b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/num_closids
new file mode 100755
index 0000000..45a4fb7
--- /dev/null
+++ b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/num_closids
@@ -0,0 +1 @@
+8
diff --git a/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/cbm_mask b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/cbm_mask
new file mode 100755
index 0000000..78031da
--- /dev/null
+++ b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/cbm_mask
@@ -0,0 +1 @@
+fffff
diff --git a/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/min_cbm_bits b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/min_cbm_bits
new file mode 100755
index 0000000..d00491f
--- /dev/null
+++ b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/min_cbm_bits
@@ -0,0 +1 @@
+1
diff --git a/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/num_closids b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/num_closids
new file mode 100755
index 0000000..45a4fb7
--- /dev/null
+++ b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/num_closids
@@ -0,0 +1 @@
+8
diff --git a/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/cpus b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/cpus
new file mode 100644
index 0000000..ede4cc2
--- /dev/null
+++ b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/cpus
@@ -0,0 +1 @@
+000000,00000000,00000000
diff --git a/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/schemata b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/schemata
new file mode 100644
index 0000000..a0ef381
--- /dev/null
+++ b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/schemata
@@ -0,0 +1,2 @@
+L3DATA:0=c0000;1=c0000
+L3CODE:0=30000;1=30000
diff --git a/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/tasks b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/tasks
new file mode 100644
index 0000000..e69de29
diff --git a/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/schemata b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/schemata
new file mode 100644
index 0000000..89dc76b
--- /dev/null
+++ b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/schemata
@@ -0,0 +1,2 @@
+L3DATA:0=fffff;1=fffff
+L3CODE:0=fffff;1=fffff
diff --git a/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/tasks b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/tasks
new file mode 100644
index 0000000..e69de29
diff --git a/tests/vircaps2xmldata/linux-resctrl-cdp/system b/tests/vircaps2xmldata/linux-resctrl-cdp/system
new file mode 120000
index 0000000..2f3a1d9
--- /dev/null
+++ b/tests/vircaps2xmldata/linux-resctrl-cdp/system
@@ -0,0 +1 @@
+../linux-resctrl/system/
\ No newline at end of file
diff --git a/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml b/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml
new file mode 100644
index 0000000..c9f460d
--- /dev/null
+++ b/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml
@@ -0,0 +1,55 @@
+<capabilities>
+
+  <host>
+    <cpu>
+      <arch>x86_64</arch>
+    </cpu>
+    <power_management/>
+    <migration_features>
+      <live/>
+    </migration_features>
+    <topology>
+      <cells num='2'>
+        <cell id='0'>
+          <memory unit='KiB'>1048576</memory>
+          <pages unit='KiB' size='4'>2048</pages>
+          <pages unit='KiB' size='2048'>4096</pages>
+          <pages unit='KiB' size='1048576'>6144</pages>
+          <cpus num='6'>
+            <cpu id='0' socket_id='0' core_id='0' siblings='0'/>
+            <cpu id='1' socket_id='0' core_id='1' siblings='1'/>
+            <cpu id='2' socket_id='0' core_id='2' siblings='2'/>
+            <cpu id='3' socket_id='0' core_id='3' siblings='3'/>
+            <cpu id='4' socket_id='0' core_id='4' siblings='4'/>
+            <cpu id='5' socket_id='0' core_id='5' siblings='5'/>
+          </cpus>
+        </cell>
+        <cell id='1'>
+          <memory unit='KiB'>2097152</memory>
+          <pages unit='KiB' size='4'>4096</pages>
+          <pages unit='KiB' size='2048'>6144</pages>
+          <pages unit='KiB' size='1048576'>8192</pages>
+          <cpus num='6'>
+            <cpu id='6' socket_id='1' core_id='0' siblings='6'/>
+            <cpu id='7' socket_id='1' core_id='1' siblings='7'/>
+            <cpu id='8' socket_id='1' core_id='2' siblings='8'/>
+            <cpu id='9' socket_id='1' core_id='3' siblings='9'/>
+            <cpu id='10' socket_id='1' core_id='4' siblings='10'/>
+            <cpu id='11' socket_id='1' core_id='5' siblings='11'/>
+          </cpus>
+        </cell>
+      </cells>
+    </topology>
+    <cache>
+      <bank id='0' level='3' type='both' size='15360' unit='KiB' cpus='0-5'>
+        <control min='768' unit='KiB' scope='code' max_allocation='8'/>
+        <control min='768' unit='KiB' scope='data' max_allocation='8'/>
+      </bank>
+      <bank id='1' level='3' type='both' size='15360' unit='KiB' cpus='6-11'>
+        <control min='768' unit='KiB' scope='code' max_allocation='8'/>
+        <control min='768' unit='KiB' scope='data' max_allocation='8'/>
+      </bank>
+    </cache>
+  </host>
+
+</capabilities>
diff --git a/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml b/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml
index 6677791..04a5eb8 100644
--- a/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml
+++ b/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml
@@ -41,8 +41,12 @@
       </cells>
     </topology>
     <cache>
-      <bank id='0' level='3' type='both' size='15360' unit='KiB' cpus='0-5'/>
-      <bank id='1' level='3' type='both' size='15360' unit='KiB' cpus='6-11'/>
+      <bank id='0' level='3' type='both' size='15360' unit='KiB' cpus='0-5'>
+        <control min='768' unit='KiB' scope='both' max_allocation='4'/>
+      </bank>
+      <bank id='1' level='3' type='both' size='15360' unit='KiB' cpus='6-11'>
+        <control min='768' unit='KiB' scope='both' max_allocation='4'/>
+      </bank>
     </cache>
   </host>
 
diff --git a/tests/vircaps2xmltest.c b/tests/vircaps2xmltest.c
index 137598e..ae1cd52 100644
--- a/tests/vircaps2xmltest.c
+++ b/tests/vircaps2xmltest.c
@@ -47,6 +47,7 @@ test_virCapabilities(const void *opaque)
     char *capsXML = NULL;
     char *path = NULL;
     char *dir = NULL;
+    char *resctrl = NULL;
     int ret = -1;
 
     /*
@@ -58,7 +59,12 @@ test_virCapabilities(const void *opaque)
                     data->resctrl ? "/system" : "") < 0)
         goto cleanup;
 
+    if (virAsprintf(&resctrl, "%s/vircaps2xmldata/linux-%s/resctrl",
+                    abs_srcdir, data->filename) < 0)
+        goto cleanup;
+
     virFileWrapperAddPrefix("/sys/devices/system", dir);
+    virFileWrapperAddPrefix("/sys/fs/resctrl", resctrl);
     caps = virCapabilitiesNew(data->arch, data->offlineMigrate, data->liveMigrate);
 
     if (!caps)
@@ -84,6 +90,7 @@ test_virCapabilities(const void *opaque)
 
  cleanup:
     VIR_FREE(dir);
+    VIR_FREE(resctrl);
     VIR_FREE(path);
     VIR_FREE(capsXML);
     virObjectUnref(caps);
@@ -112,6 +119,7 @@ mymain(void)
     DO_TEST("caches", VIR_ARCH_X86_64);
 
     DO_TEST_FULL("resctrl", VIR_ARCH_X86_64, true, true, true);
+    DO_TEST_FULL("resctrl-cdp", VIR_ARCH_X86_64, true, true, true);
 
     return ret;
 }
-- 
1.9.1

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

Re: [libvirt] [V4 RESEND PATCH] Expose resource control capabilites on cache bank

Posted by Eli Qiao 6 years, 10 months ago

ping 


On Wednesday, 17 May 2017 at 5:08 PM, taget wrote:

> From: Eli Qiao <liyong.qiao@intel.com (mailto:liyong.qiao@intel.com)>
> 
> * This patch amends the cache bank capability as follow:
> 
> <cache>
> <bank id='0' level='3' type='unified' size='15360' unit='KiB' cpus='0-5'>
> <control min='768' unit='KiB' scope='both' max_allocation='4'/>
> </bank>
> <bank id='1' level='3' type='unified' size='15360' unit='KiB' cpus='6-11'>
> <control min='768' unit='KiB' scope='both' max_allocation='4'/>
> </bank>
> </cache>
> 
> For CDP enabled on x86 arch, we will have:
> <cache>
> <bank id='0' level='3' type='unified' size='15360' unit='KiB' cpus='0-5'>
> <control min='768' unit='KiB' scope='code' max_allocation='4'/>
> <control min='768' unit='KiB' scope='data' max_allocation='4'/>
> </bank>
> ...
> 
> * Added a new testdata directory `linux-resctrl-cdp` to test CDP enabled
> case.
> 
> * Along with vircaps2xmltest case updated.
> 
> P.S. here the scope is from /sys/fs/resctrl/info/L3{"" "code" "data"}, I
> keep it capital upper as I need to use a macro to convert from enum to
> string easily.
> 
> Signed-off-by: Eli Qiao <liyong.qiao@intel.com (mailto:liyong.qiao@intel.com)>
> ---
> docs/schemas/capability.rng | 20 ++++
> src/conf/capabilities.c | 133 ++++++++++++++++++++-
> src/conf/capabilities.h | 10 ++
> .../vircaps2xmldata/linux-resctrl-cdp/resctrl/cpus | 1 +
> .../linux-resctrl-cdp/resctrl/info/L3CODE/cbm_mask | 1 +
> .../resctrl/info/L3CODE/min_cbm_bits | 1 +
> .../resctrl/info/L3CODE/num_closids | 1 +
> .../linux-resctrl-cdp/resctrl/info/L3DATA/cbm_mask | 1 +
> .../resctrl/info/L3DATA/min_cbm_bits | 1 +
> .../resctrl/info/L3DATA/num_closids | 1 +
> .../linux-resctrl-cdp/resctrl/manualres/cpus | 1 +
> .../linux-resctrl-cdp/resctrl/manualres/schemata | 2 +
> .../linux-resctrl-cdp/resctrl/manualres/tasks | 0
> .../linux-resctrl-cdp/resctrl/schemata | 2 +
> .../linux-resctrl-cdp/resctrl/tasks | 0
> tests/vircaps2xmldata/linux-resctrl-cdp/system | 1 +
> .../vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml | 55 +++++++++
> tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml | 8 +-
> tests/vircaps2xmltest.c | 8 ++
> 19 files changed, 244 insertions(+), 3 deletions(-)
> create mode 100644 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/cpus
> create mode 100755 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/cbm_mask
> create mode 100755 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/min_cbm_bits
> create mode 100755 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/num_closids
> create mode 100755 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/cbm_mask
> create mode 100755 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/min_cbm_bits
> create mode 100755 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/num_closids
> create mode 100644 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/cpus
> create mode 100644 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/schemata
> create mode 100644 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/tasks
> create mode 100644 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/schemata
> create mode 100644 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/tasks
> create mode 120000 tests/vircaps2xmldata/linux-resctrl-cdp/system
> create mode 100644 tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml
> 
> diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng
> index 26f0aa2..927fc18 100644
> --- a/docs/schemas/capability.rng
> +++ b/docs/schemas/capability.rng
> @@ -277,6 +277,26 @@
> <attribute name='cpus'>
> <ref name='cpuset'/>
> </attribute>
> + <zeroOrMore>
> + <element name='control'>
> + <attribute name='min'>
> + <ref name='unsignedInt'/>
> + </attribute>
> + <attribute name='unit'>
> + <ref name='unit'/>
> + </attribute>
> + <attribute name='scope'>
> + <choice>
> + <value>both</value>
> + <value>code</value>
> + <value>data</value>
> + </choice>
> + </attribute>
> + <attribute name='max_allocation'>
> + <ref name='unsignedInt'/>
> + </attribute>
> + </element>
> + </zeroOrMore>
> </element>
> </oneOrMore>
> </element>
> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
> index d699b08..c4a1fdf 100644
> --- a/src/conf/capabilities.c
> +++ b/src/conf/capabilities.c
> @@ -51,6 +51,7 @@
> #define VIR_FROM_THIS VIR_FROM_CAPABILITIES
> 
> #define SYSFS_SYSTEM_PATH "/sys/devices/system"
> +#define SYSFS_RESCTRL_PATH "/sys/fs/resctrl"
> 
> VIR_LOG_INIT("conf.capabilities")
> 
> @@ -872,6 +873,9 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
> virCapsHostCacheBankPtr *caches)
> {
> size_t i = 0;
> + size_t j = 0;
> + int indent = virBufferGetIndent(buf, false);
> + virBuffer controlBuf = VIR_BUFFER_INITIALIZER;
> 
> if (!ncaches)
> return 0;
> @@ -893,13 +897,35 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
> */
> virBufferAsprintf(buf,
> "<bank id='%u' level='%u' type='%s' "
> - "size='%llu' unit='%s' cpus='%s'/>\n",
> + "size='%llu' unit='%s' cpus='%s'",
> bank->id, bank->level,
> virCacheTypeToString(bank->type),
> bank->size >> (kilos * 10),
> kilos ? "KiB" : "B",
> cpus_str);
> 
> + virBufferAdjustIndent(&controlBuf, indent + 4);
> + for (j = 0; j < bank->ncontrols; j++) {
> + bool min_kilos = !(bank->controls[j]->min % 1024);
> + virBufferAsprintf(&controlBuf,
> + "<control min='%llu' unit='%s' "
> + "scope='%s' max_allocation='%u'/>\n",
> + bank->controls[j]->min >> (min_kilos * 10),
> + min_kilos ? "KiB" : "B",
> + virCacheTypeToString(bank->controls[j]->scope),
> + bank->controls[j]->max_allocation);
> + }
> +
> + if (virBufferUse(&controlBuf)) {
> + virBufferAddLit(buf, ">\n");
> + virBufferAddBuffer(buf, &controlBuf);
> + virBufferAddLit(buf, "</bank>\n");
> +
> + } else {
> + virBufferAddLit(buf, "/>\n");
> + }
> +
> + virBufferFreeAndReset(&controlBuf);
> VIR_FREE(cpus_str);
> }
> 
> @@ -1519,13 +1545,102 @@ virCapsHostCacheBankEquals(virCapsHostCacheBankPtr a,
> void
> virCapsHostCacheBankFree(virCapsHostCacheBankPtr ptr)
> {
> + size_t i;
> +
> if (!ptr)
> return;
> 
> virBitmapFree(ptr->cpus);
> + for (i = 0; i < ptr->ncontrols; i++)
> + VIR_FREE(ptr->controls[i]);
> + VIR_FREE(ptr->controls);
> VIR_FREE(ptr);
> }
> 
> +/* test which TYPE of cache control supported
> + * -1: don't support
> + * 0: cat
> + * 1: cdp
> + */
> +static int
> +virCapabilitiesGetCacheControlType(virCapsHostCacheBankPtr bank)
> +{
> + int ret = -1;
> + char *path = NULL;
> + if (virAsprintf(&path,
> + SYSFS_RESCTRL_PATH "/info/L%u",
> + bank->level) < 0)
> + return -1;
> +
> + if (virFileExists(path)) {
> + ret = 0;
> + } else {
> + VIR_FREE(path);
> + /* for CDP enabled case, CODE and DATA will show together */
> + if (virAsprintf(&path,
> + SYSFS_RESCTRL_PATH "/info/L%uCODE",
> + bank->level) < 0)
> + return -1;
> + if (virFileExists(path))
> + ret = 1;
> + }
> +
> + VIR_FREE(path);
> + return ret;
> +}
> +
> +static int
> +virCapabilitiesGetCacheControl(virCapsHostCacheBankPtr bank,
> + virCacheType scope)
> +{
> + int ret = -1;
> + char *path = NULL;
> + char *cbm_mask = NULL;
> + char *type_upper = NULL;
> + virCapsHostCacheControlPtr control;
> +
> + if (VIR_ALLOC(control) < 0)
> + goto cleanup;
> +
> + if ((scope > VIR_CACHE_TYPE_BOTH)
> + && (virStringToUpper(&type_upper, virCacheTypeToString(scope)) < 0))
> + goto cleanup;
> +
> + if (virFileReadValueUint(&control->max_allocation,
> + SYSFS_RESCTRL_PATH "/info/L%u%s/num_closids",
> + bank->level,
> + type_upper ? type_upper : "") < 0)
> + goto cleanup;
> +
> + if (virFileReadValueString(&cbm_mask,
> + SYSFS_RESCTRL_PATH
> + "/info/L%u%s/cbm_mask",
> + bank->level,
> + type_upper ? type_upper: "") < 0)
> + goto cleanup;
> +
> + virStringTrimOptionalNewline(cbm_mask);
> +
> + /* cbm_mask: cache bit mask, it's in hex, eg: fffff */
> + control->min = bank->size / (strlen(cbm_mask) * 4);
> +
> + control->scope = scope;
> +
> + if (VIR_APPEND_ELEMENT(bank->controls,
> + bank->ncontrols,
> + control) < 0)
> + goto cleanup;
> +
> + ret = 0;
> +
> + cleanup:
> + VIR_FREE(path);
> + VIR_FREE(cbm_mask);
> + VIR_FREE(type_upper);
> + VIR_FREE(control);
> + return ret;
> +}
> +
> int
> virCapabilitiesInitCaches(virCapsPtr caps)
> {
> @@ -1534,6 +1649,7 @@ virCapabilitiesInitCaches(virCapsPtr caps)
> ssize_t pos = -1;
> DIR *dirp = NULL;
> int ret = -1;
> + int typeret;
> char *path = NULL;
> char *type = NULL;
> struct dirent *ent = NULL;
> @@ -1607,12 +1723,27 @@ virCapabilitiesInitCaches(virCapsPtr caps)
> SYSFS_SYSTEM_PATH, pos, ent->d_name) < 0)
> goto cleanup;
> 
> + typeret = virCapabilitiesGetCacheControlType(bank);
> +
> + if (typeret == 0) {
> + if (virCapabilitiesGetCacheControl(bank,
> + VIR_CACHE_TYPE_BOTH) < 0)
> + goto cleanup;
> + } else if (typeret == 1) {
> + if ((virCapabilitiesGetCacheControl(bank,
> + VIR_CACHE_TYPE_CODE) < 0) ||
> + (virCapabilitiesGetCacheControl(bank,
> + VIR_CACHE_TYPE_DATA) < 0))
> + goto cleanup;
> + }
> +
> kernel_type = virCacheKernelTypeFromString(type);
> if (kernel_type < 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("Unknown cache type '%s'"), type);
> goto cleanup;
> }
> +
> bank->type = kernel_type;
> VIR_FREE(type);
> 
> diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h
> index a8cccf7..ee87d59 100644
> --- a/src/conf/capabilities.h
> +++ b/src/conf/capabilities.h
> @@ -148,6 +148,14 @@ typedef enum {
> 
> VIR_ENUM_DECL(virCache);
> 
> +typedef struct _virCapsHostCacheControl virCapsHostCacheControl;
> +typedef virCapsHostCacheControl *virCapsHostCacheControlPtr;
> +struct _virCapsHostCacheControl {
> + unsigned long long min; /* minimum cache control size in B */
> + virCacheType scope; /* data, code or both */
> + unsigned int max_allocation; /* max number of supported allocations */
> +};
> +
> typedef struct _virCapsHostCacheBank virCapsHostCacheBank;
> typedef virCapsHostCacheBank *virCapsHostCacheBankPtr;
> struct _virCapsHostCacheBank {
> @@ -156,6 +164,8 @@ struct _virCapsHostCacheBank {
> unsigned long long size; /* B */
> virCacheType type; /* Data, Instruction or Unified */
> virBitmapPtr cpus; /* All CPUs that share this bank */
> + size_t ncontrols;
> + virCapsHostCacheControlPtr *controls;
> };
> 
> typedef struct _virCapsHost virCapsHost;
> diff --git a/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/cpus b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/cpus
> new file mode 100644
> index 0000000..b3a79aa
> --- /dev/null
> +++ b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/cpus
> @@ -0,0 +1 @@
> +ffffff,ffffffff,ffffffff
> diff --git a/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/cbm_mask b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/cbm_mask
> new file mode 100755
> index 0000000..78031da
> --- /dev/null
> +++ b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/cbm_mask
> @@ -0,0 +1 @@
> +fffff
> diff --git a/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/min_cbm_bits b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/min_cbm_bits
> new file mode 100755
> index 0000000..d00491f
> --- /dev/null
> +++ b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/min_cbm_bits
> @@ -0,0 +1 @@
> +1
> diff --git a/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/num_closids b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/num_closids
> new file mode 100755
> index 0000000..45a4fb7
> --- /dev/null
> +++ b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/num_closids
> @@ -0,0 +1 @@
> +8
> diff --git a/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/cbm_mask b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/cbm_mask
> new file mode 100755
> index 0000000..78031da
> --- /dev/null
> +++ b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/cbm_mask
> @@ -0,0 +1 @@
> +fffff
> diff --git a/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/min_cbm_bits b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/min_cbm_bits
> new file mode 100755
> index 0000000..d00491f
> --- /dev/null
> +++ b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/min_cbm_bits
> @@ -0,0 +1 @@
> +1
> diff --git a/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/num_closids b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/num_closids
> new file mode 100755
> index 0000000..45a4fb7
> --- /dev/null
> +++ b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/num_closids
> @@ -0,0 +1 @@
> +8
> diff --git a/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/cpus b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/cpus
> new file mode 100644
> index 0000000..ede4cc2
> --- /dev/null
> +++ b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/cpus
> @@ -0,0 +1 @@
> +000000,00000000,00000000
> diff --git a/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/schemata b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/schemata
> new file mode 100644
> index 0000000..a0ef381
> --- /dev/null
> +++ b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/schemata
> @@ -0,0 +1,2 @@
> +L3DATA:0=c0000;1=c0000
> +L3CODE:0=30000;1=30000
> diff --git a/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/tasks b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/tasks
> new file mode 100644
> index 0000000..e69de29
> diff --git a/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/schemata b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/schemata
> new file mode 100644
> index 0000000..89dc76b
> --- /dev/null
> +++ b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/schemata
> @@ -0,0 +1,2 @@
> +L3DATA:0=fffff;1=fffff
> +L3CODE:0=fffff;1=fffff
> diff --git a/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/tasks b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/tasks
> new file mode 100644
> index 0000000..e69de29
> diff --git a/tests/vircaps2xmldata/linux-resctrl-cdp/system b/tests/vircaps2xmldata/linux-resctrl-cdp/system
> new file mode 120000
> index 0000000..2f3a1d9
> --- /dev/null
> +++ b/tests/vircaps2xmldata/linux-resctrl-cdp/system
> @@ -0,0 +1 @@
> +../linux-resctrl/system/
> \ No newline at end of file
> diff --git a/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml b/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml
> new file mode 100644
> index 0000000..c9f460d
> --- /dev/null
> +++ b/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml
> @@ -0,0 +1,55 @@
> +<capabilities>
> +
> + <host>
> + <cpu>
> + <arch>x86_64</arch>
> + </cpu>
> + <power_management/>
> + <migration_features>
> + <live/>
> + </migration_features>
> + <topology>
> + <cells num='2'>
> + <cell id='0'>
> + <memory unit='KiB'>1048576</memory>
> + <pages unit='KiB' size='4'>2048</pages>
> + <pages unit='KiB' size='2048'>4096</pages>
> + <pages unit='KiB' size='1048576'>6144</pages>
> + <cpus num='6'>
> + <cpu id='0' socket_id='0' core_id='0' siblings='0'/>
> + <cpu id='1' socket_id='0' core_id='1' siblings='1'/>
> + <cpu id='2' socket_id='0' core_id='2' siblings='2'/>
> + <cpu id='3' socket_id='0' core_id='3' siblings='3'/>
> + <cpu id='4' socket_id='0' core_id='4' siblings='4'/>
> + <cpu id='5' socket_id='0' core_id='5' siblings='5'/>
> + </cpus>
> + </cell>
> + <cell id='1'>
> + <memory unit='KiB'>2097152</memory>
> + <pages unit='KiB' size='4'>4096</pages>
> + <pages unit='KiB' size='2048'>6144</pages>
> + <pages unit='KiB' size='1048576'>8192</pages>
> + <cpus num='6'>
> + <cpu id='6' socket_id='1' core_id='0' siblings='6'/>
> + <cpu id='7' socket_id='1' core_id='1' siblings='7'/>
> + <cpu id='8' socket_id='1' core_id='2' siblings='8'/>
> + <cpu id='9' socket_id='1' core_id='3' siblings='9'/>
> + <cpu id='10' socket_id='1' core_id='4' siblings='10'/>
> + <cpu id='11' socket_id='1' core_id='5' siblings='11'/>
> + </cpus>
> + </cell>
> + </cells>
> + </topology>
> + <cache>
> + <bank id='0' level='3' type='both' size='15360' unit='KiB' cpus='0-5'>
> + <control min='768' unit='KiB' scope='code' max_allocation='8'/>
> + <control min='768' unit='KiB' scope='data' max_allocation='8'/>
> + </bank>
> + <bank id='1' level='3' type='both' size='15360' unit='KiB' cpus='6-11'>
> + <control min='768' unit='KiB' scope='code' max_allocation='8'/>
> + <control min='768' unit='KiB' scope='data' max_allocation='8'/>
> + </bank>
> + </cache>
> + </host>
> +
> +</capabilities>
> diff --git a/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml b/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml
> index 6677791..04a5eb8 100644
> --- a/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml
> +++ b/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml
> @@ -41,8 +41,12 @@
> </cells>
> </topology>
> <cache>
> - <bank id='0' level='3' type='both' size='15360' unit='KiB' cpus='0-5'/>
> - <bank id='1' level='3' type='both' size='15360' unit='KiB' cpus='6-11'/>
> + <bank id='0' level='3' type='both' size='15360' unit='KiB' cpus='0-5'>
> + <control min='768' unit='KiB' scope='both' max_allocation='4'/>
> + </bank>
> + <bank id='1' level='3' type='both' size='15360' unit='KiB' cpus='6-11'>
> + <control min='768' unit='KiB' scope='both' max_allocation='4'/>
> + </bank>
> </cache>
> </host>
> 
> diff --git a/tests/vircaps2xmltest.c b/tests/vircaps2xmltest.c
> index 137598e..ae1cd52 100644
> --- a/tests/vircaps2xmltest.c
> +++ b/tests/vircaps2xmltest.c
> @@ -47,6 +47,7 @@ test_virCapabilities(const void *opaque)
> char *capsXML = NULL;
> char *path = NULL;
> char *dir = NULL;
> + char *resctrl = NULL;
> int ret = -1;
> 
> /*
> @@ -58,7 +59,12 @@ test_virCapabilities(const void *opaque)
> data->resctrl ? "/system" : "") < 0)
> goto cleanup;
> 
> + if (virAsprintf(&resctrl, "%s/vircaps2xmldata/linux-%s/resctrl",
> + abs_srcdir, data->filename) < 0)
> + goto cleanup;
> +
> virFileWrapperAddPrefix("/sys/devices/system", dir);
> + virFileWrapperAddPrefix("/sys/fs/resctrl", resctrl);
> caps = virCapabilitiesNew(data->arch, data->offlineMigrate, data->liveMigrate);
> 
> if (!caps)
> @@ -84,6 +90,7 @@ test_virCapabilities(const void *opaque)
> 
> cleanup:
> VIR_FREE(dir);
> + VIR_FREE(resctrl);
> VIR_FREE(path);
> VIR_FREE(capsXML);
> virObjectUnref(caps);
> @@ -112,6 +119,7 @@ mymain(void)
> DO_TEST("caches", VIR_ARCH_X86_64);
> 
> DO_TEST_FULL("resctrl", VIR_ARCH_X86_64, true, true, true);
> + DO_TEST_FULL("resctrl-cdp", VIR_ARCH_X86_64, true, true, true);
> 
> return ret;
> }
> -- 
> 1.9.1
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com (mailto:libvir-list@redhat.com)
> https://www.redhat.com/mailman/listinfo/libvir-list
> 
> 


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

Re: [libvirt] [V4 RESEND PATCH] Expose resource control capabilites on cache bank

Posted by Martin Kletzander 6 years, 10 months ago

Sorry for the delays.  Apart from some details (some even mentioned
before) this looks quite alright.  I'll fix up few things here and there
not to bother you with another version and resend it with one fix that's
also needed in my previous code.

On Wed, May 31, 2017 at 09:24:37AM +0800, Eli Qiao wrote:
>ping
>
>
>On Wednesday, 17 May 2017 at 5:08 PM, taget wrote:
>
>> From: Eli Qiao <liyong.qiao@intel.com (mailto:liyong.qiao@intel.com)>
>>
>> * This patch amends the cache bank capability as follow:
>>
>> <cache>
>> <bank id='0' level='3' type='unified' size='15360' unit='KiB' cpus='0-5'>
>> <control min='768' unit='KiB' scope='both' max_allocation='4'/>
>> </bank>
>> <bank id='1' level='3' type='unified' size='15360' unit='KiB' cpus='6-11'>
>> <control min='768' unit='KiB' scope='both' max_allocation='4'/>
>> </bank>
>> </cache>
>>
>> For CDP enabled on x86 arch, we will have:
>> <cache>
>> <bank id='0' level='3' type='unified' size='15360' unit='KiB' cpus='0-5'>
>> <control min='768' unit='KiB' scope='code' max_allocation='4'/>
>> <control min='768' unit='KiB' scope='data' max_allocation='4'/>
>> </bank>
>> ...
>>
>> * Added a new testdata directory `linux-resctrl-cdp` to test CDP enabled
>> case.
>>
>> * Along with vircaps2xmltest case updated.
>>
>> P.S. here the scope is from /sys/fs/resctrl/info/L3{"" "code" "data"}, I
>> keep it capital upper as I need to use a macro to convert from enum to
>> string easily.
>>
>> Signed-off-by: Eli Qiao <liyong.qiao@intel.com (mailto:liyong.qiao@intel.com)>
>> ---
>> docs/schemas/capability.rng | 20 ++++
>> src/conf/capabilities.c | 133 ++++++++++++++++++++-
>> src/conf/capabilities.h | 10 ++
>> .../vircaps2xmldata/linux-resctrl-cdp/resctrl/cpus | 1 +
>> .../linux-resctrl-cdp/resctrl/info/L3CODE/cbm_mask | 1 +
>> .../resctrl/info/L3CODE/min_cbm_bits | 1 +
>> .../resctrl/info/L3CODE/num_closids | 1 +
>> .../linux-resctrl-cdp/resctrl/info/L3DATA/cbm_mask | 1 +
>> .../resctrl/info/L3DATA/min_cbm_bits | 1 +
>> .../resctrl/info/L3DATA/num_closids | 1 +
>> .../linux-resctrl-cdp/resctrl/manualres/cpus | 1 +
>> .../linux-resctrl-cdp/resctrl/manualres/schemata | 2 +
>> .../linux-resctrl-cdp/resctrl/manualres/tasks | 0
>> .../linux-resctrl-cdp/resctrl/schemata | 2 +
>> .../linux-resctrl-cdp/resctrl/tasks | 0
>> tests/vircaps2xmldata/linux-resctrl-cdp/system | 1 +
>> .../vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml | 55 +++++++++
>> tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml | 8 +-
>> tests/vircaps2xmltest.c | 8 ++
>> 19 files changed, 244 insertions(+), 3 deletions(-)
>> create mode 100644 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/cpus
>> create mode 100755 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/cbm_mask
>> create mode 100755 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/min_cbm_bits
>> create mode 100755 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/num_closids
>> create mode 100755 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/cbm_mask
>> create mode 100755 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/min_cbm_bits
>> create mode 100755 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/num_closids
>> create mode 100644 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/cpus
>> create mode 100644 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/schemata
>> create mode 100644 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/tasks
>> create mode 100644 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/schemata
>> create mode 100644 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/tasks
>> create mode 120000 tests/vircaps2xmldata/linux-resctrl-cdp/system
>> create mode 100644 tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml
>>
>> diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng
>> index 26f0aa2..927fc18 100644
>> --- a/docs/schemas/capability.rng
>> +++ b/docs/schemas/capability.rng
>> @@ -277,6 +277,26 @@
>> <attribute name='cpus'>
>> <ref name='cpuset'/>
>> </attribute>
>> + <zeroOrMore>
>> + <element name='control'>
>> + <attribute name='min'>
>> + <ref name='unsignedInt'/>
>> + </attribute>
>> + <attribute name='unit'>
>> + <ref name='unit'/>
>> + </attribute>
>> + <attribute name='scope'>
>> + <choice>
>> + <value>both</value>
>> + <value>code</value>
>> + <value>data</value>
>> + </choice>
>> + </attribute>
>> + <attribute name='max_allocation'>
>> + <ref name='unsignedInt'/>
>> + </attribute>
>> + </element>
>> + </zeroOrMore>
>> </element>
>> </oneOrMore>
>> </element>
>> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
>> index d699b08..c4a1fdf 100644
>> --- a/src/conf/capabilities.c
>> +++ b/src/conf/capabilities.c
>> @@ -51,6 +51,7 @@
>> #define VIR_FROM_THIS VIR_FROM_CAPABILITIES
>>
>> #define SYSFS_SYSTEM_PATH "/sys/devices/system"
>> +#define SYSFS_RESCTRL_PATH "/sys/fs/resctrl"
>>
>> VIR_LOG_INIT("conf.capabilities")
>>
>> @@ -872,6 +873,9 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
>> virCapsHostCacheBankPtr *caches)
>> {
>> size_t i = 0;
>> + size_t j = 0;
>> + int indent = virBufferGetIndent(buf, false);
>> + virBuffer controlBuf = VIR_BUFFER_INITIALIZER;
>>
>> if (!ncaches)
>> return 0;
>> @@ -893,13 +897,35 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
>> */
>> virBufferAsprintf(buf,
>> "<bank id='%u' level='%u' type='%s' "
>> - "size='%llu' unit='%s' cpus='%s'/>\n",
>> + "size='%llu' unit='%s' cpus='%s'",
>> bank->id, bank->level,
>> virCacheTypeToString(bank->type),
>> bank->size >> (kilos * 10),
>> kilos ? "KiB" : "B",
>> cpus_str);
>>
>> + virBufferAdjustIndent(&controlBuf, indent + 4);
>> + for (j = 0; j < bank->ncontrols; j++) {
>> + bool min_kilos = !(bank->controls[j]->min % 1024);
>> + virBufferAsprintf(&controlBuf,
>> + "<control min='%llu' unit='%s' "
>> + "scope='%s' max_allocation='%u'/>\n",
>> + bank->controls[j]->min >> (min_kilos * 10),
>> + min_kilos ? "KiB" : "B",
>> + virCacheTypeToString(bank->controls[j]->scope),
>> + bank->controls[j]->max_allocation);
>> + }
>> +
>> + if (virBufferUse(&controlBuf)) {
>> + virBufferAddLit(buf, ">\n");
>> + virBufferAddBuffer(buf, &controlBuf);
>> + virBufferAddLit(buf, "</bank>\n");
>> +
>> + } else {
>> + virBufferAddLit(buf, "/>\n");
>> + }
>> +
>> + virBufferFreeAndReset(&controlBuf);
>> VIR_FREE(cpus_str);
>> }
>>
>> @@ -1519,13 +1545,102 @@ virCapsHostCacheBankEquals(virCapsHostCacheBankPtr a,
>> void
>> virCapsHostCacheBankFree(virCapsHostCacheBankPtr ptr)
>> {
>> + size_t i;
>> +
>> if (!ptr)
>> return;
>>
>> virBitmapFree(ptr->cpus);
>> + for (i = 0; i < ptr->ncontrols; i++)
>> + VIR_FREE(ptr->controls[i]);
>> + VIR_FREE(ptr->controls);
>> VIR_FREE(ptr);
>> }
>>
>> +/* test which TYPE of cache control supported
>> + * -1: don't support
>> + * 0: cat
>> + * 1: cdp
>> + */
>> +static int
>> +virCapabilitiesGetCacheControlType(virCapsHostCacheBankPtr bank)
>> +{
>> + int ret = -1;
>> + char *path = NULL;
>> + if (virAsprintf(&path,
>> + SYSFS_RESCTRL_PATH "/info/L%u",
>> + bank->level) < 0)
>> + return -1;
>> +
>> + if (virFileExists(path)) {
>> + ret = 0;
>> + } else {
>> + VIR_FREE(path);
>> + /* for CDP enabled case, CODE and DATA will show together */
>> + if (virAsprintf(&path,
>> + SYSFS_RESCTRL_PATH "/info/L%uCODE",
>> + bank->level) < 0)
>> + return -1;
>> + if (virFileExists(path))
>> + ret = 1;
>> + }
>> +
>> + VIR_FREE(path);
>> + return ret;
>> +}
>> +
>> +static int
>> +virCapabilitiesGetCacheControl(virCapsHostCacheBankPtr bank,
>> + virCacheType scope)
>> +{
>> + int ret = -1;
>> + char *path = NULL;
>> + char *cbm_mask = NULL;
>> + char *type_upper = NULL;
>> + virCapsHostCacheControlPtr control;
>> +
>> + if (VIR_ALLOC(control) < 0)
>> + goto cleanup;
>> +
>> + if ((scope > VIR_CACHE_TYPE_BOTH)
>> + && (virStringToUpper(&type_upper, virCacheTypeToString(scope)) < 0))
>> + goto cleanup;
>> +
>> + if (virFileReadValueUint(&control->max_allocation,
>> + SYSFS_RESCTRL_PATH "/info/L%u%s/num_closids",
>> + bank->level,
>> + type_upper ? type_upper : "") < 0)
>> + goto cleanup;
>> +
>> + if (virFileReadValueString(&cbm_mask,
>> + SYSFS_RESCTRL_PATH
>> + "/info/L%u%s/cbm_mask",
>> + bank->level,
>> + type_upper ? type_upper: "") < 0)
>> + goto cleanup;
>> +
>> + virStringTrimOptionalNewline(cbm_mask);
>> +
>> + /* cbm_mask: cache bit mask, it's in hex, eg: fffff */
>> + control->min = bank->size / (strlen(cbm_mask) * 4);
>> +
>> + control->scope = scope;
>> +
>> + if (VIR_APPEND_ELEMENT(bank->controls,
>> + bank->ncontrols,
>> + control) < 0)
>> + goto cleanup;
>> +
>> + ret = 0;
>> +
>> + cleanup:
>> + VIR_FREE(path);
>> + VIR_FREE(cbm_mask);
>> + VIR_FREE(type_upper);
>> + VIR_FREE(control);
>> + return ret;
>> +}
>> +
>> int
>> virCapabilitiesInitCaches(virCapsPtr caps)
>> {
>> @@ -1534,6 +1649,7 @@ virCapabilitiesInitCaches(virCapsPtr caps)
>> ssize_t pos = -1;
>> DIR *dirp = NULL;
>> int ret = -1;
>> + int typeret;
>> char *path = NULL;
>> char *type = NULL;
>> struct dirent *ent = NULL;
>> @@ -1607,12 +1723,27 @@ virCapabilitiesInitCaches(virCapsPtr caps)
>> SYSFS_SYSTEM_PATH, pos, ent->d_name) < 0)
>> goto cleanup;
>>
>> + typeret = virCapabilitiesGetCacheControlType(bank);
>> +
>> + if (typeret == 0) {
>> + if (virCapabilitiesGetCacheControl(bank,
>> + VIR_CACHE_TYPE_BOTH) < 0)
>> + goto cleanup;
>> + } else if (typeret == 1) {
>> + if ((virCapabilitiesGetCacheControl(bank,
>> + VIR_CACHE_TYPE_CODE) < 0) ||
>> + (virCapabilitiesGetCacheControl(bank,
>> + VIR_CACHE_TYPE_DATA) < 0))
>> + goto cleanup;
>> + }
>> +
>> kernel_type = virCacheKernelTypeFromString(type);
>> if (kernel_type < 0) {
>> virReportError(VIR_ERR_INTERNAL_ERROR,
>> _("Unknown cache type '%s'"), type);
>> goto cleanup;
>> }
>> +
>> bank->type = kernel_type;
>> VIR_FREE(type);
>>
>> diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h
>> index a8cccf7..ee87d59 100644
>> --- a/src/conf/capabilities.h
>> +++ b/src/conf/capabilities.h
>> @@ -148,6 +148,14 @@ typedef enum {
>>
>> VIR_ENUM_DECL(virCache);
>>
>> +typedef struct _virCapsHostCacheControl virCapsHostCacheControl;
>> +typedef virCapsHostCacheControl *virCapsHostCacheControlPtr;
>> +struct _virCapsHostCacheControl {
>> + unsigned long long min; /* minimum cache control size in B */
>> + virCacheType scope; /* data, code or both */
>> + unsigned int max_allocation; /* max number of supported allocations */
>> +};
>> +
>> typedef struct _virCapsHostCacheBank virCapsHostCacheBank;
>> typedef virCapsHostCacheBank *virCapsHostCacheBankPtr;
>> struct _virCapsHostCacheBank {
>> @@ -156,6 +164,8 @@ struct _virCapsHostCacheBank {
>> unsigned long long size; /* B */
>> virCacheType type; /* Data, Instruction or Unified */
>> virBitmapPtr cpus; /* All CPUs that share this bank */
>> + size_t ncontrols;
>> + virCapsHostCacheControlPtr *controls;
>> };
>>
>> typedef struct _virCapsHost virCapsHost;
>> diff --git a/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/cpus b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/cpus
>> new file mode 100644
>> index 0000000..b3a79aa
>> --- /dev/null
>> +++ b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/cpus
>> @@ -0,0 +1 @@
>> +ffffff,ffffffff,ffffffff
>> diff --git a/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/cbm_mask b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/cbm_mask
>> new file mode 100755
>> index 0000000..78031da
>> --- /dev/null
>> +++ b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/cbm_mask
>> @@ -0,0 +1 @@
>> +fffff
>> diff --git a/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/min_cbm_bits b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/min_cbm_bits
>> new file mode 100755
>> index 0000000..d00491f
>> --- /dev/null
>> +++ b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/min_cbm_bits
>> @@ -0,0 +1 @@
>> +1
>> diff --git a/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/num_closids b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/num_closids
>> new file mode 100755
>> index 0000000..45a4fb7
>> --- /dev/null
>> +++ b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/num_closids
>> @@ -0,0 +1 @@
>> +8
>> diff --git a/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/cbm_mask b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/cbm_mask
>> new file mode 100755
>> index 0000000..78031da
>> --- /dev/null
>> +++ b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/cbm_mask
>> @@ -0,0 +1 @@
>> +fffff
>> diff --git a/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/min_cbm_bits b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/min_cbm_bits
>> new file mode 100755
>> index 0000000..d00491f
>> --- /dev/null
>> +++ b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/min_cbm_bits
>> @@ -0,0 +1 @@
>> +1
>> diff --git a/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/num_closids b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/num_closids
>> new file mode 100755
>> index 0000000..45a4fb7
>> --- /dev/null
>> +++ b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/num_closids
>> @@ -0,0 +1 @@
>> +8
>> diff --git a/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/cpus b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/cpus
>> new file mode 100644
>> index 0000000..ede4cc2
>> --- /dev/null
>> +++ b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/cpus
>> @@ -0,0 +1 @@
>> +000000,00000000,00000000
>> diff --git a/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/schemata b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/schemata
>> new file mode 100644
>> index 0000000..a0ef381
>> --- /dev/null
>> +++ b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/schemata
>> @@ -0,0 +1,2 @@
>> +L3DATA:0=c0000;1=c0000
>> +L3CODE:0=30000;1=30000
>> diff --git a/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/tasks b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/tasks
>> new file mode 100644
>> index 0000000..e69de29
>> diff --git a/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/schemata b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/schemata
>> new file mode 100644
>> index 0000000..89dc76b
>> --- /dev/null
>> +++ b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/schemata
>> @@ -0,0 +1,2 @@
>> +L3DATA:0=fffff;1=fffff
>> +L3CODE:0=fffff;1=fffff
>> diff --git a/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/tasks b/tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/tasks
>> new file mode 100644
>> index 0000000..e69de29
>> diff --git a/tests/vircaps2xmldata/linux-resctrl-cdp/system b/tests/vircaps2xmldata/linux-resctrl-cdp/system
>> new file mode 120000
>> index 0000000..2f3a1d9
>> --- /dev/null
>> +++ b/tests/vircaps2xmldata/linux-resctrl-cdp/system
>> @@ -0,0 +1 @@
>> +../linux-resctrl/system/
>> \ No newline at end of file
>> diff --git a/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml b/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml
>> new file mode 100644
>> index 0000000..c9f460d
>> --- /dev/null
>> +++ b/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml
>> @@ -0,0 +1,55 @@
>> +<capabilities>
>> +
>> + <host>
>> + <cpu>
>> + <arch>x86_64</arch>
>> + </cpu>
>> + <power_management/>
>> + <migration_features>
>> + <live/>
>> + </migration_features>
>> + <topology>
>> + <cells num='2'>
>> + <cell id='0'>
>> + <memory unit='KiB'>1048576</memory>
>> + <pages unit='KiB' size='4'>2048</pages>
>> + <pages unit='KiB' size='2048'>4096</pages>
>> + <pages unit='KiB' size='1048576'>6144</pages>
>> + <cpus num='6'>
>> + <cpu id='0' socket_id='0' core_id='0' siblings='0'/>
>> + <cpu id='1' socket_id='0' core_id='1' siblings='1'/>
>> + <cpu id='2' socket_id='0' core_id='2' siblings='2'/>
>> + <cpu id='3' socket_id='0' core_id='3' siblings='3'/>
>> + <cpu id='4' socket_id='0' core_id='4' siblings='4'/>
>> + <cpu id='5' socket_id='0' core_id='5' siblings='5'/>
>> + </cpus>
>> + </cell>
>> + <cell id='1'>
>> + <memory unit='KiB'>2097152</memory>
>> + <pages unit='KiB' size='4'>4096</pages>
>> + <pages unit='KiB' size='2048'>6144</pages>
>> + <pages unit='KiB' size='1048576'>8192</pages>
>> + <cpus num='6'>
>> + <cpu id='6' socket_id='1' core_id='0' siblings='6'/>
>> + <cpu id='7' socket_id='1' core_id='1' siblings='7'/>
>> + <cpu id='8' socket_id='1' core_id='2' siblings='8'/>
>> + <cpu id='9' socket_id='1' core_id='3' siblings='9'/>
>> + <cpu id='10' socket_id='1' core_id='4' siblings='10'/>
>> + <cpu id='11' socket_id='1' core_id='5' siblings='11'/>
>> + </cpus>
>> + </cell>
>> + </cells>
>> + </topology>
>> + <cache>
>> + <bank id='0' level='3' type='both' size='15360' unit='KiB' cpus='0-5'>
>> + <control min='768' unit='KiB' scope='code' max_allocation='8'/>
>> + <control min='768' unit='KiB' scope='data' max_allocation='8'/>
>> + </bank>
>> + <bank id='1' level='3' type='both' size='15360' unit='KiB' cpus='6-11'>
>> + <control min='768' unit='KiB' scope='code' max_allocation='8'/>
>> + <control min='768' unit='KiB' scope='data' max_allocation='8'/>
>> + </bank>
>> + </cache>
>> + </host>
>> +
>> +</capabilities>
>> diff --git a/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml b/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml
>> index 6677791..04a5eb8 100644
>> --- a/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml
>> +++ b/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml
>> @@ -41,8 +41,12 @@
>> </cells>
>> </topology>
>> <cache>
>> - <bank id='0' level='3' type='both' size='15360' unit='KiB' cpus='0-5'/>
>> - <bank id='1' level='3' type='both' size='15360' unit='KiB' cpus='6-11'/>
>> + <bank id='0' level='3' type='both' size='15360' unit='KiB' cpus='0-5'>
>> + <control min='768' unit='KiB' scope='both' max_allocation='4'/>
>> + </bank>
>> + <bank id='1' level='3' type='both' size='15360' unit='KiB' cpus='6-11'>
>> + <control min='768' unit='KiB' scope='both' max_allocation='4'/>
>> + </bank>
>> </cache>
>> </host>
>>
>> diff --git a/tests/vircaps2xmltest.c b/tests/vircaps2xmltest.c
>> index 137598e..ae1cd52 100644
>> --- a/tests/vircaps2xmltest.c
>> +++ b/tests/vircaps2xmltest.c
>> @@ -47,6 +47,7 @@ test_virCapabilities(const void *opaque)
>> char *capsXML = NULL;
>> char *path = NULL;
>> char *dir = NULL;
>> + char *resctrl = NULL;
>> int ret = -1;
>>
>> /*
>> @@ -58,7 +59,12 @@ test_virCapabilities(const void *opaque)
>> data->resctrl ? "/system" : "") < 0)
>> goto cleanup;
>>
>> + if (virAsprintf(&resctrl, "%s/vircaps2xmldata/linux-%s/resctrl",
>> + abs_srcdir, data->filename) < 0)
>> + goto cleanup;
>> +
>> virFileWrapperAddPrefix("/sys/devices/system", dir);
>> + virFileWrapperAddPrefix("/sys/fs/resctrl", resctrl);
>> caps = virCapabilitiesNew(data->arch, data->offlineMigrate, data->liveMigrate);
>>
>> if (!caps)
>> @@ -84,6 +90,7 @@ test_virCapabilities(const void *opaque)
>>
>> cleanup:
>> VIR_FREE(dir);
>> + VIR_FREE(resctrl);
>> VIR_FREE(path);
>> VIR_FREE(capsXML);
>> virObjectUnref(caps);
>> @@ -112,6 +119,7 @@ mymain(void)
>> DO_TEST("caches", VIR_ARCH_X86_64);
>>
>> DO_TEST_FULL("resctrl", VIR_ARCH_X86_64, true, true, true);
>> + DO_TEST_FULL("resctrl-cdp", VIR_ARCH_X86_64, true, true, true);
>>
>> return ret;
>> }
>> --
>> 1.9.1
>>
>> --
>> libvir-list mailing list
>> libvir-list@redhat.com (mailto:libvir-list@redhat.com)
>> https://www.redhat.com/mailman/listinfo/libvir-list
>>
>>
>
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [V4 RESEND PATCH] Expose resource control capabilites on cache bank

Posted by Martin Kletzander 6 years, 10 months ago

On Wed, May 17, 2017 at 05:08:33PM +0800, taget wrote:
>From: Eli Qiao <liyong.qiao@intel.com>
>
>* This patch amends the cache bank capability as follow:
>
><cache>
>  <bank id='0' level='3' type='unified' size='15360' unit='KiB' cpus='0-5'>
>    <control min='768' unit='KiB' scope='both' max_allocation='4'/>
>  </bank>
>  <bank id='1' level='3' type='unified' size='15360' unit='KiB' cpus='6-11'>
>    <control min='768' unit='KiB' scope='both' max_allocation='4'/>
>  </bank>
></cache>
>
>For CDP enabled on x86 arch, we will have:
><cache>
>  <bank id='0' level='3' type='unified' size='15360' unit='KiB' cpus='0-5'>
>    <control min='768' unit='KiB' scope='code' max_allocation='4'/>
>    <control min='768' unit='KiB' scope='data' max_allocation='4'/>
>  </bank>
>...
>
>* Added a new testdata directory `linux-resctrl-cdp` to test CDP enabled
>case.
>
>* Along with vircaps2xmltest case updated.
>
>P.S. here the scope is from /sys/fs/resctrl/info/L3{"" "code" "data"}, I
>keep it capital upper as I need to use a macro to convert from enum to
>string easily.
>
>Signed-off-by: Eli Qiao <liyong.qiao@intel.com>
>---
> docs/schemas/capability.rng                        |  20 ++++
> src/conf/capabilities.c                            | 133 ++++++++++++++++++++-
> src/conf/capabilities.h                            |  10 ++
> .../vircaps2xmldata/linux-resctrl-cdp/resctrl/cpus |   1 +
> .../linux-resctrl-cdp/resctrl/info/L3CODE/cbm_mask |   1 +
> .../resctrl/info/L3CODE/min_cbm_bits               |   1 +
> .../resctrl/info/L3CODE/num_closids                |   1 +
> .../linux-resctrl-cdp/resctrl/info/L3DATA/cbm_mask |   1 +
> .../resctrl/info/L3DATA/min_cbm_bits               |   1 +
> .../resctrl/info/L3DATA/num_closids                |   1 +
> .../linux-resctrl-cdp/resctrl/manualres/cpus       |   1 +
> .../linux-resctrl-cdp/resctrl/manualres/schemata   |   2 +
> .../linux-resctrl-cdp/resctrl/manualres/tasks      |   0
> .../linux-resctrl-cdp/resctrl/schemata             |   2 +
> .../linux-resctrl-cdp/resctrl/tasks                |   0
> tests/vircaps2xmldata/linux-resctrl-cdp/system     |   1 +
> .../vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml |  55 +++++++++
> tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml   |   8 +-
> tests/vircaps2xmltest.c                            |   8 ++
> 19 files changed, 244 insertions(+), 3 deletions(-)
> create mode 100644 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/cpus
> create mode 100755 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/cbm_mask
> create mode 100755 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/min_cbm_bits
> create mode 100755 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/num_closids
> create mode 100755 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/cbm_mask
> create mode 100755 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/min_cbm_bits
> create mode 100755 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/num_closids
> create mode 100644 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/cpus
> create mode 100644 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/schemata
> create mode 100644 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/tasks
> create mode 100644 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/schemata
> create mode 100644 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/tasks
> create mode 120000 tests/vircaps2xmldata/linux-resctrl-cdp/system
> create mode 100644 tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml
>
>diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng
>index 26f0aa2..927fc18 100644
>--- a/docs/schemas/capability.rng
>+++ b/docs/schemas/capability.rng
>@@ -277,6 +277,26 @@
>           <attribute name='cpus'>
>             <ref name='cpuset'/>
>           </attribute>
>+          <zeroOrMore>
>+            <element name='control'>
>+              <attribute name='min'>
>+                <ref name='unsignedInt'/>
>+              </attribute>
>+              <attribute name='unit'>
>+                <ref name='unit'/>
>+              </attribute>
>+              <attribute name='scope'>

This should be 'type', as discussed before.

>+                <choice>
>+                  <value>both</value>
>+                  <value>code</value>
>+                  <value>data</value>
>+                </choice>
>+              </attribute>

And hence this could be its own new definition, since it is already used
in the bank definition.

>+              <attribute name='max_allocation'>

Since we are trying to prefer camelCase, even in the XML, and this
should be plural, I would suggest 'maxAllocs'.

>+                <ref name='unsignedInt'/>
>+              </attribute>
>+            </element>
>+          </zeroOrMore>
>         </element>
>       </oneOrMore>
>     </element>
>diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
>index d699b08..c4a1fdf 100644
>--- a/src/conf/capabilities.c
>+++ b/src/conf/capabilities.c
>@@ -51,6 +51,7 @@
> #define VIR_FROM_THIS VIR_FROM_CAPABILITIES
>
> #define SYSFS_SYSTEM_PATH "/sys/devices/system"
>+#define SYSFS_RESCTRL_PATH "/sys/fs/resctrl"
>
> VIR_LOG_INIT("conf.capabilities")
>
>@@ -872,6 +873,9 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
>                             virCapsHostCacheBankPtr *caches)
> {
>     size_t i = 0;
>+    size_t j = 0;
>+    int indent = virBufferGetIndent(buf, false);
>+    virBuffer controlBuf = VIR_BUFFER_INITIALIZER;
>
>     if (!ncaches)
>         return 0;
>@@ -893,13 +897,35 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
>          */
>         virBufferAsprintf(buf,
>                           "<bank id='%u' level='%u' type='%s' "
>-                          "size='%llu' unit='%s' cpus='%s'/>\n",
>+                          "size='%llu' unit='%s' cpus='%s'",
>                           bank->id, bank->level,
>                           virCacheTypeToString(bank->type),
>                           bank->size >> (kilos * 10),
>                           kilos ? "KiB" : "B",
>                           cpus_str);
>
>+        virBufferAdjustIndent(&controlBuf, indent + 4);
>+        for (j = 0; j < bank->ncontrols; j++) {
>+            bool min_kilos = !(bank->controls[j]->min % 1024);
>+            virBufferAsprintf(&controlBuf,
>+                              "<control min='%llu' unit='%s' "
>+                              "scope='%s' max_allocation='%u'/>\n",
>+                              bank->controls[j]->min >> (min_kilos * 10),
>+                              min_kilos ? "KiB" : "B",
>+                              virCacheTypeToString(bank->controls[j]->scope),
>+                              bank->controls[j]->max_allocation);
>+        }
>+
>+        if (virBufferUse(&controlBuf)) {
>+            virBufferAddLit(buf, ">\n");
>+            virBufferAddBuffer(buf, &controlBuf);
>+            virBufferAddLit(buf, "</bank>\n");
>+

Spurious line.

>+        } else {
>+            virBufferAddLit(buf, "/>\n");
>+        }
>+
>+        virBufferFreeAndReset(&controlBuf);

No need for this, it is already done by virBufferAddBuffer.

>         VIR_FREE(cpus_str);
>     }
>
>@@ -1519,13 +1545,102 @@ virCapsHostCacheBankEquals(virCapsHostCacheBankPtr a,
> void
> virCapsHostCacheBankFree(virCapsHostCacheBankPtr ptr)
> {
>+    size_t i;
>+
>     if (!ptr)
>         return;
>
>     virBitmapFree(ptr->cpus);
>+    for (i = 0; i < ptr->ncontrols; i++)
>+        VIR_FREE(ptr->controls[i]);
>+    VIR_FREE(ptr->controls);
>     VIR_FREE(ptr);
> }
>
>+/* test which TYPE of cache control supported
>+ * -1: don't support
>+ *  0: cat
>+ *  1: cdp
>+ */

I would slightly reword this comment.

>+static int
>+virCapabilitiesGetCacheControlType(virCapsHostCacheBankPtr bank)
>+{
>+    int ret = -1;
>+    char *path = NULL;

Empty line here

>+    if (virAsprintf(&path,
>+                    SYSFS_RESCTRL_PATH "/info/L%u",
>+                    bank->level) < 0)
>+        return -1;
>+
>+    if (virFileExists(path)) {
>+        ret = 0;
>+    } else {
>+        VIR_FREE(path);
>+        /* for CDP enabled case, CODE and DATA will show together */

"If CDP is enabled, there will be both CODE and DATA, but it's enough to
check one of those only."

>+        if (virAsprintf(&path,
>+                        SYSFS_RESCTRL_PATH "/info/L%uCODE",
>+                        bank->level) < 0)
>+            return -1;
>+        if (virFileExists(path))
>+            ret = 1;
>+    }
>+
>+    VIR_FREE(path);
>+    return ret;
>+}
>+
>+static int
>+virCapabilitiesGetCacheControl(virCapsHostCacheBankPtr bank,
>+                               virCacheType scope)
>+{
>+    int ret = -1;
>+    char *path = NULL;
>+    char *cbm_mask = NULL;
>+    char *type_upper = NULL;
>+    virCapsHostCacheControlPtr control;
>+
>+    if (VIR_ALLOC(control) < 0)
>+        goto cleanup;
>+
>+    if ((scope > VIR_CACHE_TYPE_BOTH)
>+            && (virStringToUpper(&type_upper, virCacheTypeToString(scope)) < 0))

Order comparison on enum values should be done only in rare cases,
and there should then be some explanation why the order must be kept and
where to add what new values.  I would just do != here instead.

Also the indentation is wrong, the && belongs to the previous line and
there's too much parentheses.

>+        goto cleanup;
>+
>+    if (virFileReadValueUint(&control->max_allocation,
>+                             SYSFS_RESCTRL_PATH "/info/L%u%s/num_closids",
>+                             bank->level,
>+                             type_upper ? type_upper : "") < 0)
>+        goto cleanup;
>+
>+    if (virFileReadValueString(&cbm_mask,
>+                               SYSFS_RESCTRL_PATH
>+                               "/info/L%u%s/cbm_mask",
>+                               bank->level,
>+                               type_upper ? type_upper: "") < 0)
>+        goto cleanup;
>+
>+    virStringTrimOptionalNewline(cbm_mask);
>+
>+    /* cbm_mask: cache bit mask, it's in hex, eg: fffff */
>+    control->min = bank->size / (strlen(cbm_mask) * 4);
>+

I believe this should be multiplied by min_cbm_bits.

>+    control->scope = scope;
>+
>+    if (VIR_APPEND_ELEMENT(bank->controls,
>+                           bank->ncontrols,
>+                           control) < 0)
>+        goto cleanup;
>+
>+    ret = 0;
>+
>+ cleanup:
>+    VIR_FREE(path);
>+    VIR_FREE(cbm_mask);
>+    VIR_FREE(type_upper);
>+    VIR_FREE(control);
>+    return ret;
>+}
>+
> int
> virCapabilitiesInitCaches(virCapsPtr caps)
> {
>@@ -1534,6 +1649,7 @@ virCapabilitiesInitCaches(virCapsPtr caps)
>     ssize_t pos = -1;
>     DIR *dirp = NULL;
>     int ret = -1;
>+    int typeret;
>     char *path = NULL;
>     char *type = NULL;
>     struct dirent *ent = NULL;
>@@ -1607,12 +1723,27 @@ virCapabilitiesInitCaches(virCapsPtr caps)
>                                        SYSFS_SYSTEM_PATH, pos, ent->d_name) < 0)
>                 goto cleanup;
>
>+            typeret = virCapabilitiesGetCacheControlType(bank);
>+
>+            if (typeret == 0) {
>+                if (virCapabilitiesGetCacheControl(bank,
>+                                                   VIR_CACHE_TYPE_BOTH) < 0)
>+                    goto cleanup;
>+            } else if (typeret == 1) {
>+                if ((virCapabilitiesGetCacheControl(bank,
>+                                                    VIR_CACHE_TYPE_CODE) < 0) ||
>+                        (virCapabilitiesGetCacheControl(bank,
>+                                                        VIR_CACHE_TYPE_DATA) < 0))

Again, wrong indentation and unnecessary parentheses.

Otherwise it looks good, so ACK with those differences and reworded
commit message.  Let me know if you agree and I can push it immediately.
The suggested diff follows:

diff --git i/docs/schemas/capability.rng w/docs/schemas/capability.rng
index 927fc184de41..e5cbfa362ec0 100644
--- i/docs/schemas/capability.rng
+++ w/docs/schemas/capability.rng
@@ -261,13 +261,7 @@
           <attribute name='level'>
             <ref name='unsignedInt'/>
           </attribute>
-          <attribute name='type'>
-            <choice>
-              <value>both</value>
-              <value>code</value>
-              <value>data</value>
-            </choice>
-          </attribute>
+          <ref name='cacheType'/>
           <attribute name='size'>
             <ref name='unsignedInt'/>
           </attribute>
@@ -285,14 +279,8 @@
               <attribute name='unit'>
                 <ref name='unit'/>
               </attribute>
-              <attribute name='scope'>
-                <choice>
-                  <value>both</value>
-                  <value>code</value>
-                  <value>data</value>
-                </choice>
-              </attribute>
-              <attribute name='max_allocation'>
+              <ref name='cacheType'/>
+              <attribute name='maxAllocs'>
                 <ref name='unsignedInt'/>
               </attribute>
             </element>
@@ -302,6 +290,16 @@
     </element>
   </define>

+  <define name='cacheType'>
+    <attribute name='type'>
+      <choice>
+        <value>both</value>
+        <value>code</value>
+        <value>data</value>
+      </choice>
+    </attribute>
+  </define>
+
   <define name='guestcaps'>
     <element name='guest'>
       <ref name='ostype'/>
diff --git i/src/conf/capabilities.c w/src/conf/capabilities.c
index 8cd2957e9c88..3becc7e18c62 100644
--- i/src/conf/capabilities.c
+++ w/src/conf/capabilities.c
@@ -909,7 +909,7 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
             bool min_kilos = !(bank->controls[j]->min % 1024);
             virBufferAsprintf(&controlBuf,
                               "<control min='%llu' unit='%s' "
-                              "scope='%s' max_allocation='%u'/>\n",
+                              "type='%s' maxAllocs='%u'/>\n",
                               bank->controls[j]->min >> (min_kilos * 10),
                               min_kilos ? "KiB" : "B",
                               virCacheTypeToString(bank->controls[j]->scope),
@@ -920,12 +920,10 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
             virBufferAddLit(buf, ">\n");
             virBufferAddBuffer(buf, &controlBuf);
             virBufferAddLit(buf, "</bank>\n");
-
         } else {
             virBufferAddLit(buf, "/>\n");
         }

-        virBufferFreeAndReset(&controlBuf);
         VIR_FREE(cpus_str);
     }

@@ -1557,16 +1555,19 @@ virCapsHostCacheBankFree(virCapsHostCacheBankPtr ptr)
     VIR_FREE(ptr);
 }

-/* test which TYPE of cache control supported
- * -1: don't support
- *  0: cat
- *  1: cdp
+/*
+ * This function tests which TYPE of cache control is supported
+ * Return values are:
+ *  -1: not supported
+ *   0: CAT
+ *   1: CDP
  */
 static int
 virCapabilitiesGetCacheControlType(virCapsHostCacheBankPtr bank)
 {
     int ret = -1;
     char *path = NULL;
+
     if (virAsprintf(&path,
                     SYSFS_RESCTRL_PATH "/info/L%u",
                     bank->level) < 0)
@@ -1576,7 +1577,10 @@ virCapabilitiesGetCacheControlType(virCapsHostCacheBankPtr bank)
         ret = 0;
     } else {
         VIR_FREE(path);
-        /* for CDP enabled case, CODE and DATA will show together */
+        /*
+         * If CDP is enabled, there will be both CODE and DATA, but it's enough
+         * to check one of those only.
+         */
         if (virAsprintf(&path,
                         SYSFS_RESCTRL_PATH "/info/L%uCODE",
                         bank->level) < 0)
@@ -1597,13 +1601,14 @@ virCapabilitiesGetCacheControl(virCapsHostCacheBankPtr bank,
     char *path = NULL;
     char *cbm_mask = NULL;
     char *type_upper = NULL;
+    unsigned int min_cbm_bits = 0;
     virCapsHostCacheControlPtr control;

     if (VIR_ALLOC(control) < 0)
         goto cleanup;

-    if ((scope > VIR_CACHE_TYPE_BOTH)
-            && (virStringToUpper(&type_upper, virCacheTypeToString(scope)) < 0))
+    if (scope != VIR_CACHE_TYPE_BOTH &&
+        virStringToUpper(&type_upper, virCacheTypeToString(scope)) < 0)
         goto cleanup;

     if (virFileReadValueUint(&control->max_allocation,
@@ -1619,10 +1624,16 @@ virCapabilitiesGetCacheControl(virCapsHostCacheBankPtr bank,
                                type_upper ? type_upper: "") < 0)
         goto cleanup;

+    if (virFileReadValueUint(&min_cbm_bits,
+                             SYSFS_RESCTRL_PATH "/info/L%u%s/min_cbm_bits",
+                             bank->level,
+                             type_upper ? type_upper : "") < 0)
+        goto cleanup;
+
     virStringTrimOptionalNewline(cbm_mask);

     /* cbm_mask: cache bit mask, it's in hex, eg: fffff */
-    control->min = bank->size / (strlen(cbm_mask) * 4);
+    control->min = min_cbm_bits * bank->size / (strlen(cbm_mask) * 4);

     control->scope = scope;

@@ -1732,10 +1743,10 @@ virCapabilitiesInitCaches(virCapsPtr caps)
                                                    VIR_CACHE_TYPE_BOTH) < 0)
                     goto cleanup;
             } else if (typeret == 1) {
-                if ((virCapabilitiesGetCacheControl(bank,
-                                                    VIR_CACHE_TYPE_CODE) < 0) ||
-                        (virCapabilitiesGetCacheControl(bank,
-                                                        VIR_CACHE_TYPE_DATA) < 0))
+                if (virCapabilitiesGetCacheControl(bank,
+                                                   VIR_CACHE_TYPE_CODE) < 0 ||
+                    virCapabilitiesGetCacheControl(bank,
+                                                   VIR_CACHE_TYPE_DATA) < 0)
                     goto cleanup;
             }

diff --git i/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml w/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml
index c9f460d8a227..49aa0b98ca88 100644
--- i/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml
+++ w/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml
@@ -42,12 +42,12 @@
     </topology>
     <cache>
       <bank id='0' level='3' type='both' size='15360' unit='KiB' cpus='0-5'>
-        <control min='768' unit='KiB' scope='code' max_allocation='8'/>
-        <control min='768' unit='KiB' scope='data' max_allocation='8'/>
+        <control min='768' unit='KiB' type='code' maxAllocs='8'/>
+        <control min='768' unit='KiB' type='data' maxAllocs='8'/>
       </bank>
       <bank id='1' level='3' type='both' size='15360' unit='KiB' cpus='6-11'>
-        <control min='768' unit='KiB' scope='code' max_allocation='8'/>
-        <control min='768' unit='KiB' scope='data' max_allocation='8'/>
+        <control min='768' unit='KiB' type='code' maxAllocs='8'/>
+        <control min='768' unit='KiB' type='data' maxAllocs='8'/>
       </bank>
     </cache>
   </host>
diff --git i/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml w/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml
index 04a5eb895727..cb78b4ab788d 100644
--- i/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml
+++ w/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml
@@ -42,10 +42,10 @@
     </topology>
     <cache>
       <bank id='0' level='3' type='both' size='15360' unit='KiB' cpus='0-5'>
-        <control min='768' unit='KiB' scope='both' max_allocation='4'/>
+        <control min='1536' unit='KiB' type='both' maxAllocs='4'/>
       </bank>
       <bank id='1' level='3' type='both' size='15360' unit='KiB' cpus='6-11'>
-        <control min='768' unit='KiB' scope='both' max_allocation='4'/>
+        <control min='1536' unit='KiB' type='both' maxAllocs='4'/>
       </bank>
     </cache>
   </host>
--
Martin
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [V4 RESEND PATCH] Expose resource control capabilites on cache bank

Posted by Eli Qiao 6 years, 10 months ago

hi Martin 
Thanks for you reviewing and I am okay with the diff as  you suggested.

Please help to push this patch.

Eli. 


On Sunday, 4 June 2017 at 5:39 PM, Martin Kletzander wrote:

> On Wed, May 17, 2017 at 05:08:33PM +0800, taget wrote:
> > From: Eli Qiao <liyong.qiao@intel.com (mailto:liyong.qiao@intel.com)>
> > 
> > * This patch amends the cache bank capability as follow:
> > 
> > <cache>
> > <bank id='0' level='3' type='unified' size='15360' unit='KiB' cpus='0-5'>
> > <control min='768' unit='KiB' scope='both' max_allocation='4'/>
> > </bank>
> > <bank id='1' level='3' type='unified' size='15360' unit='KiB' cpus='6-11'>
> > <control min='768' unit='KiB' scope='both' max_allocation='4'/>
> > </bank>
> > </cache>
> > 
> > For CDP enabled on x86 arch, we will have:
> > <cache>
> > <bank id='0' level='3' type='unified' size='15360' unit='KiB' cpus='0-5'>
> > <control min='768' unit='KiB' scope='code' max_allocation='4'/>
> > <control min='768' unit='KiB' scope='data' max_allocation='4'/>
> > </bank>
> > ...
> > 
> > * Added a new testdata directory `linux-resctrl-cdp` to test CDP enabled
> > case.
> > 
> > * Along with vircaps2xmltest case updated.
> > 
> > P.S. here the scope is from /sys/fs/resctrl/info/L3{"" "code" "data"}, I
> > keep it capital upper as I need to use a macro to convert from enum to
> > string easily.
> > 
> > Signed-off-by: Eli Qiao <liyong.qiao@intel.com (mailto:liyong.qiao@intel.com)>
> > ---
> > docs/schemas/capability.rng | 20 ++++
> > src/conf/capabilities.c | 133 ++++++++++++++++++++-
> > src/conf/capabilities.h | 10 ++
> > .../vircaps2xmldata/linux-resctrl-cdp/resctrl/cpus | 1 +
> > .../linux-resctrl-cdp/resctrl/info/L3CODE/cbm_mask | 1 +
> > .../resctrl/info/L3CODE/min_cbm_bits | 1 +
> > .../resctrl/info/L3CODE/num_closids | 1 +
> > .../linux-resctrl-cdp/resctrl/info/L3DATA/cbm_mask | 1 +
> > .../resctrl/info/L3DATA/min_cbm_bits | 1 +
> > .../resctrl/info/L3DATA/num_closids | 1 +
> > .../linux-resctrl-cdp/resctrl/manualres/cpus | 1 +
> > .../linux-resctrl-cdp/resctrl/manualres/schemata | 2 +
> > .../linux-resctrl-cdp/resctrl/manualres/tasks | 0
> > .../linux-resctrl-cdp/resctrl/schemata | 2 +
> > .../linux-resctrl-cdp/resctrl/tasks | 0
> > tests/vircaps2xmldata/linux-resctrl-cdp/system | 1 +
> > .../vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml | 55 +++++++++
> > tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml | 8 +-
> > tests/vircaps2xmltest.c | 8 ++
> > 19 files changed, 244 insertions(+), 3 deletions(-)
> > create mode 100644 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/cpus
> > create mode 100755 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/cbm_mask
> > create mode 100755 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/min_cbm_bits
> > create mode 100755 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/num_closids
> > create mode 100755 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/cbm_mask
> > create mode 100755 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/min_cbm_bits
> > create mode 100755 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/num_closids
> > create mode 100644 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/cpus
> > create mode 100644 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/schemata
> > create mode 100644 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/tasks
> > create mode 100644 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/schemata
> > create mode 100644 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/tasks
> > create mode 120000 tests/vircaps2xmldata/linux-resctrl-cdp/system
> > create mode 100644 tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml
> > 
> > diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng
> > index 26f0aa2..927fc18 100644
> > --- a/docs/schemas/capability.rng
> > +++ b/docs/schemas/capability.rng
> > @@ -277,6 +277,26 @@
> > <attribute name='cpus'>
> > <ref name='cpuset'/>
> > </attribute>
> > + <zeroOrMore>
> > + <element name='control'>
> > + <attribute name='min'>
> > + <ref name='unsignedInt'/>
> > + </attribute>
> > + <attribute name='unit'>
> > + <ref name='unit'/>
> > + </attribute>
> > + <attribute name='scope'>
> > 
> 
> 
> This should be 'type', as discussed before.
> 
> > + <choice>
> > + <value>both</value>
> > + <value>code</value>
> > + <value>data</value>
> > + </choice>
> > + </attribute>
> > 
> 
> 
> And hence this could be its own new definition, since it is already used
> in the bank definition.
> 
> > + <attribute name='max_allocation'>
> 
> Since we are trying to prefer camelCase, even in the XML, and this
> should be plural, I would suggest 'maxAllocs'.
> 
> > + <ref name='unsignedInt'/>
> > + </attribute>
> > + </element>
> > + </zeroOrMore>
> > </element>
> > </oneOrMore>
> > </element>
> > diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
> > index d699b08..c4a1fdf 100644
> > --- a/src/conf/capabilities.c
> > +++ b/src/conf/capabilities.c
> > @@ -51,6 +51,7 @@
> > #define VIR_FROM_THIS VIR_FROM_CAPABILITIES
> > 
> > #define SYSFS_SYSTEM_PATH "/sys/devices/system"
> > +#define SYSFS_RESCTRL_PATH "/sys/fs/resctrl"
> > 
> > VIR_LOG_INIT("conf.capabilities")
> > 
> > @@ -872,6 +873,9 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
> > virCapsHostCacheBankPtr *caches)
> > {
> > size_t i = 0;
> > + size_t j = 0;
> > + int indent = virBufferGetIndent(buf, false);
> > + virBuffer controlBuf = VIR_BUFFER_INITIALIZER;
> > 
> > if (!ncaches)
> > return 0;
> > @@ -893,13 +897,35 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
> > */
> > virBufferAsprintf(buf,
> > "<bank id='%u' level='%u' type='%s' "
> > - "size='%llu' unit='%s' cpus='%s'/>\n",
> > + "size='%llu' unit='%s' cpus='%s'",
> > bank->id, bank->level,
> > virCacheTypeToString(bank->type),
> > bank->size >> (kilos * 10),
> > kilos ? "KiB" : "B",
> > cpus_str);
> > 
> > + virBufferAdjustIndent(&controlBuf, indent + 4);
> > + for (j = 0; j < bank->ncontrols; j++) {
> > + bool min_kilos = !(bank->controls[j]->min % 1024);
> > + virBufferAsprintf(&controlBuf,
> > + "<control min='%llu' unit='%s' "
> > + "scope='%s' max_allocation='%u'/>\n",
> > + bank->controls[j]->min >> (min_kilos * 10),
> > + min_kilos ? "KiB" : "B",
> > + virCacheTypeToString(bank->controls[j]->scope),
> > + bank->controls[j]->max_allocation);
> > + }
> > +
> > + if (virBufferUse(&controlBuf)) {
> > + virBufferAddLit(buf, ">\n");
> > + virBufferAddBuffer(buf, &controlBuf);
> > + virBufferAddLit(buf, "</bank>\n");
> > +
> > 
> 
> 
> Spurious line.
> 
> > + } else {
> > + virBufferAddLit(buf, "/>\n");
> > + }
> > +
> > + virBufferFreeAndReset(&controlBuf);
> > 
> 
> 
> No need for this, it is already done by virBufferAddBuffer.
> 
> > VIR_FREE(cpus_str);
> > }
> > 
> > @@ -1519,13 +1545,102 @@ virCapsHostCacheBankEquals(virCapsHostCacheBankPtr a,
> > void
> > virCapsHostCacheBankFree(virCapsHostCacheBankPtr ptr)
> > {
> > + size_t i;
> > +
> > if (!ptr)
> > return;
> > 
> > virBitmapFree(ptr->cpus);
> > + for (i = 0; i < ptr->ncontrols; i++)
> > + VIR_FREE(ptr->controls[i]);
> > + VIR_FREE(ptr->controls);
> > VIR_FREE(ptr);
> > }
> > 
> > +/* test which TYPE of cache control supported
> > + * -1: don't support
> > + * 0: cat
> > + * 1: cdp
> > + */
> > 
> 
> 
> I would slightly reword this comment.
> 
> > +static int
> > +virCapabilitiesGetCacheControlType(virCapsHostCacheBankPtr bank)
> > +{
> > + int ret = -1;
> > + char *path = NULL;
> > 
> 
> 
> Empty line here
> 
> > + if (virAsprintf(&path,
> > + SYSFS_RESCTRL_PATH "/info/L%u",
> > + bank->level) < 0)
> > + return -1;
> > +
> > + if (virFileExists(path)) {
> > + ret = 0;
> > + } else {
> > + VIR_FREE(path);
> > + /* for CDP enabled case, CODE and DATA will show together */
> > 
> 
> 
> "If CDP is enabled, there will be both CODE and DATA, but it's enough to
> check one of those only."
> 
> > + if (virAsprintf(&path,
> > + SYSFS_RESCTRL_PATH "/info/L%uCODE",
> > + bank->level) < 0)
> > + return -1;
> > + if (virFileExists(path))
> > + ret = 1;
> > + }
> > +
> > + VIR_FREE(path);
> > + return ret;
> > +}
> > +
> > +static int
> > +virCapabilitiesGetCacheControl(virCapsHostCacheBankPtr bank,
> > + virCacheType scope)
> > +{
> > + int ret = -1;
> > + char *path = NULL;
> > + char *cbm_mask = NULL;
> > + char *type_upper = NULL;
> > + virCapsHostCacheControlPtr control;
> > +
> > + if (VIR_ALLOC(control) < 0)
> > + goto cleanup;
> > +
> > + if ((scope > VIR_CACHE_TYPE_BOTH)
> > + && (virStringToUpper(&type_upper, virCacheTypeToString(scope)) < 0))
> > 
> 
> 
> Order comparison on enum values should be done only in rare cases,
> and there should then be some explanation why the order must be kept and
> where to add what new values. I would just do != here instead.
> 
> Also the indentation is wrong, the && belongs to the previous line and
> there's too much parentheses.
> 
> > + goto cleanup;
> > +
> > + if (virFileReadValueUint(&control->max_allocation,
> > + SYSFS_RESCTRL_PATH "/info/L%u%s/num_closids",
> > + bank->level,
> > + type_upper ? type_upper : "") < 0)
> > + goto cleanup;
> > +
> > + if (virFileReadValueString(&cbm_mask,
> > + SYSFS_RESCTRL_PATH
> > + "/info/L%u%s/cbm_mask",
> > + bank->level,
> > + type_upper ? type_upper: "") < 0)
> > + goto cleanup;
> > +
> > + virStringTrimOptionalNewline(cbm_mask);
> > +
> > + /* cbm_mask: cache bit mask, it's in hex, eg: fffff */
> > + control->min = bank->size / (strlen(cbm_mask) * 4);
> > +
> > 
> 
> 
> I believe this should be multiplied by min_cbm_bits.
> 
> > + control->scope = scope;
> > +
> > + if (VIR_APPEND_ELEMENT(bank->controls,
> > + bank->ncontrols,
> > + control) < 0)
> > + goto cleanup;
> > +
> > + ret = 0;
> > +
> > + cleanup:
> > + VIR_FREE(path);
> > + VIR_FREE(cbm_mask);
> > + VIR_FREE(type_upper);
> > + VIR_FREE(control);
> > + return ret;
> > +}
> > +
> > int
> > virCapabilitiesInitCaches(virCapsPtr caps)
> > {
> > @@ -1534,6 +1649,7 @@ virCapabilitiesInitCaches(virCapsPtr caps)
> > ssize_t pos = -1;
> > DIR *dirp = NULL;
> > int ret = -1;
> > + int typeret;
> > char *path = NULL;
> > char *type = NULL;
> > struct dirent *ent = NULL;
> > @@ -1607,12 +1723,27 @@ virCapabilitiesInitCaches(virCapsPtr caps)
> > SYSFS_SYSTEM_PATH, pos, ent->d_name) < 0)
> > goto cleanup;
> > 
> > + typeret = virCapabilitiesGetCacheControlType(bank);
> > +
> > + if (typeret == 0) {
> > + if (virCapabilitiesGetCacheControl(bank,
> > + VIR_CACHE_TYPE_BOTH) < 0)
> > + goto cleanup;
> > + } else if (typeret == 1) {
> > + if ((virCapabilitiesGetCacheControl(bank,
> > + VIR_CACHE_TYPE_CODE) < 0) ||
> > + (virCapabilitiesGetCacheControl(bank,
> > + VIR_CACHE_TYPE_DATA) < 0))
> > 
> 
> 
> Again, wrong indentation and unnecessary parentheses.
> 
> Otherwise it looks good, so ACK with those differences and reworded
> commit message. Let me know if you agree and I can push it immediately.
> The suggested diff follows:
> 
> diff --git i/docs/schemas/capability.rng w/docs/schemas/capability.rng
> index 927fc184de41..e5cbfa362ec0 100644
> --- i/docs/schemas/capability.rng
> +++ w/docs/schemas/capability.rng
> @@ -261,13 +261,7 @@
> <attribute name='level'>
> <ref name='unsignedInt'/>
> </attribute>
> - <attribute name='type'>
> - <choice>
> - <value>both</value>
> - <value>code</value>
> - <value>data</value>
> - </choice>
> - </attribute>
> + <ref name='cacheType'/>
> <attribute name='size'>
> <ref name='unsignedInt'/>
> </attribute>
> @@ -285,14 +279,8 @@
> <attribute name='unit'>
> <ref name='unit'/>
> </attribute>
> - <attribute name='scope'>
> - <choice>
> - <value>both</value>
> - <value>code</value>
> - <value>data</value>
> - </choice>
> - </attribute>
> - <attribute name='max_allocation'>
> + <ref name='cacheType'/>
> + <attribute name='maxAllocs'>
> <ref name='unsignedInt'/>
> </attribute>
> </element>
> @@ -302,6 +290,16 @@
> </element>
> </define>
> 
> + <define name='cacheType'>
> + <attribute name='type'>
> + <choice>
> + <value>both</value>
> + <value>code</value>
> + <value>data</value>
> + </choice>
> + </attribute>
> + </define>
> +
> <define name='guestcaps'>
> <element name='guest'>
> <ref name='ostype'/>
> diff --git i/src/conf/capabilities.c w/src/conf/capabilities.c
> index 8cd2957e9c88..3becc7e18c62 100644
> --- i/src/conf/capabilities.c
> +++ w/src/conf/capabilities.c
> @@ -909,7 +909,7 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
> bool min_kilos = !(bank->controls[j]->min % 1024);
> virBufferAsprintf(&controlBuf,
> "<control min='%llu' unit='%s' "
> - "scope='%s' max_allocation='%u'/>\n",
> + "type='%s' maxAllocs='%u'/>\n",
> bank->controls[j]->min >> (min_kilos * 10),
> min_kilos ? "KiB" : "B",
> virCacheTypeToString(bank->controls[j]->scope),
> @@ -920,12 +920,10 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
> virBufferAddLit(buf, ">\n");
> virBufferAddBuffer(buf, &controlBuf);
> virBufferAddLit(buf, "</bank>\n");
> -
> } else {
> virBufferAddLit(buf, "/>\n");
> }
> 
> - virBufferFreeAndReset(&controlBuf);
> VIR_FREE(cpus_str);
> }
> 
> @@ -1557,16 +1555,19 @@ virCapsHostCacheBankFree(virCapsHostCacheBankPtr ptr)
> VIR_FREE(ptr);
> }
> 
> -/* test which TYPE of cache control supported
> - * -1: don't support
> - * 0: cat
> - * 1: cdp
> +/*
> + * This function tests which TYPE of cache control is supported
> + * Return values are:
> + * -1: not supported
> + * 0: CAT
> + * 1: CDP
> */
> static int
> virCapabilitiesGetCacheControlType(virCapsHostCacheBankPtr bank)
> {
> int ret = -1;
> char *path = NULL;
> +
> if (virAsprintf(&path,
> SYSFS_RESCTRL_PATH "/info/L%u",
> bank->level) < 0)
> @@ -1576,7 +1577,10 @@ virCapabilitiesGetCacheControlType(virCapsHostCacheBankPtr bank)
> ret = 0;
> } else {
> VIR_FREE(path);
> - /* for CDP enabled case, CODE and DATA will show together */
> + /*
> + * If CDP is enabled, there will be both CODE and DATA, but it's enough
> + * to check one of those only.
> + */
> if (virAsprintf(&path,
> SYSFS_RESCTRL_PATH "/info/L%uCODE",
> bank->level) < 0)
> @@ -1597,13 +1601,14 @@ virCapabilitiesGetCacheControl(virCapsHostCacheBankPtr bank,
> char *path = NULL;
> char *cbm_mask = NULL;
> char *type_upper = NULL;
> + unsigned int min_cbm_bits = 0;
> virCapsHostCacheControlPtr control;
> 
> if (VIR_ALLOC(control) < 0)
> goto cleanup;
> 
> - if ((scope > VIR_CACHE_TYPE_BOTH)
> - && (virStringToUpper(&type_upper, virCacheTypeToString(scope)) < 0))
> + if (scope != VIR_CACHE_TYPE_BOTH &&
> + virStringToUpper(&type_upper, virCacheTypeToString(scope)) < 0)
> goto cleanup;
> 
> if (virFileReadValueUint(&control->max_allocation,
> @@ -1619,10 +1624,16 @@ virCapabilitiesGetCacheControl(virCapsHostCacheBankPtr bank,
> type_upper ? type_upper: "") < 0)
> goto cleanup;
> 
> + if (virFileReadValueUint(&min_cbm_bits,
> + SYSFS_RESCTRL_PATH "/info/L%u%s/min_cbm_bits",
> + bank->level,
> + type_upper ? type_upper : "") < 0)
> + goto cleanup;
> +
> virStringTrimOptionalNewline(cbm_mask);
> 
> /* cbm_mask: cache bit mask, it's in hex, eg: fffff */
> - control->min = bank->size / (strlen(cbm_mask) * 4);
> + control->min = min_cbm_bits * bank->size / (strlen(cbm_mask) * 4);
> 
> control->scope = scope;
> 
> @@ -1732,10 +1743,10 @@ virCapabilitiesInitCaches(virCapsPtr caps)
> VIR_CACHE_TYPE_BOTH) < 0)
> goto cleanup;
> } else if (typeret == 1) {
> - if ((virCapabilitiesGetCacheControl(bank,
> - VIR_CACHE_TYPE_CODE) < 0) ||
> - (virCapabilitiesGetCacheControl(bank,
> - VIR_CACHE_TYPE_DATA) < 0))
> + if (virCapabilitiesGetCacheControl(bank,
> + VIR_CACHE_TYPE_CODE) < 0 ||
> + virCapabilitiesGetCacheControl(bank,
> + VIR_CACHE_TYPE_DATA) < 0)
> goto cleanup;
> }
> 
> diff --git i/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml w/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml
> index c9f460d8a227..49aa0b98ca88 100644
> --- i/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml
> +++ w/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml
> @@ -42,12 +42,12 @@
> </topology>
> <cache>
> <bank id='0' level='3' type='both' size='15360' unit='KiB' cpus='0-5'>
> - <control min='768' unit='KiB' scope='code' max_allocation='8'/>
> - <control min='768' unit='KiB' scope='data' max_allocation='8'/>
> + <control min='768' unit='KiB' type='code' maxAllocs='8'/>
> + <control min='768' unit='KiB' type='data' maxAllocs='8'/>
> </bank>
> <bank id='1' level='3' type='both' size='15360' unit='KiB' cpus='6-11'>
> - <control min='768' unit='KiB' scope='code' max_allocation='8'/>
> - <control min='768' unit='KiB' scope='data' max_allocation='8'/>
> + <control min='768' unit='KiB' type='code' maxAllocs='8'/>
> + <control min='768' unit='KiB' type='data' maxAllocs='8'/>
> </bank>
> </cache>
> </host>
> diff --git i/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml w/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml
> index 04a5eb895727..cb78b4ab788d 100644
> --- i/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml
> +++ w/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml
> @@ -42,10 +42,10 @@
> </topology>
> <cache>
> <bank id='0' level='3' type='both' size='15360' unit='KiB' cpus='0-5'>
> - <control min='768' unit='KiB' scope='both' max_allocation='4'/>
> + <control min='1536' unit='KiB' type='both' maxAllocs='4'/>
> </bank>
> <bank id='1' level='3' type='both' size='15360' unit='KiB' cpus='6-11'>
> - <control min='768' unit='KiB' scope='both' max_allocation='4'/>
> + <control min='1536' unit='KiB' type='both' maxAllocs='4'/>
> </bank>
> </cache>
> </host>
> --
> Martin
> 
> 


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

Re: [libvirt] [V4 RESEND PATCH] Expose resource control capabilites on cache bank

Posted by Eli Qiao 6 years, 10 months ago

Martin:

Please hold on merge this first.

I found we still need some modification.  

> >  
> > Again, wrong indentation and unnecessary parentheses.
> >  
> > Otherwise it looks good, so ACK with those differences and reworded
> > commit message. Let me know if you agree and I can push it immediately.
> > The suggested diff follows:
> >  
> > diff --git i/docs/schemas/capability.rng w/docs/schemas/capability.rng
> > index 927fc184de41..e5cbfa362ec0 100644
> > --- i/docs/schemas/capability.rng
> > +++ w/docs/schemas/capability.rng
> > @@ -261,13 +261,7 @@
> > <attribute name='level'>
> > <ref name='unsignedInt'/>
> > </attribute>
> > - <attribute name='type'>
> > - <choice>
> > - <value>both</value>
> > - <value>code</value>
> > - <value>data</value>
> > - </choice>
> > - </attribute>
> > + <ref name='cacheType'/>
> > <attribute name='size'>
> > <ref name='unsignedInt'/>
> > </attribute>
> > @@ -285,14 +279,8 @@
> > <attribute name='unit'>
> > <ref name='unit'/>
> > </attribute>
> > - <attribute name='scope'>
> > - <choice>
> > - <value>both</value>
> > - <value>code</value>
> > - <value>data</value>
> > - </choice>
> > - </attribute>
> > - <attribute name='max_allocation'>
> > + <ref name='cacheType'/>
> > + <attribute name='maxAllocs'>
> > <ref name='unsignedInt'/>
> > </attribute>
> > </element>
> > @@ -302,6 +290,16 @@
> > </element>
> > </define>
> >  
> > + <define name='cacheType'>
> > + <attribute name='type'>
> > + <choice>
> > + <value>both</value>
> > + <value>code</value>
> > + <value>data</value>
> > + </choice>
> > + </attribute>
> > + </define>
> > +
> > <define name='guestcaps'>
> > <element name='guest'>
> > <ref name='ostype'/>
> > diff --git i/src/conf/capabilities.c w/src/conf/capabilities.c
> > index 8cd2957e9c88..3becc7e18c62 100644
> > --- i/src/conf/capabilities.c
> > +++ w/src/conf/capabilities.c
> > @@ -909,7 +909,7 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
> > bool min_kilos = !(bank->controls[j]->min % 1024);
> > virBufferAsprintf(&controlBuf,
> > "<control min='%llu' unit='%s' "
> > - "scope='%s' max_allocation='%u'/>\n",
> > + "type='%s' maxAllocs='%u'/>\n",
> > bank->controls[j]->min >> (min_kilos * 10),
> > min_kilos ? "KiB" : "B",
> > virCacheTypeToString(bank->controls[j]->scope),
> > @@ -920,12 +920,10 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
> > virBufferAddLit(buf, ">\n");
> > virBufferAddBuffer(buf, &controlBuf);
> > virBufferAddLit(buf, "</bank>\n");
> > -
> > } else {
> > virBufferAddLit(buf, "/>\n");
> > }
> >  
> > - virBufferFreeAndReset(&controlBuf);
> > VIR_FREE(cpus_str);
> > }
> >  
> > @@ -1557,16 +1555,19 @@ virCapsHostCacheBankFree(virCapsHostCacheBankPtr ptr)
> > VIR_FREE(ptr);
> > }
> >  
> > -/* test which TYPE of cache control supported
> > - * -1: don't support
> > - * 0: cat
> > - * 1: cdp
> > +/*
> > + * This function tests which TYPE of cache control is supported
> > + * Return values are:
> > + * -1: not supported
> > + * 0: CAT
> > + * 1: CDP
> > */
> > static int
> > virCapabilitiesGetCacheControlType(virCapsHostCacheBankPtr bank)
> > {
> > int ret = -1;
> > char *path = NULL;
> > +
> > if (virAsprintf(&path,
> > SYSFS_RESCTRL_PATH "/info/L%u",
> > bank->level) < 0)
> > @@ -1576,7 +1577,10 @@ virCapabilitiesGetCacheControlType(virCapsHostCacheBankPtr bank)
> > ret = 0;
> > } else {
> > VIR_FREE(path);
> > - /* for CDP enabled case, CODE and DATA will show together */
> > + /*
> > + * If CDP is enabled, there will be both CODE and DATA, but it's enough
> > + * to check one of those only.
> > + */
> > if (virAsprintf(&path,
> > SYSFS_RESCTRL_PATH "/info/L%uCODE",
> > bank->level) < 0)
> > @@ -1597,13 +1601,14 @@ virCapabilitiesGetCacheControl(virCapsHostCacheBankPtr bank,
> > char *path = NULL;
> > char *cbm_mask = NULL;
> > char *type_upper = NULL;
> > + unsigned int min_cbm_bits = 0;
> > virCapsHostCacheControlPtr control;
> >  
> > if (VIR_ALLOC(control) < 0)
> > goto cleanup;
> >  
> > - if ((scope > VIR_CACHE_TYPE_BOTH)
> > - && (virStringToUpper(&type_upper, virCacheTypeToString(scope)) < 0))
> > + if (scope != VIR_CACHE_TYPE_BOTH &&
> > + virStringToUpper(&type_upper, virCacheTypeToString(scope)) < 0)
> > goto cleanup;
> >  
> > if (virFileReadValueUint(&control->max_allocation,
> > @@ -1619,10 +1624,16 @@ virCapabilitiesGetCacheControl(virCapsHostCacheBankPtr bank,
> > type_upper ? type_upper: "") < 0)
> > goto cleanup;
> >  
> > + if (virFileReadValueUint(&min_cbm_bits,
> > + SYSFS_RESCTRL_PATH "/info/L%u%s/min_cbm_bits",
> > + bank->level,
> > + type_upper ? type_upper : "") < 0)
> > + goto cleanup;
> > +
> > virStringTrimOptionalNewline(cbm_mask);
> >  
> > /* cbm_mask: cache bit mask, it's in hex, eg: fffff */
> > - control->min = bank->size / (strlen(cbm_mask) * 4);
> > + control->min = min_cbm_bits * bank->size / (strlen(cbm_mask) * 4);
> >  

This could be wrong on new platform in Intel’s SKX CPU after check with platform guys.

The cbm_mask is “7ff” (11 bits) on SKX. I will refine this by counting the bits.

We can virFileReadValueString() then convert it to unsigned int, then count the bits of ‘1’.

thought ? or a new utils function in virfile is required?

I am not sure min_cbm_bits should be used.

Besides, on old platform (haswell), the min_cbm_bits is 2, but we still can specify a cbm like
“0x7” ) 3bit while do the cache allocation.

Later we need this on doing cache allocation(instead of read these value from the host again), we will
need
1. total cache size
2. what the cache size of 1 (CBM) bit stand for, so that we can calculate how many bits we want.

Maybe we need to add another filed like ’step’ (to indicate 1 bits stand for) ?

> >      control->scope = scope;
> >  
> > @@ -1732,10 +1743,10 @@ virCapabilitiesInitCaches(virCapsPtr caps)
> > VIR_CACHE_TYPE_BOTH) < 0)
> > goto cleanup;
> > } else if (typeret == 1) {
> > - if ((virCapabilitiesGetCacheControl(bank,
> > - VIR_CACHE_TYPE_CODE) < 0) ||
> > - (virCapabilitiesGetCacheControl(bank,
> > - VIR_CACHE_TYPE_DATA) < 0))
> > + if (virCapabilitiesGetCacheControl(bank,
> > + VIR_CACHE_TYPE_CODE) < 0 ||
> > + virCapabilitiesGetCacheControl(bank,
> > + VIR_CACHE_TYPE_DATA) < 0)
> > goto cleanup;
> > }
> >  
> > diff --git i/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml w/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml
> > index c9f460d8a227..49aa0b98ca88 100644
> > --- i/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml
> > +++ w/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml
> > @@ -42,12 +42,12 @@
> > </topology>
> > <cache>
> > <bank id='0' level='3' type='both' size='15360' unit='KiB' cpus='0-5'>
> > - <control min='768' unit='KiB' scope='code' max_allocation='8'/>
> > - <control min='768' unit='KiB' scope='data' max_allocation='8'/>
> > + <control min='768' unit='KiB' type='code' maxAllocs='8'/>
> > + <control min='768' unit='KiB' type='data' maxAllocs='8'/>
> > </bank>
> > <bank id='1' level='3' type='both' size='15360' unit='KiB' cpus='6-11'>
> > - <control min='768' unit='KiB' scope='code' max_allocation='8'/>
> > - <control min='768' unit='KiB' scope='data' max_allocation='8'/>
> > + <control min='768' unit='KiB' type='code' maxAllocs='8'/>
> > + <control min='768' unit='KiB' type='data' maxAllocs='8'/>
> > </bank>
> > </cache>
> > </host>
> > diff --git i/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml w/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml
> > index 04a5eb895727..cb78b4ab788d 100644
> > --- i/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml
> > +++ w/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml
> > @@ -42,10 +42,10 @@
> > </topology>
> > <cache>
> > <bank id='0' level='3' type='both' size='15360' unit='KiB' cpus='0-5'>
> > - <control min='768' unit='KiB' scope='both' max_allocation='4'/>
> > + <control min='1536' unit='KiB' type='both' maxAllocs='4'/>
> > </bank>
> > <bank id='1' level='3' type='both' size='15360' unit='KiB' cpus='6-11'>
> > - <control min='768' unit='KiB' scope='both' max_allocation='4'/>
> > + <control min='1536' unit='KiB' type='both' maxAllocs='4'/>
> >  
> >  
> >  
>  
>  
>  

  
> > </bank>
> > </cache>
> > </host>
> > --
> > Martin
> >  
> >  
> >  
>  
>  

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

Re: [libvirt] [V4 RESEND PATCH] Expose resource control capabilites on cache bank

Posted by Martin Kletzander 6 years, 10 months ago

On Mon, Jun 05, 2017 at 03:20:56PM +0800, Eli Qiao wrote:
>Martin:
>
>Please hold on merge this first.
>
>I found we still need some modification.
>
>> >
>> > Again, wrong indentation and unnecessary parentheses.
>> >
>> > Otherwise it looks good, so ACK with those differences and reworded
>> > commit message. Let me know if you agree and I can push it immediately.
>> > The suggested diff follows:
>> >
>> > diff --git i/docs/schemas/capability.rng w/docs/schemas/capability.rng
>> > index 927fc184de41..e5cbfa362ec0 100644
>> > --- i/docs/schemas/capability.rng
>> > +++ w/docs/schemas/capability.rng
>> > @@ -261,13 +261,7 @@
>> > <attribute name='level'>
>> > <ref name='unsignedInt'/>
>> > </attribute>
>> > - <attribute name='type'>
>> > - <choice>
>> > - <value>both</value>
>> > - <value>code</value>
>> > - <value>data</value>
>> > - </choice>
>> > - </attribute>
>> > + <ref name='cacheType'/>
>> > <attribute name='size'>
>> > <ref name='unsignedInt'/>
>> > </attribute>
>> > @@ -285,14 +279,8 @@
>> > <attribute name='unit'>
>> > <ref name='unit'/>
>> > </attribute>
>> > - <attribute name='scope'>
>> > - <choice>
>> > - <value>both</value>
>> > - <value>code</value>
>> > - <value>data</value>
>> > - </choice>
>> > - </attribute>
>> > - <attribute name='max_allocation'>
>> > + <ref name='cacheType'/>
>> > + <attribute name='maxAllocs'>
>> > <ref name='unsignedInt'/>
>> > </attribute>
>> > </element>
>> > @@ -302,6 +290,16 @@
>> > </element>
>> > </define>
>> >
>> > + <define name='cacheType'>
>> > + <attribute name='type'>
>> > + <choice>
>> > + <value>both</value>
>> > + <value>code</value>
>> > + <value>data</value>
>> > + </choice>
>> > + </attribute>
>> > + </define>
>> > +
>> > <define name='guestcaps'>
>> > <element name='guest'>
>> > <ref name='ostype'/>
>> > diff --git i/src/conf/capabilities.c w/src/conf/capabilities.c
>> > index 8cd2957e9c88..3becc7e18c62 100644
>> > --- i/src/conf/capabilities.c
>> > +++ w/src/conf/capabilities.c
>> > @@ -909,7 +909,7 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
>> > bool min_kilos = !(bank->controls[j]->min % 1024);
>> > virBufferAsprintf(&controlBuf,
>> > "<control min='%llu' unit='%s' "
>> > - "scope='%s' max_allocation='%u'/>\n",
>> > + "type='%s' maxAllocs='%u'/>\n",
>> > bank->controls[j]->min >> (min_kilos * 10),
>> > min_kilos ? "KiB" : "B",
>> > virCacheTypeToString(bank->controls[j]->scope),
>> > @@ -920,12 +920,10 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
>> > virBufferAddLit(buf, ">\n");
>> > virBufferAddBuffer(buf, &controlBuf);
>> > virBufferAddLit(buf, "</bank>\n");
>> > -
>> > } else {
>> > virBufferAddLit(buf, "/>\n");
>> > }
>> >
>> > - virBufferFreeAndReset(&controlBuf);
>> > VIR_FREE(cpus_str);
>> > }
>> >
>> > @@ -1557,16 +1555,19 @@ virCapsHostCacheBankFree(virCapsHostCacheBankPtr ptr)
>> > VIR_FREE(ptr);
>> > }
>> >
>> > -/* test which TYPE of cache control supported
>> > - * -1: don't support
>> > - * 0: cat
>> > - * 1: cdp
>> > +/*
>> > + * This function tests which TYPE of cache control is supported
>> > + * Return values are:
>> > + * -1: not supported
>> > + * 0: CAT
>> > + * 1: CDP
>> > */
>> > static int
>> > virCapabilitiesGetCacheControlType(virCapsHostCacheBankPtr bank)
>> > {
>> > int ret = -1;
>> > char *path = NULL;
>> > +
>> > if (virAsprintf(&path,
>> > SYSFS_RESCTRL_PATH "/info/L%u",
>> > bank->level) < 0)
>> > @@ -1576,7 +1577,10 @@ virCapabilitiesGetCacheControlType(virCapsHostCacheBankPtr bank)
>> > ret = 0;
>> > } else {
>> > VIR_FREE(path);
>> > - /* for CDP enabled case, CODE and DATA will show together */
>> > + /*
>> > + * If CDP is enabled, there will be both CODE and DATA, but it's enough
>> > + * to check one of those only.
>> > + */
>> > if (virAsprintf(&path,
>> > SYSFS_RESCTRL_PATH "/info/L%uCODE",
>> > bank->level) < 0)
>> > @@ -1597,13 +1601,14 @@ virCapabilitiesGetCacheControl(virCapsHostCacheBankPtr bank,
>> > char *path = NULL;
>> > char *cbm_mask = NULL;
>> > char *type_upper = NULL;
>> > + unsigned int min_cbm_bits = 0;
>> > virCapsHostCacheControlPtr control;
>> >
>> > if (VIR_ALLOC(control) < 0)
>> > goto cleanup;
>> >
>> > - if ((scope > VIR_CACHE_TYPE_BOTH)
>> > - && (virStringToUpper(&type_upper, virCacheTypeToString(scope)) < 0))
>> > + if (scope != VIR_CACHE_TYPE_BOTH &&
>> > + virStringToUpper(&type_upper, virCacheTypeToString(scope)) < 0)
>> > goto cleanup;
>> >
>> > if (virFileReadValueUint(&control->max_allocation,
>> > @@ -1619,10 +1624,16 @@ virCapabilitiesGetCacheControl(virCapsHostCacheBankPtr bank,
>> > type_upper ? type_upper: "") < 0)
>> > goto cleanup;
>> >
>> > + if (virFileReadValueUint(&min_cbm_bits,
>> > + SYSFS_RESCTRL_PATH "/info/L%u%s/min_cbm_bits",
>> > + bank->level,
>> > + type_upper ? type_upper : "") < 0)
>> > + goto cleanup;
>> > +
>> > virStringTrimOptionalNewline(cbm_mask);
>> >
>> > /* cbm_mask: cache bit mask, it's in hex, eg: fffff */
>> > - control->min = bank->size / (strlen(cbm_mask) * 4);
>> > + control->min = min_cbm_bits * bank->size / (strlen(cbm_mask) * 4);
>> >
>
>This could be wrong on new platform in Intel’s SKX CPU after check with platform guys.
>
>The cbm_mask is “7ff” (11 bits) on SKX. I will refine this by counting the bits.
>
>We can virFileReadValueString() then convert it to unsigned int, then count the bits of ‘1’.
>
>thought ? or a new utils function in virfile is required?
>

I missed this patch, sorry.  I was thinking about changing it to bit
counting, but I thought it will always be divisible by 4.  I will send a
trivial update to this, but it would be nice to have test cases for it.
Could you get some of that data from such a machine n the meantime?  If
not, I can just coy what we have and change the mask.

>I am not sure min_cbm_bits should be used.
>
>Besides, on old platform (haswell), the min_cbm_bits is 2, but we still can specify a cbm like
>“0x7” ) 3bit while do the cache allocation.
>

Yes, you can.  min_cbm_bits is not a granularity.  it is the minimum
number of consecutive bits that you need to have when allocating.  We
should also add the granularity there.  I'll fix that as well then.

>Later we need this on doing cache allocation(instead of read these value from the host again), we will
>need
>1. total cache size
>2. what the cache size of 1 (CBM) bit stand for, so that we can calculate how many bits we want.
>
>Maybe we need to add another filed like ’step’ (to indicate 1 bits stand for) ?
>
>> >      control->scope = scope;
>> >
>> > @@ -1732,10 +1743,10 @@ virCapabilitiesInitCaches(virCapsPtr caps)
>> > VIR_CACHE_TYPE_BOTH) < 0)
>> > goto cleanup;
>> > } else if (typeret == 1) {
>> > - if ((virCapabilitiesGetCacheControl(bank,
>> > - VIR_CACHE_TYPE_CODE) < 0) ||
>> > - (virCapabilitiesGetCacheControl(bank,
>> > - VIR_CACHE_TYPE_DATA) < 0))
>> > + if (virCapabilitiesGetCacheControl(bank,
>> > + VIR_CACHE_TYPE_CODE) < 0 ||
>> > + virCapabilitiesGetCacheControl(bank,
>> > + VIR_CACHE_TYPE_DATA) < 0)
>> > goto cleanup;
>> > }
>> >
>> > diff --git i/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml w/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml
>> > index c9f460d8a227..49aa0b98ca88 100644
>> > --- i/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml
>> > +++ w/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml
>> > @@ -42,12 +42,12 @@
>> > </topology>
>> > <cache>
>> > <bank id='0' level='3' type='both' size='15360' unit='KiB' cpus='0-5'>
>> > - <control min='768' unit='KiB' scope='code' max_allocation='8'/>
>> > - <control min='768' unit='KiB' scope='data' max_allocation='8'/>
>> > + <control min='768' unit='KiB' type='code' maxAllocs='8'/>
>> > + <control min='768' unit='KiB' type='data' maxAllocs='8'/>
>> > </bank>
>> > <bank id='1' level='3' type='both' size='15360' unit='KiB' cpus='6-11'>
>> > - <control min='768' unit='KiB' scope='code' max_allocation='8'/>
>> > - <control min='768' unit='KiB' scope='data' max_allocation='8'/>
>> > + <control min='768' unit='KiB' type='code' maxAllocs='8'/>
>> > + <control min='768' unit='KiB' type='data' maxAllocs='8'/>
>> > </bank>
>> > </cache>
>> > </host>
>> > diff --git i/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml w/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml
>> > index 04a5eb895727..cb78b4ab788d 100644
>> > --- i/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml
>> > +++ w/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml
>> > @@ -42,10 +42,10 @@
>> > </topology>
>> > <cache>
>> > <bank id='0' level='3' type='both' size='15360' unit='KiB' cpus='0-5'>
>> > - <control min='768' unit='KiB' scope='both' max_allocation='4'/>
>> > + <control min='1536' unit='KiB' type='both' maxAllocs='4'/>
>> > </bank>
>> > <bank id='1' level='3' type='both' size='15360' unit='KiB' cpus='6-11'>
>> > - <control min='768' unit='KiB' scope='both' max_allocation='4'/>
>> > + <control min='1536' unit='KiB' type='both' maxAllocs='4'/>
>> >
>> >
>> >
>>
>>
>>
>
>
>> > </bank>
>> > </cache>
>> > </host>
>> > --
>> > Martin
>> >
>> >
>> >
>>
>>
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [V4 RESEND PATCH] Expose resource control capabilites on cache bank

Posted by Eli Qiao 6 years, 10 months ago

> >  
> > This could be wrong on new platform in Intel’s SKX CPU after check with platform guys.
> >  
> > The cbm_mask is “7ff” (11 bits) on SKX. I will refine this by counting the bits.
> >  
> > We can virFileReadValueString() then convert it to unsigned int, then count the bits of ‘1’.
> >  
> > thought ? or a new utils function in virfile is required?
>  
> I missed this patch, sorry. I was thinking about changing it to bit
> counting, but I thought it will always be divisible by 4. I will send a
> trivial update to this, but it would be nice to have test cases for it.
> Could you get some of that data from such a machine n the meantime? If
> not, I can just coy what we have and change the mask.
>  
I don’t have a SKX machine by hand, I ask from other guys, the only different is  

cat /sys/fs/resctrl/info/L3/cbm_mask

7ff

it’s 11 bits, which can not divided by 4.  :( I don’t understand why the hardware designed as this.

////////////////////////// below are the /sys/fs/resctrl contents on SKX /////////////////////////

# cd /sys/fs/resctrl

# mkdir p1

# tree /sys/fs/resctrl/

/sys/fs/resctrl/

├── cpus

├── info

│   └── L3

│       ├── cbm_mask

│       ├── min_cbm_bits

│       └── num_closids

├── p1

│   ├── cpus

│   ├── schemata

│   └── tasks

├── schemata

└── tasks

  

# cat /sys/fs/resctrl/info/L3/cbm_mask

7ff

  

# cat /sys/fs/resctrl/info/L3/min_cbm_bits

1

  

# cat /sys/fs/resctrl/info/L3/num_closids

16

  

# cat /sys/fs/resctrl/cpus

00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,ffffffff,ffffffff,ffffffff

  

# cat /sys/fs/resctrl/schemata

L3:0=7ff;1=7ff
  
>  
> > I am not sure min_cbm_bits should be used.
> >  
> > Besides, on old platform (haswell), the min_cbm_bits is 2, but we still can specify a cbm like
> > “0x7” ) 3bit while do the cache allocation.
> >  
>  
>  
> Yes, you can. min_cbm_bits is not a granularity. it is the minimum
> number of consecutive bits that you need to have when allocating. We
> should also add the granularity there. I'll fix that as well then.
>  

Great,
for “haswell" platform, which is old and has bad support for CAT,  
 https://github.com/torvalds/linux/blob/master/arch/x86/kernel/cpu/intel_rdt.c#L166

I think we can deal with this in libvirt as a special case ?
  

  
>  
> > Later we need this on doing cache allocation(instead of read these value from the host again), we will
> > need
> > 1. total cache size
> > 2. what the cache size of 1 (CBM) bit stand for, so that we can calculate how many bits we want.
> >  
> > Maybe we need to add another filed like ’step’ (to indicate 1 bits stand for) ?
> >  
> > > > control->scope = scope;
> > > >  
> > > > @@ -1732,10 +1743,10 @@ virCapabilitiesInitCaches(virCapsPtr caps)
> > > > VIR_CACHE_TYPE_BOTH) < 0)
> > > > goto cleanup;
> > > > } else if (typeret == 1) {
> > > > - if ((virCapabilitiesGetCacheControl(bank,
> > > > - VIR_CACHE_TYPE_CODE) < 0) ||
> > > > - (virCapabilitiesGetCacheControl(bank,
> > > > - VIR_CACHE_TYPE_DATA) < 0))
> > > > + if (virCapabilitiesGetCacheControl(bank,
> > > > + VIR_CACHE_TYPE_CODE) < 0 ||
> > > > + virCapabilitiesGetCacheControl(bank,
> > > > + VIR_CACHE_TYPE_DATA) < 0)
> > > > goto cleanup;
> > > > }
> > > >  
> > > > diff --git i/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml w/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml
> > > > index c9f460d8a227..49aa0b98ca88 100644
> > > > --- i/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml
> > > > +++ w/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml
> > > > @@ -42,12 +42,12 @@
> > > > </topology>
> > > > <cache>
> > > > <bank id='0' level='3' type='both' size='15360' unit='KiB' cpus='0-5'>
> > > > - <control min='768' unit='KiB' scope='code' max_allocation='8'/>
> > > > - <control min='768' unit='KiB' scope='data' max_allocation='8'/>
> > > > + <control min='768' unit='KiB' type='code' maxAllocs='8'/>
> > > > + <control min='768' unit='KiB' type='data' maxAllocs='8'/>
> > > > </bank>
> > > > <bank id='1' level='3' type='both' size='15360' unit='KiB' cpus='6-11'>
> > > > - <control min='768' unit='KiB' scope='code' max_allocation='8'/>
> > > > - <control min='768' unit='KiB' scope='data' max_allocation='8'/>
> > > > + <control min='768' unit='KiB' type='code' maxAllocs='8'/>
> > > > + <control min='768' unit='KiB' type='data' maxAllocs='8'/>
> > > > </bank>
> > > > </cache>
> > > > </host>
> > > > diff --git i/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml w/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml
> > > > index 04a5eb895727..cb78b4ab788d 100644
> > > > --- i/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml
> > > > +++ w/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml
> > > > @@ -42,10 +42,10 @@
> > > > </topology>
> > > > <cache>
> > > > <bank id='0' level='3' type='both' size='15360' unit='KiB' cpus='0-5'>
> > > > - <control min='768' unit='KiB' scope='both' max_allocation='4'/>
> > > > + <control min='1536' unit='KiB' type='both' maxAllocs='4'/>
> > > > </bank>
> > > > <bank id='1' level='3' type='both' size='15360' unit='KiB' cpus='6-11'>
> > > > - <control min='768' unit='KiB' scope='both' max_allocation='4'/>
> > > > + <control min='1536' unit='KiB' type='both' maxAllocs='4'/>
> > > >  
> > >  
> >  
> >  
> >  
> > > > </bank>
> > > > </cache>
> > > > </host>
> > > > --
> > > > Martin
> > > >  
> > >  
> >  
> >  
>  
>  
>  


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

Re: [libvirt] [V4 RESEND PATCH] Expose resource control capabilites on cache bank

Posted by Martin Kletzander 6 years, 10 months ago

Pushed now.

On Mon, Jun 05, 2017 at 09:31:17AM +0800, Eli Qiao wrote:
>hi Martin
>Thanks for you reviewing and I am okay with the diff as  you suggested.
>
>Please help to push this patch.
>
>Eli.
>
>
>On Sunday, 4 June 2017 at 5:39 PM, Martin Kletzander wrote:
>
>> On Wed, May 17, 2017 at 05:08:33PM +0800, taget wrote:
>> > From: Eli Qiao <liyong.qiao@intel.com (mailto:liyong.qiao@intel.com)>
>> >
>> > * This patch amends the cache bank capability as follow:
>> >
>> > <cache>
>> > <bank id='0' level='3' type='unified' size='15360' unit='KiB' cpus='0-5'>
>> > <control min='768' unit='KiB' scope='both' max_allocation='4'/>
>> > </bank>
>> > <bank id='1' level='3' type='unified' size='15360' unit='KiB' cpus='6-11'>
>> > <control min='768' unit='KiB' scope='both' max_allocation='4'/>
>> > </bank>
>> > </cache>
>> >
>> > For CDP enabled on x86 arch, we will have:
>> > <cache>
>> > <bank id='0' level='3' type='unified' size='15360' unit='KiB' cpus='0-5'>
>> > <control min='768' unit='KiB' scope='code' max_allocation='4'/>
>> > <control min='768' unit='KiB' scope='data' max_allocation='4'/>
>> > </bank>
>> > ...
>> >
>> > * Added a new testdata directory `linux-resctrl-cdp` to test CDP enabled
>> > case.
>> >
>> > * Along with vircaps2xmltest case updated.
>> >
>> > P.S. here the scope is from /sys/fs/resctrl/info/L3{"" "code" "data"}, I
>> > keep it capital upper as I need to use a macro to convert from enum to
>> > string easily.
>> >
>> > Signed-off-by: Eli Qiao <liyong.qiao@intel.com (mailto:liyong.qiao@intel.com)>
>> > ---
>> > docs/schemas/capability.rng | 20 ++++
>> > src/conf/capabilities.c | 133 ++++++++++++++++++++-
>> > src/conf/capabilities.h | 10 ++
>> > .../vircaps2xmldata/linux-resctrl-cdp/resctrl/cpus | 1 +
>> > .../linux-resctrl-cdp/resctrl/info/L3CODE/cbm_mask | 1 +
>> > .../resctrl/info/L3CODE/min_cbm_bits | 1 +
>> > .../resctrl/info/L3CODE/num_closids | 1 +
>> > .../linux-resctrl-cdp/resctrl/info/L3DATA/cbm_mask | 1 +
>> > .../resctrl/info/L3DATA/min_cbm_bits | 1 +
>> > .../resctrl/info/L3DATA/num_closids | 1 +
>> > .../linux-resctrl-cdp/resctrl/manualres/cpus | 1 +
>> > .../linux-resctrl-cdp/resctrl/manualres/schemata | 2 +
>> > .../linux-resctrl-cdp/resctrl/manualres/tasks | 0
>> > .../linux-resctrl-cdp/resctrl/schemata | 2 +
>> > .../linux-resctrl-cdp/resctrl/tasks | 0
>> > tests/vircaps2xmldata/linux-resctrl-cdp/system | 1 +
>> > .../vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml | 55 +++++++++
>> > tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml | 8 +-
>> > tests/vircaps2xmltest.c | 8 ++
>> > 19 files changed, 244 insertions(+), 3 deletions(-)
>> > create mode 100644 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/cpus
>> > create mode 100755 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/cbm_mask
>> > create mode 100755 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/min_cbm_bits
>> > create mode 100755 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3CODE/num_closids
>> > create mode 100755 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/cbm_mask
>> > create mode 100755 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/min_cbm_bits
>> > create mode 100755 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/info/L3DATA/num_closids
>> > create mode 100644 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/cpus
>> > create mode 100644 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/schemata
>> > create mode 100644 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/manualres/tasks
>> > create mode 100644 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/schemata
>> > create mode 100644 tests/vircaps2xmldata/linux-resctrl-cdp/resctrl/tasks
>> > create mode 120000 tests/vircaps2xmldata/linux-resctrl-cdp/system
>> > create mode 100644 tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml
>> >
>> > diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng
>> > index 26f0aa2..927fc18 100644
>> > --- a/docs/schemas/capability.rng
>> > +++ b/docs/schemas/capability.rng
>> > @@ -277,6 +277,26 @@
>> > <attribute name='cpus'>
>> > <ref name='cpuset'/>
>> > </attribute>
>> > + <zeroOrMore>
>> > + <element name='control'>
>> > + <attribute name='min'>
>> > + <ref name='unsignedInt'/>
>> > + </attribute>
>> > + <attribute name='unit'>
>> > + <ref name='unit'/>
>> > + </attribute>
>> > + <attribute name='scope'>
>> >
>>
>>
>> This should be 'type', as discussed before.
>>
>> > + <choice>
>> > + <value>both</value>
>> > + <value>code</value>
>> > + <value>data</value>
>> > + </choice>
>> > + </attribute>
>> >
>>
>>
>> And hence this could be its own new definition, since it is already used
>> in the bank definition.
>>
>> > + <attribute name='max_allocation'>
>>
>> Since we are trying to prefer camelCase, even in the XML, and this
>> should be plural, I would suggest 'maxAllocs'.
>>
>> > + <ref name='unsignedInt'/>
>> > + </attribute>
>> > + </element>
>> > + </zeroOrMore>
>> > </element>
>> > </oneOrMore>
>> > </element>
>> > diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
>> > index d699b08..c4a1fdf 100644
>> > --- a/src/conf/capabilities.c
>> > +++ b/src/conf/capabilities.c
>> > @@ -51,6 +51,7 @@
>> > #define VIR_FROM_THIS VIR_FROM_CAPABILITIES
>> >
>> > #define SYSFS_SYSTEM_PATH "/sys/devices/system"
>> > +#define SYSFS_RESCTRL_PATH "/sys/fs/resctrl"
>> >
>> > VIR_LOG_INIT("conf.capabilities")
>> >
>> > @@ -872,6 +873,9 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
>> > virCapsHostCacheBankPtr *caches)
>> > {
>> > size_t i = 0;
>> > + size_t j = 0;
>> > + int indent = virBufferGetIndent(buf, false);
>> > + virBuffer controlBuf = VIR_BUFFER_INITIALIZER;
>> >
>> > if (!ncaches)
>> > return 0;
>> > @@ -893,13 +897,35 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
>> > */
>> > virBufferAsprintf(buf,
>> > "<bank id='%u' level='%u' type='%s' "
>> > - "size='%llu' unit='%s' cpus='%s'/>\n",
>> > + "size='%llu' unit='%s' cpus='%s'",
>> > bank->id, bank->level,
>> > virCacheTypeToString(bank->type),
>> > bank->size >> (kilos * 10),
>> > kilos ? "KiB" : "B",
>> > cpus_str);
>> >
>> > + virBufferAdjustIndent(&controlBuf, indent + 4);
>> > + for (j = 0; j < bank->ncontrols; j++) {
>> > + bool min_kilos = !(bank->controls[j]->min % 1024);
>> > + virBufferAsprintf(&controlBuf,
>> > + "<control min='%llu' unit='%s' "
>> > + "scope='%s' max_allocation='%u'/>\n",
>> > + bank->controls[j]->min >> (min_kilos * 10),
>> > + min_kilos ? "KiB" : "B",
>> > + virCacheTypeToString(bank->controls[j]->scope),
>> > + bank->controls[j]->max_allocation);
>> > + }
>> > +
>> > + if (virBufferUse(&controlBuf)) {
>> > + virBufferAddLit(buf, ">\n");
>> > + virBufferAddBuffer(buf, &controlBuf);
>> > + virBufferAddLit(buf, "</bank>\n");
>> > +
>> >
>>
>>
>> Spurious line.
>>
>> > + } else {
>> > + virBufferAddLit(buf, "/>\n");
>> > + }
>> > +
>> > + virBufferFreeAndReset(&controlBuf);
>> >
>>
>>
>> No need for this, it is already done by virBufferAddBuffer.
>>
>> > VIR_FREE(cpus_str);
>> > }
>> >
>> > @@ -1519,13 +1545,102 @@ virCapsHostCacheBankEquals(virCapsHostCacheBankPtr a,
>> > void
>> > virCapsHostCacheBankFree(virCapsHostCacheBankPtr ptr)
>> > {
>> > + size_t i;
>> > +
>> > if (!ptr)
>> > return;
>> >
>> > virBitmapFree(ptr->cpus);
>> > + for (i = 0; i < ptr->ncontrols; i++)
>> > + VIR_FREE(ptr->controls[i]);
>> > + VIR_FREE(ptr->controls);
>> > VIR_FREE(ptr);
>> > }
>> >
>> > +/* test which TYPE of cache control supported
>> > + * -1: don't support
>> > + * 0: cat
>> > + * 1: cdp
>> > + */
>> >
>>
>>
>> I would slightly reword this comment.
>>
>> > +static int
>> > +virCapabilitiesGetCacheControlType(virCapsHostCacheBankPtr bank)
>> > +{
>> > + int ret = -1;
>> > + char *path = NULL;
>> >
>>
>>
>> Empty line here
>>
>> > + if (virAsprintf(&path,
>> > + SYSFS_RESCTRL_PATH "/info/L%u",
>> > + bank->level) < 0)
>> > + return -1;
>> > +
>> > + if (virFileExists(path)) {
>> > + ret = 0;
>> > + } else {
>> > + VIR_FREE(path);
>> > + /* for CDP enabled case, CODE and DATA will show together */
>> >
>>
>>
>> "If CDP is enabled, there will be both CODE and DATA, but it's enough to
>> check one of those only."
>>
>> > + if (virAsprintf(&path,
>> > + SYSFS_RESCTRL_PATH "/info/L%uCODE",
>> > + bank->level) < 0)
>> > + return -1;
>> > + if (virFileExists(path))
>> > + ret = 1;
>> > + }
>> > +
>> > + VIR_FREE(path);
>> > + return ret;
>> > +}
>> > +
>> > +static int
>> > +virCapabilitiesGetCacheControl(virCapsHostCacheBankPtr bank,
>> > + virCacheType scope)
>> > +{
>> > + int ret = -1;
>> > + char *path = NULL;
>> > + char *cbm_mask = NULL;
>> > + char *type_upper = NULL;
>> > + virCapsHostCacheControlPtr control;
>> > +
>> > + if (VIR_ALLOC(control) < 0)
>> > + goto cleanup;
>> > +
>> > + if ((scope > VIR_CACHE_TYPE_BOTH)
>> > + && (virStringToUpper(&type_upper, virCacheTypeToString(scope)) < 0))
>> >
>>
>>
>> Order comparison on enum values should be done only in rare cases,
>> and there should then be some explanation why the order must be kept and
>> where to add what new values. I would just do != here instead.
>>
>> Also the indentation is wrong, the && belongs to the previous line and
>> there's too much parentheses.
>>
>> > + goto cleanup;
>> > +
>> > + if (virFileReadValueUint(&control->max_allocation,
>> > + SYSFS_RESCTRL_PATH "/info/L%u%s/num_closids",
>> > + bank->level,
>> > + type_upper ? type_upper : "") < 0)
>> > + goto cleanup;
>> > +
>> > + if (virFileReadValueString(&cbm_mask,
>> > + SYSFS_RESCTRL_PATH
>> > + "/info/L%u%s/cbm_mask",
>> > + bank->level,
>> > + type_upper ? type_upper: "") < 0)
>> > + goto cleanup;
>> > +
>> > + virStringTrimOptionalNewline(cbm_mask);
>> > +
>> > + /* cbm_mask: cache bit mask, it's in hex, eg: fffff */
>> > + control->min = bank->size / (strlen(cbm_mask) * 4);
>> > +
>> >
>>
>>
>> I believe this should be multiplied by min_cbm_bits.
>>
>> > + control->scope = scope;
>> > +
>> > + if (VIR_APPEND_ELEMENT(bank->controls,
>> > + bank->ncontrols,
>> > + control) < 0)
>> > + goto cleanup;
>> > +
>> > + ret = 0;
>> > +
>> > + cleanup:
>> > + VIR_FREE(path);
>> > + VIR_FREE(cbm_mask);
>> > + VIR_FREE(type_upper);
>> > + VIR_FREE(control);
>> > + return ret;
>> > +}
>> > +
>> > int
>> > virCapabilitiesInitCaches(virCapsPtr caps)
>> > {
>> > @@ -1534,6 +1649,7 @@ virCapabilitiesInitCaches(virCapsPtr caps)
>> > ssize_t pos = -1;
>> > DIR *dirp = NULL;
>> > int ret = -1;
>> > + int typeret;
>> > char *path = NULL;
>> > char *type = NULL;
>> > struct dirent *ent = NULL;
>> > @@ -1607,12 +1723,27 @@ virCapabilitiesInitCaches(virCapsPtr caps)
>> > SYSFS_SYSTEM_PATH, pos, ent->d_name) < 0)
>> > goto cleanup;
>> >
>> > + typeret = virCapabilitiesGetCacheControlType(bank);
>> > +
>> > + if (typeret == 0) {
>> > + if (virCapabilitiesGetCacheControl(bank,
>> > + VIR_CACHE_TYPE_BOTH) < 0)
>> > + goto cleanup;
>> > + } else if (typeret == 1) {
>> > + if ((virCapabilitiesGetCacheControl(bank,
>> > + VIR_CACHE_TYPE_CODE) < 0) ||
>> > + (virCapabilitiesGetCacheControl(bank,
>> > + VIR_CACHE_TYPE_DATA) < 0))
>> >
>>
>>
>> Again, wrong indentation and unnecessary parentheses.
>>
>> Otherwise it looks good, so ACK with those differences and reworded
>> commit message. Let me know if you agree and I can push it immediately.
>> The suggested diff follows:
>>
>> diff --git i/docs/schemas/capability.rng w/docs/schemas/capability.rng
>> index 927fc184de41..e5cbfa362ec0 100644
>> --- i/docs/schemas/capability.rng
>> +++ w/docs/schemas/capability.rng
>> @@ -261,13 +261,7 @@
>> <attribute name='level'>
>> <ref name='unsignedInt'/>
>> </attribute>
>> - <attribute name='type'>
>> - <choice>
>> - <value>both</value>
>> - <value>code</value>
>> - <value>data</value>
>> - </choice>
>> - </attribute>
>> + <ref name='cacheType'/>
>> <attribute name='size'>
>> <ref name='unsignedInt'/>
>> </attribute>
>> @@ -285,14 +279,8 @@
>> <attribute name='unit'>
>> <ref name='unit'/>
>> </attribute>
>> - <attribute name='scope'>
>> - <choice>
>> - <value>both</value>
>> - <value>code</value>
>> - <value>data</value>
>> - </choice>
>> - </attribute>
>> - <attribute name='max_allocation'>
>> + <ref name='cacheType'/>
>> + <attribute name='maxAllocs'>
>> <ref name='unsignedInt'/>
>> </attribute>
>> </element>
>> @@ -302,6 +290,16 @@
>> </element>
>> </define>
>>
>> + <define name='cacheType'>
>> + <attribute name='type'>
>> + <choice>
>> + <value>both</value>
>> + <value>code</value>
>> + <value>data</value>
>> + </choice>
>> + </attribute>
>> + </define>
>> +
>> <define name='guestcaps'>
>> <element name='guest'>
>> <ref name='ostype'/>
>> diff --git i/src/conf/capabilities.c w/src/conf/capabilities.c
>> index 8cd2957e9c88..3becc7e18c62 100644
>> --- i/src/conf/capabilities.c
>> +++ w/src/conf/capabilities.c
>> @@ -909,7 +909,7 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
>> bool min_kilos = !(bank->controls[j]->min % 1024);
>> virBufferAsprintf(&controlBuf,
>> "<control min='%llu' unit='%s' "
>> - "scope='%s' max_allocation='%u'/>\n",
>> + "type='%s' maxAllocs='%u'/>\n",
>> bank->controls[j]->min >> (min_kilos * 10),
>> min_kilos ? "KiB" : "B",
>> virCacheTypeToString(bank->controls[j]->scope),
>> @@ -920,12 +920,10 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
>> virBufferAddLit(buf, ">\n");
>> virBufferAddBuffer(buf, &controlBuf);
>> virBufferAddLit(buf, "</bank>\n");
>> -
>> } else {
>> virBufferAddLit(buf, "/>\n");
>> }
>>
>> - virBufferFreeAndReset(&controlBuf);
>> VIR_FREE(cpus_str);
>> }
>>
>> @@ -1557,16 +1555,19 @@ virCapsHostCacheBankFree(virCapsHostCacheBankPtr ptr)
>> VIR_FREE(ptr);
>> }
>>
>> -/* test which TYPE of cache control supported
>> - * -1: don't support
>> - * 0: cat
>> - * 1: cdp
>> +/*
>> + * This function tests which TYPE of cache control is supported
>> + * Return values are:
>> + * -1: not supported
>> + * 0: CAT
>> + * 1: CDP
>> */
>> static int
>> virCapabilitiesGetCacheControlType(virCapsHostCacheBankPtr bank)
>> {
>> int ret = -1;
>> char *path = NULL;
>> +
>> if (virAsprintf(&path,
>> SYSFS_RESCTRL_PATH "/info/L%u",
>> bank->level) < 0)
>> @@ -1576,7 +1577,10 @@ virCapabilitiesGetCacheControlType(virCapsHostCacheBankPtr bank)
>> ret = 0;
>> } else {
>> VIR_FREE(path);
>> - /* for CDP enabled case, CODE and DATA will show together */
>> + /*
>> + * If CDP is enabled, there will be both CODE and DATA, but it's enough
>> + * to check one of those only.
>> + */
>> if (virAsprintf(&path,
>> SYSFS_RESCTRL_PATH "/info/L%uCODE",
>> bank->level) < 0)
>> @@ -1597,13 +1601,14 @@ virCapabilitiesGetCacheControl(virCapsHostCacheBankPtr bank,
>> char *path = NULL;
>> char *cbm_mask = NULL;
>> char *type_upper = NULL;
>> + unsigned int min_cbm_bits = 0;
>> virCapsHostCacheControlPtr control;
>>
>> if (VIR_ALLOC(control) < 0)
>> goto cleanup;
>>
>> - if ((scope > VIR_CACHE_TYPE_BOTH)
>> - && (virStringToUpper(&type_upper, virCacheTypeToString(scope)) < 0))
>> + if (scope != VIR_CACHE_TYPE_BOTH &&
>> + virStringToUpper(&type_upper, virCacheTypeToString(scope)) < 0)
>> goto cleanup;
>>
>> if (virFileReadValueUint(&control->max_allocation,
>> @@ -1619,10 +1624,16 @@ virCapabilitiesGetCacheControl(virCapsHostCacheBankPtr bank,
>> type_upper ? type_upper: "") < 0)
>> goto cleanup;
>>
>> + if (virFileReadValueUint(&min_cbm_bits,
>> + SYSFS_RESCTRL_PATH "/info/L%u%s/min_cbm_bits",
>> + bank->level,
>> + type_upper ? type_upper : "") < 0)
>> + goto cleanup;
>> +
>> virStringTrimOptionalNewline(cbm_mask);
>>
>> /* cbm_mask: cache bit mask, it's in hex, eg: fffff */
>> - control->min = bank->size / (strlen(cbm_mask) * 4);
>> + control->min = min_cbm_bits * bank->size / (strlen(cbm_mask) * 4);
>>
>> control->scope = scope;
>>
>> @@ -1732,10 +1743,10 @@ virCapabilitiesInitCaches(virCapsPtr caps)
>> VIR_CACHE_TYPE_BOTH) < 0)
>> goto cleanup;
>> } else if (typeret == 1) {
>> - if ((virCapabilitiesGetCacheControl(bank,
>> - VIR_CACHE_TYPE_CODE) < 0) ||
>> - (virCapabilitiesGetCacheControl(bank,
>> - VIR_CACHE_TYPE_DATA) < 0))
>> + if (virCapabilitiesGetCacheControl(bank,
>> + VIR_CACHE_TYPE_CODE) < 0 ||
>> + virCapabilitiesGetCacheControl(bank,
>> + VIR_CACHE_TYPE_DATA) < 0)
>> goto cleanup;
>> }
>>
>> diff --git i/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml w/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml
>> index c9f460d8a227..49aa0b98ca88 100644
>> --- i/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml
>> +++ w/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml
>> @@ -42,12 +42,12 @@
>> </topology>
>> <cache>
>> <bank id='0' level='3' type='both' size='15360' unit='KiB' cpus='0-5'>
>> - <control min='768' unit='KiB' scope='code' max_allocation='8'/>
>> - <control min='768' unit='KiB' scope='data' max_allocation='8'/>
>> + <control min='768' unit='KiB' type='code' maxAllocs='8'/>
>> + <control min='768' unit='KiB' type='data' maxAllocs='8'/>
>> </bank>
>> <bank id='1' level='3' type='both' size='15360' unit='KiB' cpus='6-11'>
>> - <control min='768' unit='KiB' scope='code' max_allocation='8'/>
>> - <control min='768' unit='KiB' scope='data' max_allocation='8'/>
>> + <control min='768' unit='KiB' type='code' maxAllocs='8'/>
>> + <control min='768' unit='KiB' type='data' maxAllocs='8'/>
>> </bank>
>> </cache>
>> </host>
>> diff --git i/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml w/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml
>> index 04a5eb895727..cb78b4ab788d 100644
>> --- i/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml
>> +++ w/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml
>> @@ -42,10 +42,10 @@
>> </topology>
>> <cache>
>> <bank id='0' level='3' type='both' size='15360' unit='KiB' cpus='0-5'>
>> - <control min='768' unit='KiB' scope='both' max_allocation='4'/>
>> + <control min='1536' unit='KiB' type='both' maxAllocs='4'/>
>> </bank>
>> <bank id='1' level='3' type='both' size='15360' unit='KiB' cpus='6-11'>
>> - <control min='768' unit='KiB' scope='both' max_allocation='4'/>
>> + <control min='1536' unit='KiB' type='both' maxAllocs='4'/>
>> </bank>
>> </cache>
>> </host>
>> --
>> Martin
>>
>>
>
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list