Re: [PATCH 06/10] conf: Add support for cputune/cachetune

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jan 24, 2018 at 03:04:37PM +0100, Pavel Hrdina wrote:
On Tue, Jan 23, 2018 at 07:05:15PM +0100, Martin Kletzander wrote:
More info in the documentation, this is basically the XML parsing/formatting
support, schemas, tests and documentation for the new cputune/cachetune element
that will get used by following patches.

Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx>
---
 docs/formatdomain.html.in                          |  54 ++++
 docs/schemas/domaincommon.rng                      |  32 +++
 src/conf/domain_conf.c                             | 295 ++++++++++++++++++++-
 src/conf/domain_conf.h                             |  13 +
 tests/genericxml2xmlindata/cachetune-cdp.xml       |  36 +++
 .../cachetune-colliding-allocs.xml                 |  30 +++
 .../cachetune-colliding-tunes.xml                  |  32 +++
 .../cachetune-colliding-types.xml                  |  30 +++
 tests/genericxml2xmlindata/cachetune-small.xml     |  29 ++
 tests/genericxml2xmlindata/cachetune.xml           |  33 +++
 tests/genericxml2xmltest.c                         |  10 +
 11 files changed, 592 insertions(+), 2 deletions(-)
 create mode 100644 tests/genericxml2xmlindata/cachetune-cdp.xml
 create mode 100644 tests/genericxml2xmlindata/cachetune-colliding-allocs.xml
 create mode 100644 tests/genericxml2xmlindata/cachetune-colliding-tunes.xml
 create mode 100644 tests/genericxml2xmlindata/cachetune-colliding-types.xml
 create mode 100644 tests/genericxml2xmlindata/cachetune-small.xml
 create mode 100644 tests/genericxml2xmlindata/cachetune.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index d272cc1ba698..7b4d9051a551 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -689,6 +689,10 @@
     &lt;iothread_quota&gt;-1&lt;/iothread_quota&gt;
     &lt;vcpusched vcpus='0-4,^3' scheduler='fifo' priority='1'/&gt;
     &lt;iothreadsched iothreads='2' scheduler='batch'/&gt;
+    &lt;cachetune vcpus='0-3'&gt;
+      &lt;cache id='0' level='3' type='both' size='3' unit='MiB'/&gt;
+      &lt;cache id='1' level='3' type='both' size='3' unit='MiB'/&gt;
+    &lt;/cachetune&gt;
   &lt;/cputune&gt;
   ...
 &lt;/domain&gt;
@@ -834,6 +838,56 @@
         <span class="since">Since 1.2.13</span>
       </dd>

+      <dt><code>cachetune</code><span class="since">Since 4.1.0</span></dt>
+      <dd>
+        Optional <code>cachetune</code> element can control allocations for CPU
+        caches using the resctrl on the host. Whether or not is this supported
+        can be gathered from capabilities where some limitations like minimum
+        size and required granularity are reported as well.  The required

s/  / /

+        attribute <code>vcpus</code> specifies to which vCPUs this allocation
+        applies. A vCPU can only be member of one <code>cachetune</code> element
+        allocations. Supported subelements are:
+        <dl>
+          <dt><code>cache</code></dt>
+          <dd>
+            This element controls the allocation of CPU cache and has the
+            following attributes:
+            <dl>
+              <dt><code>level</code></dt>
+              <dd>
+                Host cache level from which to allocate.
+              </dd>
+              <dt><code>id</code></dt>
+              <dd>
+                Host cache id from which to allocate.
+              </dd>
+              <dt><code>type</code></dt>
+              <dd>
+                Type of allocation.  Can be <code>code</code> for code

s/  / /

+                (instructions), <code>data</code> for data or <code>both</code>
+                for both code and data (unified).  Currently the allocation can

s/  / /


Fixed all three.

/me wonders why people still insist on this.  Is someone editing these
   files with proportional font?  Do we have any other output than
   HTML?  I guess this will be yet another thing I have to get used to
   and surrender.

