RE: [PATCH V2 14/19] selftests/resctrl: Skip the test if requested resctrl feature is not supported

Hi Reinette,

-----Original Message----- From: Reinette Chatre reinette.chatre@intel.com Sent: Wednesday, May 20, 2020 4:46 PM To: Prakhya, Sai Praneeth sai.praneeth.prakhya@intel.com; shuah@kernel.org; skhan@linuxfoundation.org; linux-kselftest@vger.kernel.org Cc: tglx@linutronix.de; mingo@redhat.com; bp@alien8.de; Luck, Tony tony.luck@intel.com; babu.moger@amd.com; james.morse@arm.com; Shankar, Ravi V ravi.v.shankar@intel.com; Yu, Fenghua fenghua.yu@intel.com; x86@kernel.org; linux-kernel@vger.kernel; dan.carpenter@oracle.com; dcb314@hotmail.com Subject: Re: [PATCH V2 14/19] selftests/resctrl: Skip the test if requested resctrl feature is not supported

Hi Sai,

[SNIP]

...

diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c index fb7703413be7..d45ae004ed77 100644 --- a/tools/testing/selftests/resctrl/resctrl_tests.c +++ b/tools/testing/selftests/resctrl/resctrl_tests.c @@ -170,6 +170,10 @@ int main(int argc, char **argv)

if (!is_amd && mbm_test) { printf("# Starting MBM BW change ...\n");

• if (!validate_resctrl_feature_request("mbm")) {
  

• 	printf("ok MBM # SKIP Hardware does not support
  

MBM or MBM is disabled\n");

...

• 	goto test_mba;
  

• }
  

  if (!has_ben) sprintf(benchmark_cmd[5], "%s", "mba"); res = mbm_bw_change(span, cpu_no, bw_report,

benchmark_cmd); @@

...

-178,8 +182,13 @@ int main(int argc, char **argv) tests_run++; }

+test_mba:

I think this particular usage of goto could make the flow of the code harder to trace. Could the tests perhaps be moved to functions to avoid needing to jump like this? Perhaps there could be a new function per test, like run_mbm_test(), run_mba_test(), etc. with each test called when requested by user and with the test exiting cleanly if feature is not supported by the hardware.

Makes sense. I will change it as suggested.

Regards, Sai