On Tue, 19 Mar 2024, Reinette Chatre wrote:
On 3/11/2024 6:52 AM, Ilpo Järvinen wrote:
The struct resctrl_val_param is there to customize behavior inside resctrl_val() which is currently not used to full extent and there are number of strcmp()s for test name in resctrl_val done by resctrl_val().
Create ->init() hook into the struct resctrl_val_param to cleanly do per test initialization.
Remove also unused branches to setup paths and the related #defines.
Signed-off-by: Ilpo Järvinen ilpo.jarvinen@linux.intel.com
tools/testing/selftests/resctrl/cmt_test.c | 12 ++ tools/testing/selftests/resctrl/mba_test.c | 24 +++- tools/testing/selftests/resctrl/mbm_test.c | 24 +++- tools/testing/selftests/resctrl/resctrl.h | 9 +- tools/testing/selftests/resctrl/resctrl_val.c | 123 ++---------------- 5 files changed, 75 insertions(+), 117 deletions(-)
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,
Strangely some tab characters sneaked in above.
Ah, fixed it now. They seemed to came directly from the old code.
@@ -826,17 +729,11 @@ int resctrl_val(const struct resctrl_test *test, if (ret) goto out;
- if (!strncmp(resctrl_val, MBM_STR, sizeof(MBM_STR)) ||
!strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR))) {ret = initialize_mem_bw_imc();
- if (param->init) {
ret = param->init(param, domain_id);
initialize_mem_bw_resctrl(param->ctrlgrp, param->mongrp,domain_id, resctrl_val);- } else if (!strncmp(resctrl_val, CMT_STR, sizeof(CMT_STR)))
initialize_llc_occu_resctrl(param->ctrlgrp, param->mongrp,domain_id, resctrl_val);
- }
/* Parent waits for child to be ready. */ close(pipefd[1]);
I am trying to make sense of what these tests are trying to do and it is starting to look like the monitor groups (as they are used here) are unnecessary.
Looking at resctrl_val()->write_bm_pid_to_resctrl() I see that the control group is created with "bm_pid" written to its "tasks" file and then a monitor group is created as child of the control group with the same "bm_pid" written to its "tasks" file. This looks unnecessary to me because when the original control group is created on a system that supports monitoring then that control group would automatically be a monitoring group also. In the resctrl kernel document these different groups are termed "CTRL_MON" and "MON" groups respectively.
For example, if user creates control group "c1": # mkdir /sys/fs/resctrl/c1 Then, automatically it would also be a monitoring group with its monitoring data available from /sys/fs/resctrl/c1/mon_data/mon_L3_XX/*
PIDs written to /sys/fs/resctrl/c1/tasks will have allocations assigned in /sys/fs/resctrl/c1/schemata and monitoring data /sys/fd/resctrl/c1/mon_data/mon_L3_XX/* ... it is not necessary to create a separate monitoring group to collect that monitoring data.
This seems to be the discrepancy between the MBA and MBM test ... if I understand correctly they end up needing the same data but the one uses the data from the CTRL_MON group while the other creates a redundant child MON group to read the same data that is available from the CTRL_MON group.
Okay, I can look into this too. I've not looked too deeply why the difference between MBA and MBM test is there.