Re: [libvirt] [PATCH 17/21] conf: cachetune

Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; --- docs/formatdomain.html.in | 24 ++ docs/schemas/domaincommon.rng | 32 +++ src/conf/domain_conf.c | 249 +++++++++++++++++++++ src/conf/domain_conf.h | 21 ++ src/util/virresctrl.c | 2 +- .../genericxml2xmlindata/generic-cachetune-cdp.xml | 36 +++ .../generic-cachetune-colliding-allocs.xml | 30 +++ .../generic-cachetune-colliding-tunes.xml | 32 +++ .../generic-cachetune-colliding-types.xml | 30 +++ .../generic-cachetune-small.xml | 29 +++ tests/genericxml2xmlindata/generic-cachetune.xml | 33 +++ tests/genericxml2xmltest.c | 10 + 12 files changed, 527 insertions(+), 1 deletion(-) create mode 100644 tests/genericxml2xmlindata/generic-cachetune-cdp.xml create mode 100644 tests/genericxml2xmlindata/generic-cachetune-colliding-allocs.xml create mode 100644 tests/genericxml2xmlindata/generic-cachetune-colliding-tunes.xml create mode 100644 tests/genericxml2xmlindata/generic-cachetune-colliding-types.xml create mode 100644 tests/genericxml2xmlindata/generic-cachetune-small.xml create mode 100644 tests/genericxml2xmlindata/generic-cachetune.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 92e14a919aba..b9d7f53b31f9 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,26 @@ <span class="since">Since 1.2.13</span> </dd> + <dt><code>cachetune</code></dt>

+ <dd> + Optional <code>cachetune</code> element can control allocations for CPU + caches using the resctrlfs 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. + Attribute <code>vcpus</code> specifies what vCPUs this allocation

+ applies to. One vCPU cannot have multiple <code>cachetune</code> + allocations.

+ For now there is only one supported + sub-element, <code>cache</code>. That element must have

+ attributes <code>level</code> and <code>id</code> - host's cache level/id + to allocate from, <code>type</code> - whether to + allocate <code>code</code>, <code>data</code> or <code>both</code> + and <code>size</code> - size of the region to allocate. Size is by + default in bytes, but optional attribute <code>unit</code> can be used + to scale the size appropriately. Each <code>cachetune</code> can have + multiple <code>cache</code> elements. <span class="since">Since + 3.10.0</span>

+ + </dd> </dl> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 9cec1a063724..5d90247799ad 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 400e900325f2..bf9e61efc8d2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2848,6 +2848,17 @@ 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; @@ -3020,6 +3031,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) @@ -18080,6 +18095,163 @@ 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 attribute id for cachetune/cache"));

+ goto cleanup; + } + if (virStrToLong_uip(tmp, NULL, 10, &cache) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid attribute id '%s' for cachetune/cache"),

+ tmp); + goto cleanup; + } + VIR_FREE(tmp); + + tmp = virXMLPropString(node, "level"); + if (!tmp) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Missing attribute level for cachetune/cache"));

+ goto cleanup; + } + if (virStrToLong_uip(tmp, NULL, 10, &level) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid attribute level '%s' for cachetune/cache"), + tmp); + goto cleanup; + } + VIR_FREE(tmp); + + tmp = virXMLPropString(node, "type"); + if (!tmp) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Missing attribute type for cachetune/cache"));

+ goto cleanup; + } + type = virCacheTypeFromString(tmp); + if (type < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid attribute type '%s' for cachetune"), + 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) +{ + xmlNodePtr oldnode = ctxt->node; + xmlNodePtr *nodes = NULL; + virBitmapPtr vcpus = NULL; + virResctrlAllocPtr alloc = NULL; + virDomainCachetuneDefPtr tmp_cachetune = NULL; + char *tmp = NULL; + char *vcpus_str = NULL; + ssize_t i = 0; + int n; + int ret = -1; + + ctxt->node = node; + + if (VIR_ALLOC(tmp_cachetune) < 0) + goto cleanup; + + vcpus_str = virXMLPropString(node, "vcpus"); + if (!vcpus_str) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Missing attribute vcpus for cachetune"));

