5:24 a.m.
Both together fix https://bugzilla.redhat.com/show_bug.cgi?id=1686274
Martin Kletzander (2):
resctrl: Do not calculate free bandwidth for MBA
resctrl: Set MBA defaults properly
src/util/virresctrl.c | 89 +++++++++++++++++++++++++------------------
1 file changed, 52 insertions(+), 37 deletions(-)
--
2.21.0
5:24 a.m.
New subject: [libvirt] [PATCH 1/2] resctrl: Do not calculate free bandwidth for MBA
For CAT we calculate unallocated parts of the cache, however with MBA this does
not make sense as the purpose of that is to limit the bandwidth and the setting
is only proportional relative to bandwidth settings for other groups.
This means it makes sense to set the values to 100% even if there are other
groups with some allocations and that we don't need to find the available
(unallocated) bandwidth in all the groups.
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
---
src/util/virresctrl.c | 52 ++++++++++++-------------------------------
1 file changed, 14 insertions(+), 38 deletions(-)
diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
index 0857d4e882be..4a812d7a8be8 100644
--- a/src/util/virresctrl.c
+++ b/src/util/virresctrl.c
@@ -1823,22 +1823,6 @@ virResctrlAllocSubtract(virResctrlAllocPtr dst,
}
-static void
-virResctrlMemoryBandwidthSubtract(virResctrlAllocPtr free,
- virResctrlAllocPtr used)
-{
- size_t i;
-
- if (!used->mem_bw)
- return;
-
- for (i = 0; i < used->mem_bw->nbandwidths; i++) {
- if (used->mem_bw->bandwidths[i])
- *(free->mem_bw->bandwidths[i]) -=
*(used->mem_bw->bandwidths[i]);
- }
-}
-
-
static virResctrlAllocPtr
virResctrlAllocNewFromInfo(virResctrlInfoPtr info)
{
@@ -1902,15 +1886,18 @@ virResctrlAllocNewFromInfo(virResctrlInfoPtr info)
}
/*
- * This function creates an allocation that represents all unused parts of
- * all caches and memory bandwidth in the system. It uses virResctrlInfo
- * for creating a new full allocation with all bits set (using the
- * virResctrlAllocNewFromInfo()), sets memory bandwidth 100%, and then 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 for CAT and
- * available memory bandwidth for MBA. It is used for two things, calculating
- * the masks and bandwidth available when creating allocations and from tests.
+ * This function creates an allocation that represents all unused parts of all
+ * caches in the system. It uses virResctrlInfo for creating a new full
+ * allocation with all bits set (using virResctrlAllocNewFromInfo()) and then
+ * 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
+ * tests.
+ *
+ * MBA (Memory Bandwidth Allocation) is not taken into account as it is a
+ * limiting setting, not an allocating one. The way it works is also vastly
+ * different from CAT.
*/
virResctrlAllocPtr
virResctrlAllocGetUnused(virResctrlInfoPtr resctrl)
@@ -1956,7 +1943,6 @@ virResctrlAllocGetUnused(virResctrlInfoPtr resctrl)
goto error;
}
- virResctrlMemoryBandwidthSubtract(ret, alloc);
virResctrlAllocSubtract(ret, alloc);
virObjectUnref(alloc);
alloc = NULL;
@@ -2109,12 +2095,10 @@ virResctrlAllocFindUnused(virResctrlAllocPtr alloc,
static int
virResctrlAllocMemoryBandwidth(virResctrlInfoPtr resctrl,
- virResctrlAllocPtr alloc,
- virResctrlAllocPtr free)
+ virResctrlAllocPtr alloc)
{
size_t i;
virResctrlAllocMemBWPtr mem_bw_alloc = alloc->mem_bw;
- virResctrlAllocMemBWPtr mem_bw_free = free->mem_bw;
virResctrlInfoMemBWPtr mem_bw_info = resctrl->membw_info;
if (!mem_bw_alloc)
@@ -2154,14 +2138,6 @@ virResctrlAllocMemoryBandwidth(virResctrlInfoPtr resctrl,
i, mem_bw_info->max_id);
return -1;
}
- if (*(mem_bw_alloc->bandwidths[i]) > *(mem_bw_free->bandwidths[i])) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Not enough room for allocation of %u%% "
- "bandwidth on node %zd, available bandwidth
%u%%"),
- *(mem_bw_alloc->bandwidths[i]), i,
- *(mem_bw_free->bandwidths[i]));
- return -1;
- }
}
return 0;
}
@@ -2228,7 +2204,7 @@ virResctrlAllocAssign(virResctrlInfoPtr resctrl,
if (!alloc_default)
goto cleanup;
- if (virResctrlAllocMemoryBandwidth(resctrl, alloc, alloc_free) < 0)
+ if (virResctrlAllocMemoryBandwidth(resctrl, alloc) < 0)
goto cleanup;
if (virResctrlAllocCopyMasks(alloc, alloc_default) < 0)
--
2.21.0
5:24 a.m.
New subject: [libvirt] [PATCH 2/2] resctrl: Set MBA defaults properly
Similarly to CAT, when you set some values in an group, remove the group and
recreate it, the previous values will be kept there. In order to not get values
from a previous setting (a previous VM, for example), we need to set them to
sensible defaults. The same way we do that for CAT, just set the same values as
the default group has.
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
---
src/util/virresctrl.c | 39 +++++++++++++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
index 4a812d7a8be8..ab977b995c4d 100644
--- a/src/util/virresctrl.c
+++ b/src/util/virresctrl.c
@@ -2143,6 +2143,42 @@ virResctrlAllocMemoryBandwidth(virResctrlInfoPtr resctrl,
}
+static int
+virResctrlAllocCopyMemBW(virResctrlAllocPtr dst,
+ virResctrlAllocPtr src)
+{
+ size_t i = 0;
+ virResctrlAllocMemBWPtr dst_bw = NULL;
+ virResctrlAllocMemBWPtr src_bw = src->mem_bw;
+
+ if (!src->mem_bw)
+ return 0;
+
+ if (!dst->mem_bw &&
+ VIR_ALLOC(dst->mem_bw) < 0)
+ return -1;
+
+ dst_bw = dst->mem_bw;
+
+ if (src_bw->nbandwidths > dst_bw->nbandwidths &&
+ VIR_EXPAND_N(dst_bw->bandwidths, dst_bw->nbandwidths,
+ src_bw->nbandwidths - dst_bw->nbandwidths) < 0)
+ return -1;
+
+ for (i = 0; i < src_bw->nbandwidths; i++) {
+ if (dst_bw->bandwidths[i]) {
+ *dst_bw->bandwidths[i] = 123;
+ continue;
+ }
+ if (VIR_ALLOC(dst_bw->bandwidths[i]) < 0)
+ return -1;
+ *dst_bw->bandwidths[i] = *src_bw->bandwidths[i];
+ }
+
+ return 0;
+}
+
+
static int
virResctrlAllocCopyMasks(virResctrlAllocPtr dst,
virResctrlAllocPtr src)
@@ -2210,6 +2246,9 @@ virResctrlAllocAssign(virResctrlInfoPtr resctrl,
if (virResctrlAllocCopyMasks(alloc, alloc_default) < 0)
goto cleanup;
+ if (virResctrlAllocCopyMemBW(alloc, alloc_default) < 0)
+ goto cleanup;
+
for (level = 0; level < alloc->nlevels; level++) {
virResctrlAllocPerLevelPtr a_level = alloc->levels[level];
virResctrlAllocPerLevelPtr f_level = NULL;
--
2.21.0
8:46 a.m.
On 3/11/19 11:24 AM, Martin Kletzander wrote:
Both together fix
https://bugzilla.redhat.com/show_bug.cgi?id=1686274
Martin Kletzander (2):
resctrl: Do not calculate free bandwidth for MBA
resctrl: Set MBA defaults properly
src/util/virresctrl.c | 89 +++++++++++++++++++++++++------------------
1 file changed, 52 insertions(+), 37 deletions(-)
ACK
Michal
2119
days inactive
2119
days old
3 comments
2 participants
participants (2)
-
Martin Kletzander -
Michal Privoznik