On 2018年07月25日 06:08, John Ferlan wrote:
On 07/18/2018 03:57 AM, bing....@intel.com wrote:
From: Bing Niu <bing....@intel.com>
Introducing virResctrlInfoMemBW for the information memory bandwidth
allocation information. Those information is used for memory
bandwidth allocating.
Last sentence is duplicitous.
Signed-off-by: Bing Niu <bing....@intel.com>
---
src/util/virresctrl.c | 104 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 104 insertions(+)
diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
index 1515de2..06e2702 100644
--- a/src/util/virresctrl.c
+++ b/src/util/virresctrl.c
@@ -80,6 +80,9 @@ typedef virResctrlInfoPerType *virResctrlInfoPerTypePtr;
typedef struct _virResctrlInfoPerLevel virResctrlInfoPerLevel;
typedef virResctrlInfoPerLevel *virResctrlInfoPerLevelPtr;
+typedef struct _virResctrlInfoMemBW virResctrlInfoMemBW;
+typedef virResctrlInfoMemBW *virResctrlInfoMemBWPtr;
+
typedef struct _virResctrlAllocPerType virResctrlAllocPerType;
typedef virResctrlAllocPerType *virResctrlAllocPerTypePtr;
@@ -116,11 +119,31 @@ struct _virResctrlInfoPerLevel {
virResctrlInfoPerTypePtr *types;
};
+/* Information about memory bandwidth allocation */
+struct _virResctrlInfoMemBW {
+ /* minimum memory bandwidth allowed */
+ unsigned int min_bandwidth;
+ /* bandwidth granularity */
+ unsigned int bandwidth_granularity;
+ /* Maximum number of simultaneous allocations */
+ unsigned int max_allocation;
+ /* level number of last level cache */
+ unsigned int last_level_cache;
+ /* max id of last level cache, this is used to track
+ * how many last level cache available in host system,
+ * the number of memory bandwidth allocation controller
+ * is identical with last level cache.
+ */
The closing */ can be on the previous line.
+ unsigned int max_id;
+};
+
struct _virResctrlInfo {
virObject parent;
virResctrlInfoPerLevelPtr *levels;
size_t nlevels;
+
+ virResctrlInfoMemBWPtr membw_info;
};
@@ -146,6 +169,7 @@ virResctrlInfoDispose(void *obj)
VIR_FREE(level);
}
+ VIR_FREE(resctrl->membw_info);
VIR_FREE(resctrl->levels);
}
@@ -442,6 +466,65 @@ virResctrlGetCacheInfo(virResctrlInfoPtr resctrl, DIR *dirp)
static int
+virResctrlGetMemoryBandwidthInfo(virResctrlInfoPtr resctrl)
+{
+ int ret = -1;
+ int rv = -1;
+ virResctrlInfoMemBWPtr i_membw = NULL;
+
+ /* query memory bandwidth allocation info */
+ if (VIR_ALLOC(i_membw) < 0)
+ goto cleanup;
+ rv = virFileReadValueUint(&i_membw->bandwidth_granularity,
+ SYSFS_RESCTRL_PATH "/info/MB/bandwidth_gran");
+ if (rv == -2) {
+ /* The file doesn't exist, so it's unusable for us,
+ * properly mba unsupported */
s/ properly/probably/
?? Is that what you meant?
Yes! probably. :)
s/mba/memory bandwidth/
s/mba/memory bandwidth allocation/
right?
+ VIR_INFO("The path '" SYSFS_RESCTRL_PATH "/info/MB/bandwidth_gran'"
+ "does not exist");
+ ret = 0;
+ goto cleanup;
+ } else if (rv < 0) {
+ /* Other failures are fatal, so just quit */
+ goto cleanup;
+ }
+
+ rv = virFileReadValueUint(&i_membw->min_bandwidth,
+ SYSFS_RESCTRL_PATH "/info/MB/min_bandwidth");
+ if (rv == -2) {
+ /* If the previous file exists, so should this one. Hence -2 is
s/ Hence/ Hence/
It's a raging debate at times on this list whether to go with 1 space or
2. I think 1 space usually wins, but I use 2 many times as well. I'll
adjust this to 1 space though ;-)
Good to know this history :). Above 2 space is just following the
existing virresctrl code.
+ * fatal in this case (errors out in next condition) - the
+ * kernel interface might've changed too much or something else is
+ * wrong. */
"kernel" can fit on previous line meaning "wrong." can fit on previous
line. Also, no need for blank line between here and virReportError
Sorry I am a bit lost. Do you mean put "kernel" in previous line? right?
I have a try in Thunderbird. Looks like put “kernel” in previous line
beyond max length of line and Thunderbird give me automatically split. :(
+
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Cannot get min bandwidth from resctrl memory info"));
+ }
+ if (rv < 0)
+ goto cleanup;
+
+ rv = virFileReadValueUint(&i_membw->max_allocation,
+ SYSFS_RESCTRL_PATH "/info/MB/num_closids");
+ if (rv == -2) {
+ /* If the previous file exists, so should this one. Hence -2 is
+ * fatal in this case as well (errors out in next condition) - the
+ * kernel interface might've changed too much or something else is
+ * wrong. */
I'd just note /* Similar reasoning as min_bandwidth above */
Short and clear!
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Cannot get max allocation from resctrl memory
info"));
+ }
+ if (rv < 0)
+ goto cleanup;
+
+ VIR_STEAL_PTR(resctrl->membw_info, i_membw);
+ ret = 0;
+ cleanup:
+ VIR_FREE(i_membw);
+ return ret;
+}
+
+
+static int
virResctrlGetInfo(virResctrlInfoPtr resctrl)
{
DIR *dirp = NULL;
@@ -451,6 +534,10 @@ virResctrlGetInfo(virResctrlInfoPtr resctrl)
if (ret <= 0)
goto cleanup;
+ ret = virResctrlGetMemoryBandwidthInfo(resctrl);
+ if (ret < 0)
+ goto cleanup;
+
ret = virResctrlGetCacheInfo(resctrl, dirp);
if (ret < 0)
goto cleanup;
@@ -492,6 +579,9 @@ virResctrlInfoIsEmpty(virResctrlInfoPtr resctrl)
if (!resctrl)
return true;
+ if (resctrl->membw_info)
+ return false;
+
for (i = 0; i < resctrl->nlevels; i++) {
virResctrlInfoPerLevelPtr i_level = resctrl->levels[i];
@@ -517,12 +607,26 @@ virResctrlInfoGetCache(virResctrlInfoPtr resctrl,
{
virResctrlInfoPerLevelPtr i_level = NULL;
virResctrlInfoPerTypePtr i_type = NULL;
+ virResctrlInfoMemBWPtr membw_info = NULL;
This can be local to the if...
size_t i = 0;
int ret = -1;
if (virResctrlInfoIsEmpty(resctrl))
return 0;
+ /* Let's take the opportunity to update the number of last level
+ * cache. This number of memory bandwidth controller is same with
+ * last level cache */
+ if (resctrl->membw_info) {
virResctrlInfoMemBWPtr membw_info = resctrl->membw_info;
Yes. Should put to here. :). I remember some compile flags will complain
if local variable not at the begin of a function.
+ membw_info = resctrl->membw_info;
+ if (level > membw_info->last_level_cache) {
+ membw_info->last_level_cache = level;
+ membw_info->max_id = 0;
+ } else if (membw_info->last_level_cache == level) {
+ membw_info->max_id++;
+ }
+ }
+
This last hunk should be it's own patch. I can split it out for you.
The rest of the patch introduces the concept, does the query, and saves
the data in the "right place".
This hunk says, hey we have some membw_info data that can change our
perception of reality, so we need to adjust. Although nothing yet has
set last_level_cache or max_id... I'll assume that's comeing.
I'll also make membw_info "local" to the if {}.
The only hmm... I have is this last hunk, I've already forgotten what we
discussed the previous series. But my hmm is related to why is it here
and what impact can it (eventually) have if the values are changed in
this method while perhaps also being changed in a different method. I'm
sure I'll learn more of that as I move forward.
Thanks for that! Let me help to recall the discuss. :)
RDT kernel module will report some parameters for MBA. They are :
"min_bandwidth": The minimum memory bandwidth percentage which
user can request.
"bandwidth_gran": The granularity in which the memory bandwidth
percentage is allocated. The allocated
b/w percentage is rounded off to the next
control step available on the hardware. The
available bandwidth control steps are:
min_bandwidth + N * bandwidth_gran.
"num_closids": The number mba group can exist simultaneous.
Those information is query from kernel interface /sys/fs/resctrl/info/MB
Above hunk is used to calculate the number of memory bandwidth
allocation controllers in system. The number of memory bandwidth
allocation controllers is same as last level cache. This number is
calculate by traversing cache hierarchy of
host(/sys/bus/cpu/devices/cpuX/cache/).
So above hunk is used to collect that information, to sanitize domain
XML about memory bandwidth allocation part. And the number of memory
bandwidth allocation controllers will not change, it just cannot query
directly from RDT kernel module.
Reviewed-by: John Ferlan <jfer...@redhat.com>
John
BTW: I'm stopping here for the evening, I'll work through the rest
hopefully tomorrow depending on interruptions.
Good evening :).
The rest are about 1. allocate memory bandwidth 2. domain XML and 3.
host capability XML.
if (level >= resctrl->nlevels)
return 0;
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
