On 2018年07月26日 06:31, John Ferlan wrote:
On 07/25/2018 12:28 AM, bing.niu wrote:
On 2018年07月25日 06:08, John Ferlan wrote:
[...]
+ * 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. :(
Yes moving kernel up a line is what I meant. It's taken care of in my
branch already.
Thanks for this!
[...]
+ 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
Right this I understand the other fields are fetch-able. Setting
last_level_cache and max_id is only ever done in virResctrlInfoGetCache.
I have to remind myself what ends up calling into here and the order of
processing for all this code.
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.
So does the following seem like a good summary for the split out patch?:
util: Add MBA check to virResctrlInfoGetCache
If we have some membw_info data, then we need to calculate the number
of MBA controllers on the system. The value cannot be obtained from a
direct query to the RDT kernel module, but it is the same as the last
level cache value which is calculated by traversing the cache hierarchy
of host(/sys/bus/cpu/devices/cpuX/cache/).
Above summary is clear and informative. :)
John
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
