On Thursday, 6 April 2017 at 5:49 PM, Martin Kletzander wrote:

On Thu, Apr 06, 2017 at 04:20:06PM +0800, Eli Qiao wrote:

This patch is based on Martin's cache branch.

This patch amends the cache bank capability as follow:

<bank id='0' level='3' type='unified' size='15360' unit='KiB' cpus='0-5'/>

<control min='768' unit='KiB' type='unified' nclos='4'/>

<bank id='1' level='3' type='unified' size='15360' unit='KiB' cpus='6-11'/>

<control min='768' unit='KiB' type='unified' nclos='4'/>

Were we exposing the number of CLoS IDs before? Was there a discussion

about it? Do we want to expose them? Probably yes, I'm just wondering.

Along with vircaps2xmltest case updated.

Signed-off-by: Eli Qiao <liyong.qiao@intel.com>

---

src/conf/capabilities.c | 112 +++++++++++++++++++++--

src/conf/capabilities.h | 13 ++-

tests/vircaps2xmldata/vircaps-x86_64-caches.xml | 2 +

tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml | 2 +

tests/vircaps2xmltest.c | 11 +++

5 files changed, 132 insertions(+), 8 deletions(-)

diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c

index 416dd1a..75c0bec 100644

--- a/src/conf/capabilities.c

+++ b/src/conf/capabilities.c

@@ -52,6 +52,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")

@@ -873,6 +874,7 @@ virCapabilitiesFormatCaches(virBufferPtr buf,

virCapsHostCacheBankPtr *caches)

{

size_t i = 0;

+ size_t j = 0;

if (!ncaches)

return 0;

@@ -900,6 +902,18 @@ virCapabilitiesFormatCaches(virBufferPtr buf,

bank->size >> (kilos * 10),

kilos ? "KiB" : "B",

cpus_str);

+ virBufferAdjustIndent(buf, 2);

+ for (j = 0; j < bank->ncontrol; j++) {

+ bool min_kilos = !(bank->controls[j]->min % 1024);

+ virBufferAsprintf(buf,

+ "<control min='%llu' unit='%s' "

+ "type='%s' nclos='%u'/>\n",

+ bank->controls[j]->min >> (min_kilos * 10),

+ min_kilos ? "KiB" : "B",

+ virCacheTypeToString(bank->controls[j]->type),

+ bank->controls[j]->nclos);

+ }

+ virBufferAdjustIndent(buf, -2);

VIR_FREE(cpus_str);

}

@@ -1516,10 +1530,93 @@ virCapsHostCacheBankFree(virCapsHostCacheBankPtr ptr)

if (!ptr)

return;

+ size_t i;

Upstream frowns upon C99 initialization in the middle of the code.

virBitmapFree(ptr->cpus);

+ for (i = 0; i < ptr->ncontrol; i++)

+ VIR_FREE(ptr->controls[i]);

+ VIR_FREE(ptr->controls);

VIR_FREE(ptr);

}

+/* test which kinds 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);

+ 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, const char* type)

+{

+ int ret = -1;

+ char *path = NULL;

+ char *cbm_mask = NULL;

+ virCapsHostCacheControlPtr control;

+

+ if (VIR_ALLOC(control) < 0)

+ goto cleanup;

+

+ if (virFileReadValueUint(&control->nclos,

+ SYSFS_RESCTRL_PATH "info/L%u%s/num_closids",

+ bank->level,

+ type) < 0)

+ goto cleanup;

+

+ if (virFileReadValueString(&cbm_mask,

+ SYSFS_RESCTRL_PATH

+ "info/L%u%s/cbm_mask",

+ bank->level,

+ type) < 0)

+ goto cleanup;

+

+ virStringTrimOptionalNewline(cbm_mask);

+

+ control->min = bank->size / (strlen(cbm_mask) * 4);

+

+ if (STREQ("", type))

+ control->type = VIR_CACHE_TYPE_UNIFIED;

+ else if (STREQ("CODE", type))

+ control->type = VIR_CACHE_TYPE_INSTRUCTION;

+ else if (STREQ("DATA", type))

+ control->type = VIR_CACHE_TYPE_DATA;

+ else

+ goto cleanup;

+

+ if (VIR_APPEND_ELEMENT(bank->controls,

+ bank->ncontrol,

+ control) < 0)

+ goto error;

+

+ ret = 0;

+

+ cleanup:

+ VIR_FREE(path);

+ return ret;

+ error:

+ VIR_FREE(control);

+ return -1;

The return value is not used anywhere.

+}

+

int

virCapabilitiesInitCaches(virCapsPtr caps)

{

@@ -1533,11 +1630,6 @@ virCapabilitiesInitCaches(virCapsPtr caps)

struct dirent *ent = NULL;

virCapsHostCacheBankPtr bank = NULL;

- /* Minimum level to expose in capabilities. Can be lowered or removed (with

- * the appropriate code below), but should not be increased, because we'd

- * lose information. */

- const int cache_min_level = 3;

-

You are removing this and only reporting it for those we can control.

But I believe the cache information can be valuable by itself, even

for those who can't change it. If this was intentional, it should be

mentioned in the commit message.

/* offline CPUs don't provide cache info */

if (virFileReadValueBitmap(&cpus, SYSFS_SYSTEM_PATH "cpu/online") < 0)

