[libvirt] [PATCH 04/10] test: add test case for resctrl monitor

[Wang, Huaqiang]

> -----Original Message-----
> From: John Ferlan [mailto:jferlan at redhat.com]
> Sent: Wednesday, September 5, 2018 7:59 PM
> To: Wang, Huaqiang <huaqiang.wang at intel.com>; libvir-list at redhat.com
> Cc: Feng, Shaohe <shaohe.feng at intel.com>; Niu, Bing <bing.niu at intel.com>;
> Ding, Jian-feng <jian-feng.ding at intel.com>; Zang, Rui <rui.zang at intel.com>
> Subject: Re: [libvirt] [PATCH 04/10] test: add test case for resctrl monitor
> 
> 
> 
> On 08/27/2018 07:23 AM, Wang Huaqiang wrote:
> > Signed-off-by: Wang Huaqiang <huaqiang.wang at intel.com>
> > ---
> >  .../linux-resctrl/resctrl/info/L3_MON/max_threshold_occupancy       | 1 +
> >  .../vircaps2xmldata/linux-resctrl/resctrl/info/L3_MON/mon_features  | 3 +++
> >  tests/vircaps2xmldata/linux-resctrl/resctrl/info/L3_MON/num_rmids   | 1 +
> >  tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml                    | 6 ++++++
> >  4 files changed, 11 insertions(+)
> >  create mode 100644
> > tests/vircaps2xmldata/linux-resctrl/resctrl/info/L3_MON/max_threshold_
> > occupancy  create mode 100644
> > tests/vircaps2xmldata/linux-resctrl/resctrl/info/L3_MON/mon_features
> >  create mode 100644
> > tests/vircaps2xmldata/linux-resctrl/resctrl/info/L3_MON/num_rmids
> >
> 
> And this would be combined with part of patch2 and patch3
> 
> > diff --git
> > a/tests/vircaps2xmldata/linux-resctrl/resctrl/info/L3_MON/max_threshol
> > d_occupancy
> > b/tests/vircaps2xmldata/linux-resctrl/resctrl/info/L3_MON/max_threshol
> > d_occupancy
> > new file mode 100644
> > index 0000000..77f05e2
> > --- /dev/null
> > +++ b/tests/vircaps2xmldata/linux-resctrl/resctrl/info/L3_MON/max_thre
> > +++ shold_occupancy
> > @@ -0,0 +1 @@
> > +270336
> > diff --git
> > a/tests/vircaps2xmldata/linux-resctrl/resctrl/info/L3_MON/mon_features
> > b/tests/vircaps2xmldata/linux-resctrl/resctrl/info/L3_MON/mon_features
> > new file mode 100644
> > index 0000000..0c57b8d
> > --- /dev/null
> > +++ b/tests/vircaps2xmldata/linux-resctrl/resctrl/info/L3_MON/mon_feat
> > +++ ures
> > @@ -0,0 +1,3 @@
> > +llc_occupancy
> > +mbm_total_bytes
> > +mbm_local_bytes
> 
> Could/should this list values that aren't prefixed by "llc_" and "mbm_"
> to validate your code?

Will add some 'fake' features to the list, and make more tests.
To be done in next version patch.

> 
> There's only 1 set of data but it's printed twice - that's the reason for my
> comment in patch2 about duplication of the same data that is unnecessary.
> What if there were 10 bank id's, 100? 1000? - lots of waste.  Only 2, no big deal.
> 

    <cache>
      <bank id='0' level='3' type='both' size='55' unit='MiB' cpus='0-21,44-65'>
        <control granularity='2816' unit='KiB' type='both' maxAllocs='16'/>
        <monitor threshold='270336' unit='B' maxAllocs='176'>
          <feature name='llc_occupancy'/>
        </monitor>
      </bank>
      <bank id='1' level='3' type='both' size='55' unit='MiB' cpus='22-43,66-87'>
        <control granularity='2816' unit='KiB' type='both' maxAllocs='16'/>
        <monitor threshold='270336' unit='B' maxAllocs='176'>
          <feature name='llc_occupancy'/>
        </monitor>
      </bank>
    </cache>

Above is the cache capabilites section, dumped from my system through
'virsh capabilities' command.
This is a 2-socket E5-2699v4 CPU(22 core with CAT/CMT enabled and CDP disabled)
system, as you said, the cache monitor capability is printed more than once.

And I need to point out that the following cache 'control' element is also
printed for twice, it met the same situation you mentioned for cache monitor.
     "<control granularity='2816' unit='KiB' type='both' maxAllocs='16'/>"

After you have read my considerations(in the reply to your review of patch 2)
for reason why I used current disign, if you still think it not wise to
make it duplicated, let's find a proper place to keep the data and eliminate
such kind of duplication.
We can make any changes to make our design more reasonable in the design
stage.


Thanks for your kind revew!

BR
Huaqiang   

> John
> 
> > diff --git
> > a/tests/vircaps2xmldata/linux-resctrl/resctrl/info/L3_MON/num_rmids
> > b/tests/vircaps2xmldata/linux-resctrl/resctrl/info/L3_MON/num_rmids
> > new file mode 100644
> > index 0000000..1057e9a
> > --- /dev/null
> > +++ b/tests/vircaps2xmldata/linux-resctrl/resctrl/info/L3_MON/num_rmid
> > +++ s
> > @@ -0,0 +1 @@
> > +176
> > diff --git a/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml
> > b/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml
> > index 9b00cf0..678fdc9 100644
> > --- a/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml
> > +++ b/tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml
> > @@ -44,9 +44,15 @@
> >      <cache>
> >        <bank id='0' level='3' type='both' size='15' unit='MiB' cpus='0-5'>
> >          <control granularity='768' min='1536' unit='KiB' type='both'
> > maxAllocs='4'/>
> > +        <monitor threshold='270336' unit='B' maxAllocs='176'>
> > +          <feature name='llc_occupancy'/>
> > +        </monitor>
> >        </bank>
> >        <bank id='1' level='3' type='both' size='15' unit='MiB' cpus='6-11'>
> >          <control granularity='768' min='1536' unit='KiB' type='both'
> > maxAllocs='4'/>
> > +        <monitor threshold='270336' unit='B' maxAllocs='176'>
> > +          <feature name='llc_occupancy'/>
> > +        </monitor>
> >        </bank>
> >      </cache>
> >      <memory_bandwidth>
> >