On Mon, Jun 05, 2017 at 06:02:25PM +0800, Eli Qiao wrote:
This patch adds cache control step size under control tag. step is calculated by total_cache_size / cbm_length. This will be needed while later doing cache allocation to calculate cbm bits. Besides, count host's cbm length by counting bit instead of checking cbm_mask length, along with test case updated.
I would rather add another test for that.
Signed-off-by: Eli Qiao <qiaoliyong@xxxxxxxxx>
---
docs/schemas/capability.rng | 3 +++
src/conf/capabilities.c | 19 ++++++++++++++++---
src/conf/capabilities.h | 1 +
.../linux-resctrl/resctrl/info/L3/cbm_mask | 2 +-
.../linux-resctrl/resctrl/info/L3/min_cbm_bits | 2 +-
.../linux-resctrl/resctrl/info/L3/num_closids | 2 +-
tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml | 8 ++++----
tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml | 4 ++--
8 files changed, 29 insertions(+), 12 deletions(-)
diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng
index e5cbfa3..6be7ac9 100644
--- a/docs/schemas/capability.rng
+++ b/docs/schemas/capability.rng
@@ -276,6 +276,9 @@
<attribute name='min'>
<ref name='unsignedInt'/>
</attribute>
+ <attribute name='step'>
+ <ref name='unsignedInt'/>
+ </attribute>
<attribute name='unit'>
<ref name='unit'/>
</attribute>
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 3becc7e..d1266e6 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -908,9 +908,10 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
for (j = 0; j < bank->ncontrols; j++) {
bool min_kilos = !(bank->controls[j]->min % 1024);
virBufferAsprintf(&controlBuf,
- "<control min='%llu' unit='%s' "
+ "<control min='%llu' step='%llu' unit='%s' "
"type='%s' maxAllocs='%u'/>\n",
bank->controls[j]->min >> (min_kilos * 10),
+ bank->controls[j]->step >> (min_kilos * 10),
min_kilos ? "KiB" : "B",
virCacheTypeToString(bank->controls[j]->scope),
bank->controls[j]->max_allocation);
Most of the time the step will be the same as the minimum, at that point it doesn't make much sense reporting both. Also, you need to check that both of them are divisible by 2^10, otherwise you might not get exact results for one of them (e.g. minimum is divisible, but step is not)
@@ -1602,6 +1603,8 @@ virCapabilitiesGetCacheControl(virCapsHostCacheBankPtr bank,
char *cbm_mask = NULL;
char *type_upper = NULL;
unsigned int min_cbm_bits = 0;
+ unsigned int cbm_mask_val = 0;
+ unsigned int cbm_mask_len = 0;
virCapsHostCacheControlPtr control;
if (VIR_ALLOC(control) < 0)
@@ -1632,8 +1635,18 @@ virCapabilitiesGetCacheControl(virCapsHostCacheBankPtr bank,
virStringTrimOptionalNewline(cbm_mask);
- /* cbm_mask: cache bit mask, it's in hex, eg: fffff */
- control->min = min_cbm_bits * bank->size / (strlen(cbm_mask) * 4);
+ /* cbm_mask: cache bit mask, it's in hex, e.g. fffff or 7ff */
+ if (virStrToLong_uip(cbm_mask, NULL, 16, &cbm_mask_val) < 0)
+ goto cleanup;
+
The hex numbers get bigger fast. I feel like the unsigned int is not the right choice for this.
+ while (cbm_mask_val & 0x1) {
Also counting bit by bit is very tiresome.
+ cbm_mask_val = cbm_mask_val >> 1; + cbm_mask_len ++;
Extra whitespace.
+ }
+
+ control->step = bank->size / cbm_mask_len;
+
+ control->min = min_cbm_bits * control->step;
control->scope = scope;
diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h
index ee87d59..2cc9bdc 100644
--- a/src/conf/capabilities.h
+++ b/src/conf/capabilities.h
@@ -152,6 +152,7 @@ typedef struct _virCapsHostCacheControl virCapsHostCacheControl;
typedef virCapsHostCacheControl *virCapsHostCacheControlPtr;
struct _virCapsHostCacheControl {
unsigned long long min; /* minimum cache control size in B */
+ unsigned long long step; /* cache control step size in B */
virCacheType scope; /* data, code or both */
unsigned int max_allocation; /* max number of supported allocations */
};
diff --git a/tests/vircaps2xmldata/linux-resctrl/resctrl/info/L3/cbm_mask b/tests/vircaps2xmldata/linux-resctrl/resctrl/info/L3/cbm_mask
index 78031da..d482bbb 100644
--- a/tests/vircaps2xmldata/linux-resctrl/resctrl/info/L3/cbm_mask
+++ b/tests/vircaps2xmldata/linux-resctrl/resctrl/info/L3/cbm_mask
@@ -1 +1 @@
-fffff
+7ff
diff --git a/tests/vircaps2xmldata/linux-resctrl/resctrl/info/L3/min_cbm_bits b/tests/vircaps2xmldata/linux-resctrl/resctrl/info/L3/min_cbm_bits
index 0cfbf08..d00491f 100644
--- a/tests/vircaps2xmldata/linux-resctrl/resctrl/info/L3/min_cbm_bits
+++ b/tests/vircaps2xmldata/linux-resctrl/resctrl/info/L3/min_cbm_bits
@@ -1 +1 @@
-2
+1
The reason for introducing new test was that with this ^^
diff --git a/tests/vircaps2xmldata/linux-resctrl/resctrl/info/L3/num_closids b/tests/vircaps2xmldata/linux-resctrl/resctrl/info/L3/num_closids
index b8626c4..b6a7d89 100644
--- a/tests/vircaps2xmldata/linux-resctrl/resctrl/info/L3/num_closids
+++ b/tests/vircaps2xmldata/linux-resctrl/resctrl/info/L3/num_closids
@@ -1 +1 @@
-4
+16
diff --git a/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml b/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml
index 49aa0b9..98893b6 100644
--- a/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml
+++ b/tests/vircaps2xmldata/vircaps-x86_64-resctrl-cdp.xml
@@ -42,12 +42,12 @@
</topology>
<cache>
<bank id='0' level='3' type='both' size='15360' unit='KiB' cpus='0-5'>
- <control min='768' unit='KiB' type='code' maxAllocs='8'/>
- <control min='768' unit='KiB' type='data' maxAllocs='8'/>
+ <control min='768' step='768' unit='KiB' type='code' maxAllocs='8'/>
+ <control min='768' step='768' unit='KiB' type='data' maxAllocs='8'/>
</bank>
<bank id='1' level='3' type='both' size='15360' unit='KiB' cpus='6-11'>
- <control min='768' unit='KiB' type='code' maxAllocs='8'/>
- <control min='768' unit='KiB' type='data' maxAllocs='8'/>
+ <control min='768' step='768' unit='KiB' type='code' maxAllocs='8'/>
+ <control min='768' step='768' unit='KiB' type='data' maxAllocs='8'/>
</bank>
</cache>
</host>
diff --git a/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml b/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml
index cb78b4a..b54f08e 100644
--- a/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml
+++ b/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml
@@ -42,10 +42,10 @@
</topology>
<cache>
<bank id='0' level='3' type='both' size='15360' unit='KiB' cpus='0-5'>
- <control min='1536' unit='KiB' type='both' maxAllocs='4'/>
+ <control min='1429876' step='1429876' unit='B' type='both' maxAllocs='16'/>
</bank>
<bank id='1' level='3' type='both' size='15360' unit='KiB' cpus='6-11'>
- <control min='1536' unit='KiB' type='both' maxAllocs='4'/>
+ <control min='1429876' step='1429876' unit='B' type='both' maxAllocs='16'/>
</bank>
</cache>
</host>
You are not actually checking that those two values are different in any of the tests. I cooked up a patch in the meantime as well (although it took me a while because I had to fix gnulib, too), so there's another idea for the same thing, let me know what you think about it as well, please: https://www.redhat.com/archives/libvir-list/2017-June/msg00229.html Have a nice day, Martin
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
