Re: [PATCH v2 08/13] selftests/resctrl: Add ->init() callback into resctrl_val_param

[Ilpo Järvinen]

On Wed, 13 Mar 2024, Maciej Wieczor-Retman wrote:

> On 2024-03-13 at 13:37:51 +0200, Ilpo Järvinen wrote:
> >On Wed, 13 Mar 2024, Maciej Wieczor-Retman wrote:
> >> On 2024-03-13 at 11:15:30 +0100, Maciej Wieczor-Retman wrote:
> >> >On 2024-03-11 at 15:52:25 +0200, Ilpo Järvinen wrote:
> >> >>diff --git a/tools/testing/selftests/resctrl/cmt_test.c b/tools/testing/selftests/resctrl/cmt_test.c
> >> >>index 241c0b129b58..e79eca9346f3 100644
> >> >>--- a/tools/testing/selftests/resctrl/cmt_test.c
> >> >>+++ b/tools/testing/selftests/resctrl/cmt_test.c
> >> >>@@ -16,6 +16,17 @@
> >> >> #define MAX_DIFF		2000000
> >> >> #define MAX_DIFF_PERCENT	15
> >> >> 
> >> >>+#define CON_MON_LCC_OCCUP_PATH		\
> >> >>+	"%s/%s/mon_groups/%s/mon_data/mon_L3_%02d/llc_occupancy"
> >> >>+
> >> >>+static int set_cmt_path(const struct resctrl_val_param *param, int domain_id)
> >> >>+{
> >> >>+	sprintf(llc_occup_path,	CON_MON_LCC_OCCUP_PATH,	RESCTRL_PATH,
> >> >>+		param->ctrlgrp, param->mongrp, domain_id);
> >> >>+
> >> >>+	return 0;
> >> >>+}
> >> >>+
> >> >
> >> >Is there an option to make this function (and the set_mbm_path()) global through
> >> >the resctrl.h?
> >> >
> >> >I'd like to use it in my SNC series [1] for looping over different nodes and
> >> >that requires changing the paths during the measure phase of the tests and that
> >> >part is currently in cache.c:measure_llc_resctrl().
> >> >
> >> >Or would you suggest some other way of changing these paths in cache?
> >> >
> >> 
> >> +forgot to add the link :b
> >> 
> >> [1] https://lore.kernel.org/all/cover.1709721159.git.maciej.wieczor-retman@xxxxxxxxx/
> >
> >Perhaps ->init() should just prepare an array of filenames to read from 
> >to support SNC. That would keep the filename preparations out of the 
> >measurement period.
> >
> >It feels slightly hacky to have an array of files but I cannot think of 
> >anything else that would be cleaner and would not require creating the 
> >filenames during the actual test.
> 
> So the array of names would be a part of "struct resctrl_val_param"?

This series has in independent of resctrl_val_param because resctrl_val() 
itself doesn't care after the separation of generic and per test logic. 

But that meant making it globals into the per test files which I didn't 
like that much either. So perhaps having it in resctrl_val_param is 
still better.

-- 
 i.

> It would have to be dynamically allocated too before running the tests.
> 
> I kinda agree, creating filenames during measurements messes the whole
> separation idea between setup and measuring. And as you said there are like 4
> nodes max so not much memory would be wasted there.
> 
> I can experiment with it and try to add it in my series - since it's much more
> tied to SNC. Unless you see it'd better fit here then that's fine with me too.