return -1;

@@ -1595,12 +1687,19 @@ virCapabilitiesInitCaches(virCapsPtr caps)

pos, ent->d_name) < 0)

goto cleanup;

- if (bank->level < cache_min_level) {

+ if ((ret = virCapabilitiesGetCacheControlType(bank)) < 0) {

virCapsHostCacheBankFree(bank);

bank = NULL;

continue;

}

+ if (ret == 0) {

+ virCapabilitiesGetCacheControl(bank, "");

+ } else if (ret == 1) {

+ virCapabilitiesGetCacheControl(bank, "CODE");

+ virCapabilitiesGetCacheControl(bank, "DATA");

+ }

+

for (tmp_c = type; *tmp_c != '\0'; tmp_c++)

*tmp_c = c_tolower(*tmp_c);

@@ -1617,6 +1716,7 @@ virCapabilitiesInitCaches(virCapsPtr caps)

if (virCapsHostCacheBankEquals(bank, caps->host.caches[i]))

break;

}

+

if (i == caps->host.ncaches) {

if (VIR_APPEND_ELEMENT(caps->host.caches,

caps->host.ncaches,

diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h

index e099ccc..13effdd 100644

--- a/src/conf/capabilities.h

+++ b/src/conf/capabilities.h

@@ -139,15 +139,22 @@ struct _virCapsHostSecModel {

};

typedef enum {

- VIR_CACHE_TYPE_DATA,

- VIR_CACHE_TYPE_INSTRUCTION,

VIR_CACHE_TYPE_UNIFIED,

+ VIR_CACHE_TYPE_INSTRUCTION,

+ VIR_CACHE_TYPE_DATA,

VIR_CACHE_TYPE_LAST

} virCacheType;

VIR_ENUM_DECL(virCache);

+typedef struct _virCapsHostCacheControl virCapsHostCacheControl;

+typedef virCapsHostCacheControl *virCapsHostCacheControlPtr;

+struct _virCapsHostCacheControl {

+ unsigned long long min; /* B */

+ virCacheType type; /* Data, Instruction or Unified */

+ unsigned int nclos; /* number class of id */

+};

typedef struct _virCapsHostCacheBank virCapsHostCacheBank;

typedef virCapsHostCacheBank *virCapsHostCacheBankPtr;

struct _virCapsHostCacheBank {

@@ -156,6 +163,8 @@ struct _virCapsHostCacheBank {

unsigned long long size; /* B */

virCacheType type; /* Data, Instruction or Unified */

virBitmapPtr cpus; /* All CPUs that share this bank */

+ size_t ncontrol;

+ virCapsHostCacheControlPtr *controls;

};

typedef struct _virCapsHost virCapsHost;

diff --git a/tests/vircaps2xmldata/vircaps-x86_64-caches.xml b/tests/vircaps2xmldata/vircaps-x86_64-caches.xml

index f2da28e..61269ea 100644

--- a/tests/vircaps2xmldata/vircaps-x86_64-caches.xml

+++ b/tests/vircaps2xmldata/vircaps-x86_64-caches.xml

@@ -30,6 +30,8 @@

</topology>

<cache>

<bank id='0' level='3' type='unified' size='8192' unit='KiB' cpus='0-7'/>

+ <control min='419430' unit='B' type='instruction' nclos='8'/>

+ <control min='419430' unit='B' type='data' nclos='8'/>

This file should have no <control/> in it. There is no resctrl for

these. I don't see the bug immeadiatelly, though.

</cache>

</host>

diff --git a/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml b/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml

index c30ea87..df27b94 100644

--- a/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml

+++ b/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml

@@ -42,7 +42,9 @@

</topology>

<cache>

<bank id='0' level='3' type='unified' size='15360' unit='KiB' cpus='0-5'/>

+ <control min='768' unit='KiB' type='unified' nclos='4'/>

<bank id='1' level='3' type='unified' size='15360' unit='KiB' cpus='6-11'/>

+ <control min='768' unit='KiB' type='unified' nclos='4'/>

</cache>

</host>

diff --git a/tests/vircaps2xmltest.c b/tests/vircaps2xmltest.c

index f590249..93f776a 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_dir = NULL;

int ret = -1;

/*

@@ -58,6 +59,15 @@ test_virCapabilities(const void *opaque)

data->resctrl ? "/system" : "") < 0)

goto cleanup;

+ if (data->resctrl) {

+ if (virAsprintf(&resctrl_dir, "%s/vircaps2xmldata/linux-%s%s",

just /resctrl instead of the last %s

+ abs_srcdir, data->filename,

+ "/resctrl") < 0)

+ goto cleanup;

+ virFileMockAddPrefix("/sys/fs/resctrl", resctrl_dir);

+ }

+

+

virFileMockAddPrefix("/sys/devices/system", dir);

caps = virCapabilitiesNew(data->arch, data->offlineMigrate, data->liveMigrate);

@@ -84,6 +94,7 @@ test_virCapabilities(const void *opaque)

cleanup:

VIR_FREE(dir);

+ VIR_FREE(resctrl_dir);

VIR_FREE(path);

VIR_FREE(capsXML);

virObjectUnref(caps);

--

1.9.1

--

libvir-list mailing list

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