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: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Tested-by: Babu Moger <babu.moger@amd.com>
Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>

diff --git a/tools/testing/selftests/resctrl/mba_test.c b/tools/testing/selftests/resctrl/mba_test.c
index 20427bdb5c6d9a6d9cc768bf657192c6bc06bac7..5538a0032c0e7e8c17b2f8be9791a0e39f5bf298 100644 (file)
--- a/tools/testing/selftests/resctrl/mba_test.c
+++ b/tools/testing/selftests/resctrl/mba_test.c
@@ -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,

diff --git a/tools/testing/selftests/resctrl/mbm_test.c b/tools/testing/selftests/resctrl/mbm_test.c
index adc205bf9e51ad8bf8710d12dd2ade50bc5a5791..40eca4a9eec9bda591b054fc97fa976d628764dc 100644 (file)
--- a/tools/testing/selftests/resctrl/mbm_test.c
+++ b/tools/testing/selftests/resctrl/mbm_test.c
@@ -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,

diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h
index d3fbb957309d694bdb4fb7e05a3389f1d75c3551..4446a0e493ef1b9c85d245d2298bfc0abd03e335 100644 (file)
--- a/tools/testing/selftests/resctrl/resctrl.h
+++ b/tools/testing/selftests/resctrl/resctrl.h
@@ -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,

diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c
index ecbf46c4f3ea0c596d320a3ba7e7d397b5bf1b17..0a0abc860a7f129df7b9d63fc116073423c0ccf1 100644 (file)
--- a/tools/testing/selftests/resctrl/resctrl_val.c
+++ b/tools/testing/selftests/resctrl/resctrl_val.c
@@ -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

diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c
index 917d677adbba4ab0b61494edc0bc618af7f3a2f1..9e4cda154d66df9569b9a67789f1c2a5fffdadc9 100644 (file)
--- a/tools/testing/selftests/resctrl/resctrlfs.c
+++ b/tools/testing/selftests/resctrl/resctrlfs.c
@@ -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,