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

[Maciej Wieczor-Retman]

Hi again :)

On 2024-03-11 at 15:52:25 +0200, Ilpo Järvinen wrote:
>diff --git a/tools/testing/selftests/resctrl/mbm_test.c b/tools/testing/selftests/resctrl/mbm_test.c
>index 17398cd3aace..ffbfcecf9bd6 100644
>--- a/tools/testing/selftests/resctrl/mbm_test.c
>+++ b/tools/testing/selftests/resctrl/mbm_test.c
>@@ -8,12 +8,19 @@
>  *    Sai Praneeth Prakhya <sai.praneeth.prakhya@xxxxxxxxx>,
>  *    Fenghua Yu <fenghua.yu@xxxxxxxxx>
>  */
>+#include <limits.h>
>+
> #include "resctrl.h"
> 
> #define RESULT_FILE_NAME	"result_mbm"
> #define MAX_DIFF_PERCENT	8
> #define NUM_OF_RUNS		5
> 
>+#define CON_MON_MBM_LOCAL_BYTES_PATH \
>+	"%s/%s/mon_groups/%s/mon_data/mon_L3_%02d/mbm_local_bytes"
>+
>+static char mbm_total_path[PATH_MAX];
>+
> static int
> show_bw_info(unsigned long *bw_imc, unsigned long *bw_resc, size_t span)
> {
>@@ -86,6 +93,20 @@ static int check_results(size_t span)
> 	return ret;
> }
> 
>+static int set_mbm_path(const struct resctrl_val_param *param, int domain_id)
>+{
>+	int ret;
>+
>+	ret = initialize_mem_bw_imc();

I just noticed this. Since there is not only path stuff here but also some imc
logic maybe the function names could be changed? Something like

	set_mbm_path -> init_mbm

The same could apply for all these init functions or at least the mba one.

>+	if (ret)
>+		return ret;
>+
>+	sprintf(mbm_total_path, CON_MON_MBM_LOCAL_BYTES_PATH,
>+		RESCTRL_PATH, param->ctrlgrp, param->mongrp, domain_id);
>+
>+	return 0;
>+}
>+
>

-- 
Kind regards
Maciej Wieczór-Retman