On Friday, 31 March 2017 at 7:19 PM, Martin Kletzander wrote:

On Fri, Mar 31, 2017 at 09:56:32AM +0800, Eli Qiao wrote:

</cells>
Okay, cool, this comes better than my patches and have some differences.
I am open with this as long as that it can meet cache allocation requires and
everyone will be happy.

I am ++ for this.

But I am not sure expose all of cache information in the capabilities XML.

??? Are you not sure we "should expose" all of cache information? Or
are you afraid we're not exposing enough information?

Well, I was saying not to expose all of the information, but after your reply, it's okay for me.

 
</topology>
+ <cache>
+ <bank id='0' level='3' type='unified' size='8192' unit='KiB' cpus='0-7'/>

eg: if enabled CDP feature on the host, what the type of level=3 cache should be like?

This has nothing to do with resctrl yet. I'm just exposing the caches
that exist on the host.

+ <bank id='0' level='2' type='unified' size='256' unit='KiB' cpus='0-1'/>
for the bank id, it’s per cache level unique right (data/instruction shares same id)?

It looks like it's per cache level/type unique. But it's precisely just
what the kernel exposes to us. I'm not doing anything on top of that.

+ <bank id='0' level='1' type='instruction' size='32' unit='KiB' cpus='0-1'/>
+ <bank id='0' level='1' type='data' size='32' unit='KiB' cpus='0-1'/>
+ <bank id='1' level='2' type='unified' size='256' unit='KiB' cpus='2-3'/>
+ <bank id='1' level='1' type='instruction' size='32' unit='KiB' cpus='2-3'/>
+ <bank id='1' level='1' type='data' size='32' unit='KiB' cpus='2-3'/>
+ <bank id='2' level='2' type='unified' size='256' unit='KiB' cpus='4-5'/>
+ <bank id='2' level='1' type='instruction' size='32' unit='KiB' cpus='4-5'/>
+ <bank id='2' level='1' type='data' size='32' unit='KiB' cpus='4-5'/>
+ <bank id='3' level='2' type='unified' size='256' unit='KiB' cpus='6-7'/>
+ <bank id='3' level='1' type='instruction' size='32' unit='KiB' cpus='6-7'/>
+ <bank id='3' level='1' type='data' size='32' unit='KiB' cpus='6-7'/>
+ </cache>


This’s really good that you have work this out by expose all these out to capabilities,
and it will be much easy to let resctrl keep focus on cache allocation.

So if util/virresctrl.c would like to access some cache abilities, it will first get virCapsPtr.host.caches,
right?

Well yeah, it'll probably extend the CacheBank struct.
+1 

but I am not sure if that’s be okay to expose all cache information which we can not
do the allocation yet.

What's the harm?


think about I have a host has 2 Socket 22 core and 2 thread per core, I will have 88 cpus

so the cache bank will be a large list 

2 for l3 , 44 for l2, 44 for l1d and 44 for l1c, not all of them are useful to users, and the capabilities XML are boring.

Even though I am Okay to expose all of them.
 
I remember that Daniel has some comments for this in the RFC to not expose all caches of the host if no cache allocation support yet.

Any way, no harm.


How can a user/admin to know from capabilities?

Easily. The XML you see above just says what cache is on the host. If
any of the banks are allocatable, then it will have a sub-element. Is
there any problem with that?
No problem, +1 to extend a sub-element .

Any suggestions for what’s it should be? 

How about for l3: 
<control min="2816" avail=“56320” cbm_len=“20” scope=‘both’ reserved=“2816"/>

</host>

</capabilities>
diff --git a/tests/vircaps2xmltest.c b/tests/vircaps2xmltest.c
index ffbe9a783811..dda0757766a8 100644
--- a/tests/vircaps2xmltest.c
+++ b/tests/vircaps2xmltest.c
@@ -58,7 +58,8 @@ test_virCapabilities(const void *opaque)
if (!caps)
goto cleanup;

- if (virCapabilitiesInitNUMA(caps) < 0)
+ if (virCapabilitiesInitNUMA(caps) < 0 ||
+ virCapabilitiesInitCaches(caps) < 0)
goto cleanup;

virSysfsSetSystemPath(NULL);
--
2.12.2

--
libvir-list mailing list
libvir-list@redhat.com (mailto:libvir-list@redhat.com)