Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; --- src/util/virresctrl.c | 79 +++++++++++++++++++++++++++++++++------------------ 1 file changed, 52 insertions(+), 27 deletions(-) diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index 42876c6e2b9b..fa7c28e9b45e 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -1041,6 +1041,53 @@ virResctrlAllocParse(virResctrlInfoPtr resctrl, } +static int +virResctrlAllocGetGroup(virResctrlInfoPtr resctrl, + const char *groupname, + virResctrlAllocPtr *alloc) +{ + char *schemata = NULL; + int rv = virFileReadValueString(&schemata, + SYSFS_RESCTRL_PATH + "/%s/schemata", + groupname); + + if (rv < 0) + return rv; + + *alloc = virResctrlAllocNew(); + if (!*alloc) + goto error; + + if (virResctrlAllocParse(resctrl, *alloc, schemata) < 0) + goto error; + + VIR_FREE(schemata); + return 0; + + error: + VIR_FREE(schemata); + virObjectUnref(*alloc); + *alloc = NULL; + return -1; +} + + +static virResctrlAllocPtr +virResctrlAllocGetDefault(virResctrlInfoPtr resctrl) +{ + virResctrlAllocPtr ret = NULL; + + if (virResctrlAllocGetGroup(resctrl, ".", &ret) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not read schemata file for the default group")); + return NULL; + } + + return ret; +} + + static void virResctrlAllocSubtractPerType(virResctrlAllocPerTypePtr dst, virResctrlAllocPerTypePtr src) @@ -1141,7 +1188,6 @@ virResctrlAllocGetUnused(virResctrlInfoPtr resctrl) virResctrlAllocPtr alloc = NULL; struct dirent *ent = NULL; DIR *dirp = NULL; - char *schemata = NULL; int rv = -1; if (virResctrlInfoIsEmpty(resctrl)) { @@ -1154,22 +1200,12 @@ virResctrlAllocGetUnused(virResctrlInfoPtr resctrl) if (!ret) return NULL; - if (virFileReadValueString(&schemata, - SYSFS_RESCTRL_PATH - "/schemata") < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Could not read schemata file for the default group")); - goto error; - } - - alloc = virResctrlAllocNew(); + alloc = virResctrlAllocGetDefault(resctrl); if (!alloc) goto error; - if (virResctrlAllocParse(resctrl, alloc, schemata) < 0) - goto error; - virResctrlAllocSubtract(ret, alloc); + virObjectUnref(alloc); if (virDirOpen(&dirp, SYSFS_RESCTRL_PATH) < 0) goto error; @@ -1178,11 +1214,7 @@ virResctrlAllocGetUnused(virResctrlInfoPtr resctrl) if (STREQ(ent->d_name, "info")) continue; - VIR_FREE(schemata); - rv = virFileReadValueString(&schemata, - SYSFS_RESCTRL_PATH - "/%s/schemata", - ent->d_name); + rv = virResctrlAllocGetGroup(resctrl, ent->d_name, &alloc); if (rv == -2) continue; @@ -1193,15 +1225,9 @@ virResctrlAllocGetUnused(virResctrlInfoPtr resctrl) goto error; } - virObjectUnref(alloc); - alloc = virResctrlAllocNew(); - if (!alloc) - goto error; - - if (virResctrlAllocParse(resctrl, alloc, schemata) < 0) - goto error; - virResctrlAllocSubtract(ret, alloc); + virObjectUnref(alloc); + alloc = NULL; } if (rv < 0) goto error; @@ -1209,7 +1235,6 @@ virResctrlAllocGetUnused(virResctrlInfoPtr resctrl) cleanup: virObjectUnref(alloc); VIR_DIR_CLOSE(dirp); - VIR_FREE(schemata); return ret; error: -- 2.16.1

