[libvirt] [PATCH 15/16] tests: Add tests for caches into vircaps2xmltest

[Eli Qiao]

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


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170331/2f87d96d/attachment-0001.htm>