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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Thu, 14 Mar 2024 17:07:24 +0100
From: Maciej Wieczor-Retman <maciej.wieczor-retman@...el.com>
To: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
CC: <linux-kselftest@...r.kernel.org>, Reinette Chatre
	<reinette.chatre@...el.com>, Shuah Khan <shuah@...nel.org>, Babu Moger
	<babu.moger@....com>, Fenghua Yu <fenghua.yu@...el.com>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 08/13] selftests/resctrl: Add ->init() callback into
 resctrl_val_param

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@...el.com>,
>  *    Fenghua Yu <fenghua.yu@...el.com>
>  */
>+#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