| Message ID | 20240520123020.18938-15-ilpo.jarvinen@linux.intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | selftests/resctrl: resctrl_val() related cleanups & improvements | expand |
Hi Ilpo, On 5/20/24 5:30 AM, Ilpo Järvinen wrote: > Nothing during MBA test uses mongrp even if it has been defined ever > since the introduction of the MBA test in the commit 01fee6b4d1f9 > ("selftests/resctrl: Add MBA test"). > > Remove the mongrp from MBA test. > > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> > --- > tools/testing/selftests/resctrl/mba_test.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/tools/testing/selftests/resctrl/mba_test.c b/tools/testing/selftests/resctrl/mba_test.c > index 9c9a4f22e529..5e0b1e794295 100644 > --- a/tools/testing/selftests/resctrl/mba_test.c > +++ b/tools/testing/selftests/resctrl/mba_test.c > @@ -166,7 +166,6 @@ static int mba_run_test(const struct resctrl_test *test, const struct user_param > struct resctrl_val_param param = { > .resctrl_val = MBA_STR, > .ctrlgrp = "c1", > - .mongrp = "m1", > .filename = RESULT_FILE_NAME, > .init = mba_init, > .setup = mba_setup, This may explain the unexpected checks that are removed in final patch? Reviewed-by: Reinette Chatre <reinette.chatre@intel.com> Reinette
On Wed, 29 May 2024, Reinette Chatre wrote: > On 5/20/24 5:30 AM, Ilpo Järvinen wrote: > > Nothing during MBA test uses mongrp even if it has been defined ever > > since the introduction of the MBA test in the commit 01fee6b4d1f9 > > ("selftests/resctrl: Add MBA test"). > > > > Remove the mongrp from MBA test. > > > > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> > > --- > > tools/testing/selftests/resctrl/mba_test.c | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/tools/testing/selftests/resctrl/mba_test.c > > b/tools/testing/selftests/resctrl/mba_test.c > > index 9c9a4f22e529..5e0b1e794295 100644 > > --- a/tools/testing/selftests/resctrl/mba_test.c > > +++ b/tools/testing/selftests/resctrl/mba_test.c > > @@ -166,7 +166,6 @@ static int mba_run_test(const struct resctrl_test *test, > > const struct user_param > > struct resctrl_val_param param = { > > .resctrl_val = MBA_STR, > > .ctrlgrp = "c1", > > - .mongrp = "m1", > > .filename = RESULT_FILE_NAME, > > .init = mba_init, > > .setup = mba_setup, > > This may explain the unexpected checks that are removed in final patch? While possible, I just have gotten a feeling that not much thought has been put on generality until now. Because of that, the solution had always been adding new ifs, no matter the place, instead of thinking how to parametrize things properly instead. It has lead to fully overlapping checks, dead code, and incomplete error handling which is hopefully now slowly getting less and less. > Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>
Hi Ilpo, On 5/30/24 4:56 AM, Ilpo Järvinen wrote: > On Wed, 29 May 2024, Reinette Chatre wrote: >> On 5/20/24 5:30 AM, Ilpo Järvinen wrote: >>> Nothing during MBA test uses mongrp even if it has been defined ever >>> since the introduction of the MBA test in the commit 01fee6b4d1f9 >>> ("selftests/resctrl: Add MBA test"). >>> >>> Remove the mongrp from MBA test. >>> >>> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> >>> --- >>> tools/testing/selftests/resctrl/mba_test.c | 1 - >>> 1 file changed, 1 deletion(-) >>> >>> diff --git a/tools/testing/selftests/resctrl/mba_test.c >>> b/tools/testing/selftests/resctrl/mba_test.c >>> index 9c9a4f22e529..5e0b1e794295 100644 >>> --- a/tools/testing/selftests/resctrl/mba_test.c >>> +++ b/tools/testing/selftests/resctrl/mba_test.c >>> @@ -166,7 +166,6 @@ static int mba_run_test(const struct resctrl_test *test, >>> const struct user_param >>> struct resctrl_val_param param = { >>> .resctrl_val = MBA_STR, >>> .ctrlgrp = "c1", >>> - .mongrp = "m1", >>> .filename = RESULT_FILE_NAME, >>> .init = mba_init, >>> .setup = mba_setup, >> >> This may explain the unexpected checks that are removed in final patch? > > While possible, I just have gotten a feeling that not much thought has > been put on generality until now. Because of that, the solution had always > been adding new ifs, no matter the place, instead of thinking how to > parametrize things properly instead. It has lead to fully overlapping > checks, dead code, and incomplete error handling which is hopefully now > slowly getting less and less. Ack. Your work is greatly appreciated. Reinette
diff --git a/tools/testing/selftests/resctrl/mba_test.c b/tools/testing/selftests/resctrl/mba_test.c index 9c9a4f22e529..5e0b1e794295 100644 --- a/tools/testing/selftests/resctrl/mba_test.c +++ b/tools/testing/selftests/resctrl/mba_test.c @@ -166,7 +166,6 @@ static int mba_run_test(const struct resctrl_test *test, const struct user_param struct resctrl_val_param param = { .resctrl_val = MBA_STR, .ctrlgrp = "c1", - .mongrp = "m1", .filename = RESULT_FILE_NAME, .init = mba_init, .setup = mba_setup,
Nothing during MBA test uses mongrp even if it has been defined ever since the introduction of the MBA test in the commit 01fee6b4d1f9 ("selftests/resctrl: Add MBA test"). Remove the mongrp from MBA test. Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> --- tools/testing/selftests/resctrl/mba_test.c | 1 - 1 file changed, 1 deletion(-)