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

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

 



-- 
Best regards 
Eli

天涯无处不重逢
a leaf duckweed belongs to the sea, where not to meet in life 
Sent with Sparrow

On Wednesday, 15 March 2017 at 8:53 PM, Martin Kletzander wrote:

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?


 I am okay to ether of them, just follow with previous discussion.
 
</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.

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.

yep, we can remove  virResCtrlAvailable.

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.

Ops, good to know that, would like to learn how to use VIR_ONCE. 
 
qemu_driver->qemuCapsCache = virQEMUCapsCacheNew(cfg->libDir,
cfg->cacheDir,
run_uid,
--
1.9.1

--
libvir-list mailing list
 

--
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