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

["Prakhya, Sai Praneeth" <sai.praneeth.prakhya@intel.com>]

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