Re: [PATCH 1/5] selftests/resctrl: Clear unused initalization code in MBM tests

Hi Shaopeng,

(typo in Subject: initalization -> initialization)

On 9/13/2022 6:51 PM, Shaopeng Tan wrote:

There is a comment "Set up shemata with 100% allocation on the first run" in function mbm_setup(), but the condition "num_of_runs == 0" will never be met and write_schemata() will never be called to set schemata to 100%.

Thanks for catching this.

Since umount/mount resctrl file system is run on each resctrl test, at the same time the default schemata will also be set to 100%.

This is the case when a test is run with struct resctrl_val_param->mum_resctrlfs == 1, but if the test is run with struct resctrl_val_param->mum_resctrlfs == 0 then resctrl filesystem will not be remounted.

I do think that this setup function should support both cases.

Clear unused initialization code in MBM test, such as CMT test.

Could the initialization code be fixed instead to increment the number of runs later, similar to cat_setup()?

Signed-off-by: Shaopeng Tan tan.shaopeng@jp.fujitsu.com

tools/testing/selftests/resctrl/mbm_test.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/tools/testing/selftests/resctrl/mbm_test.c b/tools/testing/selftests/resctrl/mbm_test.c index 8392e5c55ed0..38a3b3ad1c76 100644 --- a/tools/testing/selftests/resctrl/mbm_test.c +++ b/tools/testing/selftests/resctrl/mbm_test.c @@ -89,24 +89,19 @@ 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);

• /* Set up shemata with 100% allocation on the first run. */
• if (num_of_runs == 0)

• ret = write_schemata(p->ctrlgrp, "100", p->cpu_no,
  

• 		     p->resctrl_val);
  

• /* Run NUM_OF_RUNS times */
• if (p->num_of_runs >= NUM_OF_RUNS)

• return -1;
  

You seem to be fixing two bugs in this patch, the first is described in the commit message and the second is to use p->num_of_runs instead of the local num_of_runs. Although, after a quick look I cannot see if struct resctrl_val_param->num_of_runs is used anywhere. Could you please add description of these changes to the changelog?

• p->num_of_runs++;

• return ret;

• return 0;

} void mbm_test_cleanup(void)

Thank you

Reinette