Re: [PATCH resend V10 02/12] Resctrl: expose cache information to capabilities

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

 



On Mon, Mar 06, 2017 at 06:06:31PM +0800, Eli Qiao wrote:
This patch expose cache information to host's capabilites xml.

For l3 cache allocation
<cache>
 <bank id='0' type='l3' size='56320' unit='KiB' cpus='0-21,44-65'>
   <control min='2816' reserved='2816' unit='KiB' scope='L3'/>
 </bank>
 <bank id='1' type='l3' size='56320' unit='KiB' cpus='22-43,66-87'>
   <control min='2816' reserved='2816' unit='KiB' scope='L3'/>
 </bank>
</cache>

For l3 cache allocation supported cdp(seperate data/code):
<cache>
 <bank id='0' type='l3' size='56320' unit='KiB' cpus='0-21,44-65'>
   <control min='2816' reserved='2816' unit='KiB' scope='L3DATA'/>
   <control min='2816' reserved='2816' unit='KiB' scope='L3CODE'/>

I know this was discussed before, but why having a vector value in a
single attribute?  Why don't we split it to two scalars?  It will also
be SOO much more readable and close to what other tools expose, e.g.:

<control scope="data"/>
<control scope="instruction"/>

or

<control scope="both"/>

I also skipped the 'l3' because that is already in the <bank/> and will
never be different, right?

 </bank>
 <bank id='1' type='l3' size='56320' unit='KiB' cpus='22-43,66-87'>
   <control min='2816' reserved='2816' unit='KiB' scope='L3DATA'/>
   <control min='2816' reserved='2816' unit='KiB' scope='L3CODE'/>
 </bank>
</cache>

RFC on mailing list.
https://www.redhat.com/archives/libvir-list/2017-January/msg00644.html

Signed-off-by: Eli Qiao <liyong.qiao@xxxxxxxxx>
---
src/conf/capabilities.c      | 56 ++++++++++++++++++++++++++++++++++++++
src/conf/capabilities.h      | 23 ++++++++++++++++
src/libvirt_private.syms     |  3 +++
src/nodeinfo.c               | 64 ++++++++++++++++++++++++++++++++++++++++++++
src/nodeinfo.h               |  1 +
src/qemu/qemu_capabilities.c |  8 ++++++
src/qemu/qemu_driver.c       |  4 +++
7 files changed, 159 insertions(+)

diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 9ab343b..23e21e4 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -198,6 +198,18 @@ virCapabilitiesClearSecModel(virCapsHostSecModelPtr secmodel)
}