Introduce virResctrlAllocCopyMasks() and use that to initially copy the default group schemata to the allocation before reserving any parts of the cache. The reason for this is that when new group is created the schemata will have unknown data in it. If there was previously group with the same CLoS ID, it will have the previous valies, if not it will have all bits set. And we need to set all unspecified (in the XML) allocations to the same one as the default group. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1289368 Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; --- src/util/virresctrl.c | 64 ++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 48 insertions(+), 16 deletions(-) diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index fa7c28e9b45e..2bdca5455783 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -1296,23 +1296,8 @@ virResctrlAllocFindUnused(virResctrlAllocPerTypePtr a_type, ssize_t last_bits = 0; ssize_t last_pos = -1; - /* If there is no reservation requested we need to set all bits. That's due - * to weird interface of the resctrl sysfs. It's also the reason why we - * cannot reserve the whole cache in one allocation. */ - if (!size) { - a_mask = virBitmapNew(i_type->bits); - if (!a_mask) - return -1; - - virBitmapSetAll(a_mask); - - if (virResctrlAllocSetMask(a_type, cache, a_mask) < 0) { - virBitmapFree(a_mask); - return -1; - } - + if (!size) return 0; - } if (cache >= f_type->nmasks) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -1415,6 +1400,44 @@ virResctrlAllocFindUnused(virResctrlAllocPerTypePtr a_type, } +static int +virResctrlAllocCopyMasks(virResctrlAllocPtr dst, + virResctrlAllocPtr src) +{ + unsigned int level = 0; + + for (level = 0; level < src->nlevels; level++) { + virResctrlAllocPerLevelPtr s_level = src->levels[level]; + unsigned int type = 0; + + if (!s_level) + continue; + + for (type = 0; type < VIR_CACHE_TYPE_LAST; type++) { + virResctrlAllocPerTypePtr s_type = s_level->types[type]; + virResctrlAllocPerTypePtr d_type = NULL; + unsigned int cache = 0; + + if (!s_type) + continue; + + d_type = virResctrlAllocGetType(dst, level, type); + if (!d_type) + return -1; + + for (cache = 0; cache < s_type->nmasks; cache++) { + virBitmapPtr mask = s_type->masks[cache]; + + if (mask && virResctrlAllocUpdateMask(dst, level, type, cache, mask) < 0) + return -1; + } + } + } + + return 0; +} + + /* * This function is called when creating an allocation in the system. What it * does is that it gets all the unused bits using virResctrlAllocGetUnused() and @@ -1428,11 +1451,19 @@ virResctrlAllocMasksAssign(virResctrlInfoPtr resctrl, int ret = -1; unsigned int level = 0; virResctrlAllocPtr alloc_free = NULL; + virResctrlAllocPtr alloc_default = NULL; alloc_free = virResctrlAllocGetUnused(resctrl); if (!alloc_free) return -1; + alloc_default = virResctrlAllocGetDefault(resctrl); + if (!alloc_default) + return -1; + + if (virResctrlAllocCopyMasks(alloc, alloc_default) < 0) + return -1; + for (level = 0; level < alloc->nlevels; level++) { virResctrlAllocPerLevelPtr a_level = alloc->levels[level]; virResctrlAllocPerLevelPtr f_level = NULL; @@ -1480,6 +1511,7 @@ virResctrlAllocMasksAssign(virResctrlInfoPtr resctrl, ret = 0; cleanup: virObjectUnref(alloc_free); + virObjectUnref(alloc_default); return ret; } -- 2.16.1

We can use this from more places later, so just a future code de-duplication. Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; --- src/libvirt_private.syms | 1 + src/util/virresctrl.c | 29 ++++++++++++++++++++--------- src/util/virresctrl.h | 4 ++++ 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 792fb60568bd..9339c2c3259d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2560,6 +2560,7 @@ virCacheTypeFromString; virCacheTypeToString; virResctrlAllocAddPID; virResctrlAllocCreate; +virResctrlAllocDeterminePath; virResctrlAllocForeachSize; virResctrlAllocFormat; virResctrlAllocGetID; diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index 14a3c459ffd3..fefca32a6710 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -1503,6 +1503,25 @@ virResctrlAllocMasksAssign(virResctrlInfoPtr resctrl, } +int +virResctrlAllocDeterminePath(virResctrlAllocPtr alloc, + const char *machinename) +{ + if (!alloc->id) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Resctrl Allocation ID must be set before creation")); + return -1; + } + + if (!alloc->path && + virAsprintf(&alloc->path, "%s/%s-%s", + SYSFS_RESCTRL_PATH, machinename, alloc->id) < 0) + return -1; + + return 0; +} + + /* This checks if the directory for the alloc exists. If not it tries to create * it and apply appropriate alloc settings. */ int @@ -1524,15 +1543,7 @@ virResctrlAllocCreate(virResctrlInfoPtr resctrl, return -1; } - if (!alloc->id) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Resctrl Allocation ID must be set before creation")); - return -1; - } - - if (!alloc->path && - virAsprintf(&alloc->path, "%s/%s-%s", - SYSFS_RESCTRL_PATH, machinename, alloc->id) < 0) + if (virResctrlAllocDeterminePath(alloc, machinename) < 0) return -1; if (virFileExists(alloc->path)) { diff --git a/src/util/virresctrl.h b/src/util/virresctrl.h index 63a954feaad1..5368ba211924 100644 --- a/src/util/virresctrl.h +++ b/src/util/virresctrl.h @@ -102,6 +102,10 @@ virResctrlAllocGetID(virResctrlAllocPtr alloc); char * virResctrlAllocFormat(virResctrlAllocPtr alloc); +int +virResctrlAllocDeterminePath(virResctrlAllocPtr alloc, + const char *machinename); + int virResctrlAllocCreate(virResctrlInfoPtr r_info, virResctrlAllocPtr alloc, -- 2.16.1

If we reorder the functions properly, we only need to have few stub functions for non-Linux platforms and it is also nicer to look at and read if they are in one place with only one preprocessor condition. The order after this patch is: - Class allocation and Static functions (used by everything, static dependencies) - Linux-only functions - Non-Linux stubs - Exported functions (as they rely on all previous functions) Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; --- src/libvirt_linux.syms | 3 + src/libvirt_private.syms | 1 - src/util/virresctrl.c | 939 +++++++++++++++++++++++------------------------ 3 files changed, 459 insertions(+), 484 deletions(-) diff --git a/src/libvirt_linux.syms b/src/libvirt_linux.syms index 5fa2c790efc1..27425e4bbebb 100644 --- a/src/libvirt_linux.syms +++ b/src/libvirt_linux.syms @@ -9,6 +9,9 @@ virHostCPUGetSiblingsList; virHostCPUGetSocket; virHostCPUGetStatsLinux; +# util/virresctrl.h +virResctrlAllocGetUnused; + # Let emacs know we want case-insensitive sorting # Local Variables: # sort-fold-case: t diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9339c2c3259d..21ec8d8c8c34 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2564,7 +2564,6 @@ virResctrlAllocDeterminePath; virResctrlAllocForeachSize; virResctrlAllocFormat; virResctrlAllocGetID; -virResctrlAllocGetUnused; virResctrlAllocNew; virResctrlAllocRemove; virResctrlAllocSetID; diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index fefca32a6710..cab7570f7b8e 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -294,346 +294,6 @@ virResctrlAllocNew(void) } -/* Common functions */ -#ifdef __linux__ -static int -virResctrlLockInternal(int op) -{ - int fd = open(SYSFS_RESCTRL_PATH, O_DIRECTORY | O_CLOEXEC); - - if (fd < 0) { - virReportSystemError(errno, "%s", _("Cannot open resctrl")); - return -1; - } - - if (flock(fd, op) < 0) { - virReportSystemError(errno, "%s", _("Cannot lock resctrl")); - VIR_FORCE_CLOSE(fd); - return -1; - } - - return fd; -} - - -static inline int -virResctrlLockWrite(void) -{ - return virResctrlLockInternal(LOCK_EX); -} - -#else - -static inline int -virResctrlLockWrite(void) -{ - virReportSystemError(ENOSYS, "%s", - _("resctrl not supported on this platform")); - return -1; -} - -#endif - - - - -static int -virResctrlUnlock(int fd) -{ - if (fd == -1) - return 0; - -#ifdef __linux__ - /* The lock gets unlocked by closing the fd, which we need to do anyway in - * order to clean up properly */ - if (VIR_CLOSE(fd) < 0) { - virReportSystemError(errno, "%s", _("Cannot close resctrl")); - - /* Trying to save the already broken */ - if (flock(fd, LOCK_UN) < 0) - virReportSystemError(errno, "%s", _("Cannot unlock resctrl")); - return -1; - } -#endif /* ! __linux__ */ - - return 0; -} - - -/* Info-related functions */ -static bool -virResctrlInfoIsEmpty(virResctrlInfoPtr resctrl) -{ - size_t i = 0; - size_t j = 0; - - if (!resctrl) - return true; - - for (i = 0; i < resctrl->nlevels; i++) { - virResctrlInfoPerLevelPtr i_level = resctrl->levels[i]; - - if (!i_level) - continue; - - for (j = 0; j < VIR_CACHE_TYPE_LAST; j++) { - if (i_level->types[j]) - return false; - } - } - - return true; -} - - -#ifdef __linux__ - -int -virResctrlGetInfo(virResctrlInfoPtr resctrl) -{ - DIR *dirp = NULL; - char *endptr = NULL; - char *tmp_str = NULL; - int ret = -1; - int rv = -1; - int type = 0; - struct dirent *ent = NULL; - unsigned int level = 0; - virResctrlInfoPerLevelPtr i_level = NULL; - virResctrlInfoPerTypePtr i_type = NULL; - - rv = virDirOpenIfExists(&dirp, SYSFS_RESCTRL_PATH "/info"); - if (rv <= 0) { - ret = rv; - goto cleanup; - } - - while ((rv = virDirRead(dirp, &ent, SYSFS_RESCTRL_PATH "/info")) > 0) { - VIR_DEBUG("Parsing info type '%s'", ent->d_name); - if (ent->d_name[0] != 'L') - continue; - - if (virStrToLong_uip(ent->d_name + 1, &endptr, 10, &level) < 0) { - VIR_DEBUG("Cannot parse resctrl cache info level '%s'", ent->d_name + 1); - continue; - } - - type = virResctrlTypeFromString(endptr); - if (type < 0) { - VIR_DEBUG("Cannot parse resctrl cache info type '%s'", endptr); - continue; - } - - if (VIR_ALLOC(i_type) < 0) - goto cleanup; - - i_type->control.scope = type; - - rv = virFileReadValueUint(&i_type->control.max_allocation, - SYSFS_RESCTRL_PATH "/info/%s/num_closids", - ent->d_name); - if (rv == -2) { - /* The file doesn't exist, so it's unusable for us, - * but we can scan further */ - VIR_WARN("The path '" SYSFS_RESCTRL_PATH "/info/%s/num_closids' " - "does not exist", - ent->d_name); - } else if (rv < 0) { - /* Other failures are fatal, so just quit */ - goto cleanup; - } - - rv = virFileReadValueString(&i_type->cbm_mask, - SYSFS_RESCTRL_PATH - "/info/%s/cbm_mask", - ent->d_name); - if (rv == -2) { - /* If the previous file exists, so should this one. Hence -2 is - * fatal in this case as well (errors out in next condition) - the - * kernel interface might've changed too much or something else is - * wrong. */ - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot get cbm_mask from resctrl cache info")); - } - if (rv < 0) - goto cleanup; - - virStringTrimOptionalNewline(i_type->cbm_mask); - - rv = virFileReadValueUint(&i_type->min_cbm_bits, - SYSFS_RESCTRL_PATH "/info/%s/min_cbm_bits", - ent->d_name); - if (rv == -2) - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot get min_cbm_bits from resctrl cache info")); - if (rv < 0) - goto cleanup; - - if (resctrl->nlevels <= level && - VIR_EXPAND_N(resctrl->levels, resctrl->nlevels, - level - resctrl->nlevels + 1) < 0) - goto cleanup; - - if (!resctrl->levels[level]) { - virResctrlInfoPerTypePtr *types = NULL; - - if (VIR_ALLOC_N(types, VIR_CACHE_TYPE_LAST) < 0) - goto cleanup; - - if (VIR_ALLOC(resctrl->levels[level]) < 0) { - VIR_FREE(types); - goto cleanup; - } - resctrl->levels[level]->types = types; - } - - i_level = resctrl->levels[level]; - - if (i_level->types[type]) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Duplicate cache type in resctrl for level %u"), - level); - goto cleanup; - } - - for (tmp_str = i_type->cbm_mask; *tmp_str != '\0'; tmp_str++) { - if (!c_isxdigit(*tmp_str)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot parse cbm_mask from resctrl cache info")); - goto cleanup; - } - - i_type->bits += count_one_bits(virHexToBin(*tmp_str)); - } - - VIR_STEAL_PTR(i_level->types[type], i_type); - } - - ret = 0; - cleanup: - VIR_DIR_CLOSE(dirp); - if (i_type) - VIR_FREE(i_type->cbm_mask); - VIR_FREE(i_type); - return ret; -} - -#else /* ! __linux__ */ - -int -virResctrlGetInfo(virResctrlInfoPtr resctrl ATTRIBUTE_UNUSED) -{ - virReportSystemError(ENOSYS, "%s", - _("Cache tune not supported on this platform")); - return -1; -} - -#endif /* ! __linux__ */ - - -int -virResctrlInfoGetCache(virResctrlInfoPtr resctrl, - unsigned int level, - unsigned long long size, - size_t *ncontrols, - virResctrlInfoPerCachePtr **controls) -{ - virResctrlInfoPerLevelPtr i_level = NULL; - virResctrlInfoPerTypePtr i_type = NULL; - size_t i = 0; - int ret = -1; - - if (virResctrlInfoIsEmpty(resctrl)) - return 0; - - if (level >= resctrl->nlevels) - return 0; - - i_level = resctrl->levels[level]; - if (!i_level) - return 0; - - for (i = 0; i < VIR_CACHE_TYPE_LAST; i++) { - i_type = i_level->types[i]; - if (!i_type) - continue; - - /* Let's take the opportunity to update our internal information about - * the cache size */ - if (!i_type->size) { - i_type->size = size; - i_type->control.granularity = size / i_type->bits; - if (i_type->min_cbm_bits != 1) - i_type->control.min = i_type->min_cbm_bits * i_type->control.granularity; - } else { - if (i_type->size != size) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("level %u cache size %llu does not match " - "expected size %llu"), - level, i_type->size, size); - goto error; - } - i_type->max_cache_id++; - } - - if (VIR_EXPAND_N(*controls, *ncontrols, 1) < 0) - goto error; - if (VIR_ALLOC((*controls)[*ncontrols - 1]) < 0) - goto error; - - memcpy((*controls)[*ncontrols - 1], &i_type->control, sizeof(i_type->control)); - } - - ret = 0; - cleanup: - return ret; - error: - while (*ncontrols) - VIR_FREE((*controls)[--*ncontrols]); - VIR_FREE(*controls); - goto cleanup; -} - - -/* Alloc-related functions */ -bool -virResctrlAllocIsEmpty(virResctrlAllocPtr resctrl) -{ - size_t i = 0; - size_t j = 0; - size_t k = 0; - - if (!resctrl) - return true; - - for (i = 0; i < resctrl->nlevels; i++) { - virResctrlAllocPerLevelPtr a_level = resctrl->levels[i]; - - if (!a_level) - continue; - - for (j = 0; j < VIR_CACHE_TYPE_LAST; j++) { - virResctrlAllocPerTypePtr a_type = a_level->types[j]; - - if (!a_type) - continue; - - for (k = 0; k < a_type->nsizes; k++) { - if (a_type->sizes[k]) - return false; - } - - for (k = 0; k < a_type->nmasks; k++) { - if (a_type->masks[k]) - return false; - } - } - } - - return true; -} - - static virResctrlAllocPerTypePtr virResctrlAllocGetType(virResctrlAllocPtr resctrl, unsigned int level, @@ -667,38 +327,6 @@ virResctrlAllocGetType(virResctrlAllocPtr resctrl, } -#ifdef __linux__ - -static int -virResctrlAllocUpdateMask(virResctrlAllocPtr resctrl, - unsigned int level, - virCacheType type, - unsigned int cache, - virBitmapPtr mask) -{ - virResctrlAllocPerTypePtr a_type = virResctrlAllocGetType(resctrl, level, type); - - if (!a_type) - return -1; - - if (a_type->nmasks <= cache && - VIR_EXPAND_N(a_type->masks, a_type->nmasks, - cache - a_type->nmasks + 1) < 0) - return -1; - - if (!a_type->masks[cache]) { - a_type->masks[cache] = virBitmapNew(virBitmapSize(mask)); - - if (!a_type->masks[cache]) - return -1; - } - - return virBitmapCopy(a_type->masks[cache], mask); -} - -#endif - - static int virResctrlAllocUpdateSize(virResctrlAllocPtr resctrl, unsigned int level, @@ -725,6 +353,31 @@ virResctrlAllocUpdateSize(virResctrlAllocPtr resctrl, } +static bool +virResctrlInfoIsEmpty(virResctrlInfoPtr resctrl) +{ + size_t i = 0; + size_t j = 0; + + if (!resctrl) + return true; + + for (i = 0; i < resctrl->nlevels; i++) { + virResctrlInfoPerLevelPtr i_level = resctrl->levels[i]; + + if (!i_level) + continue; + + for (j = 0; j < VIR_CACHE_TYPE_LAST; j++) { + if (i_level->types[j]) + return false; + } + } + + return true; +} + + /* * Check if there is an allocation for this level/type/cache already. Called * before updating the structure. VIR_CACHE_TYPE_BOTH collides with any type, @@ -775,150 +428,223 @@ virResctrlAllocCheckCollision(virResctrlAllocPtr alloc, } else { a_type = a_level->types[type]; - if (a_type && a_type->nsizes > cache && a_type->sizes[cache]) - return true; + if (a_type && a_type->nsizes > cache && a_type->sizes[cache]) + return true; + } + + return false; +} + + +#ifdef __linux__ + +static int +virResctrlLockInternal(int op) +{ + int fd = open(SYSFS_RESCTRL_PATH, O_DIRECTORY | O_CLOEXEC); + + if (fd < 0) { + virReportSystemError(errno, "%s", _("Cannot open resctrl")); + return -1; + } + + if (flock(fd, op) < 0) { + virReportSystemError(errno, "%s", _("Cannot lock resctrl")); + VIR_FORCE_CLOSE(fd); + return -1; } - return false; + return fd; } -int -virResctrlAllocSetSize(virResctrlAllocPtr resctrl, - unsigned int level, - virCacheType type, - unsigned int cache, - unsigned long long size) +static inline int +virResctrlLockWrite(void) { - if (virResctrlAllocCheckCollision(resctrl, level, type, cache)) { - virReportError(VIR_ERR_XML_ERROR, - _("Colliding cache allocations for cache " - "level '%u' id '%u', type '%s'"), - level, cache, virCacheTypeToString(type)); + return virResctrlLockInternal(LOCK_EX); +} + + +static int +virResctrlUnlock(int fd) +{ + if (fd == -1) + return 0; + + /* The lock gets unlocked by closing the fd, which we need to do anyway in + * order to clean up properly */ + if (VIR_CLOSE(fd) < 0) { + virReportSystemError(errno, "%s", _("Cannot close resctrl")); + + /* Trying to save the already broken */ + if (flock(fd, LOCK_UN) < 0) + virReportSystemError(errno, "%s", _("Cannot unlock resctrl")); return -1; } - return virResctrlAllocUpdateSize(resctrl, level, type, cache, size); + return 0; } int -virResctrlAllocForeachSize(virResctrlAllocPtr resctrl, - virResctrlAllocForeachSizeCallback cb, - void *opaque) +virResctrlGetInfo(virResctrlInfoPtr resctrl) { - int ret = 0; + DIR *dirp = NULL; + char *endptr = NULL; + char *tmp_str = NULL; + int ret = -1; + int rv = -1; + int type = 0; + struct dirent *ent = NULL; unsigned int level = 0; - unsigned int type = 0; - unsigned int cache = 0; - - if (!resctrl) - return 0; + virResctrlInfoPerLevelPtr i_level = NULL; + virResctrlInfoPerTypePtr i_type = NULL; - for (level = 0; level < resctrl->nlevels; level++) { - virResctrlAllocPerLevelPtr a_level = resctrl->levels[level]; + rv = virDirOpenIfExists(&dirp, SYSFS_RESCTRL_PATH "/info"); + if (rv <= 0) { + ret = rv; + goto cleanup; + } - if (!a_level) + while ((rv = virDirRead(dirp, &ent, SYSFS_RESCTRL_PATH "/info")) > 0) { + VIR_DEBUG("Parsing info type '%s'", ent->d_name); + if (ent->d_name[0] != 'L') continue; - for (type = 0; type < VIR_CACHE_TYPE_LAST; type++) { - virResctrlAllocPerTypePtr a_type = a_level->types[type]; + if (virStrToLong_uip(ent->d_name + 1, &endptr, 10, &level) < 0) { + VIR_DEBUG("Cannot parse resctrl cache info level '%s'", ent->d_name + 1); + continue; + } - if (!a_type) - continue; + type = virResctrlTypeFromString(endptr); + if (type < 0) { + VIR_DEBUG("Cannot parse resctrl cache info type '%s'", endptr); + continue; + } - for (cache = 0; cache < a_type->nsizes; cache++) { - unsigned long long *size = a_type->sizes[cache]; + if (VIR_ALLOC(i_type) < 0) + goto cleanup; - if (!size) - continue; + i_type->control.scope = type; - ret = cb(level, type, cache, *size, opaque); - if (ret < 0) - return ret; - } + rv = virFileReadValueUint(&i_type->control.max_allocation, + SYSFS_RESCTRL_PATH "/info/%s/num_closids", + ent->d_name); + if (rv == -2) { + /* The file doesn't exist, so it's unusable for us, + * but we can scan further */ + VIR_WARN("The path '" SYSFS_RESCTRL_PATH "/info/%s/num_closids' " + "does not exist", + ent->d_name); + } else if (rv < 0) { + /* Other failures are fatal, so just quit */ + goto cleanup; } - } - return 0; -} + rv = virFileReadValueString(&i_type->cbm_mask, + SYSFS_RESCTRL_PATH + "/info/%s/cbm_mask", + ent->d_name); + if (rv == -2) { + /* If the previous file exists, so should this one. Hence -2 is + * fatal in this case as well (errors out in next condition) - the + * kernel interface might've changed too much or something else is + * wrong. */ + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot get cbm_mask from resctrl cache info")); + } + if (rv < 0) + goto cleanup; + virStringTrimOptionalNewline(i_type->cbm_mask); -int -virResctrlAllocSetID(virResctrlAllocPtr alloc, - const char *id) -{ - if (!id) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Resctrl allocation 'id' cannot be NULL")); - return -1; - } + rv = virFileReadValueUint(&i_type->min_cbm_bits, + SYSFS_RESCTRL_PATH "/info/%s/min_cbm_bits", + ent->d_name); + if (rv == -2) + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot get min_cbm_bits from resctrl cache info")); + if (rv < 0) + goto cleanup; - return VIR_STRDUP(alloc->id, id); -} + if (resctrl->nlevels <= level && + VIR_EXPAND_N(resctrl->levels, resctrl->nlevels, + level - resctrl->nlevels + 1) < 0) + goto cleanup; + if (!resctrl->levels[level]) { + virResctrlInfoPerTypePtr *types = NULL; -const char * -virResctrlAllocGetID(virResctrlAllocPtr alloc) -{ - return alloc->id; -} + if (VIR_ALLOC_N(types, VIR_CACHE_TYPE_LAST) < 0) + goto cleanup; + if (VIR_ALLOC(resctrl->levels[level]) < 0) { + VIR_FREE(types); + goto cleanup; + } + resctrl->levels[level]->types = types; + } -char * -virResctrlAllocFormat(virResctrlAllocPtr resctrl) -{ - virBuffer buf = VIR_BUFFER_INITIALIZER; - unsigned int level = 0; - unsigned int type = 0; - unsigned int cache = 0; + i_level = resctrl->levels[level]; - if (!resctrl) - return NULL; + if (i_level->types[type]) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Duplicate cache type in resctrl for level %u"), + level); + goto cleanup; + } - for (level = 0; level < resctrl->nlevels; level++) { - virResctrlAllocPerLevelPtr a_level = resctrl->levels[level]; + for (tmp_str = i_type->cbm_mask; *tmp_str != '\0'; tmp_str++) { + if (!c_isxdigit(*tmp_str)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot parse cbm_mask from resctrl cache info")); + goto cleanup; + } - if (!a_level) - continue; + i_type->bits += count_one_bits(virHexToBin(*tmp_str)); + } - for (type = 0; type < VIR_CACHE_TYPE_LAST; type++) { - virResctrlAllocPerTypePtr a_type = a_level->types[type]; + VIR_STEAL_PTR(i_level->types[type], i_type); + } - if (!a_type) - continue; + ret = 0; + cleanup: + VIR_DIR_CLOSE(dirp); + if (i_type) + VIR_FREE(i_type->cbm_mask); + VIR_FREE(i_type); + return ret; +} - virBufferAsprintf(&buf, "L%u%s:", level, virResctrlTypeToString(type)); - for (cache = 0; cache < a_type->nmasks; cache++) { - virBitmapPtr mask = a_type->masks[cache]; - char *mask_str = NULL; +static int +virResctrlAllocUpdateMask(virResctrlAllocPtr resctrl, + unsigned int level, + virCacheType type, + unsigned int cache, + virBitmapPtr mask) +{ + virResctrlAllocPerTypePtr a_type = virResctrlAllocGetType(resctrl, level, type); - if (!mask) - continue; + if (!a_type) + return -1; - mask_str = virBitmapToString(mask, false, true); - if (!mask_str) { - virBufferFreeAndReset(&buf); - return NULL; - } + if (a_type->nmasks <= cache && + VIR_EXPAND_N(a_type->masks, a_type->nmasks, + cache - a_type->nmasks + 1) < 0) + return -1; - virBufferAsprintf(&buf, "%u=%s;", cache, mask_str); - VIR_FREE(mask_str); - } + if (!a_type->masks[cache]) { + a_type->masks[cache] = virBitmapNew(virBitmapSize(mask)); - virBufferTrim(&buf, ";", 1); - virBufferAddChar(&buf, '\n'); - } + if (!a_type->masks[cache]) + return -1; } - virBufferCheckError(&buf); - return virBufferContentAndReset(&buf); + return virBitmapCopy(a_type->masks[cache], mask); } -#ifdef __linux__ - static int virResctrlAllocParseProcessCache(virResctrlInfoPtr resctrl, virResctrlAllocPtr alloc, @@ -1171,6 +897,7 @@ virResctrlAllocNewFromInfo(virResctrlInfoPtr info) goto cleanup; } + /* * This function creates an allocation that represents all unused parts of all * caches in the system. It uses virResctrlInfo for creating a new full @@ -1238,23 +965,11 @@ virResctrlAllocGetUnused(virResctrlInfoPtr resctrl) return ret; error: - virObjectUnref(ret); - ret = NULL; - goto cleanup; -} - -#else /* ! __linux__ */ - -virResctrlAllocPtr -virResctrlAllocGetUnused(virResctrlInfoPtr resctrl ATTRIBUTE_UNUSED) -{ - virReportSystemError(ENOSYS, "%s", - _("Cache tune not supported on this platform")); - return NULL; + virObjectUnref(ret); + ret = NULL; + goto cleanup; } -#endif /* ! __linux__ */ - /* * Given the information about requested allocation type `a_type`, the host @@ -1591,6 +1306,264 @@ virResctrlAllocCreate(virResctrlInfoPtr resctrl, return ret; } +#else /* ! __linux__ */ + +int +virResctrlGetInfo(virResctrlInfoPtr resctrl ATTRIBUTE_UNUSED) +{ + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Resource control is not supported on this host")); + return -1; +} + +int +virResctrlAllocCreate(virResctrlInfoPtr resctrl ATTRIBUTE_UNUSED, + virResctrlAllocPtr alloc ATTRIBUTE_UNUSED, + const char *machinename ATTRIBUTE_UNUSED) +{ + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Resource control is not supported on this host")); + return -1; +} + +#endif /* ! __linux__ */ + + +int +virResctrlInfoGetCache(virResctrlInfoPtr resctrl, + unsigned int level, + unsigned long long size, + size_t *ncontrols, + virResctrlInfoPerCachePtr **controls) +{ + virResctrlInfoPerLevelPtr i_level = NULL; + virResctrlInfoPerTypePtr i_type = NULL; + size_t i = 0; + int ret = -1; + + if (virResctrlInfoIsEmpty(resctrl)) + return 0; + + if (level >= resctrl->nlevels) + return 0; + + i_level = resctrl->levels[level]; + if (!i_level) + return 0; + + for (i = 0; i < VIR_CACHE_TYPE_LAST; i++) { + i_type = i_level->types[i]; + if (!i_type) + continue; + + /* Let's take the opportunity to update our internal information about + * the cache size */ + if (!i_type->size) { + i_type->size = size; + i_type->control.granularity = size / i_type->bits; + if (i_type->min_cbm_bits != 1) + i_type->control.min = i_type->min_cbm_bits * i_type->control.granularity; + } else { + if (i_type->size != size) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("level %u cache size %llu does not match " + "expected size %llu"), + level, i_type->size, size); + goto error; + } + i_type->max_cache_id++; + } + + if (VIR_EXPAND_N(*controls, *ncontrols, 1) < 0) + goto error; + if (VIR_ALLOC((*controls)[*ncontrols - 1]) < 0) + goto error; + + memcpy((*controls)[*ncontrols - 1], &i_type->control, sizeof(i_type->control)); + } + + ret = 0; + cleanup: + return ret; + error: + while (*ncontrols) + VIR_FREE((*controls)[--*ncontrols]); + VIR_FREE(*controls); + goto cleanup; +} + + +bool +virResctrlAllocIsEmpty(virResctrlAllocPtr resctrl) +{ + size_t i = 0; + size_t j = 0; + size_t k = 0; + + if (!resctrl) + return true; + + for (i = 0; i < resctrl->nlevels; i++) { + virResctrlAllocPerLevelPtr a_level = resctrl->levels[i]; + + if (!a_level) + continue; + + for (j = 0; j < VIR_CACHE_TYPE_LAST; j++) { + virResctrlAllocPerTypePtr a_type = a_level->types[j]; + + if (!a_type) + continue; + + for (k = 0; k < a_type->nsizes; k++) { + if (a_type->sizes[k]) + return false; + } + + for (k = 0; k < a_type->nmasks; k++) { + if (a_type->masks[k]) + return false; + } + } + } + + return true; +} + + +int +virResctrlAllocSetSize(virResctrlAllocPtr resctrl, + unsigned int level, + virCacheType type, + unsigned int cache, + unsigned long long size) +{ + if (virResctrlAllocCheckCollision(resctrl, level, type, cache)) { + virReportError(VIR_ERR_XML_ERROR, + _("Colliding cache allocations for cache " + "level '%u' id '%u', type '%s'"), + level, cache, virCacheTypeToString(type)); + return -1; + } + + return virResctrlAllocUpdateSize(resctrl, level, type, cache, size); +} + + +int +virResctrlAllocForeachSize(virResctrlAllocPtr resctrl, + virResctrlAllocForeachSizeCallback cb, + void *opaque) +{ + int ret = 0; + unsigned int level = 0; + unsigned int type = 0; + unsigned int cache = 0; + + if (!resctrl) + return 0; + + for (level = 0; level < resctrl->nlevels; level++) { + virResctrlAllocPerLevelPtr a_level = resctrl->levels[level]; + + if (!a_level) + continue; + + for (type = 0; type < VIR_CACHE_TYPE_LAST; type++) { + virResctrlAllocPerTypePtr a_type = a_level->types[type]; + + if (!a_type) + continue; + + for (cache = 0; cache < a_type->nsizes; cache++) { + unsigned long long *size = a_type->sizes[cache]; + + if (!size) + continue; + + ret = cb(level, type, cache, *size, opaque); + if (ret < 0) + return ret; + } + } + } + + return 0; +} + + +int +virResctrlAllocSetID(virResctrlAllocPtr alloc, + const char *id) +{ + if (!id) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Resctrl allocation 'id' cannot be NULL")); + return -1; + } + + return VIR_STRDUP(alloc->id, id); +} + + +const char * +virResctrlAllocGetID(virResctrlAllocPtr alloc) +{ + return alloc->id; +} + + +char * +virResctrlAllocFormat(virResctrlAllocPtr resctrl) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + unsigned int level = 0; + unsigned int type = 0; + unsigned int cache = 0; + + if (!resctrl) + return NULL; + + for (level = 0; level < resctrl->nlevels; level++) { + virResctrlAllocPerLevelPtr a_level = resctrl->levels[level]; + + if (!a_level) + continue; + + for (type = 0; type < VIR_CACHE_TYPE_LAST; type++) { + virResctrlAllocPerTypePtr a_type = a_level->types[type]; + + if (!a_type) + continue; + + virBufferAsprintf(&buf, "L%u%s:", level, virResctrlTypeToString(type)); + + for (cache = 0; cache < a_type->nmasks; cache++) { + virBitmapPtr mask = a_type->masks[cache]; + char *mask_str = NULL; + + if (!mask) + continue; + + mask_str = virBitmapToString(mask, false, true); + if (!mask_str) { + virBufferFreeAndReset(&buf); + return NULL; + } + + virBufferAsprintf(&buf, "%u=%s;", cache, mask_str); + VIR_FREE(mask_str); + } + + virBufferTrim(&buf, ";", 1); + virBufferAddChar(&buf, '\n'); + } + } + + virBufferCheckError(&buf); + return virBufferContentAndReset(&buf); +} + int virResctrlAllocAddPID(virResctrlAllocPtr alloc, -- 2.16.1

This was just forgotten when doing more renames in the past when working in the reviews, I believe. Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; --- src/util/virresctrl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index cab7570f7b8e..2948f2b9569c 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -618,13 +618,13 @@ virResctrlGetInfo(virResctrlInfoPtr resctrl) static int -virResctrlAllocUpdateMask(virResctrlAllocPtr resctrl, +virResctrlAllocUpdateMask(virResctrlAllocPtr alloc, unsigned int level, virCacheType type, unsigned int cache, virBitmapPtr mask) { - virResctrlAllocPerTypePtr a_type = virResctrlAllocGetType(resctrl, level, type); + virResctrlAllocPerTypePtr a_type = virResctrlAllocGetType(alloc, level, type); if (!a_type) return -1; -- 2.16.1