On Mon, Nov 13, 2017 at 09:50:31AM +0100, Martin Kletzander wrote:
> With this commit we finally have a way to read and manipulate basic resctrl
> settings. Locking is done only on exposed functions that read/write from/to
> resctrlfs. Not in fuctions that are exposed in virresctrlpriv.h as those are
> only supposed to be used from tests.
>
> Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx>
> ---
> src/Makefile.am | 2 +-
> src/libvirt_private.syms | 12 +
> src/util/virresctrl.c | 1012 ++++++++++++++++++++++++++++++++++++++++++++-
> src/util/virresctrl.h | 59 +++
> src/util/virresctrlpriv.h | 32 ++
> 5 files changed, 1115 insertions(+), 2 deletions(-)
> create mode 100644 src/util/virresctrlpriv.h
>
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 1d24231249de..ad113262fbb0 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -167,7 +167,7 @@ UTIL_SOURCES = \
> util/virprocess.c util/virprocess.h \
> util/virqemu.c util/virqemu.h \
> util/virrandom.h util/virrandom.c \
> - util/virresctrl.h util/virresctrl.c \
> + util/virresctrl.h util/virresctrl.c util/virresctrlpriv.h \
Use only single space instead of tab after "util/virresctrl.c" and
"util/virresctrlpriv.h".
> util/virrotatingfile.h util/virrotatingfile.c \
> util/virscsi.c util/virscsi.h \
> util/virscsihost.c util/virscsihost.h \
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index b24728ce4a1d..37bac41e618b 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -2532,6 +2532,18 @@ virRandomInt;
> # util/virresctrl.h
> virCacheTypeFromString;
> virCacheTypeToString;
> +virResctrlAllocAddPID;
> +virResctrlAllocCreate;
> +virResctrlAllocForeachSize;
> +virResctrlAllocFormat;
> +virResctrlAllocGetFree;
> +virResctrlAllocMasksAssign;
> +virResctrlAllocNewFromInfo;
> +virResctrlAllocRemove;
> +virResctrlAllocSetID;
> +virResctrlAllocSetSize;
> +virResctrlAllocUpdateMask;
> +virResctrlAllocUpdateSize;
> virResctrlGetInfo;
> virResctrlInfoGetCache;
>
> diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
> index 6c6692e78f42..ac1b38436bb2 100644
> --- a/src/util/virresctrl.c
> +++ b/src/util/virresctrl.c
> @@ -23,7 +23,7 @@
> #include <sys/stat.h>
> #include <fcntl.h>
>
> -#include "virresctrl.h"
> +#include "virresctrlpriv.h"
>
> #include "c-ctype.h"
> #include "count-one-bits.h"
> @@ -151,6 +151,153 @@ virResctrlInfoNew(void)
> }
>
>
> +/* Alloc-related definitions and AllocClass-related functions */
> +typedef struct _virResctrlAllocPerType virResctrlAllocPerType;
> +typedef virResctrlAllocPerType *virResctrlAllocPerTypePtr;
> +struct _virResctrlAllocPerType {
> + /* There could be bool saying whether this is set or not, but since everything
> + * in virResctrlAlloc (and most of libvirt) goes with pointer arrays we would
> + * have to have one more level of allocation anyway, so this stays faithful to
> + * the concept */
> + unsigned long long **sizes;
> + size_t nsizes;
> +
> + /* Mask for each cache */
> + virBitmapPtr *masks;
> + size_t nmasks;
> +};
> +
> +typedef struct _virResctrlAllocPerLevel virResctrlAllocPerLevel;
> +typedef virResctrlAllocPerLevel *virResctrlAllocPerLevelPtr;
> +struct _virResctrlAllocPerLevel {
> + virResctrlAllocPerTypePtr *types; /* Indexed with enum virCacheType */
> +};
> +
> +struct _virResctrlAlloc {
> + virObject parent;
> +
> + virResctrlAllocPerLevelPtr *levels;
> + size_t nlevels;
> +
> + char *id; /* The identifier (any unique string for now) */
> + char *path;
> +};
> +
> +static virClassPtr virResctrlAllocClass;
> +
> +static void
> +virResctrlAllocDispose(void *obj)
> +{
> + size_t i = 0;
> + size_t j = 0;
> + size_t k = 0;
> +
> + virResctrlAllocPtr resctrl = obj;
> +
> + for (i = 0; i < resctrl->nlevels; i++) {
> + virResctrlAllocPerLevelPtr level = resctrl->levels[--resctrl->nlevels];
> +
> + if (!level)
> + continue;
> +
> + for (j = 0; j < VIR_CACHE_TYPE_LAST; j++) {
> + virResctrlAllocPerTypePtr type = level->types[j];
> +
> + if (!type)
> + continue;
> +
> + for (k = 0; k < type->nsizes; k++)
> + VIR_FREE(type->sizes[k]);
> +
> + VIR_FREE(type->sizes);
> + VIR_FREE(type);
> + }
> + VIR_FREE(level->types);
> + VIR_FREE(level);
> + }
> +
> + VIR_FREE(resctrl->id);
> + VIR_FREE(resctrl->levels);
> +}
> +
> +static int virResctrlAllocOnceInit(void)
> +{
> + if (!(virResctrlAllocClass = virClassNew(virClassForObject(),
> + "virResctrlAlloc",
> + sizeof(virResctrlAlloc),
> + virResctrlAllocDispose)))
> + return -1;
> +
> + return 0;
> +}
> +
> +VIR_ONCE_GLOBAL_INIT(virResctrlAlloc)
> +
> +static virResctrlAllocPtr
> +virResctrlAllocNew(void)
> +{
> + if (virResctrlAllocInitialize() < 0)
> + return NULL;
> +
> + return virObjectNew(virResctrlAllocClass);
> +}
> +
> +
> +/* Common functions */
> +static int
> +virResctrlLockInternal(int op)
> +{
> + int fd = open(SYSFS_RESCTRL_PATH, O_DIRECTORY | O_CLOEXEC);
> +
> + if (fd < 0) {
> + virReportSystemError(errno, "%s", _("Cannot open resctrlfs"));
> + return -1;
> + }
> +
> + if (flock(fd, op) < 0) {
> + virReportSystemError(errno, "%s", _("Cannot lock resctrlfs"));
> + VIR_FORCE_CLOSE(fd);
> + return -1;
> + }
> +
> + return fd;
> +}
> +
> +static inline int
> +virResctrlLockRead(void)
> +{
> + return virResctrlLockInternal(LOCK_SH);
> +}
> +
> +static inline int
> +virResctrlLockWrite(void)
> +{
> + return virResctrlLockInternal(LOCK_EX);
> +}
> +
> +static int
> +virResctrlUnlock(int fd)
> +{
> + int ret = -1;
> +
> + 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 resctrlfs"));
> +
> + /* Trying to save the already broken */
> + if (flock(fd, LOCK_UN) < 0)
> + virReportSystemError(errno, "%s", _("Cannot unlock resctrlfs"));
> + return -1;
> + }
> +
> + return ret;
> +}
> +
> +
> /* Info-related functions */
> int
> virResctrlGetInfo(virResctrlInfoPtr *resctrl)
> @@ -318,3 +465,866 @@ virResctrlInfoGetCache(virResctrlInfoPtr resctrl,
> VIR_FREE(*controls);
> goto cleanup;
> }
> +
> +
> +/* Alloc-related functions */
> +static virResctrlAllocPerTypePtr
> +virResctrlAllocFindType(virResctrlAllocPtr *resctrl,
> + unsigned int level,
> + virCacheType type,
> + bool alloc)
> +{
I don't like this implementation, it's too complex and it does two
different things based on a bool attribute. I see the benefit that
it's convenient but IMHO it's ugly.
The only call path that doesn't need allocation is from
virResctrlAllocCheckCollision(). The remaining two calls
virResctrlAllocUpdateMask() and virResctrlAllocUpdateSize() needs
to allocate the internal structures of *virResctrlAllocPtr* object.
Another point is that there is no need to have this function create
new *virResctrlAllocPtr* object on demand, I would prefer creating
that object in advance before we even start filling all the data.
> + virResctrlAllocPerLevelPtr a_level = NULL;
> + virResctrlAllocPtr tmp = NULL;
> +
> + if (!*resctrl) {
> + if (!alloc || !(*resctrl = virResctrlAllocNew()))
> + return NULL;
> + }
> +
> + tmp = *resctrl;
> +
> + /* Per-level array */
> + if (tmp->nlevels <= level) {
> + if (!alloc || VIR_EXPAND_N(tmp->levels, tmp->nlevels,
> + level - tmp->nlevels + 1) < 0)
> + return NULL;
> + }
> +
> + if (!tmp->levels[level]) {
> + if (!alloc ||
> + VIR_ALLOC(tmp->levels[level]) < 0 ||
> + VIR_ALLOC_N(tmp->levels[level]->types, VIR_CACHE_TYPE_LAST) < 0)
> + return NULL;
> + }
> + a_level = tmp->levels[level];
> +
> + if (!a_level->types[type]) {
> + if (!alloc || VIR_ALLOC(a_level->types[type]) < 0)
> + return NULL;
> + }
> +
> + return a_level->types[type];
> +}
> +
> +static virBitmapPtr *
> +virResctrlAllocFindMask(virResctrlAllocPtr *resctrl,
> + unsigned int level,
> + virCacheType type,
> + unsigned int cache,
> + bool alloc)
> +{
The code of this function can be merged into virResctrlAllocUpdateMask()
and again only allocate the structures and set the mask. Currently
there is no need for "Find" function if we will need it in the future
it should definitely only find the mask, not allocate it.
> + virResctrlAllocPerTypePtr a_type = virResctrlAllocFindType(resctrl, level,
> + type, alloc);
> +
> + if (!a_type)
> + return NULL;
> +
> + if (!a_type->masks) {
> + if (!alloc || VIR_ALLOC_N(a_type->masks, cache + 1) < 0)
> + return NULL;
> + a_type->nmasks = cache + 1;
> + } else if (a_type->nmasks <= cache) {
> + if (!alloc || VIR_EXPAND_N(a_type->masks, a_type->nmasks,
> + cache - a_type->nmasks + 1) < 0)
> + return NULL;
> + }
> +
> + return a_type->masks + cache;
> +}
> +
> +static unsigned long long *
> +virResctrlAllocFindSize(virResctrlAllocPtr *resctrl,
> + unsigned int level,
> + virCacheType type,
> + unsigned int cache,
> + bool alloc)
> +{
> + virResctrlAllocPerTypePtr a_type = virResctrlAllocFindType(resctrl, level,
> + type, alloc);
Same as for virResctrlAllocFindMask(). With the exception that we
actually need lookup function so create a one, that will only check
whether there is some size set or not.
> +
> + if (!a_type)
> + return NULL;
> +
> + if (!a_type->sizes) {
> + if (!alloc || VIR_ALLOC_N(a_type->sizes, cache + 1) < 0)
> + return NULL;
> + a_type->nsizes = cache + 1;
> + } else if (a_type->nsizes <= cache) {
> + if (!alloc || VIR_EXPAND_N(a_type->sizes, a_type->nsizes,
> + cache - a_type->nsizes + 1) < 0)
> + return NULL;
> + }
> +
> + if (!a_type->sizes[cache]) {
> + if (!alloc || VIR_ALLOC(a_type->sizes[cache]) < 0)
> + return NULL;
> + }
> +
> + return a_type->sizes[cache];
> +}
> +
> +int
> +virResctrlAllocUpdateMask(virResctrlAllocPtr *resctrl,
> + unsigned int level,
> + virCacheType type,
> + unsigned int cache,
> + virBitmapPtr mask)
> +{
> + virBitmapPtr *found = virResctrlAllocFindMask(resctrl, level, type, cache,
> + true);
> +
> + if (!found)
> + return -1;
> +
> + virBitmapFree(*found);
> +
> + *found = virBitmapNew(virBitmapSize(mask));
> + if (!*found)
> + return -1;
> +
> + return virBitmapCopy(*found, mask);
> +}
> +
> +int
> +virResctrlAllocUpdateSize(virResctrlAllocPtr *resctrl,
> + unsigned int level,
> + virCacheType type,
> + unsigned int cache,
> + unsigned long long size)
> +{
> + unsigned long long *found = virResctrlAllocFindSize(resctrl, level, type,
> + cache, true);
> +
> + if (!found)
> + return -1;
> +
> + *found = size;
> + return 0;
> +}
> +
> +static bool
> +virResctrlAllocCheckCollision(virResctrlAllocPtr a,
> + unsigned int level,
> + virCacheType type,
> + unsigned int cache)
> +{
> + /* If there is an allocation for type 'both', there can be no other
> + * allocation for the same cache */
> + if (virResctrlAllocFindSize(&a, level, VIR_CACHE_TYPE_BOTH, cache, false))
> + return true;
> +
> + if (type == VIR_CACHE_TYPE_BOTH) {
> + if (virResctrlAllocFindSize(&a, level, VIR_CACHE_TYPE_CODE, cache, false))
> + return true;
> + if (virResctrlAllocFindSize(&a, level, VIR_CACHE_TYPE_DATA, cache, false))
> + return true;
> + }
> +
> + /* And never two allocations for the same type */
> + if (virResctrlAllocFindSize(&a, level, type, cache, false))
> + return true;
> +
> + return false;
> +}
> +
> +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);
> +}
This is how I would expect all the other public functions to look like.
Simple, does one thing and there is no magic.
> +
> +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);
> + }
> +
> + virBufferTrim(&buf, ";", 1);
> + virBufferAddChar(&buf, '\n');
> + }
> + }
> +
> + virBufferCheckError(&buf);
> + return virBufferContentAndReset(&buf);
> +}
> +
> +static int
> +virResctrlAllocParseProcessCache(virResctrlAllocPtr *resctrl,
> + unsigned int level,
> + virCacheType type,
> + char *cache)
> +{
> + char *tmp = strchr(cache, '=');
> + unsigned int cache_id = 0;
> + virBitmapPtr mask = NULL;
> + int ret = -1;
> +
> + if (!tmp)
> + return 0;
> +
> + *tmp = '\0';
> + tmp++;
> +
> + if (virStrToLong_uip(cache, NULL, 10, &cache_id) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Invalid cache id '%s'"), cache);
> + return -1;
> + }
> +
> + mask = virBitmapNewString(tmp);
> + if (!mask)
> + return -1;
> +
> + if (virResctrlAllocUpdateMask(resctrl, level, type, cache_id, mask) < 0)
> + goto cleanup;
> +
> + ret = 0;
> + cleanup:
> + virBitmapFree(mask);
> + return ret;
> +}
> +
> +static int
> +virResctrlAllocParseProcessLine(virResctrlAllocPtr *resctrl,
> + char *line)
> +{
> + char **caches = NULL;
> + char *tmp = NULL;
> + unsigned int level = 0;
> + int type = -1;
> + size_t ncaches = 0;
> + size_t i = 0;
> + int ret = -1;
> +
> + /* Skip lines that don't concern caches, e.g. MB: etc. */
> + if (line[0] != 'L')
> + return 0;
> +
> + /* And lines that we can't parse too */
> + tmp = strchr(line, ':');
> + if (!tmp)
> + return 0;
> +
> + *tmp = '\0';
> + tmp++;
> +
> + if (virStrToLong_uip(line + 1, &line, 10, &level) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Cannot parse resctrl schema level '%s'"),
> + line + 1);
> + return -1;
> + }
> +
> + type = virResctrlTypeFromString(line);
> + if (type < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Cannot parse resctrl schema level '%s'"),
> + line + 1);
> + return -1;
> + }
> +
> + caches = virStringSplitCount(tmp, ";", 0, &ncaches);
> + if (!caches)
> + return 0;
> +
> + for (i = 0; i < ncaches; i++) {
> + if (virResctrlAllocParseProcessCache(resctrl, level, type, caches[i]) < 0)
> + goto cleanup;
> + }
> +
> + ret = 0;
> + cleanup:
> + virStringListFree(caches);
> + return ret;
> +}
> +
> +static int
> +virResctrlAllocParse(virResctrlAllocPtr *alloc,
> + const char *schemata)
> +{
The virResctrlAllocPtr object should already exists and this function
should only parse the data into existing object.
> + virResctrlAllocPtr tmp = NULL;
> + char **lines = NULL;
> + size_t nlines = 0;
> + size_t i = 0;
> + int ret = -1;
> +
> + lines = virStringSplitCount(schemata, "\n", 0, &nlines);
> + for (i = 0; i < nlines; i++) {
> + if (virResctrlAllocParseProcessLine(&tmp, lines[i]) < 0)
> + goto cleanup;
> + }
> +
> + *alloc = tmp;
> + tmp = NULL;
> + ret = 0;
> + cleanup:
> + virStringListFree(lines);
> + virObjectUnref(tmp);
> + return ret;
> +}
> +
> +static void
> +virResctrlAllocSubtractPerType(virResctrlAllocPerTypePtr a,
> + virResctrlAllocPerTypePtr b)
> +{
> + size_t i = 0;
> +
> + if (!a || !b)
> + return;
> +
> + for (i = 0; i < a->nmasks && i < b->nmasks; ++i) {
> + if (a->masks[i] && b->masks[i])
> + virBitmapSubtract(a->masks[i], b->masks[i]);
> + }
> +}
> +
> +static void
> +virResctrlAllocSubtract(virResctrlAllocPtr a,
> + virResctrlAllocPtr b)
> +{
> + size_t i = 0;
> + size_t j = 0;
> +
> + if (!b)
> + return;
> +
> + for (i = 0; i < a->nlevels && b->nlevels; ++i) {
> + if (a->levels[i] && b->levels[i]) {
> + /* Here we rely on all the system allocations to use the same types.
> + * We kind of _hope_ it's the case. If this is left here until the
> + * review and someone finds it, then suggest only removing this last
> + * sentence. */
> + for (j = 0; j < VIR_CACHE_TYPE_LAST; j++) {
> + virResctrlAllocSubtractPerType(a->levels[i]->types[j],
> + b->levels[i]->types[j]);
> + }
> + }
> + }
> +}
> +
> +virResctrlAllocPtr
> +virResctrlAllocNewFromInfo(virResctrlInfoPtr info)
> +{
> + size_t i = 0;
> + size_t j = 0;
> + size_t k = 0;
> + virResctrlAllocPtr ret = NULL;
> + virBitmapPtr mask = NULL;
> +
> + for (i = 0; i < info->nlevels; i++) {
> + virResctrlInfoPerLevelPtr i_level = info->levels[i];
> +
> + if (!i_level)
> + continue;
> +
> + for (j = 0; j < VIR_CACHE_TYPE_LAST; j++) {
> + virResctrlInfoPerTypePtr i_type = i_level->types[j];
> +
> + if (!i_type)
> + continue;
> +
> + virBitmapFree(mask);
> + mask = virBitmapNew(i_type->bits);
> + if (!mask)
> + goto error;
> + virBitmapSetAll(mask);
> +
> + for (k = 0; k <= i_type->max_cache_id; k++) {
> + if (virResctrlAllocUpdateMask(&ret, i, j, k, mask) < 0)
> + goto error;
> + }
> + }
> + }
> +
> + cleanup:
> + virBitmapFree(mask);
> + return ret;
> + error:
> + virObjectUnref(ret);
> + ret = NULL;
> + goto cleanup;
> +}
> +
> +virResctrlAllocPtr
> +virResctrlAllocGetFree(virResctrlInfoPtr resctrl)
> +{
> + virResctrlAllocPtr ret = NULL;
> + virResctrlAllocPtr alloc = NULL;
> + virBitmapPtr mask = NULL;
> + struct dirent *ent = NULL;
> + DIR *dirp = NULL;
> + char *schemata = NULL;
> + int rv = -1;
> +
> + ret = virResctrlAllocNewFromInfo(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;
> + }
> +
> + if (virResctrlAllocParse(&alloc, schemata) < 0)
> + goto error;
> + if (!alloc) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("No schemata for default resctrlfs group"));
> + goto error;
> + }
> + virResctrlAllocSubtract(ret, alloc);
> +
> + if (virDirOpen(&dirp, SYSFS_RESCTRL_PATH) < 0)
> + goto error;
> +
> + while ((rv = virDirRead(dirp, &ent, SYSFS_RESCTRL_PATH)) > 0) {
> + if (ent->d_type != DT_DIR)
> + continue;
> +
> + if (STREQ(ent->d_name, "info"))
> + continue;
> +
> + VIR_FREE(schemata);
> + if (virFileReadValueString(&schemata,
> + SYSFS_RESCTRL_PATH
> + "/%s/schemata",
> + ent->d_name) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Could not read schemata file for group %s"),
> + ent->d_name);
> + goto error;
> + }
> +
> + virObjectUnref(alloc);
> + alloc = NULL;
> + if (virResctrlAllocParse(&alloc, schemata) < 0)
> + goto error;
> +
> + virResctrlAllocSubtract(ret, alloc);
> +
> + VIR_FREE(schemata);
> + }
> + if (rv < 0)
> + goto error;
> +
> + cleanup:
> + virObjectUnref(alloc);
> + VIR_DIR_CLOSE(dirp);
> + VIR_FREE(schemata);
> + virBitmapFree(mask);
> + return ret;
> +
> + error:
> + virObjectUnref(ret);
> + ret = NULL;
> + goto cleanup;
> +}
> +
> +int
> +virResctrlAllocMasksAssign(virResctrlInfoPtr r_info,
> + virResctrlAllocPtr alloc,
> + void **save_ptr)
> +{
> + int ret = -1;
> + unsigned int level = 0;
> + virResctrlAllocPtr alloc_free = NULL;
> +
> + if (!r_info) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("Resource control is not supported on this host"));
> + return -1;
> + }
I'm wondering whether this error message can be moved to
virResctrlAllocCreate() or somewhere else to hit it as soon as possible.
> +
> + if (!save_ptr) {
> + alloc_free = virResctrlAllocGetFree(r_info);
> + } else {
> + if (!*save_ptr)
> + *save_ptr = virResctrlAllocGetFree(r_info);
> +
> + alloc_free = *save_ptr;
> + }
This code and the *save_ptr* is here only for tests purposes. Would it
be possible to get rid of it? How much time it takes to calculate
free allocation?
> +
> + if (!alloc_free)
> + return -1;
> +
> + for (level = 0; level < alloc->nlevels; level++) {
> + virResctrlAllocPerLevelPtr a_level = alloc->levels[level];
> + virResctrlAllocPerLevelPtr f_level = NULL;
> + unsigned int type = 0;
> +
> + if (!a_level)
> + continue;
> +
> + if (level < alloc_free->nlevels)
> + f_level = alloc_free->levels[level];
> +
> + if (!f_level) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("Cache level %d does not support tuning"),
> + level);
> + goto cleanup;
> + }
> +
> + for (type = 0; type < VIR_CACHE_TYPE_LAST; type++) {
> + virResctrlAllocPerTypePtr a_type = a_level->types[type];
> + virResctrlAllocPerTypePtr f_type = f_level->types[type];
> + unsigned int cache = 0;
> +
> + if (!a_type)
> + continue;
> +
> + if (!f_type) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("Cache level %d does not support tuning for "
> + "scope type '%s'"),
> + level, virCacheTypeToString(type));
> + goto cleanup;
> + }
> +
> + for (cache = 0; cache < a_type->nsizes; cache++) {
> + unsigned long long *size = a_type->sizes[cache];
> + virBitmapPtr a_mask = NULL;
> + virBitmapPtr f_mask = f_type->masks[cache];
> + virResctrlInfoPerLevelPtr i_level = r_info->levels[level];
> + virResctrlInfoPerTypePtr i_type = i_level->types[type];
> + unsigned long long granularity;
> + unsigned long long need_bits;
> + size_t i = 0;
> + ssize_t pos = -1;
> + ssize_t last_bits = 0;
> + ssize_t last_pos = -1;
> +
> + if (!size)
> + continue;
> +
> + if (cache >= f_type->nmasks) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("Cache with id %u does not exists for level %d"),
> + cache, level);
> + goto cleanup;
> + }
> +
> + f_mask = f_type->masks[cache];
> + if (!f_mask) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("Cache level %d id %u does not support tuning for "
> + "scope type '%s'"),
> + level, cache, virCacheTypeToString(type));
> + goto cleanup;
> + }
> +
> + granularity = i_type->size / i_type->bits;
> + need_bits = *size / granularity;
> +
> + if (*size % granularity) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("Cache allocation of size %llu is not "
> + "divisible by granularity %llu"),
> + *size, granularity);
> + goto cleanup;
> + }
> +
> + if (need_bits < i_type->min_cbm_bits) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("Cache allocation of size %llu is smaller "
> + "than the minimum allowed allocation %llu"),
> + *size, granularity * i_type->min_cbm_bits);
> + goto cleanup;
> + }
> +
> + while ((pos = virBitmapNextSetBit(f_mask, pos)) >= 0) {
> + ssize_t pos_clear = virBitmapNextClearBit(f_mask, pos);
> + ssize_t bits;
> +
> + if (pos_clear < 0)
> + pos_clear = virBitmapSize(f_mask);
> +
> + bits = pos_clear - pos;
> +
> + /* Not enough bits, move on and skip all of them */
> + if (bits < need_bits) {
> + pos = pos_clear;
> + continue;
> + }
> +
> + /* This fits perfectly */
> + if (bits == need_bits) {
> + last_pos = pos;
> + break;
> + }
> +
> + /* Remember the smaller region if we already found on before */
> + if (last_pos < 0 || (last_bits && bits < last_bits)) {
> + last_bits = bits;
> + last_pos = pos;
> + }
> +
> + pos = pos_clear;
> + }
> +
> + if (last_pos < 0) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("Not enough room for allocation of "
> + "%llu bytes for level %u cache %u "
> + "scope type '%s'"),
> + *size, level, cache,
> + virCacheTypeToString(type));
> + goto cleanup;
> + }
> +
> + a_mask = virBitmapNew(i_type->bits);
> + for (i = last_pos; i < last_pos + need_bits; i++) {
> + ignore_value(virBitmapSetBit(a_mask, i));
> + ignore_value(virBitmapClearBit(f_mask, i));
> + }
> +
> + if (a_type->nmasks <= cache) {
> + if (VIR_EXPAND_N(a_type->masks, a_type->nmasks,
> + cache - a_type->nmasks + 1) < 0) {
> + virBitmapFree(a_mask);
> + goto cleanup;
> + }
> + }
> + a_type->masks[cache] = a_mask;
> + }
> + }
> + }
> +
> + ret = 0;
> + cleanup:
> + if (!save_ptr)
> + virObjectUnref(alloc_free);
> + return ret;
> +}
> +
> +/* This checks if the directory for the alloc exists. If not it tries to create
> + * it and apply appropriate alloc settings. */
> +int
> +virResctrlAllocCreate(virResctrlInfoPtr r_info,
> + virResctrlAllocPtr alloc,
> + const char *drivername,
> + const char *machinename)
> +{
> + char *alloc_path = NULL;
> + char *schemata_path = NULL;
> + bool dir_created = false;
> + char *alloc_str = NULL;
> + int ret = -1;
> + int lockfd = -1;
> +
> + if (!alloc)
> + return 0;
> +
> + if (!alloc->path &&
> + virAsprintf(&alloc->path, "%s/%s-%s-%s",
> + SYSFS_RESCTRL_PATH, drivername, machinename, alloc->id) < 0)
> + return -1;
> +
> + /* Check if this allocation was already created */
> + if (virFileIsDir(alloc->path)) {
> + VIR_FREE(alloc_path);
> + return 0;
> + }
> +
> + if (virFileExists(alloc->path)) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Path '%s' for resctrl allocation exists and is not a "
> + "directory"), alloc->path);
> + goto cleanup;
> + }
> +
> + lockfd = virResctrlLockWrite();
> + if (lockfd < 0)
> + goto cleanup;
> +
> + if (virResctrlAllocMasksAssign(r_info, alloc, NULL) < 0)
> + goto cleanup;
> +
> + alloc_str = virResctrlAllocFormat(alloc);
> + if (!alloc_str)
> + return -1;
> +
> + if (virAsprintf(&schemata_path, "%s/schemata", alloc->path) < 0)
> + goto cleanup;
> +
> + if (virFileMakePath(alloc->path) < 0) {
> + virReportSystemError(errno,
> + _("Cannot create resctrl directory '%s'"),
> + alloc->path);
> + goto cleanup;
> + }
> + dir_created = true;
> +
> + if (virFileWriteStr(schemata_path, alloc_str, 0) < 0) {
> + virReportSystemError(errno,
> + _("Cannot write into schemata file '%s'"),
> + schemata_path);
> + goto cleanup;
> + }
> +
> + ret = 0;
> + cleanup:
> + if (ret < 0 && dir_created)
> + rmdir(alloc->path);
> + virResctrlUnlock(lockfd);
> + VIR_FREE(alloc_str);
> + VIR_FREE(alloc_path);
> + VIR_FREE(schemata_path);
> + return ret;
> +}
> +
> +int
> +virResctrlAllocAddPID(virResctrlAllocPtr alloc,
> + pid_t pid)
> +{
> + char *tasks = NULL;
> + char *pidstr = NULL;
> + int ret = 0;
> +
> + if (!alloc->path) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Cannot add pid to non-existing resctrl allocation"));
> + return -1;
> + }
> +
> + if (virAsprintf(&tasks, "%s/tasks", alloc->path) < 0)
> + return -1;
> +
> + if (virAsprintf(&pidstr, "%lld", (long long int) pid) < 0)
> + goto cleanup;
> +
> + if (virFileWriteStr(tasks, pidstr, 0) < 0) {
> + virReportSystemError(errno,
> + _("Cannot write pid in tasks file '%s'"),
> + tasks);
> + goto cleanup;
> + }
> +
> + ret = 0;
> + cleanup:
> + VIR_FREE(tasks);
> + VIR_FREE(pidstr);
> + return ret;
> +}
> +
> +int
> +virResctrlAllocRemove(virResctrlAllocPtr alloc)
> +{
> + int ret = 0;
> +
> + if (!alloc->path)
> + return 0;
> +
> + VIR_DEBUG("Removing resctrl allocation %s", alloc->path);
> + if (rmdir(alloc->path) != 0 && errno != ENOENT) {
> + ret = -errno;
> + VIR_ERROR(_("Unable to remove %s (%d)"), alloc->path, errno);
> + }
> +
> + return ret;
> +}
> diff --git a/src/util/virresctrl.h b/src/util/virresctrl.h
> index c4df88f23c3a..b233eca41c03 100644
> --- a/src/util/virresctrl.h
> +++ b/src/util/virresctrl.h
> @@ -62,4 +62,63 @@ virResctrlInfoGetCache(virResctrlInfoPtr resctrl,
> size_t *ncontrols,
> virResctrlInfoPerCachePtr **controls);
>
> +/* Alloc-related things */
> +typedef struct _virResctrlAlloc virResctrlAlloc;
> +typedef virResctrlAlloc *virResctrlAllocPtr;
> +
> +typedef int virResctrlAllocForeachSizeCallback(unsigned int level,
> + virCacheType type,
> + unsigned int cache,
> + unsigned long long size,
> + void *opaque);
> +
> +virResctrlAllocPtr
> +virResctrlAllocNewFromInfo(virResctrlInfoPtr info);
> +
> +int
> +virResctrlAllocUpdateSize(virResctrlAllocPtr *resctrl,
> + unsigned int level,
> + virCacheType type,
> + unsigned int cache,
> + unsigned long long size);
> +
> +int
> +virResctrlAllocUpdateMask(virResctrlAllocPtr *resctrl,
> + unsigned int level,
> + virCacheType type,
> + unsigned int cache,
> + virBitmapPtr mask);
> +
> +int
> +virResctrlAllocSetSize(virResctrlAllocPtr *resctrl,
> + unsigned int level,
> + virCacheType type,
> + unsigned int cache,
> + unsigned long long size);
> +
> +int
> +virResctrlAllocForeachSize(virResctrlAllocPtr resctrl,
> + virResctrlAllocForeachSizeCallback cb,
> + void *opaque);
> +
> +int
> +virResctrlAllocSetID(virResctrlAllocPtr alloc,
> + const char *id);
> +
> +char *
> +virResctrlAllocFormat(virResctrlAllocPtr alloc);
> +
> +int
> +virResctrlAllocCreate(virResctrlInfoPtr r_info,
> + virResctrlAllocPtr alloc,
> + const char *drivername,
> + const char *machinename);
> +
> +int
> +virResctrlAllocAddPID(virResctrlAllocPtr alloc,
> + pid_t pid);
> +
> +int
> +virResctrlAllocRemove(virResctrlAllocPtr alloc);
> +
> #endif /* __VIR_RESCTRL_H__ */
> diff --git a/src/util/virresctrlpriv.h b/src/util/virresctrlpriv.h
> new file mode 100644
> index 000000000000..4255ad496302
> --- /dev/null
> +++ b/src/util/virresctrlpriv.h
> @@ -0,0 +1,32 @@
> +/*
> + * virresctrlpriv.h:
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library. If not, see
> + * <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef __VIR_RESCTRL_PRIV_H__
> +# define __VIR_RESCTRL_PRIV_H__
> +
> +# include "virresctrl.h"
> +
> +virResctrlAllocPtr
> +virResctrlAllocGetFree(virResctrlInfoPtr resctrl);
> +
> +int
> +virResctrlAllocMasksAssign(virResctrlInfoPtr r_info,
> + virResctrlAllocPtr alloc,
> + void **save_ptr);
> +
> +#endif /* __VIR_RESCTRL_PRIV_H__ */
> --
> 2.15.0
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