static void
+virCapabilitiesClearCacheBank(virCapsHostCacheBankPtr cachebank)
+{
+    size_t i;
+    for (i = 0; i < cachebank->ncontrol; i++)
+        VIR_FREE(cachebank->control[i].scope);
+
+    VIR_FREE(cachebank->type);
+    VIR_FREE(cachebank->cpus);
+}
+
+
+static void
virCapabilitiesDispose(void *object)
{
    virCapsPtr caps = object;
@@ -221,6 +233,10 @@ virCapabilitiesDispose(void *object)
        virCapabilitiesClearSecModel(&caps->host.secModels[i]);
    VIR_FREE(caps->host.secModels);

+    for (i = 0; i < caps->host.ncachebank; i++)
+        virCapabilitiesClearCacheBank(caps->host.cachebank[i]);
+    VIR_FREE(caps->host.cachebank);
+
    VIR_FREE(caps->host.netprefix);
    VIR_FREE(caps->host.pagesSize);
    virCPUDefFree(caps->host.cpu);
@@ -844,6 +860,41 @@ virCapabilitiesFormatNUMATopology(virBufferPtr buf,
    return 0;
}

+static int
+virCapabilitiesFormatCache(virBufferPtr buf,
+                           size_t ncachebank,
+                           virCapsHostCacheBankPtr *cachebank)
+{
+    size_t i;
+    size_t j;
+
+    virBufferAddLit(buf, "<cache>\n");
+    virBufferAdjustIndent(buf, 2);
+
+    for (i = 0; i < ncachebank; i++) {
+        virBufferAsprintf(buf,
+                          "<bank id='%u' type='%s' size='%llu' unit='KiB' cpus='%s'>\n",
+                          cachebank[i]->id,
+                          cachebank[i]->type,
+                          cachebank[i]->size,
+                          cachebank[i]->cpus);
+
+        virBufferAdjustIndent(buf, 2);
+        for (j = 0; j < cachebank[i]->ncontrol; j++) {
+            virBufferAsprintf(buf,
+                              "<control min='%llu' reserved='%llu' unit='KiB' scope='%s'/>\n",
+                              cachebank[i]->control[j].min,
+                              cachebank[i]->control[j].reserved,
+                              cachebank[i]->control[j].scope);
+        }
+        virBufferAdjustIndent(buf, -2);
+        virBufferAddLit(buf, "</bank>\n");
+    }
+    virBufferAdjustIndent(buf, -2);
+    virBufferAddLit(buf, "</cache>\n");
+    return 0;
+}
+
/**
 * virCapabilitiesFormatXML:
 * @caps: capabilities to format
@@ -931,6 +982,11 @@ virCapabilitiesFormatXML(virCapsPtr caps)
        virBufferAddLit(&buf, "</migration_features>\n");
    }

+    if (caps->host.ncachebank &&
+            virCapabilitiesFormatCache(&buf, caps->host.ncachebank,
+                                       caps->host.cachebank) < 0)
+        return NULL;
+
    if (caps->host.netprefix)
        virBufferAsprintf(&buf, "<netprefix>%s</netprefix>\n",
                          caps->host.netprefix);
diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h
index cfdc34a..b446de5 100644
--- a/src/conf/capabilities.h
+++ b/src/conf/capabilities.h
@@ -138,6 +138,25 @@ struct _virCapsHostSecModel {
    virCapsHostSecModelLabelPtr labels;
};

+typedef struct _virCapsHostCacheControl virCapsHostCacheControl;
+typedef virCapsHostCacheControl *virCapsHostCacheControlPtr;
+struct _virCapsHostCacheControl {
+    unsigned long long min;
+    unsigned long long reserved;
+    char* scope;
+};
+
+typedef struct _virCapsHostCacheBank virCapsHostCacheBank;
+typedef virCapsHostCacheBank *virCapsHostCacheBankPtr;
+struct _virCapsHostCacheBank {
+    unsigned int id;
+    char* type;
+    char* cpus;
+    unsigned long long size;
+    size_t ncontrol;
+    virCapsHostCacheControlPtr control;
+};
+
typedef struct _virCapsHost virCapsHost;
typedef virCapsHost *virCapsHostPtr;
struct _virCapsHost {
@@ -160,6 +179,10 @@ struct _virCapsHost {
    size_t nsecModels;
    virCapsHostSecModelPtr secModels;

+    size_t ncachebank;
+    size_t ncachebank_max;
+    virCapsHostCacheBankPtr *cachebank;
+
    char *netprefix;
    virCPUDefPtr cpu;
    int nPagesSize;             /* size of pagesSize array */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index bb7c3ad..b8445ef 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1125,6 +1125,7 @@ virLogManagerNew;
# nodeinfo.h
nodeCapsInitNUMA;
nodeGetInfo;
+virCapsInitCache;
virHostCPUGetCount;
virHostCPUGetKVMMaxVCPUs;
virHostCPUGetMap;
@@ -2322,8 +2323,10 @@ virRandomInt;

# util/virresctrl.h
virResCtrlAvailable;
+virResCtrlGet;
virResCtrlInit;

+
# util/virrotatingfile.h
virRotatingFileReaderConsume;
virRotatingFileReaderFree;
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index f2ded02..a001cd0 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -48,6 +48,7 @@
#include "virstring.h"
#include "virnuma.h"
#include "virlog.h"
+#include "virresctrl.h"

#define VIR_FROM_THIS VIR_FROM_NONE

@@ -416,3 +417,66 @@ nodeCapsInitNUMA(virCapsPtr caps)
    VIR_FREE(pageinfo);
    return ret;
}
+
+int
+virCapsInitCache(virCapsPtr caps)
+{
+    size_t i, j;
+    virResCtrlPtr resctrl;
+    virCapsHostCacheBankPtr bank;
+
+    for (i = 0; i < VIR_RDT_RESOURCE_LAST; i++) {
+        /* L3DATA and L3CODE share L3 resources */
+        if (i == VIR_RDT_RESOURCE_L3CODE)
+            continue;
+
+        resctrl = virResCtrlGet(i);
+
+        if (resctrl->enabled) {
+            for (j = 0; j < resctrl->num_banks; j++) {
+                if (VIR_RESIZE_N(caps->host.cachebank, caps->host.ncachebank_max,
+                                caps->host.ncachebank, 1) < 0)
+                    return -1;
+
+                if (VIR_ALLOC(bank) < 0)
+                    return -1;
+
+                bank->id = resctrl->cache_banks[j].host_id;
+                if (VIR_STRDUP(bank->type, resctrl->cache_level) < 0)
+                    goto err;
+                if (VIR_STRDUP(bank->cpus, virBitmapFormat(resctrl->cache_banks[j].cpu_mask)) < 0)
+                    goto err;
+                bank->size = resctrl->cache_banks[j].cache_size;
+                /*L3DATA and L3CODE shares L3 cache resources, so fill them to the control element*/
+                if (i == VIR_RDT_RESOURCE_L3DATA) {
+                    if (VIR_EXPAND_N(bank->control, bank->ncontrol, 2) < 0)
+                        goto err;
+
+                    bank->control[0].min = virResCtrlGet(VIR_RDT_RESOURCE_L3DATA)->cache_banks[j].cache_min;
+                    bank->control[0].reserved = bank->control[0].min * (virResCtrlGet(VIR_RDT_RESOURCE_L3DATA)->min_cbm_bits);
+                    if (VIR_STRDUP(bank->control[0].scope,
+                                  virResCtrlGet(VIR_RDT_RESOURCE_L3DATA)->name) < 0)
+                        goto err;
+
+                    bank->control[1].min = virResCtrlGet(VIR_RDT_RESOURCE_L3CODE)->cache_banks[j].cache_min;
+                    bank->control[1].reserved = bank->control[1].min * (virResCtrlGet(VIR_RDT_RESOURCE_L3CODE)->min_cbm_bits);
+                    if (VIR_STRDUP(bank->control[1].scope,
+                                   virResCtrlGet(VIR_RDT_RESOURCE_L3CODE)->name) < 0)
+                        goto err;
+                } else {
+                    if (VIR_EXPAND_N(bank->control, bank->ncontrol, 1) < 0)
+                        goto err;
+                    bank->control[0].min = resctrl->cache_banks[j].cache_min;
+                    bank->control[0].reserved = bank->control[0].min * resctrl->min_cbm_bits;
+                    if (VIR_STRDUP(bank->control[0].scope, resctrl->name) < 0)
+                        goto err;
+                }
+                caps->host.cachebank[caps->host.ncachebank++] = bank;
+            }
+        }
+    }
+    return 0;
+ err:
+    VIR_FREE(bank);
+    return -1;
+}
diff --git a/src/nodeinfo.h b/src/nodeinfo.h
index 3c4dc46..5eb0f83 100644
--- a/src/nodeinfo.h
+++ b/src/nodeinfo.h
@@ -28,5 +28,6 @@

