> -----Original Message----- > From: John Ferlan [mailto:jferlan@xxxxxxxxxx] > Sent: Saturday, September 8, 2018 1:14 AM > To: Wang, Huaqiang <huaqiang.wang@xxxxxxxxx>; libvir-list@xxxxxxxxxx > Cc: Feng, Shaohe <shaohe.feng@xxxxxxxxx>; Niu, Bing <bing.niu@xxxxxxxxx>; > Ding, Jian-feng <jian-feng.ding@xxxxxxxxx>; Zang, Rui <rui.zang@xxxxxxxxx> > Subject: Re: [PATCH 04/10] test: add test case for resctrl monitor > > > > On 09/07/2018 05:12 AM, Wang, Huaqiang wrote: > > > > > >> -----Original Message----- > >> From: John Ferlan [mailto:jferlan@xxxxxxxxxx] > >> Sent: Wednesday, September 5, 2018 7:59 PM > >> To: Wang, Huaqiang <huaqiang.wang@xxxxxxxxx>; libvir-list@xxxxxxxxxx > >> Cc: Feng, Shaohe <shaohe.feng@xxxxxxxxx>; Niu, Bing > >> <bing.niu@xxxxxxxxx>; Ding, Jian-feng <jian-feng.ding@xxxxxxxxx>; > >> Zang, Rui <rui.zang@xxxxxxxxx> > >> Subject: Re: [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@xxxxxxxxx> > >>> --- > >>> .../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_threshol > >>> d_ > >>> 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_thresh > >>> ol > >>> d_occupancy > >>> b/tests/vircaps2xmldata/linux-resctrl/resctrl/info/L3_MON/max_thresh > >>> ol > >>> d_occupancy > >>> new file mode 100644 > >>> index 0000000..77f05e2 > >>> --- /dev/null > >>> +++ b/tests/vircaps2xmldata/linux-resctrl/resctrl/info/L3_MON/max_th > >>> +++ re > >>> +++ shold_occupancy > >>> @@ -0,0 +1 @@ > >>> +270336 > >>> diff --git > >>> a/tests/vircaps2xmldata/linux-resctrl/resctrl/info/L3_MON/mon_featur > >>> es > >>> b/tests/vircaps2xmldata/linux-resctrl/resctrl/info/L3_MON/mon_featur > >>> es > >>> new file mode 100644 > >>> index 0000000..0c57b8d > >>> --- /dev/null > >>> +++ b/tests/vircaps2xmldata/linux-resctrl/resctrl/info/L3_MON/mon_fe > >>> +++ at > >>> +++ 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'/>" > > > > I think we covered this earlier... I'm still not really clear on what <control> per > <bank> really provides other than it's a calculated value based on a few different > numbers and honestly I had a hard time following that logic all over the place. > Understood. cache <control> information might be alerted by the cache size or something else. Have proposed new cache monitor layout in previous email's update. Hope it looks better. Thanks for review. Huaqiang > John > > > 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_rm > >>> +++ id > >>> +++ 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> > >>> -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
