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) > > > https://www.redhat.com/mailman/listinfo/libvir-list > > > > > > > >
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list