Commit fa1116d0 authored by Ilpo Järvinen's avatar Ilpo Järvinen Committed by Shuah Khan
Browse files

selftests/resctrl: Simplify bandwidth report type handling 



bw_report is only needed for selecting the correct value from the
values IMC measured. It is a member in the resctrl_val_param struct and
is always set to "reads". The value is then checked in resctrl_val()
using validate_bw_report_request() that besides validating the input,
assumes it can mutate the string which is questionable programming
practice.

Simplify handling bw_report:

- Convert validate_bw_report_request() into get_bw_report_type() that
  inputs and returns const char *. Use NULL to indicate error.

- Validate the report types inside measure_mem_bw(), not in
  resctrl_val().

- Pass bw_report to measure_mem_bw() from ->measure() hook because
  resctrl_val() no longer needs bw_report for anything.

Signed-off-by: default avatarIlpo Järvinen <ilpo.jarvinen@linux.intel.com>
Tested-by: default avatarBabu Moger <babu.moger@amd.com>
Reviewed-by: default avatarReinette Chatre <reinette.chatre@intel.com>
Signed-off-by: default avatarShuah Khan <skhan@linuxfoundation.org>
parent aef5efa6

tools/testing/selftests/resctrl/mba_test.c

+1 −2
Original line number Diff line number Diff line
@@ -67,7 +67,7 @@ static int mba_setup(const struct resctrl_test *test, 
static int mba_measure(const struct user_params *uparams,

		       struct resctrl_val_param *param, pid_t bm_pid)

{

	return measure_mem_bw(uparams, param, bm_pid);

	return measure_mem_bw(uparams, param, bm_pid, "reads");

}



static bool show_mba_info(unsigned long *bw_imc, unsigned long *bw_resc)
@@ -168,7 +168,6 @@ static int mba_run_test(const struct resctrl_test *test, const struct user_param 
		.ctrlgrp	= "c1",

		.mongrp		= "m1",

		.filename	= RESULT_FILE_NAME,

		.bw_report	= "reads",

		.init		= mba_init,

		.setup		= mba_setup,

		.measure	= mba_measure,

tools/testing/selftests/resctrl/mbm_test.c

+1 −2
Original line number Diff line number Diff line
@@ -121,7 +121,7 @@ static int mbm_setup(const struct resctrl_test *test, 
static int mbm_measure(const struct user_params *uparams,

		       struct resctrl_val_param *param, pid_t bm_pid)

{

	return measure_mem_bw(uparams, param, bm_pid);

	return measure_mem_bw(uparams, param, bm_pid, "reads");

}



static void mbm_test_cleanup(void)
@@ -135,7 +135,6 @@ static int mbm_run_test(const struct resctrl_test *test, const struct user_param 
		.resctrl_val	= MBM_STR,

		.ctrlgrp	= "c1",

		.filename	= RESULT_FILE_NAME,

		.bw_report	= "reads",

		.init		= mbm_init,

		.setup		= mbm_setup,

		.measure	= mbm_measure,

tools/testing/selftests/resctrl/resctrl.h

+3 −4
Original line number Diff line number Diff line
@@ -85,7 +85,6 @@ struct resctrl_test { 
 * @ctrlgrp:		Name of the control monitor group (con_mon grp)

 * @mongrp:		Name of the monitor group (mon grp)

 * @filename:		Name of file to which the o/p should be written

 * @bw_report:		Bandwidth report type (reads vs writes)

 * @init:		Callback function to initialize test environment

 * @setup:		Callback function to setup per test run environment

 * @measure:		Callback that performs the measurement (a single test)
@@ -95,7 +94,6 @@ struct resctrl_val_param { 
	char		ctrlgrp[64];

	char		mongrp[64];

	char		filename[64];

	char		*bw_report;

	unsigned long	mask;

	int		num_of_runs;

	int		(*init)(const struct resctrl_val_param *param,
@@ -135,7 +133,7 @@ int filter_dmesg(void); 
int get_domain_id(const char *resource, int cpu_no, int *domain_id);

int mount_resctrlfs(void);

int umount_resctrlfs(void);

int validate_bw_report_request(char *bw_report);

const char *get_bw_report_type(const char *bw_report);

bool resctrl_resource_exists(const char *resource);

bool resctrl_mon_feature_exists(const char *resource, const char *feature);

bool resource_info_file_exists(const char *resource, const char *file);
@@ -154,7 +152,8 @@ void fill_cache_read(unsigned char *buf, size_t buf_size, bool once); 
int run_fill_buf(size_t buf_size, int memflush, int op, bool once);

int initialize_mem_bw_imc(void);

int measure_mem_bw(const struct user_params *uparams,

		   struct resctrl_val_param *param, pid_t bm_pid);

		   struct resctrl_val_param *param, pid_t bm_pid,

		   const char *bw_report);

void initialize_mem_bw_resctrl(const struct resctrl_val_param *param,

			       int domain_id);

int resctrl_val(const struct resctrl_test *test,

tools/testing/selftests/resctrl/resctrl_val.c

+9 −10
Original line number Diff line number Diff line
@@ -349,7 +349,7 @@ static void do_imc_mem_bw_test(void) 
 *

 * Return: = 0 on success. < 0 on failure.

 */

static int get_mem_bw_imc(char *bw_report, float *bw_imc)

static int get_mem_bw_imc(const char *bw_report, float *bw_imc)

{

	float reads, writes, of_mul_read, of_mul_write;

	int imc;
@@ -556,6 +556,7 @@ static int print_results_bw(char *filename, pid_t bm_pid, float bw_imc, 
 * @uparams:		User supplied parameters

 * @param:		Parameters passed to resctrl_val()

 * @bm_pid:		PID that runs the benchmark

 * @bw_report:		Bandwidth report type (reads, writes)

 *

 * Measure memory bandwidth from resctrl and from another source which is

 * perf imc value or could be something else if perf imc event is not
@@ -563,13 +564,18 @@ static int print_results_bw(char *filename, pid_t bm_pid, float bw_imc, 
 * 1 sec to measure the data.

 */

int measure_mem_bw(const struct user_params *uparams,

		   struct resctrl_val_param *param, pid_t bm_pid)

		   struct resctrl_val_param *param, pid_t bm_pid,

		   const char *bw_report)

{

	unsigned long bw_resc, bw_resc_start, bw_resc_end;

	FILE *mem_bw_fp;

	float bw_imc;

	int ret;



	bw_report = get_bw_report_type(bw_report);

	if (!bw_report)

		return -1;



	mem_bw_fp = open_mem_bw_resctrl(mbm_total_path);

	if (!mem_bw_fp)

		return -1;
@@ -590,7 +596,7 @@ int measure_mem_bw(const struct user_params *uparams, 
	if (ret < 0)

		goto close_imc;



	ret = get_mem_bw_imc(param->bw_report, &bw_imc);

	ret = get_mem_bw_imc(bw_report, &bw_imc);

	if (ret < 0)

		goto close_imc;
@@ -694,13 +700,6 @@ int resctrl_val(const struct resctrl_test *test, 
		return ret;

	}



	if (!strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR)) ||

	    !strncmp(resctrl_val, MBM_STR, sizeof(MBM_STR))) {

		ret = validate_bw_report_request(param->bw_report);

		if (ret)

			return ret;

	}



	/*

	 * If benchmark wasn't successfully started by child, then child should

	 * kill parent, so save parent's pid

tools/testing/selftests/resctrl/resctrlfs.c

+6 −7
Original line number Diff line number Diff line
@@ -837,22 +837,21 @@ int filter_dmesg(void) 
	return 0;

}



int validate_bw_report_request(char *bw_report)

const char *get_bw_report_type(const char *bw_report)

{

	if (strcmp(bw_report, "reads") == 0)

		return 0;

		return bw_report;

	if (strcmp(bw_report, "writes") == 0)

		return 0;

		return bw_report;

	if (strcmp(bw_report, "nt-writes") == 0) {

		strcpy(bw_report, "writes");

		return 0;

		return "writes";

	}

	if (strcmp(bw_report, "total") == 0)

		return 0;

		return bw_report;



	fprintf(stderr, "Requested iMC bandwidth report type unavailable\n");



	return -1;

	return NULL;

}



int perf_event_open(struct perf_event_attr *hw_event, pid_t pid, int cpu,