int nodeGetInfo(virNodeInfoPtr nodeinfo);
int nodeCapsInitNUMA(virCapsPtr caps);
+int virCapsInitCache(virCapsPtr caps);

#endif /* __VIR_NODEINFO_H__*/
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 359a0d8..a0d7254 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -1100,6 +1100,11 @@ virQEMUCapsInitCPU(virCapsPtr caps,
    goto cleanup;
}

+static int
+virQEMUCapsInitCache(virCapsPtr caps)
+{
+    return virCapsInitCache(caps);
+}

static int
virQEMUCapsInitPages(virCapsPtr caps)
@@ -1146,6 +1151,9 @@ virCapsPtr virQEMUCapsInit(virQEMUCapsCachePtr cache)
    if (virQEMUCapsInitCPU(caps, hostarch) < 0)
        VIR_WARN("Failed to get host CPU");

+    if (virQEMUCapsInitCache(caps) < 0)
+        VIR_WARN("Failed to get host cache");
+
    /* Add the power management features of the host */
    if (virNodeSuspendGetTargetMask(&caps->host.powerMgmt) < 0)
        VIR_WARN("Failed to get host power management capabilities");
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 77d8175..3bfb4ec 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -105,6 +105,7 @@
#include "vircgroup.h"
#include "virperf.h"
#include "virnuma.h"
+#include "virresctrl.h"
#include "dirname.h"
#include "network/bridge_driver.h"

@@ -849,6 +850,9 @@ qemuStateInitialize(bool privileged,
        run_gid = cfg->group;
    }

+    if (virResCtrlAvailable() && virResCtrlInit() < 0)
+        VIR_WARN("Faild to initialize resource control.");
+

s/Faild/Failed/

Also the Available() call seems unnecessary since Init() is graceful to
unavailability IIRC.

Anyway if this is to be done once per daemon lifetime, it should be
wrapped using VIR_ONCE (see VIR_ONCE_GLOBAL_INIT all over the
codebase).  I believe I mentioned this already in some previous version.

    qemu_driver->qemuCapsCache = virQEMUCapsCacheNew(cfg->libDir,
                                                     cfg->cacheDir,
                                                     run_uid,
--
1.9.1

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

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