+                be done only with the same type as the host supports, meaning
+                you cannot request <code>both</code> for host with CDP
+                (code/data prioritization) enabled.
+              </dd>
+              <dt><code>size</code></dt>
+              <dd>
+                The size of the region to allocate. The value by default is in
+                bytes, but the <code>unit</code> attribute can be used to scale
+                the value.
+              </dd>
+              <dt><code>unit</code> (optional)</dt>
+              <dd>
+                If specified it is the unit such as KiB, MiB, GiB, or TiB
+                (described in the <code>memory</code> element
+                for <a href="#elementsMemoryAllocation">Memory Allocation</a>)
+                in which <code>size</code> is specified, defaults to bytes.
+              </dd>
+            </dl>
+          </dd>
+        </dl>
+
+      </dd>
     </dl>


diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index f22c932f6c09..252f58f4379c 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -900,6 +900,38 @@
             <ref name="schedparam"/>
           </element>
         </zeroOrMore>
+        <zeroOrMore>
+          <element name="cachetune">
+            <attribute name="vcpus">
+              <ref name='cpuset'/>
+            </attribute>
+            <oneOrMore>
+              <element name="cache">
+                <attribute name="id">
+                  <ref name='unsignedInt'/>
+                </attribute>
+                <attribute name="level">
+                  <ref name='unsignedInt'/>
+                </attribute>
+                <attribute name="type">
+                  <choice>
+                    <value>both</value>
+                    <value>code</value>
+                    <value>data</value>
+                  </choice>
+                </attribute>
+                <attribute name="size">
+                  <ref name='unsignedLong'/>
+                </attribute>
+                <optional>
+                  <attribute name='unit'>
+                    <ref name='unit'/>
+                  </attribute>
+                </optional>
+              </element>
+            </oneOrMore>
+          </element>
+        </zeroOrMore>
       </interleave>
     </element>
   </define>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a1c25060f9e9..27665d0372a7 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2883,6 +2883,19 @@ virDomainLoaderDefFree(virDomainLoaderDefPtr loader)
     VIR_FREE(loader);
 }

