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

> -----Original Message----- > From: John Ferlan [mailto:jferlan@redhat.com] > Sent: Wednesday, September 5, 2018 7:59 PM > To: Wang, Huaqiang <huaqiang.wang(a)intel.com>; libvir-list(a)redhat.com > Cc: Feng, Shaohe <shaohe.feng(a)intel.com>; Niu, Bing <bing.niu(a)intel.com>; > Ding, Jian-feng <jian-feng.ding(a)intel.com>; Zang, Rui <rui.zang(a)intel.com&gt; > 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(a)intel.com&gt; >> --- >> .../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> >>