+ goto cleanup; + } + if (virBitmapParse(vcpus_str, &vcpus, VIR_DOMAIN_CPUMASK_LEN) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid attribute vcpus '%s' for cachetune"), + vcpus_str); + goto cleanup; + } + + if ((n = virXPathNodeSet("./cache", ctxt, &nodes)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot extract cachetune/cache nodes")); + goto cleanup; + } + + for (i = 0; i < n; i++) { + if (virDomainCachetuneDefParseCache(ctxt, nodes[i], &alloc) < 0) + goto cleanup; + } + + if (!alloc) { + ret = 0; + goto cleanup; + } + + virBitmapShrink(vcpus, def->maxvcpus); + + 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; + } + } + + if (virResctrlAllocSetID(alloc, vcpus_str) < 0) + goto cleanup; + + tmp_cachetune->vcpus = vcpus; + tmp_cachetune->alloc = alloc; + vcpus = NULL; + alloc = NULL; + + if (VIR_APPEND_ELEMENT(def->cachetunes, def->ncachetunes, tmp_cachetune) < 0) + goto cleanup; + + ret = 0; + cleanup: + ctxt->node = oldnode; + virDomainCachetuneDefFree(tmp_cachetune); + virObjectUnref(alloc); + virBitmapFree(vcpus); + VIR_FREE(vcpus_str); + VIR_FREE(nodes); + VIR_FREE(tmp); + return ret; +} + + static virDomainDefPtr virDomainDefParseXML(xmlDocPtr xml, xmlNodePtr root, @@ -18632,6 +18804,18 @@ virDomainDefParseXML(xmlDocPtr xml, } VIR_FREE(nodes); + if ((n = virXPathNodeSet("./cputune/cachetune", ctxt, &nodes)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot extract cachetune nodes")); + goto error; + } + + for (i = 0; i < n; i++) { + if (virDomainCachetuneDefParse(def, ctxt, nodes[i]) < 0) + goto error; + } + VIR_FREE(nodes); + if (virCPUDefParseXML(ctxt, "./cpu[1]", VIR_CPU_TYPE_GUEST, &def->cpu) < 0) goto error; @@ -25444,6 +25628,68 @@ virDomainSchedulerFormat(virBufferPtr buf, } +struct virCachetuneHelperData { + virBufferPtr buf; + size_t vcpu_id; +}; + +static int +virDomainCachetuneDefFormatHelper(unsigned int level, + virCacheType type, + unsigned int cache, + unsigned long long size, + void *opaque) +{ + const char *unit; + virBufferPtr buf = opaque; + unsigned long long short_size = virPrettySize(size, &unit); + + virBufferAsprintf(buf, + "<cache id='%u' level='%u' type='%s' " + "size='%llu' unit='%s'/>\n", + cache, level, virCacheTypeToString(type), + short_size, unit); + + return 0; +} + + +static int +virDomainCachetuneDefFormat(virBufferPtr buf, + virDomainCachetuneDefPtr cachetune) +{ + virBuffer childrenBuf = VIR_BUFFER_INITIALIZER; + char *vcpus = NULL; + int ret = -1; + + virBufferSetChildIndent(&childrenBuf, buf); + virResctrlAllocForeachSize(cachetune->alloc, + virDomainCachetuneDefFormatHelper, + &childrenBuf); + + + if (virBufferCheckError(&childrenBuf) < 0) + goto cleanup; + + if (!virBufferUse(&childrenBuf)) { + ret = 0; + goto cleanup; + } + + vcpus = virBitmapFormat(cachetune->vcpus); + + virBufferAsprintf(buf, "<cachetune vcpus='%s'>\n", vcpus); + virBufferAddBuffer(buf, &childrenBuf); + virBufferAddLit(buf, "</cachetune>\n"); + + ret = 0; + cleanup: + virBufferFreeAndReset(&childrenBuf); + VIR_FREE(vcpus); + return ret; +} + +

static int virDomainCputuneDefFormat(virBufferPtr buf, virDomainDefPtr def) @@ -25545,6 +25791,9 @@ virDomainCputuneDefFormat(virBufferPtr buf, def->iothreadids[i]->iothread_id); } + for (i = 0; i < def->ncachetunes; i++) + virDomainCachetuneDefFormat(&childrenBuf, def->cachetunes[i]); + if (virBufferCheckError(&childrenBuf) < 0) return -1; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index be38792c6942..bda83d72e7ed 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -56,6 +56,7 @@ # include "virperf.h" # include "virtypedparam.h" # include "virsavecookie.h" +# include "virresctrl.h" /* forward declarations of all device types, required by * virDomainDeviceDef @@ -2167,6 +2168,15 @@ struct _virDomainCputune { }; +typedef struct _virDomainCachetuneDef virDomainCachetuneDef; +typedef virDomainCachetuneDef *virDomainCachetuneDefPtr; + +struct _virDomainCachetuneDef { + virBitmapPtr vcpus; + virResctrlAllocPtr alloc; +}; + + typedef struct _virDomainVcpuDef virDomainVcpuDef; typedef virDomainVcpuDef *virDomainVcpuDefPtr; @@ -2294,6 +2304,17 @@ struct _virDomainDef { virDomainIOThreadIDDefPtr *iothreadids; virDomainCputune cputune; + /* + * Both cachetune as well as runtime allocation (the same structure, but + * updated) for all threads is kept here. It is specified here instead of + * specifying it in virDomainVcpuDef _deliberately_ because it needs to be + * set at once under one flock() and not for each thread separately. Also + * if one cachetune is specified for multiple vCPUs, it must be created as + * one due to limited number of concurrent cachetune settings (number of + * CLoS IDs) provided by hardware. + */

+ virDomainCachetuneDefPtr *cachetunes; + size_t ncachetunes; virDomainNumaPtr numa; virDomainResourceDefPtr resource; diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index ac1b38436bb2..8753b1dca325 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -693,7 +693,7 @@ virResctrlAllocSetID(virResctrlAllocPtr alloc, { if (!id) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Resctrl allocation id cannot be NULL")); + _("Resctrl allocation 'id' cannot be NULL"));

return -1; }