+
+static void
+virDomainCachetuneDefFree(virDomainCachetuneDefPtr cachetune)
+{
+    if (!cachetune)
+        return;
+
+    virObjectUnref(cachetune->alloc);
+    virBitmapFree(cachetune->vcpus);
+    VIR_FREE(cachetune);
+}
+
+
 void virDomainDefFree(virDomainDefPtr def)
 {
     size_t i;
@@ -3055,6 +3068,10 @@ void virDomainDefFree(virDomainDefPtr def)
         virDomainShmemDefFree(def->shmems[i]);
     VIR_FREE(def->shmems);

+    for (i = 0; i < def->ncachetunes; i++)
+        virDomainCachetuneDefFree(def->cachetunes[i]);
+    VIR_FREE(def->cachetunes);
+
     VIR_FREE(def->keywrap);

     if (def->namespaceData && def->ns.free)
@@ -18247,6 +18264,194 @@ virDomainDefParseBootOptions(virDomainDefPtr def,
 }


+static int
+virDomainCachetuneDefParseCache(xmlXPathContextPtr ctxt,
+                                xmlNodePtr node,
+                                virResctrlAllocPtr alloc)
+{
+    xmlNodePtr oldnode = ctxt->node;
+    unsigned int level;
+    unsigned int cache;
+    int type;
+    unsigned long long size;
+    char *tmp = NULL;
+    int ret = -1;
+
+    ctxt->node = node;
+
+    tmp = virXMLPropString(node, "id");
+    if (!tmp) {
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("Missing cachetune attribute 'id'"));
+        goto cleanup;
+    }
+    if (virStrToLong_uip(tmp, NULL, 10, &cache) < 0) {
+        virReportError(VIR_ERR_XML_ERROR,
+                       _("Invalid cachetune attribute 'id' value '%s'"),
+                       tmp);
+        goto cleanup;
+    }
+    VIR_FREE(tmp);
+
+    tmp = virXMLPropString(node, "level");
+    if (!tmp) {
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("Missing cachetune attribute 'level'"));
+        goto cleanup;
+    }
+    if (virStrToLong_uip(tmp, NULL, 10, &level) < 0) {
+        virReportError(VIR_ERR_XML_ERROR,
+                       _("Invalid cachetune attribute 'level' value '%s'"),
+                       tmp);
+        goto cleanup;
+    }
+    VIR_FREE(tmp);
+
+    tmp = virXMLPropString(node, "type");
+    if (!tmp) {
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("Missing cachetune attribute 'type'"));
+        goto cleanup;
+    }
+    type = virCacheTypeFromString(tmp);
+    if (type < 0) {
+        virReportError(VIR_ERR_XML_ERROR,
+                       _("Invalid cachetune attribute 'type' value '%s'"),
+                       tmp);
+        goto cleanup;
+    }
+    VIR_FREE(tmp);
+
+    if (virDomainParseScaledValue("./@size", "./@unit",
+                                  ctxt, &size, 1024,
+                                  ULLONG_MAX, true) < 0)
+        goto cleanup;
+
+    if (virResctrlAllocSetSize(alloc, level, type, cache, size) < 0)
+        goto cleanup;
+
+    ret = 0;
+ cleanup:
+    ctxt->node = oldnode;
+    VIR_FREE(tmp);
+    return ret;
+}
+
+
+static int
+virDomainCachetuneDefParse(virDomainDefPtr def,
+                           xmlXPathContextPtr ctxt,
+                           xmlNodePtr node,
+                           unsigned int flags)
+{
+    xmlNodePtr oldnode = ctxt->node;
+    xmlNodePtr *nodes = NULL;
+    virBitmapPtr vcpus = NULL;
+    virResctrlAllocPtr alloc = virResctrlAllocNew();
+    virDomainCachetuneDefPtr tmp_cachetune = NULL;
+    char *tmp = NULL;
+    char *vcpus_str = NULL;
+    char *alloc_id = NULL;
+    ssize_t i = 0;
+    int n;
+    int ret = -1;
+
+    ctxt->node = node;
+
+    if (!alloc)
+        goto cleanup;
+
+    if (VIR_ALLOC(tmp_cachetune) < 0)
+        goto cleanup;
+
+    vcpus_str = virXMLPropString(node, "vcpus");
+    if (!vcpus_str) {
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("Missing cachetune attribute 'vcpus'"));
+        goto cleanup;
+    }
+    if (virBitmapParse(vcpus_str, &vcpus, VIR_DOMAIN_CPUMASK_LEN) < 0) {
+        virReportError(VIR_ERR_XML_ERROR,
+                       _("Invalid cachetune attribute 'vcpus' value '%s'"),
+                       vcpus_str);
+        goto cleanup;
+    }
+
+    /* We need to limit the bitmap to number of vCPUs.  If there's nothing left,
+     * then we can just clean up and return 0 immediately */
+    virBitmapShrink(vcpus, def->maxvcpus);
+    if (virBitmapIsAllClear(vcpus)) {
+        ret = 0;
+        goto cleanup;
+    }
+
+    if ((n = virXPathNodeSet("./cache", ctxt, &nodes)) < 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("Cannot extract cache nodes under cachetune"));
+        goto cleanup;
+    }
+
+    for (i = 0; i < n; i++) {
+        if (virDomainCachetuneDefParseCache(ctxt, nodes[i], alloc) < 0)
+            goto cleanup;
+    }
+
+    if (virResctrlAllocIsEmpty(alloc)) {
+        ret = 0;
+        goto cleanup;
+    }

Can this ever happen? The


Sure, for example:

 <cachetune/>

+
+    for (i = 0; i < def->ncachetunes; i++) {
+        if (virBitmapOverlaps(def->cachetunes[i]->vcpus, vcpus)) {
+            virReportError(VIR_ERR_XML_ERROR, "%s",
+                           _("Overlapping vcpus in cachetunes"));
+            goto cleanup;
+        }
+    }
+
+    /* We need to format it back because we need to be consistent in the naming
+     * even when users specify some "sub-optimal" string there. */
+    VIR_FREE(vcpus_str);
+    vcpus_str = virBitmapFormat(vcpus);
+    if (!vcpus_str)
+        goto cleanup;
+
+    if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE))
+        alloc_id = virXMLPropString(node, "id");
+
+    if (!alloc_id) {
+        /* The number of allocatios is limited and the directory structure is flat,

s/allocatios/allocations/


Fixed.

Reviewed-by: Pavel Hrdina <phrdina@xxxxxxxxxx>


Attachment: signature.asc
Description: Digital signature

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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux