Hi Reinette,
> -----Original Message-----
> From: Reinette Chatre <reinette.chatre@xxxxxxxxx>
> Sent: Wednesday, May 20, 2020 4:46 PM
> To: Prakhya, Sai Praneeth <sai.praneeth.prakhya@xxxxxxxxx>;
> shuah@xxxxxxxxxx; skhan@xxxxxxxxxxxxxxxxxxx; linux-kselftest@xxxxxxxxxxxxxxx
> Cc: tglx@xxxxxxxxxxxxx; mingo@xxxxxxxxxx; bp@xxxxxxxxx; Luck, Tony
> <tony.luck@xxxxxxxxx>; babu.moger@xxxxxxx; james.morse@xxxxxxx;
> Shankar, Ravi V <ravi.v.shankar@xxxxxxxxx>; Yu, Fenghua
> <fenghua.yu@xxxxxxxxx>; x86@xxxxxxxxxx; linux-kernel@vger.kernel;
> dan.carpenter@xxxxxxxxxx; dcb314@xxxxxxxxxxx
> 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
