Just to stay consistent. Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; --- src/util/virresctrl.c | 64 +++++++++++++++++++++---------------------- src/util/virresctrl.h | 6 ++-- 2 files changed, 35 insertions(+), 35 deletions(-) diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index fc1163508ac1..c140276c0909 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -231,10 +231,10 @@ virResctrlAllocDispose(void *obj) size_t j = 0; size_t k = 0; - virResctrlAllocPtr resctrl = obj; + virResctrlAllocPtr alloc = obj; - for (i = 0; i < resctrl->nlevels; i++) { - virResctrlAllocPerLevelPtr level = resctrl->levels[i]; + for (i = 0; i < alloc->nlevels; i++) { + virResctrlAllocPerLevelPtr level = alloc->levels[i]; if (!level) continue; @@ -259,9 +259,9 @@ virResctrlAllocDispose(void *obj) VIR_FREE(level); } - VIR_FREE(resctrl->id); - VIR_FREE(resctrl->path); - VIR_FREE(resctrl->levels); + VIR_FREE(alloc->id); + VIR_FREE(alloc->path); + VIR_FREE(alloc->levels); } @@ -591,17 +591,17 @@ virResctrlInfoGetCache(virResctrlInfoPtr resctrl, /* Alloc-related functions */ bool -virResctrlAllocIsEmpty(virResctrlAllocPtr resctrl) +virResctrlAllocIsEmpty(virResctrlAllocPtr alloc) { size_t i = 0; size_t j = 0; size_t k = 0; - if (!resctrl) + if (!alloc) return true; - for (i = 0; i < resctrl->nlevels; i++) { - virResctrlAllocPerLevelPtr a_level = resctrl->levels[i]; + for (i = 0; i < alloc->nlevels; i++) { + virResctrlAllocPerLevelPtr a_level = alloc->levels[i]; if (!a_level) continue; @@ -629,30 +629,30 @@ virResctrlAllocIsEmpty(virResctrlAllocPtr resctrl) static virResctrlAllocPerTypePtr -virResctrlAllocGetType(virResctrlAllocPtr resctrl, +virResctrlAllocGetType(virResctrlAllocPtr alloc, unsigned int level, virCacheType type) { virResctrlAllocPerLevelPtr a_level = NULL; - if (resctrl->nlevels <= level && - VIR_EXPAND_N(resctrl->levels, resctrl->nlevels, level - resctrl->nlevels + 1) < 0) + if (alloc->nlevels <= level && + VIR_EXPAND_N(alloc->levels, alloc->nlevels, level - alloc->nlevels + 1) < 0) return NULL; - if (!resctrl->levels[level]) { + if (!alloc->levels[level]) { virResctrlAllocPerTypePtr *types = NULL; if (VIR_ALLOC_N(types, VIR_CACHE_TYPE_LAST) < 0) return NULL; - if (VIR_ALLOC(resctrl->levels[level]) < 0) { + if (VIR_ALLOC(alloc->levels[level]) < 0) { VIR_FREE(types); return NULL; } - resctrl->levels[level]->types = types; + alloc->levels[level]->types = types; } - a_level = resctrl->levels[level]; + a_level = alloc->levels[level]; if (!a_level->types[type] && VIR_ALLOC(a_level->types[type]) < 0) return NULL; @@ -662,13 +662,13 @@ virResctrlAllocGetType(virResctrlAllocPtr 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; @@ -690,13 +690,13 @@ virResctrlAllocUpdateMask(virResctrlAllocPtr resctrl, static int -virResctrlAllocUpdateSize(virResctrlAllocPtr resctrl, +virResctrlAllocUpdateSize(virResctrlAllocPtr alloc, unsigned int level, virCacheType type, unsigned int cache, unsigned long long size) { - virResctrlAllocPerTypePtr a_type = virResctrlAllocGetType(resctrl, level, type); + virResctrlAllocPerTypePtr a_type = virResctrlAllocGetType(alloc, level, type); if (!a_type) return -1; @@ -774,13 +774,13 @@ virResctrlAllocCheckCollision(virResctrlAllocPtr alloc, int -virResctrlAllocSetSize(virResctrlAllocPtr resctrl, +virResctrlAllocSetSize(virResctrlAllocPtr alloc, unsigned int level, virCacheType type, unsigned int cache, unsigned long long size) { - if (virResctrlAllocCheckCollision(resctrl, level, type, cache)) { + if (virResctrlAllocCheckCollision(alloc, level, type, cache)) { virReportError(VIR_ERR_XML_ERROR, _("Colliding cache allocations for cache " "level '%u' id '%u', type '%s'"), @@ -788,12 +788,12 @@ virResctrlAllocSetSize(virResctrlAllocPtr resctrl, return -1; } - return virResctrlAllocUpdateSize(resctrl, level, type, cache, size); + return virResctrlAllocUpdateSize(alloc, level, type, cache, size); } int -virResctrlAllocForeachSize(virResctrlAllocPtr resctrl, +virResctrlAllocForeachSize(virResctrlAllocPtr alloc, virResctrlAllocForeachSizeCallback cb, void *opaque) { @@ -802,11 +802,11 @@ virResctrlAllocForeachSize(virResctrlAllocPtr resctrl, unsigned int type = 0; unsigned int cache = 0; - if (!resctrl) + if (!alloc) return 0; - for (level = 0; level < resctrl->nlevels; level++) { - virResctrlAllocPerLevelPtr a_level = resctrl->levels[level]; + for (level = 0; level < alloc->nlevels; level++) { + virResctrlAllocPerLevelPtr a_level = alloc->levels[level]; if (!a_level) continue; @@ -856,18 +856,18 @@ virResctrlAllocGetID(virResctrlAllocPtr alloc) char * -virResctrlAllocFormat(virResctrlAllocPtr resctrl) +virResctrlAllocFormat(virResctrlAllocPtr alloc) { virBuffer buf = VIR_BUFFER_INITIALIZER; unsigned int level = 0; unsigned int type = 0; unsigned int cache = 0; - if (!resctrl) + if (!alloc) return NULL; - for (level = 0; level < resctrl->nlevels; level++) { - virResctrlAllocPerLevelPtr a_level = resctrl->levels[level]; + for (level = 0; level < alloc->nlevels; level++) { + virResctrlAllocPerLevelPtr a_level = alloc->levels[level]; if (!a_level) continue; diff --git a/src/util/virresctrl.h b/src/util/virresctrl.h index 5368ba211924..cc648b521e8f 100644 --- a/src/util/virresctrl.h +++ b/src/util/virresctrl.h @@ -79,17 +79,17 @@ virResctrlAllocPtr virResctrlAllocNew(void); bool -virResctrlAllocIsEmpty(virResctrlAllocPtr resctrl); +virResctrlAllocIsEmpty(virResctrlAllocPtr alloc); int -virResctrlAllocSetSize(virResctrlAllocPtr resctrl, +virResctrlAllocSetSize(virResctrlAllocPtr alloc, unsigned int level, virCacheType type, unsigned int cache, unsigned long long size); int -virResctrlAllocForeachSize(virResctrlAllocPtr resctrl, +virResctrlAllocForeachSize(virResctrlAllocPtr alloc, virResctrlAllocForeachSizeCallback cb, void *opaque); -- 2.17.1

Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; --- src/util/virresctrl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index c140276c0909..9a118fc3595d 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -941,7 +941,7 @@ virResctrlAllocParseProcessCache(virResctrlInfoPtr resctrl, !resctrl->levels[level]->types[type]) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Missing or inconsistent resctrl info for " - "level '%ud' type '%s'"), + "level '%u' type '%s'"), level, virCacheTypeToString(type)); goto cleanup; } -- 2.17.1

We already have virFileLock(), but we are now using flock() in the code as well (due to requirements for mutual exclusion between libvirt and other programs using flock() as well), so let's have a function for that as well so we don't need to have stubs for unsupported platforms in other files. Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; --- src/libvirt_private.syms | 1 + src/util/virfile.c | 40 ++++++++++++++++++++++++++++++++++++++++ src/util/virfile.h | 2 ++ 3 files changed, 43 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index b4ab1f36290c..3a9680b3cd2b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1783,6 +1783,7 @@ virFileFindHugeTLBFS; virFileFindMountPoint; virFileFindResource; virFileFindResourceFull; +virFileFlock; virFileFreeACLs; virFileGetACLs; virFileGetHugepageSize; diff --git a/src/util/virfile.c b/src/util/virfile.c index 12b41a64e030..9296ccbe2a4f 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -51,6 +51,7 @@ #if HAVE_SYS_ACL_H # include <sys/acl.h> #endif +#include <sys/file.h> #ifdef __linux__ # if HAVE_LINUX_MAGIC_H @@ -362,6 +363,7 @@ virFileWrapperFdFree(virFileWrapperFdPtr wfd) #ifndef WIN32 + /** * virFileLock: * @fd: file descriptor to acquire the lock on @@ -430,7 +432,32 @@ int virFileUnlock(int fd, off_t start, off_t len) return 0; } + + +/** + * virFileFlock: + * @fd: file descriptor to call flock on + * @lock: true for lock, false for unlock + * @shared: true if shared, false for exclusive, ignored if `@lock == false` + * + * This is just a simple wrapper around flock(2) that errors out on unsupported + * platforms. + * + * The lock will be released when @fd is closed or this function is called with + * `@lock == false`. + * + * Returns 0 on success, -1 otherwise (with errno set) + */ +int virFileFlock(int fd, bool lock, bool shared) +{ + if (lock) + return flock(fd, shared ? LOCK_SH : LOCK_EX); + + return flock(fd, LOCK_UN); +} + #else + int virFileLock(int fd ATTRIBUTE_UNUSED, bool shared ATTRIBUTE_UNUSED, off_t start ATTRIBUTE_UNUSED, @@ -439,14 +466,27 @@ int virFileLock(int fd ATTRIBUTE_UNUSED, { return -ENOSYS; } + + int virFileUnlock(int fd ATTRIBUTE_UNUSED, off_t start ATTRIBUTE_UNUSED, off_t len ATTRIBUTE_UNUSED) { return -ENOSYS; } + + +int virFileFlock(int fd ATTRIBUTE_UNUSED, + bool lock ATTRIBUTE_UNUSED, + bool shared ATTRIBUTE_UNUSED) +{ + errno = ENOSYS; + return -1; +} + #endif + int virFileRewrite(const char *path, mode_t mode, diff --git a/src/util/virfile.h b/src/util/virfile.h index 59c14b97a6ac..6f1e802fde78 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -104,6 +104,8 @@ void virFileWrapperFdFree(virFileWrapperFdPtr dfd); int virFileLock(int fd, bool shared, off_t start, off_t len, bool waitForLock); int virFileUnlock(int fd, off_t start, off_t len); +int virFileFlock(int fd, bool lock, bool shared); + typedef int (*virFileRewriteFunc)(int fd, const void *opaque); int virFileRewrite(const char *path, mode_t mode, -- 2.17.1

By exporting virResctrlAllocMasksAssign() we can remove most of the conditionals. And that function should be used from the tests later anyway. Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; --- src/libvirt_private.syms | 1 + src/util/virresctrl.c | 30 +----------------------------- src/util/virresctrlpriv.h | 4 ++++ 3 files changed, 6 insertions(+), 29 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3a9680b3cd2b..6a8b40bbbdb1 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2628,6 +2628,7 @@ virResctrlAllocForeachSize; virResctrlAllocFormat; virResctrlAllocGetID; virResctrlAllocGetUnused; +virResctrlAllocMasksAssign; virResctrlAllocNew; virResctrlAllocRemove; virResctrlAllocSetID; diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index 9a118fc3595d..787db31b6126 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -380,8 +380,6 @@ virResctrlInfoIsEmpty(virResctrlInfoPtr resctrl) } -#ifdef __linux__ - int virResctrlGetInfo(virResctrlInfoPtr resctrl) { @@ -512,18 +510,6 @@ virResctrlGetInfo(virResctrlInfoPtr resctrl) 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, @@ -1089,8 +1075,6 @@ virResctrlAllocGetDefault(virResctrlInfoPtr resctrl) } -#ifdef __linux__ - static void virResctrlAllocSubtractPerType(virResctrlAllocPerTypePtr dst, virResctrlAllocPerTypePtr src) @@ -1246,18 +1230,6 @@ virResctrlAllocGetUnused(virResctrlInfoPtr resctrl) goto cleanup; } -#else /* ! __linux__ */ - -virResctrlAllocPtr -virResctrlAllocGetUnused(virResctrlInfoPtr resctrl ATTRIBUTE_UNUSED) -{ - virReportSystemError(ENOSYS, "%s", - _("Cache tune not supported on this platform")); - return NULL; -} - -#endif /* ! __linux__ */ - /* * Given the information about requested allocation type `a_type`, the host @@ -1434,7 +1406,7 @@ virResctrlAllocCopyMasks(virResctrlAllocPtr dst, * then tries to find a proper space for every requested allocation effectively * transforming `sizes` into `masks`. */ -static int +int virResctrlAllocMasksAssign(virResctrlInfoPtr resctrl, virResctrlAllocPtr alloc) { diff --git a/src/util/virresctrlpriv.h b/src/util/virresctrlpriv.h index 72b115afd13e..80ddd4b875a3 100644 --- a/src/util/virresctrlpriv.h +++ b/src/util/virresctrlpriv.h @@ -24,4 +24,8 @@ virResctrlAllocPtr virResctrlAllocGetUnused(virResctrlInfoPtr resctrl); +int +virResctrlAllocMasksAssign(virResctrlInfoPtr resctrl, + virResctrlAllocPtr alloc); + #endif /* __VIR_RESCTRL_PRIV_H__ */ -- 2.17.1

That way we get rid of the last preprocessor conditional so the code compiles on all platforms. Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; --- src/util/virresctrl.c | 31 ++++--------------------------- 1 file changed, 4 insertions(+), 27 deletions(-) diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index 787db31b6126..ef649a08bf73 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -289,9 +289,8 @@ virResctrlAllocNew(void) /* Common functions */ -#ifdef __linux__ static int -virResctrlLockInternal(int op) +virResctrlLockWrite(void) { int fd = open(SYSFS_RESCTRL_PATH, O_DIRECTORY | O_CLOEXEC); @@ -300,7 +299,7 @@ virResctrlLockInternal(int op) return -1; } - if (flock(fd, op) < 0) { + if (virFileFlock(fd, true, true) < 0) { virReportSystemError(errno, "%s", _("Cannot lock resctrl")); VIR_FORCE_CLOSE(fd); return -1; @@ -310,45 +309,23 @@ virResctrlLockInternal(int op) } -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) + if (virFileFlock(fd, false, false) < 0) virReportSystemError(errno, "%s", _("Cannot unlock resctrl")); + return -1; } -#endif /* ! __linux__ */ return 0; } -- 2.17.1

Move description of the purpose of the file before any definition. One empty line between related enum definitions. All typedefs before all structs. This is exception from the usual, but not the only one, we already have something similar for some other structs. This way we can move contents between structs and reorder some parts nicely without moving all definitions of one type before another one just so it's defined. Define all classes in one place. Have one initialization function for all classes in the file. Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; --- src/util/virresctrl.c | 156 +++++++++++++++++++++--------------------- 1 file changed, 79 insertions(+), 77 deletions(-) diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index ef649a08bf73..0266fee4e959 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -37,21 +37,22 @@ VIR_LOG_INIT("util.virresctrl") -/* Common definitions */ -#define SYSFS_RESCTRL_PATH "/sys/fs/resctrl" - /* Resctrl is short for Resource Control. It might be implemented for various * resources, but at the time of this writing this is only supported for cache * allocation technology (aka CAT). Hence the reson for leaving 'Cache' out of * all the structure and function names for now (can be added later if needed. */ + +/* Common definitions */ +#define SYSFS_RESCTRL_PATH "/sys/fs/resctrl" + + /* Our naming for cache types and scopes */ VIR_ENUM_IMPL(virCache, VIR_CACHE_TYPE_LAST, "both", "code", "data") - /* * This is the same enum, but for the resctrl naming * of the type (L<level><type>) @@ -63,9 +64,28 @@ VIR_ENUM_IMPL(virResctrl, VIR_CACHE_TYPE_LAST, "DATA") -/* Info-related definitions and InfoClass-related functions */ +/* All private typedefs so that they exist for all later definitions. This way + * structs can be included in one or another without reorganizing the code every + * time. */ typedef struct _virResctrlInfoPerType virResctrlInfoPerType; typedef virResctrlInfoPerType *virResctrlInfoPerTypePtr; + +typedef struct _virResctrlInfoPerLevel virResctrlInfoPerLevel; +typedef virResctrlInfoPerLevel *virResctrlInfoPerLevelPtr; + +typedef struct _virResctrlAllocPerType virResctrlAllocPerType; +typedef virResctrlAllocPerType *virResctrlAllocPerTypePtr; + +typedef struct _virResctrlAllocPerLevel virResctrlAllocPerLevel; +typedef virResctrlAllocPerLevel *virResctrlAllocPerLevelPtr; + + +/* Class definitions and initializations */ +static virClassPtr virResctrlInfoClass; +static virClassPtr virResctrlAllocClass; + + +/* virResctrlInfo */ struct _virResctrlInfoPerType { /* Kernel-provided information */ char *cbm_mask; @@ -86,8 +106,6 @@ struct _virResctrlInfoPerType { virResctrlInfoPerCache control; }; -typedef struct _virResctrlInfoPerLevel virResctrlInfoPerLevel; -typedef virResctrlInfoPerLevel *virResctrlInfoPerLevelPtr; struct _virResctrlInfoPerLevel { virResctrlInfoPerTypePtr *types; }; @@ -99,7 +117,6 @@ struct _virResctrlInfo { size_t nlevels; }; -static virClassPtr virResctrlInfoClass; static void virResctrlInfoDispose(void *obj) @@ -130,30 +147,7 @@ virResctrlInfoDispose(void *obj) } -static int -virResctrlInfoOnceInit(void) -{ - if (!VIR_CLASS_NEW(virResctrlInfo, virClassForObject())) - return -1; - - return 0; -} - - -VIR_ONCE_GLOBAL_INIT(virResctrlInfo) - - -virResctrlInfoPtr -virResctrlInfoNew(void) -{ - if (virResctrlInfoInitialize() < 0) - return NULL; - - return virObjectNew(virResctrlInfoClass); -} - - -/* Alloc-related definitions and AllocClass-related functions */ +/* virResctrlAlloc */ /* * virResctrlAlloc represents one allocation (in XML under cputune/cachetune and @@ -186,8 +180,6 @@ virResctrlInfoNew(void) * virBitmaps named `masks` indexed the same way as `sizes`. The upper bounds * of the sparse arrays are stored in nmasks or nsizes, respectively. */ -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 @@ -201,8 +193,6 @@ struct _virResctrlAllocPerType { size_t nmasks; }; -typedef struct _virResctrlAllocPerLevel virResctrlAllocPerLevel; -typedef virResctrlAllocPerLevel *virResctrlAllocPerLevelPtr; struct _virResctrlAllocPerLevel { virResctrlAllocPerTypePtr *types; /* Indexed with enum virCacheType */ /* There is no `ntypes` member variable as it is always allocated for @@ -222,7 +212,6 @@ struct _virResctrlAlloc { char *path; }; -static virClassPtr virResctrlAllocClass; static void virResctrlAllocDispose(void *obj) @@ -265,27 +254,20 @@ virResctrlAllocDispose(void *obj) } +/* Global initialization for classes */ static int -virResctrlAllocOnceInit(void) +virResctrlOnceInit(void) { + if (!VIR_CLASS_NEW(virResctrlInfo, virClassForObject())) + return -1; + if (!VIR_CLASS_NEW(virResctrlAlloc, virClassForObject())) return -1; return 0; } - -VIR_ONCE_GLOBAL_INIT(virResctrlAlloc) - - -virResctrlAllocPtr -virResctrlAllocNew(void) -{ - if (virResctrlAllocInitialize() < 0) - return NULL; - - return virObjectNew(virResctrlAllocClass); -} +VIR_ONCE_GLOBAL_INIT(virResctrl) /* Common functions */ @@ -331,32 +313,7 @@ virResctrlUnlock(int fd) } -/* 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; -} - - +/* virResctrlInfo-related definitions */ int virResctrlGetInfo(virResctrlInfoPtr resctrl) { @@ -488,6 +445,41 @@ virResctrlGetInfo(virResctrlInfoPtr resctrl) } +virResctrlInfoPtr +virResctrlInfoNew(void) +{ + if (virResctrlInitialize() < 0) + return NULL; + + return virObjectNew(virResctrlInfoClass); +} + + +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; +} + + int virResctrlInfoGetCache(virResctrlInfoPtr resctrl, unsigned int level, @@ -552,7 +544,17 @@ virResctrlInfoGetCache(virResctrlInfoPtr resctrl, } -/* Alloc-related functions */ +/* virResctrlAlloc-related definitions */ +virResctrlAllocPtr +virResctrlAllocNew(void) +{ + if (virResctrlInitialize() < 0) + return NULL; + + return virObjectNew(virResctrlAllocClass); +} + + bool virResctrlAllocIsEmpty(virResctrlAllocPtr alloc) { -- 2.17.1

There is no need to have virResctrlGetInfo() when it must be called after virResctrlInfoNew() anyway, otherwise it's just an unusable object. When we wrap the logic inside the New() function we'll save some calls later as well. Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; --- src/conf/capabilities.c | 2 +- src/libvirt_private.syms | 1 - src/util/virresctrl.c | 15 +++++++++++++-- src/util/virresctrl.h | 3 --- 4 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index d5f6f007300a..978edf871943 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -1604,7 +1604,7 @@ virCapabilitiesInitResctrl(virCapsPtr caps) if (!caps->host.resctrl) return -1; - return virResctrlGetInfo(caps->host.resctrl); + return 0; } diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6a8b40bbbdb1..20ec8760ec27 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2633,7 +2633,6 @@ virResctrlAllocNew; virResctrlAllocRemove; virResctrlAllocSetID; virResctrlAllocSetSize; -virResctrlGetInfo; virResctrlInfoGetCache; virResctrlInfoNew; diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index 0266fee4e959..4e35b19649ca 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -314,7 +314,7 @@ virResctrlUnlock(int fd) /* virResctrlInfo-related definitions */ -int +static int virResctrlGetInfo(virResctrlInfoPtr resctrl) { DIR *dirp = NULL; @@ -448,10 +448,21 @@ virResctrlGetInfo(virResctrlInfoPtr resctrl) virResctrlInfoPtr virResctrlInfoNew(void) { + virResctrlInfoPtr ret = NULL; + if (virResctrlInitialize() < 0) return NULL; - return virObjectNew(virResctrlInfoClass); + ret = virObjectNew(virResctrlInfoClass); + if (!ret) + return NULL; + + if (virResctrlGetInfo(ret) < 0) { + virObjectUnref(ret); + return NULL; + } + + return ret; } diff --git a/src/util/virresctrl.h b/src/util/virresctrl.h index cc648b521e8f..a348fa7ab195 100644 --- a/src/util/virresctrl.h +++ b/src/util/virresctrl.h @@ -55,9 +55,6 @@ typedef virResctrlInfo *virResctrlInfoPtr; virResctrlInfoPtr virResctrlInfoNew(void); -int -virResctrlGetInfo(virResctrlInfoPtr resctrl); - int virResctrlInfoGetCache(virResctrlInfoPtr resctrl, unsigned int level, -- 2.17.1

It will be used in that file later on, plus it makes sense for all the implementations to be in same place. Also comment each one of them nicely and add a comment explaining why they all need to end with the same _LAST value. Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; --- src/conf/capabilities.c | 6 ------ src/libvirt_private.syms | 2 ++ src/util/virresctrl.c | 19 ++++++++++++++----- src/util/virresctrl.h | 1 + 4 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 978edf871943..7a810efa6662 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -1544,12 +1544,6 @@ virCapabilitiesInitPages(virCapsPtr caps) return ret; } -/* Cache name mapping for Linux kernel naming */ -VIR_ENUM_DECL(virCacheKernel); -VIR_ENUM_IMPL(virCacheKernel, VIR_CACHE_TYPE_LAST, - "Unified", - "Instruction", - "Data") bool virCapsHostCacheBankEquals(virCapsHostCacheBankPtr a, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 20ec8760ec27..a4a0c95b474d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2619,6 +2619,8 @@ virRandomInt; # util/virresctrl.h +virCacheKernelTypeFromString; +virCacheKernelTypeToString; virCacheTypeFromString; virCacheTypeToString; virResctrlAllocAddPID; diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index 4e35b19649ca..481577bbb9f6 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -48,15 +48,24 @@ VIR_LOG_INIT("util.virresctrl") #define SYSFS_RESCTRL_PATH "/sys/fs/resctrl" -/* Our naming for cache types and scopes */ +/* Following are three different enum implementations for the same enum. Each + * one of them helps translating to/from strings for different interfaces. The + * delimiter must be VIR_CACHE_TYPE_LAST for all of them in order to stay + * consistent in between all of them. */ + +/* Cache name mapping for Linux kernel naming. */ +VIR_ENUM_IMPL(virCacheKernel, VIR_CACHE_TYPE_LAST, + "Unified", + "Instruction", + "Data") + +/* Cache name mapping for our XML naming. */ VIR_ENUM_IMPL(virCache, VIR_CACHE_TYPE_LAST, "both", "code", "data") -/* - * This is the same enum, but for the resctrl naming - * of the type (L<level><type>) - */ + +/* Cache name mapping for resctrl interface naming. */ VIR_ENUM_DECL(virResctrl) VIR_ENUM_IMPL(virResctrl, VIR_CACHE_TYPE_LAST, "", diff --git a/src/util/virresctrl.h b/src/util/virresctrl.h index a348fa7ab195..9052a2b19a02 100644 --- a/src/util/virresctrl.h +++ b/src/util/virresctrl.h @@ -34,6 +34,7 @@ typedef enum { } virCacheType; VIR_ENUM_DECL(virCache); +VIR_ENUM_DECL(virCacheKernel); typedef struct _virResctrlInfoPerCache virResctrlInfoPerCache; -- 2.17.1

It was used just temporarily to do a calculation, no need to keep that around. Also use virBitmap in the code instead of reimplementing two of its existing functions. And move the counting part next to where the value is read. Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; --- src/util/virresctrl.c | 37 ++++++++++++++++--------------------- 1 file changed, 16 insertions(+), 21 deletions(-) diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index 481577bbb9f6..2b72a3b36bae 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -24,8 +24,6 @@ #include <fcntl.h> #include "virresctrlpriv.h" -#include "c-ctype.h" -#include "count-one-bits.h" #include "viralloc.h" #include "virfile.h" #include "virlog.h" @@ -97,7 +95,6 @@ static virClassPtr virResctrlAllocClass; /* virResctrlInfo */ struct _virResctrlInfoPerType { /* Kernel-provided information */ - char *cbm_mask; unsigned int min_cbm_bits; /* Our computed information from the above */ @@ -142,11 +139,8 @@ virResctrlInfoDispose(void *obj) continue; if (level->types) { - for (j = 0; j < VIR_CACHE_TYPE_LAST; j++) { - if (level->types[j]) - VIR_FREE(level->types[j]->cbm_mask); + for (j = 0; j < VIR_CACHE_TYPE_LAST; j++) VIR_FREE(level->types[j]); - } } VIR_FREE(level->types); VIR_FREE(level); @@ -334,6 +328,7 @@ virResctrlGetInfo(virResctrlInfoPtr resctrl) int type = 0; struct dirent *ent = NULL; unsigned int level = 0; + virBitmapPtr tmp_map = NULL; virResctrlInfoPerLevelPtr i_level = NULL; virResctrlInfoPerTypePtr i_type = NULL; @@ -378,7 +373,7 @@ virResctrlGetInfo(virResctrlInfoPtr resctrl) goto cleanup; } - rv = virFileReadValueString(&i_type->cbm_mask, + rv = virFileReadValueString(&tmp_str, SYSFS_RESCTRL_PATH "/info/%s/cbm_mask", ent->d_name); @@ -393,7 +388,19 @@ virResctrlGetInfo(virResctrlInfoPtr resctrl) if (rv < 0) goto cleanup; - virStringTrimOptionalNewline(i_type->cbm_mask); + virStringTrimOptionalNewline(tmp_str); + + tmp_map = virBitmapNewString(tmp_str); + VIR_FREE(tmp_str); + if (!tmp_map) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot parse cbm_mask from resctrl cache info")); + goto cleanup; + } + + i_type->bits = virBitmapCountBits(tmp_map); + virBitmapFree(tmp_map); + tmp_map = NULL; rv = virFileReadValueUint(&i_type->min_cbm_bits, SYSFS_RESCTRL_PATH "/info/%s/min_cbm_bits", @@ -431,24 +438,12 @@ virResctrlGetInfo(virResctrlInfoPtr resctrl) 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; } -- 2.17.1

Introduce virCache{Info,Bank} as structures describing host cache information including the ways they can be managed by resctrl. This has couple of advantages. First and foremost the virResctrlAllocCreate() is no longer dependent on a data structure initialized in another part of the code and the data is automatically fresh. This couldn't be done previously since the code initializing that was in conf/. And it also makes more sense for functions that parse sysfs files to be in util/. Last, but not least, the code doesn't duplicate any data or keep around stale information that needs to be recalculated anyway. Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; --- src/conf/capabilities.c | 397 +++++++++---------------------- src/conf/capabilities.h | 21 +- src/libvirt_private.syms | 7 +- src/qemu/qemu_process.c | 26 +- src/util/virresctrl.c | 485 ++++++++++++++++++++++++++++++-------- src/util/virresctrl.h | 54 +++-- src/util/virresctrlpriv.h | 4 +- tests/virresctrltest.c | 18 +- 8 files changed, 560 insertions(+), 452 deletions(-) diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 7a810efa6662..58769393e821 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -235,14 +235,10 @@ virCapsDispose(void *object) virCapabilitiesClearSecModel(&caps->host.secModels[i]); VIR_FREE(caps->host.secModels); - for (i = 0; i < caps->host.ncaches; i++) - virCapsHostCacheBankFree(caps->host.caches[i]); - VIR_FREE(caps->host.caches); - VIR_FREE(caps->host.netprefix); VIR_FREE(caps->host.pagesSize); virCPUDefFree(caps->host.cpu); - virObjectUnref(caps->host.resctrl); + virObjectUnref(caps->host.caches); } /** @@ -863,96 +859,135 @@ virCapabilitiesFormatNUMATopology(virBufferPtr buf, return 0; } + static int -virCapabilitiesFormatCaches(virBufferPtr buf, - size_t ncaches, - virCapsHostCacheBankPtr *caches) +virCapabilitiesFormatCacheControlHelper(unsigned long long granularity, + unsigned long long min, + virCacheType scope, + unsigned int max_allocations, + void *opaque) { - size_t i = 0; - size_t j = 0; - virBuffer controlBuf = VIR_BUFFER_INITIALIZER; + virBufferPtr buf = opaque; + const char *unit; + const char *min_unit; + unsigned long long gran_short_size = virFormatIntPretty(granularity, &unit); + unsigned long long min_short_size = virFormatIntPretty(min, &min_unit); + + /* Only use the smaller unit if they are different */ + if (min != granularity) { + unsigned long long gran_div; + unsigned long long min_div; + + gran_div = granularity / gran_short_size; + min_div = min / min_short_size; + + if (min_div > gran_div) { + min_short_size *= min_div / gran_div; + } else if (min_div < gran_div) { + unit = min_unit; + gran_short_size *= gran_div / min_div; + } + } - if (!ncaches) - return 0; + virBufferAsprintf(buf, + "<control granularity='%llu'", + gran_short_size); - virBufferAddLit(buf, "<cache>\n"); - virBufferAdjustIndent(buf, 2); + if (min != granularity) + virBufferAsprintf(buf, " min='%llu'", min_short_size); - for (i = 0; i < ncaches; i++) { - virCapsHostCacheBankPtr bank = caches[i]; - char *cpus_str = virBitmapFormat(bank->cpus); - const char *unit = NULL; - unsigned long long short_size = virFormatIntPretty(bank->size, &unit); + virBufferAsprintf(buf, + " unit='%s' type='%s' maxAllocs='%u'/>\n", + unit, + virCacheTypeToString(scope), + max_allocations); - if (!cpus_str) - return -1; + return 0; +} - /* - * Let's just *hope* the size is aligned to KiBs so that it does not - * bite is back in the future - */ - virBufferAsprintf(buf, - "<bank id='%u' level='%u' type='%s' " - "size='%llu' unit='%s' cpus='%s'", - bank->id, bank->level, - virCacheTypeToString(bank->type), - short_size, unit, cpus_str); - VIR_FREE(cpus_str); - - virBufferSetChildIndent(&controlBuf, buf); - for (j = 0; j < bank->ncontrols; j++) { - const char *min_unit; - virResctrlInfoPerCachePtr controls = bank->controls[j]; - unsigned long long gran_short_size = controls->granularity; - unsigned long long min_short_size = controls->min; - - gran_short_size = virFormatIntPretty(gran_short_size, &unit); - min_short_size = virFormatIntPretty(min_short_size, &min_unit); - - /* Only use the smaller unit if they are different */ - if (min_short_size) { - unsigned long long gran_div; - unsigned long long min_div; - - gran_div = controls->granularity / gran_short_size; - min_div = controls->min / min_short_size; - - if (min_div > gran_div) { - min_short_size *= min_div / gran_div; - } else if (min_div < gran_div) { - unit = min_unit; - gran_short_size *= gran_div / min_div; - } - } - virBufferAsprintf(&controlBuf, - "<control granularity='%llu'", - gran_short_size); +static int +virCapabilitiesFormatCacheHelper(virCacheInfoPtr caches, + unsigned int level, + virCacheType type, + unsigned int id, + unsigned long long size, + virBitmapPtr cpus, + void *opaque) +{ + virBufferPtr buf = opaque; + virBuffer controlBuf = VIR_BUFFER_INITIALIZER; + char *cpus_str = virBitmapFormat(cpus); + const char *unit = NULL; + unsigned long long short_size = virFormatIntPretty(size, &unit); - if (min_short_size) - virBufferAsprintf(&controlBuf, " min='%llu'", min_short_size); + if (!cpus_str) + return -1; - virBufferAsprintf(&controlBuf, - " unit='%s' type='%s' maxAllocs='%u'/>\n", - unit, - virCacheTypeToString(controls->scope), - controls->max_allocation); - } + virBufferAsprintf(buf, + "<bank id='%u' level='%u' type='%s' " + "size='%llu' unit='%s' cpus='%s'", + id, level, virCacheTypeToString(type), + short_size, unit, cpus_str); + VIR_FREE(cpus_str); - if (virBufferCheckError(&controlBuf) < 0) - return -1; + virBufferSetChildIndent(&controlBuf, buf); - if (virBufferUse(&controlBuf)) { - virBufferAddLit(buf, ">\n"); - virBufferAddBuffer(buf, &controlBuf); - virBufferAddLit(buf, "</bank>\n"); - } else { - virBufferAddLit(buf, "/>\n"); - } + if (virCacheControlForeach(caches, level, type, id, + virCapabilitiesFormatCacheControlHelper, + &controlBuf) < 0) + return -1; + + if (virBufferCheckError(&controlBuf) < 0) + return -1; + + if (virBufferUse(&controlBuf)) { + virBufferAddLit(buf, ">\n"); + virBufferAddBuffer(buf, &controlBuf); + virBufferAddLit(buf, "</bank>\n"); + } else { + virBufferAddLit(buf, "/>\n"); } - virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, "</cache>\n"); + return 0; +} + + +int +virCapabilitiesInitCaches(virCapsPtr caps) +{ + if (caps->host.caches) + return 0; + + caps->host.caches = virCacheInfoNew(); + if (!caps->host.caches) + return -1; + + return 0; +} + + +static int +virCapabilitiesFormatCaches(virBufferPtr buf, + virCacheInfoPtr caches) +{ + virBuffer bankBuf = VIR_BUFFER_INITIALIZER; + + virBufferSetChildIndent(&bankBuf, buf); + + if (virCacheForeachBank(caches, + virCapabilitiesFormatCacheHelper, + &bankBuf) < 0) + return -1; + + if (virBufferCheckError(&bankBuf) < 0) + return -1; + + if (virBufferUse(&bankBuf)) { + virBufferAddLit(buf, "<cache>\n"); + virBufferAddBuffer(buf, &bankBuf); + virBufferAddLit(buf, "</cache>\n"); + } return 0; } @@ -1056,8 +1091,7 @@ virCapabilitiesFormatXML(virCapsPtr caps) caps->host.numaCell) < 0) goto error; - if (virCapabilitiesFormatCaches(&buf, caps->host.ncaches, - caps->host.caches) < 0) + if (virCapabilitiesFormatCaches(&buf, caps->host.caches) < 0) goto error; for (i = 0; i < caps->host.nsecModels; i++) { @@ -1545,203 +1579,6 @@ virCapabilitiesInitPages(virCapsPtr caps) } -bool -virCapsHostCacheBankEquals(virCapsHostCacheBankPtr a, - virCapsHostCacheBankPtr b) -{ - return (a->id == b->id && - a->level == b->level && - a->type == b->type && - a->size == b->size && - virBitmapEqual(a->cpus, b->cpus)); -} - -void -virCapsHostCacheBankFree(virCapsHostCacheBankPtr ptr) -{ - size_t i; - - if (!ptr) - return; - - virBitmapFree(ptr->cpus); - for (i = 0; i < ptr->ncontrols; i++) - VIR_FREE(ptr->controls[i]); - VIR_FREE(ptr->controls); - VIR_FREE(ptr); -} - - -static int -virCapsHostCacheBankSorter(const void *a, - const void *b) -{ - virCapsHostCacheBankPtr ca = *(virCapsHostCacheBankPtr *)a; - virCapsHostCacheBankPtr cb = *(virCapsHostCacheBankPtr *)b; - - if (ca->level < cb->level) - return -1; - if (ca->level > cb->level) - return 1; - - return ca->id - cb->id; -} - - -static int -virCapabilitiesInitResctrl(virCapsPtr caps) -{ - if (caps->host.resctrl) - return 0; - - caps->host.resctrl = virResctrlInfoNew(); - if (!caps->host.resctrl) - return -1; - - return 0; -} - - -int -virCapabilitiesInitCaches(virCapsPtr caps) -{ - size_t i = 0; - virBitmapPtr cpus = NULL; - ssize_t pos = -1; - DIR *dirp = NULL; - int ret = -1; - char *path = NULL; - char *type = NULL; - 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; - - if (virCapabilitiesInitResctrl(caps) < 0) - return -1; - - /* offline CPUs don't provide cache info */ - if (virFileReadValueBitmap(&cpus, "%s/cpu/online", SYSFS_SYSTEM_PATH) < 0) - return -1; - - while ((pos = virBitmapNextSetBit(cpus, pos)) >= 0) { - int rv = -1; - - VIR_FREE(path); - if (virAsprintf(&path, "%s/cpu/cpu%zd/cache/", SYSFS_SYSTEM_PATH, pos) < 0) - goto cleanup; - - VIR_DIR_CLOSE(dirp); - - rv = virDirOpenIfExists(&dirp, path); - if (rv < 0) - goto cleanup; - - if (!dirp) - continue; - - while ((rv = virDirRead(dirp, &ent, path)) > 0) { - int kernel_type; - unsigned int level; - - if (!STRPREFIX(ent->d_name, "index")) - continue; - - if (virFileReadValueUint(&level, - "%s/cpu/cpu%zd/cache/%s/level", - SYSFS_SYSTEM_PATH, pos, ent->d_name) < 0) - goto cleanup; - - if (level < cache_min_level) - continue; - - if (VIR_ALLOC(bank) < 0) - goto cleanup; - - bank->level = level; - - if (virFileReadValueUint(&bank->id, - "%s/cpu/cpu%zd/cache/%s/id", - SYSFS_SYSTEM_PATH, pos, ent->d_name) < 0) - goto cleanup; - - if (virFileReadValueUint(&bank->level, - "%s/cpu/cpu%zd/cache/%s/level", - SYSFS_SYSTEM_PATH, pos, ent->d_name) < 0) - goto cleanup; - - if (virFileReadValueString(&type, - "%s/cpu/cpu%zd/cache/%s/type", - SYSFS_SYSTEM_PATH, pos, ent->d_name) < 0) - goto cleanup; - - if (virFileReadValueScaledInt(&bank->size, - "%s/cpu/cpu%zd/cache/%s/size", - SYSFS_SYSTEM_PATH, pos, ent->d_name) < 0) - goto cleanup; - - if (virFileReadValueBitmap(&bank->cpus, - "%s/cpu/cpu%zd/cache/%s/shared_cpu_list", - SYSFS_SYSTEM_PATH, pos, ent->d_name) < 0) - goto cleanup; - - kernel_type = virCacheKernelTypeFromString(type); - if (kernel_type < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unknown cache type '%s'"), type); - goto cleanup; - } - - bank->type = kernel_type; - VIR_FREE(type); - - for (i = 0; i < caps->host.ncaches; i++) { - if (virCapsHostCacheBankEquals(bank, caps->host.caches[i])) - break; - } - if (i == caps->host.ncaches) { - /* If it is a new cache, then update its resctrl information. */ - if (virResctrlInfoGetCache(caps->host.resctrl, - bank->level, - bank->size, - &bank->ncontrols, - &bank->controls) < 0) - goto cleanup; - - if (VIR_APPEND_ELEMENT(caps->host.caches, - caps->host.ncaches, - bank) < 0) { - goto cleanup; - } - } - - virCapsHostCacheBankFree(bank); - bank = NULL; - } - if (rv < 0) - goto cleanup; - } - - /* Sort the array in order for the tests to be predictable. This way we can - * still traverse the directory instead of guessing names (in case there is - * 'index1' and 'index3' but no 'index2'). */ - qsort(caps->host.caches, caps->host.ncaches, - sizeof(*caps->host.caches), virCapsHostCacheBankSorter); - - ret = 0; - cleanup: - VIR_FREE(type); - VIR_FREE(path); - VIR_DIR_CLOSE(dirp); - virCapsHostCacheBankFree(bank); - virBitmapFree(cpus); - return ret; -} - - void virCapabilitiesHostInitIOMMU(virCapsPtr caps) { diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h index fe1b9ea45539..b617bb5df2f5 100644 --- a/src/conf/capabilities.h +++ b/src/conf/capabilities.h @@ -139,18 +139,6 @@ struct _virCapsHostSecModel { virCapsHostSecModelLabelPtr labels; }; -typedef struct _virCapsHostCacheBank virCapsHostCacheBank; -typedef virCapsHostCacheBank *virCapsHostCacheBankPtr; -struct _virCapsHostCacheBank { - unsigned int id; - unsigned int level; /* 1=L1, 2=L2, 3=L3, etc. */ - unsigned long long size; /* B */ - virCacheType type; /* Data, Instruction or Unified */ - virBitmapPtr cpus; /* All CPUs that share this bank */ - size_t ncontrols; - virResctrlInfoPerCachePtr *controls; -}; - typedef struct _virCapsHost virCapsHost; typedef virCapsHost *virCapsHostPtr; struct _virCapsHost { @@ -170,10 +158,7 @@ struct _virCapsHost { size_t nnumaCell_max; virCapsHostNUMACellPtr *numaCell; - virResctrlInfoPtr resctrl; - - size_t ncaches; - virCapsHostCacheBankPtr *caches; + virCacheInfoPtr caches; size_t nsecModels; virCapsHostSecModelPtr secModels; @@ -322,10 +307,6 @@ int virCapabilitiesInitPages(virCapsPtr caps); int virCapabilitiesInitNUMA(virCapsPtr caps); -bool virCapsHostCacheBankEquals(virCapsHostCacheBankPtr a, - virCapsHostCacheBankPtr b); -void virCapsHostCacheBankFree(virCapsHostCacheBankPtr ptr); - int virCapabilitiesInitCaches(virCapsPtr caps); void virCapabilitiesHostInitIOMMU(virCapsPtr caps); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a4a0c95b474d..07bf63489e20 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2619,8 +2619,9 @@ virRandomInt; # util/virresctrl.h -virCacheKernelTypeFromString; -virCacheKernelTypeToString; +virCacheControlForeach; +virCacheForeachBank; +virCacheInfoNew; virCacheTypeFromString; virCacheTypeToString; virResctrlAllocAddPID; @@ -2635,8 +2636,6 @@ virResctrlAllocNew; virResctrlAllocRemove; virResctrlAllocSetID; virResctrlAllocSetSize; -virResctrlInfoGetCache; -virResctrlInfoNew; # util/virrotatingfile.h diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 1606f4cfe931..d945f6177456 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2439,34 +2439,18 @@ qemuProcessSetupEmulator(virDomainObjPtr vm) static int -qemuProcessResctrlCreate(virQEMUDriverPtr driver, - virDomainObjPtr vm) +qemuProcessResctrlCreate(virDomainObjPtr vm) { - int ret = -1; size_t i = 0; - virCapsPtr caps = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; - if (!vm->def->ncachetunes) - return 0; - - /* Force capability refresh since resctrl info can change - * XXX: move cache info into virresctrl so caps are not needed */ - caps = virQEMUDriverGetCapabilities(driver, true); - if (!caps) - return -1; - for (i = 0; i < vm->def->ncachetunes; i++) { - if (virResctrlAllocCreate(caps->host.resctrl, - vm->def->cachetunes[i]->alloc, + if (virResctrlAllocCreate(vm->def->cachetunes[i]->alloc, priv->machineName) < 0) - goto cleanup; + return -1; } - ret = 0; - cleanup: - virObjectUnref(caps); - return ret; + return 0; } @@ -6227,7 +6211,7 @@ qemuProcessLaunch(virConnectPtr conn, goto cleanup; VIR_DEBUG("Setting up resctrl"); - if (qemuProcessResctrlCreate(driver, vm) < 0) + if (qemuProcessResctrlCreate(vm) < 0) goto cleanup; VIR_DEBUG("Setting up managed PR daemon"); diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index 2b72a3b36bae..c41f90b94919 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -38,12 +38,16 @@ VIR_LOG_INIT("util.virresctrl") /* Resctrl is short for Resource Control. It might be implemented for various * resources, but at the time of this writing this is only supported for cache * allocation technology (aka CAT). Hence the reson for leaving 'Cache' out of - * all the structure and function names for now (can be added later if needed. + * all the structure and function names for now (can be added later if needed). + * + * We now have virCache structures and functions as well, they are supposed to + * cover all host cache information. */ /* Common definitions */ #define SYSFS_RESCTRL_PATH "/sys/fs/resctrl" +#define SYSFS_SYSTEM_PATH "/sys/devices/system" /* Following are three different enum implementations for the same enum. Each @@ -52,6 +56,7 @@ VIR_LOG_INIT("util.virresctrl") * consistent in between all of them. */ /* Cache name mapping for Linux kernel naming. */ +VIR_ENUM_DECL(virCacheKernel); VIR_ENUM_IMPL(virCacheKernel, VIR_CACHE_TYPE_LAST, "Unified", "Instruction", @@ -74,12 +79,18 @@ VIR_ENUM_IMPL(virResctrl, VIR_CACHE_TYPE_LAST, /* All private typedefs so that they exist for all later definitions. This way * structs can be included in one or another without reorganizing the code every * time. */ +typedef struct _virCacheBank virCacheBank; +typedef virCacheBank *virCacheBankPtr; + typedef struct _virResctrlInfoPerType virResctrlInfoPerType; typedef virResctrlInfoPerType *virResctrlInfoPerTypePtr; typedef struct _virResctrlInfoPerLevel virResctrlInfoPerLevel; typedef virResctrlInfoPerLevel *virResctrlInfoPerLevelPtr; +typedef struct _virResctrlInfo virResctrlInfo; +typedef virResctrlInfo *virResctrlInfoPtr; + typedef struct _virResctrlAllocPerType virResctrlAllocPerType; typedef virResctrlAllocPerType *virResctrlAllocPerTypePtr; @@ -88,28 +99,69 @@ typedef virResctrlAllocPerLevel *virResctrlAllocPerLevelPtr; /* Class definitions and initializations */ +static virClassPtr virCacheInfoClass; +static virClassPtr virCacheBankClass; static virClassPtr virResctrlInfoClass; static virClassPtr virResctrlAllocClass; +/* virCacheInfo */ +struct _virCacheInfo { + virObject parent; + + virCacheBankPtr *banks; + size_t nbanks; + + virResctrlInfoPtr resctrl; +}; + + +static void +virCacheInfoDispose(void *obj) +{ + size_t i = 0; + + virCacheInfoPtr ci = obj; + + for (i = 0; i < ci->nbanks; i++) + virObjectUnref(ci->banks[i]); + + virObjectUnref(ci->resctrl); + VIR_FREE(ci->banks); +} + + +/* virCacheBank */ +struct _virCacheBank { + virObject parent; + + unsigned int id; + unsigned int level; /* 1=L1, 2=L2, 3=L3, etc. */ + unsigned long long size; /* B */ + virCacheType type; /* Data, Instruction or Unified */ + virBitmapPtr cpus; /* All CPUs that share this bank */ +}; + + +static void +virCacheBankDispose(void *obj) +{ + virCacheBankPtr bank = obj; + + virBitmapFree(bank->cpus); +} + + /* virResctrlInfo */ struct _virResctrlInfoPerType { /* Kernel-provided information */ unsigned int min_cbm_bits; - /* Our computed information from the above */ - unsigned int bits; - unsigned int max_cache_id; - - /* In order to be self-sufficient we need size information per cache. - * Funnily enough, one of the outcomes of the resctrl design is that it - * does not account for different sizes per cache on the same level. So - * for the sake of easiness, let's copy that, for now. */ - unsigned long long size; + /* Number of bits in the cbm mask */ + size_t bits; - /* Information that we will return upon request (this is public struct) as - * until now all the above is internal to this module */ - virResctrlInfoPerCache control; + /* Maximum number of simultaneous allocations */ + unsigned int max_allocations; }; struct _virResctrlInfoPerLevel { @@ -261,6 +313,12 @@ virResctrlAllocDispose(void *obj) static int virResctrlOnceInit(void) { + if (!VIR_CLASS_NEW(virCacheInfo, virClassForObject())) + return -1; + + if (!VIR_CLASS_NEW(virCacheBank, virClassForObject())) + return -1; + if (!VIR_CLASS_NEW(virResctrlInfo, virClassForObject())) return -1; @@ -357,9 +415,7 @@ virResctrlGetInfo(virResctrlInfoPtr resctrl) if (VIR_ALLOC(i_type) < 0) goto cleanup; - i_type->control.scope = type; - - rv = virFileReadValueUint(&i_type->control.max_allocation, + rv = virFileReadValueUint(&i_type->max_allocations, SYSFS_RESCTRL_PATH "/info/%s/num_closids", ent->d_name); if (rv == -2) { @@ -443,13 +499,14 @@ virResctrlGetInfo(virResctrlInfoPtr resctrl) ret = 0; cleanup: + virBitmapFree(tmp_map); VIR_DIR_CLOSE(dirp); VIR_FREE(i_type); return ret; } -virResctrlInfoPtr +static virResctrlInfoPtr virResctrlInfoNew(void) { virResctrlInfoPtr ret = NULL; @@ -495,67 +552,289 @@ virResctrlInfoIsEmpty(virResctrlInfoPtr resctrl) } -int -virResctrlInfoGetCache(virResctrlInfoPtr resctrl, - unsigned int level, - unsigned long long size, - size_t *ncontrols, - virResctrlInfoPerCachePtr **controls) +/* virCacheBank-related definitions */ +static virCacheBankPtr +virCacheBankNew(void) +{ + if (virResctrlInitialize() < 0) + return NULL; + + return virObjectNew(virCacheBankClass); +} + + +static bool +virCacheBankEquals(virCacheBankPtr a, + virCacheBankPtr b) +{ + return (a->id == b->id && + a->level == b->level && + a->type == b->type); +} + + +static int +virCacheBankSorter(const void *a, + const void *b) +{ + virCacheBankPtr ca = *(virCacheBankPtr *)a; + virCacheBankPtr cb = *(virCacheBankPtr *)b; + + if (ca->level < cb->level) + return -1; + if (ca->level > cb->level) + return 1; + + return ca->id - cb->id; +} + + +/* virCacheInfo-related definitions */ +static int +virCacheGetInfo(virCacheInfoPtr ci) { - virResctrlInfoPerLevelPtr i_level = NULL; - virResctrlInfoPerTypePtr i_type = NULL; size_t i = 0; + virBitmapPtr cpus = NULL; + ssize_t pos = -1; + DIR *dirp = NULL; int ret = -1; + char *path = NULL; + char *type = NULL; + struct dirent *ent = NULL; + virCacheBankPtr bank = NULL; - if (virResctrlInfoIsEmpty(resctrl)) - return 0; + /* 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; - if (level >= resctrl->nlevels) - return 0; + ci->resctrl = virResctrlInfoNew(); + if (!ci->resctrl) + return -1; - i_level = resctrl->levels[level]; - if (!i_level) - return 0; + /* offline CPUs don't provide cache info */ + if (virFileReadValueBitmap(&cpus, "%s/cpu/online", SYSFS_SYSTEM_PATH) < 0) + return -1; - for (i = 0; i < VIR_CACHE_TYPE_LAST; i++) { - i_type = i_level->types[i]; - if (!i_type) + while ((pos = virBitmapNextSetBit(cpus, pos)) >= 0) { + int rv = -1; + + VIR_FREE(path); + if (virAsprintf(&path, "%s/cpu/cpu%zd/cache/", SYSFS_SYSTEM_PATH, pos) < 0) + goto cleanup; + + VIR_DIR_CLOSE(dirp); + + rv = virDirOpenIfExists(&dirp, path); + if (rv < 0) + goto cleanup; + + if (!dirp) 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) { + while ((rv = virDirRead(dirp, &ent, path)) > 0) { + int kernel_type; + unsigned int level; + + if (!STRPREFIX(ent->d_name, "index")) + continue; + + if (virFileReadValueUint(&level, + "%s/cpu/cpu%zd/cache/%s/level", + SYSFS_SYSTEM_PATH, pos, ent->d_name) < 0) + goto cleanup; + + if (level < cache_min_level) + continue; + + bank = virCacheBankNew(); + if (!bank) + goto cleanup; + + bank->level = level; + + if (virFileReadValueUint(&bank->id, + "%s/cpu/cpu%zd/cache/%s/id", + SYSFS_SYSTEM_PATH, pos, ent->d_name) < 0) + goto cleanup; + + if (virFileReadValueUint(&bank->level, + "%s/cpu/cpu%zd/cache/%s/level", + SYSFS_SYSTEM_PATH, pos, ent->d_name) < 0) + goto cleanup; + + if (virFileReadValueString(&type, + "%s/cpu/cpu%zd/cache/%s/type", + SYSFS_SYSTEM_PATH, pos, ent->d_name) < 0) + goto cleanup; + + if (virFileReadValueScaledInt(&bank->size, + "%s/cpu/cpu%zd/cache/%s/size", + SYSFS_SYSTEM_PATH, pos, ent->d_name) < 0) + goto cleanup; + + if (virFileReadValueBitmap(&bank->cpus, + "%s/cpu/cpu%zd/cache/%s/shared_cpu_list", + SYSFS_SYSTEM_PATH, pos, ent->d_name) < 0) + goto cleanup; + + kernel_type = virCacheKernelTypeFromString(type); + if (kernel_type < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("level %u cache size %llu does not match " - "expected size %llu"), - level, i_type->size, size); - goto error; + _("Unknown cache type '%s'"), type); + goto cleanup; } - i_type->max_cache_id++; - } - if (VIR_EXPAND_N(*controls, *ncontrols, 1) < 0) - goto error; - if (VIR_ALLOC((*controls)[*ncontrols - 1]) < 0) - goto error; + bank->type = kernel_type; + VIR_FREE(type); + + for (i = 0; i < ci->nbanks; i++) { + if (virCacheBankEquals(bank, ci->banks[i])) + break; + } + if (i == ci->nbanks && + VIR_APPEND_ELEMENT(ci->banks, ci->nbanks, bank) < 0) - memcpy((*controls)[*ncontrols - 1], &i_type->control, sizeof(i_type->control)); + virObjectUnref(bank); + bank = NULL; + } + if (rv < 0) + goto cleanup; } + /* Sort the array in order for the tests to be predictable. This way we can + * still traverse the directory instead of guessing names (in case there is + * 'index1' and 'index3' but no 'index2'). */ + qsort(ci->banks, ci->nbanks, sizeof(*ci->banks), virCacheBankSorter); + ret = 0; cleanup: + VIR_FREE(type); + VIR_FREE(path); + VIR_DIR_CLOSE(dirp); + virObjectUnref(bank); + virBitmapFree(cpus); return ret; - error: - while (*ncontrols) - VIR_FREE((*controls)[--*ncontrols]); - VIR_FREE(*controls); - goto cleanup; +} + + +virCacheInfoPtr +virCacheInfoNew(void) +{ + virCacheInfoPtr ret = NULL; + + if (virResctrlInitialize() < 0) + return NULL; + + ret = virObjectNew(virCacheInfoClass); + if (!ret) + return NULL; + + if (virCacheGetInfo(ret) < 0) { + virObjectUnref(ret); + return NULL; + } + + return ret; +} + + +static virCacheBankPtr +virCacheInfoGetBankForType(virCacheInfoPtr ci, + unsigned int level, + unsigned int cache, + virCacheType type) +{ + size_t i = 0; + + for (i = 0; i < ci->nbanks; i++) { + virCacheBankPtr bank = ci->banks[i]; + + if (bank->level != level) + continue; + + if (bank->id != cache) + continue; + + /* + * We're looking for a particular type. If it is _BOTH we can only + * return that, but since CAT can support _CODE/_DATA allocation on + * cache with type _BOTH, we need to be able to find that as well. + * + * Think of this as a function that returns a particular cache that + * takes care of caching type @type on a level @level, but we're + * restricted to cache id @cache. Or rather don't think about since + * it's also confusing. + */ + if (bank->type == type || bank->type == VIR_CACHE_TYPE_BOTH) + return bank; + } + + return NULL; +} + + +int +virCacheControlForeach(virCacheInfoPtr ci, + unsigned int level, + virCacheType type, + unsigned int id, + virCacheForeachControlCallback cb, + void *opaque) +{ + virResctrlInfoPerLevelPtr r_level = NULL; + size_t i = 0; + + if (!ci || + !ci->resctrl || + !ci->resctrl->levels || + level >= ci->resctrl->nlevels) + return 0; + + r_level = ci->resctrl->levels[level]; + if (!r_level || !r_level->types) + return 0; + + for (i = 0; i < VIR_CACHE_TYPE_LAST; i++) { + virResctrlInfoPerTypePtr r_type = r_level->types[i]; + virCacheBankPtr bank = virCacheInfoGetBankForType(ci, level, id, type); + unsigned long long granularity; + unsigned long long min; + + if (!r_type || !bank) + continue; + + granularity = bank->size / r_type->bits; + min = r_type->min_cbm_bits * granularity; + + int rv = cb(granularity, min, i, r_type->max_allocations, opaque); + if (rv < 0) + return rv; + } + + return 0; +} + + +int +virCacheForeachBank(virCacheInfoPtr ci, + virCacheForeachBankCallback cb, + void *opaque) +{ + size_t i = 0; + + if (!ci) + return 0; + + for (i = 0; i < ci->nbanks; i++) { + virCacheBankPtr bank = ci->banks[i]; + int rv = cb(ci, bank->level, bank->type, bank->id, bank->size, bank->cpus, opaque); + + if (rv < 0) + return rv; + } + + return 0; } @@ -1107,25 +1386,25 @@ virResctrlAllocSubtract(virResctrlAllocPtr dst, static virResctrlAllocPtr -virResctrlAllocNewFromInfo(virResctrlInfoPtr info) +virResctrlAllocNewFromInfo(virCacheInfoPtr ci) { + size_t level = 0; + size_t type = 0; size_t i = 0; - size_t j = 0; - size_t k = 0; virResctrlAllocPtr ret = virResctrlAllocNew(); virBitmapPtr mask = NULL; if (!ret) return NULL; - for (i = 0; i < info->nlevels; i++) { - virResctrlInfoPerLevelPtr i_level = info->levels[i]; + for (level = 0; level < ci->resctrl->nlevels; level++) { + virResctrlInfoPerLevelPtr i_level = ci->resctrl->levels[level]; if (!i_level) continue; - for (j = 0; j < VIR_CACHE_TYPE_LAST; j++) { - virResctrlInfoPerTypePtr i_type = i_level->types[j]; + for (type = 0; type < VIR_CACHE_TYPE_LAST; type++) { + virResctrlInfoPerTypePtr i_type = i_level->types[type]; if (!i_type) continue; @@ -1136,8 +1415,13 @@ virResctrlAllocNewFromInfo(virResctrlInfoPtr info) goto error; virBitmapSetAll(mask); - for (k = 0; k <= i_type->max_cache_id; k++) { - if (virResctrlAllocUpdateMask(ret, i, j, k, mask) < 0) + for (i = 0; i < ci->nbanks; i++) { + virCacheBankPtr bank = ci->banks[i]; + + if (bank->level != level) + continue; + + if (virResctrlAllocUpdateMask(ret, level, type, bank->id, mask) < 0) goto error; } } @@ -1159,12 +1443,13 @@ virResctrlAllocNewFromInfo(virResctrlInfoPtr info) * scans for all allocations under /sys/fs/resctrl and subtracts each one of * them from it. That way it can then return an allocation with only bit set * being those that are not mentioned in any other allocation. It is used for - * two things, a) calculating the masks when creating allocations and b) from + * two things, a) calculating the masks when creating allocations and cb) from * tests. */ virResctrlAllocPtr -virResctrlAllocGetUnused(virResctrlInfoPtr resctrl) +virResctrlAllocGetUnused(virCacheInfoPtr ci) { + virResctrlInfoPtr resctrl = ci->resctrl; virResctrlAllocPtr ret = NULL; virResctrlAllocPtr alloc = NULL; struct dirent *ent = NULL; @@ -1177,7 +1462,7 @@ virResctrlAllocGetUnused(virResctrlInfoPtr resctrl) return NULL; } - ret = virResctrlAllocNewFromInfo(resctrl); + ret = virResctrlAllocNewFromInfo(ci); if (!ret) return NULL; @@ -1239,13 +1524,15 @@ static int virResctrlAllocFindUnused(virResctrlAllocPtr alloc, virResctrlInfoPerTypePtr i_type, virResctrlAllocPerTypePtr f_type, + unsigned long long size, unsigned int level, unsigned int type, unsigned int cache) { - unsigned long long *size = alloc->levels[level]->types[type]->sizes[cache]; + unsigned long long *need_size = alloc->levels[level]->types[type]->sizes[cache]; virBitmapPtr a_mask = NULL; virBitmapPtr f_mask = NULL; + unsigned long long granularity = size / i_type->bits; unsigned long long need_bits; size_t i = 0; ssize_t pos = -1; @@ -1253,7 +1540,7 @@ virResctrlAllocFindUnused(virResctrlAllocPtr alloc, ssize_t last_pos = -1; int ret = -1; - if (!size) + if (!need_size) return 0; if (cache >= f_type->nmasks) { @@ -1272,30 +1559,30 @@ virResctrlAllocFindUnused(virResctrlAllocPtr alloc, return -1; } - if (*size == i_type->size) { + if (*need_size == size) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Cache allocation for the whole cache is not " "possible, specify size smaller than %llu"), - i_type->size); + size); return -1; } - need_bits = *size / i_type->control.granularity; - - if (*size % i_type->control.granularity) { + if (*need_size % granularity) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Cache allocation of size %llu is not " "divisible by granularity %llu"), - *size, i_type->control.granularity); + *need_size, granularity); return -1; } + need_bits = *need_size / granularity; + 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, - i_type->control.granularity * i_type->min_cbm_bits); + *need_size, + granularity * i_type->min_cbm_bits); return -1; } @@ -1334,7 +1621,7 @@ virResctrlAllocFindUnused(virResctrlAllocPtr alloc, _("Not enough room for allocation of " "%llu bytes for level %u cache %u " "scope type '%s'"), - *size, level, cache, + *need_size, level, cache, virCacheTypeToString(type)); return -1; } @@ -1401,15 +1688,16 @@ virResctrlAllocCopyMasks(virResctrlAllocPtr dst, * transforming `sizes` into `masks`. */ int -virResctrlAllocMasksAssign(virResctrlInfoPtr resctrl, +virResctrlAllocMasksAssign(virCacheInfoPtr ci, virResctrlAllocPtr alloc) { int ret = -1; unsigned int level = 0; + virResctrlInfoPtr resctrl = ci->resctrl; virResctrlAllocPtr alloc_free = NULL; virResctrlAllocPtr alloc_default = NULL; - alloc_free = virResctrlAllocGetUnused(resctrl); + alloc_free = virResctrlAllocGetUnused(ci); if (!alloc_free) return -1; @@ -1457,8 +1745,18 @@ virResctrlAllocMasksAssign(virResctrlInfoPtr resctrl, for (cache = 0; cache < a_type->nsizes; cache++) { virResctrlInfoPerLevelPtr i_level = resctrl->levels[level]; virResctrlInfoPerTypePtr i_type = i_level->types[type]; + virCacheBankPtr bank = virCacheInfoGetBankForType(ci, level, cache, type); + + if (!bank) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Cache level %u id '%u' does not support tuning for " + "scope type '%s'"), + level, cache, virCacheTypeToString(type)); + goto cleanup; + } - if (virResctrlAllocFindUnused(alloc, i_type, f_type, level, type, cache) < 0) + if (virResctrlAllocFindUnused(alloc, i_type, f_type, bank->size, + level, type, cache) < 0) goto cleanup; } } @@ -1494,10 +1792,10 @@ virResctrlAllocDeterminePath(virResctrlAllocPtr alloc, /* This checks if the directory for the alloc exists. If not it tries to create * it and apply appropriate alloc settings. */ int -virResctrlAllocCreate(virResctrlInfoPtr resctrl, - virResctrlAllocPtr alloc, +virResctrlAllocCreate(virResctrlAllocPtr alloc, const char *machinename) { + virCacheInfoPtr ci = NULL; char *schemata_path = NULL; char *alloc_str = NULL; int ret = -1; @@ -1506,14 +1804,16 @@ virResctrlAllocCreate(virResctrlInfoPtr resctrl, if (!alloc) return 0; - if (virResctrlInfoIsEmpty(resctrl)) { + ci = virCacheInfoNew(); + + if (virResctrlInfoIsEmpty(ci->resctrl)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Resource control is not supported on this host")); - return -1; + goto cleanup; } if (virResctrlAllocDeterminePath(alloc, machinename) < 0) - return -1; + goto cleanup; if (virFileExists(alloc->path)) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -1526,7 +1826,7 @@ virResctrlAllocCreate(virResctrlInfoPtr resctrl, if (lockfd < 0) goto cleanup; - if (virResctrlAllocMasksAssign(resctrl, alloc) < 0) + if (virResctrlAllocMasksAssign(ci, alloc) < 0) goto cleanup; alloc_str = virResctrlAllocFormat(alloc); @@ -1554,6 +1854,7 @@ virResctrlAllocCreate(virResctrlInfoPtr resctrl, ret = 0; cleanup: + virObjectUnref(ci); virResctrlUnlock(lockfd); VIR_FREE(alloc_str); VIR_FREE(schemata_path); diff --git a/src/util/virresctrl.h b/src/util/virresctrl.h index 9052a2b19a02..ef1fef8cf622 100644 --- a/src/util/virresctrl.h +++ b/src/util/virresctrl.h @@ -34,34 +34,43 @@ typedef enum { } virCacheType; VIR_ENUM_DECL(virCache); -VIR_ENUM_DECL(virCacheKernel); +typedef struct _virCacheInfo virCacheInfo; +typedef virCacheInfo *virCacheInfoPtr; -typedef struct _virResctrlInfoPerCache virResctrlInfoPerCache; -typedef virResctrlInfoPerCache *virResctrlInfoPerCachePtr; -struct _virResctrlInfoPerCache { - /* Smallest possible increase of the allocation size in bytes */ - unsigned long long granularity; - /* Minimal allocatable size in bytes (if different from granularity) */ - unsigned long long min; - /* Type of the allocation */ - virCacheType scope; - /* Maximum number of simultaneous allocations */ - unsigned int max_allocation; -}; +virCacheInfoPtr +virCacheInfoNew(void); -typedef struct _virResctrlInfo virResctrlInfo; -typedef virResctrlInfo *virResctrlInfoPtr; +typedef int virCacheForeachControlCallback(unsigned long long granularity, + unsigned long long min, + virCacheType scope, + unsigned int max_allocations, + void *opaque); -virResctrlInfoPtr -virResctrlInfoNew(void); int -virResctrlInfoGetCache(virResctrlInfoPtr resctrl, +virCacheControlForeach(virCacheInfoPtr ci, unsigned int level, - unsigned long long size, - size_t *ncontrols, - virResctrlInfoPerCachePtr **controls); + virCacheType type, + unsigned int id, + virCacheForeachControlCallback cb, + void *opaque); + + +typedef int virCacheForeachBankCallback(virCacheInfoPtr ci, + unsigned int level, + virCacheType type, + unsigned int id, + unsigned long long size, + virBitmapPtr cpus, + void *opaque); + + +int +virCacheForeachBank(virCacheInfoPtr ci, + virCacheForeachBankCallback cb, + void *opaque); + /* Alloc-related things */ typedef struct _virResctrlAlloc virResctrlAlloc; @@ -105,8 +114,7 @@ virResctrlAllocDeterminePath(virResctrlAllocPtr alloc, const char *machinename); int -virResctrlAllocCreate(virResctrlInfoPtr r_info, - virResctrlAllocPtr alloc, +virResctrlAllocCreate(virResctrlAllocPtr alloc, const char *machinename); int diff --git a/src/util/virresctrlpriv.h b/src/util/virresctrlpriv.h index 80ddd4b875a3..258f3a0e3f74 100644 --- a/src/util/virresctrlpriv.h +++ b/src/util/virresctrlpriv.h @@ -22,10 +22,10 @@ # include "virresctrl.h" virResctrlAllocPtr -virResctrlAllocGetUnused(virResctrlInfoPtr resctrl); +virResctrlAllocGetUnused(virCacheInfoPtr ci); int -virResctrlAllocMasksAssign(virResctrlInfoPtr resctrl, +virResctrlAllocMasksAssign(virCacheInfoPtr ci, virResctrlAllocPtr alloc); #endif /* __VIR_RESCTRL_PRIV_H__ */ diff --git a/tests/virresctrltest.c b/tests/virresctrltest.c index 99e20d5dec99..397e4f9a644e 100644 --- a/tests/virresctrltest.c +++ b/tests/virresctrltest.c @@ -18,13 +18,13 @@ static int test_virResctrlGetUnused(const void *opaque) { struct virResctrlData *data = (struct virResctrlData *) opaque; - char *system_dir = NULL; - char *resctrl_dir = NULL; - int ret = -1; + virCacheInfoPtr ci = NULL; virResctrlAllocPtr alloc = NULL; char *schemata_str = NULL; char *schemata_file; - virCapsPtr caps = NULL; + char *system_dir = NULL; + char *resctrl_dir = NULL; + int ret = -1; if (virAsprintf(&system_dir, "%s/vircaps2xmldata/linux-%s/system", abs_srcdir, data->filename) < 0) @@ -41,13 +41,11 @@ test_virResctrlGetUnused(const void *opaque) virFileWrapperAddPrefix("/sys/devices/system", system_dir); virFileWrapperAddPrefix("/sys/fs/resctrl", resctrl_dir); - caps = virCapabilitiesNew(VIR_ARCH_X86_64, false, false); - if (!caps || virCapabilitiesInitCaches(caps) < 0) { - fprintf(stderr, "Could not initialize capabilities"); + ci = virCacheInfoNew(); + if (!ci) goto cleanup; - } - alloc = virResctrlAllocGetUnused(caps->host.resctrl); + alloc = virResctrlAllocGetUnused(ci); virFileWrapperClearPrefixes(); @@ -68,7 +66,7 @@ test_virResctrlGetUnused(const void *opaque) ret = 0; cleanup: - virObjectUnref(caps); + virObjectUnref(ci); virObjectUnref(alloc); VIR_FREE(system_dir); VIR_FREE(resctrl_dir); -- 2.17.1