| Message ID | 20221101094341.3383073-2-tan.shaopeng@jp.fujitsu.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Some improvements of resctrl selftest | expand |
Hi Shaopeng, On 11/1/2022 2:43 AM, Shaopeng Tan wrote: > There is a comment "Set up shemata with 100% allocation on the first run" > in function mbm_setup(), but there is an increment bug and the condition > "num_of_runs == 0" will never be met and write_schemata() will never be > called to set schemata to 100%. Even if write_schemata() is called in MBM > test, since it is not supported for MBM test it does not set the schemata. > This is currently fine because resctrl_val_parm->mum_resctrlfs is always 1 > and umount/mount will be run in each test to set the schemata to 100%. > > To support the usage when MBM test does not unmount/remount resctrl > filesystem before the test starts, fix to call write_schemata() and > set schemata properly when the function is called for the first time. > > Also, remove static local variable 'num_of_runs' because this is not > needed as there is resctrl_val_param->num_of_runs which should be used > instead like in cat_setup(). > > Signed-off-by: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com> Thank you very much. Reviewed-by: Reinette Chatre <reinette.chatre@intel.com> Reinette
diff --git a/tools/testing/selftests/resctrl/mbm_test.c b/tools/testing/selftests/resctrl/mbm_test.c index 8392e5c55ed0..6d550f012829 100644 --- a/tools/testing/selftests/resctrl/mbm_test.c +++ b/tools/testing/selftests/resctrl/mbm_test.c @@ -89,23 +89,24 @@ static int check_results(int span) static int mbm_setup(int num, ...) { struct resctrl_val_param *p; - static int num_of_runs; va_list param; int ret = 0; - /* Run NUM_OF_RUNS times */ - if (num_of_runs++ >= NUM_OF_RUNS) - return -1; - va_start(param, num); p = va_arg(param, struct resctrl_val_param *); va_end(param); + /* Run NUM_OF_RUNS times */ + if (p->num_of_runs >= NUM_OF_RUNS) + return -1; + /* Set up shemata with 100% allocation on the first run. */ - if (num_of_runs == 0) + if (p->num_of_runs == 0) ret = write_schemata(p->ctrlgrp, "100", p->cpu_no, p->resctrl_val); + p->num_of_runs++; + return ret; } diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c index 6f543e470ad4..8546bc9f1786 100644 --- a/tools/testing/selftests/resctrl/resctrlfs.c +++ b/tools/testing/selftests/resctrl/resctrlfs.c @@ -498,6 +498,7 @@ int write_schemata(char *ctrlgrp, char *schemata, int cpu_no, char *resctrl_val) FILE *fp; if (strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR)) && + strncmp(resctrl_val, MBM_STR, sizeof(MBM_STR)) && strncmp(resctrl_val, CAT_STR, sizeof(CAT_STR)) && strncmp(resctrl_val, CMT_STR, sizeof(CMT_STR))) return -ENOENT; @@ -523,7 +524,8 @@ int write_schemata(char *ctrlgrp, char *schemata, int cpu_no, char *resctrl_val) if (!strncmp(resctrl_val, CAT_STR, sizeof(CAT_STR)) || !strncmp(resctrl_val, CMT_STR, sizeof(CMT_STR))) sprintf(schema, "%s%d%c%s", "L3:", resource_id, '=', schemata); - if (!strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR))) + if (!strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR)) || + !strncmp(resctrl_val, MBM_STR, sizeof(MBM_STR))) sprintf(schema, "%s%d%c%s", "MB:", resource_id, '=', schemata); fp = fopen(controlgroup, "w");
There is a comment "Set up shemata with 100% allocation on the first run" in function mbm_setup(), but there is an increment bug and the condition "num_of_runs == 0" will never be met and write_schemata() will never be called to set schemata to 100%. Even if write_schemata() is called in MBM test, since it is not supported for MBM test it does not set the schemata. This is currently fine because resctrl_val_parm->mum_resctrlfs is always 1 and umount/mount will be run in each test to set the schemata to 100%. To support the usage when MBM test does not unmount/remount resctrl filesystem before the test starts, fix to call write_schemata() and set schemata properly when the function is called for the first time. Also, remove static local variable 'num_of_runs' because this is not needed as there is resctrl_val_param->num_of_runs which should be used instead like in cat_setup(). Signed-off-by: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com> --- tools/testing/selftests/resctrl/mbm_test.c | 13 +++++++------ tools/testing/selftests/resctrl/resctrlfs.c | 4 +++- 2 files changed, 10 insertions(+), 7 deletions(-)