Received-SPF: pass (google.com: domain of linux-kernel+bounces-195329-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>
Date: Thu, 30 May 2024 14:56:16 +0300 (EEST)
To: Reinette Chatre <reinette.chatre@intel.com>
cc: linux-kselftest@vger.kernel.org, Shuah Khan <shuah@kernel.org>, 
    Babu Moger <babu.moger@amd.com>, 
    =?ISO-8859-15?Q?Maciej_Wiecz=F3r-Retman?= <maciej.wieczor-retman@intel.com>, 
    LKML <linux-kernel@vger.kernel.org>, Fenghua Yu <fenghua.yu@intel.com>, 
    Shuah Khan <skhan@linuxfoundation.org>
Subject: Re: [PATCH v4 14/16] selftests/resctrl: Remove mongrp from MBA
 test
In-Reply-To: <f2a8e8ad-311c-4302-a200-03fc9f02b20b@intel.com>
Message-ID: <feec5ce3-cad9-6ace-3588-67e7da9641d6@linux.intel.com>
References: <20240520123020.18938-1-ilpo.jarvinen@linux.intel.com> <20240520123020.18938-15-ilpo.jarvinen@linux.intel.com> <f2a8e8ad-311c-4302-a200-03fc9f02b20b@intel.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/mixed; BOUNDARY="8323328-1244305515-1717062694=:1113"
Content-ID: <ad75cbad-443e-d9d9-5090-23354f3468f7@linux.intel.com>

  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

--8323328-1244305515-1717062694=:1113
Content-Type: text/plain; CHARSET=ISO-8859-15
Content-Transfer-Encoding: QUOTED-PRINTABLE
Content-ID: <dc718522-6640-a0a6-7777-2864c166fa6f@linux.intel.com>

On Wed, 29 May 2024, Reinette Chatre wrote:
> On 5/20/24 5:30 AM, Ilpo J=E4rvinen 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").
> >=20
> > Remove the mongrp from MBA test.
> >=20
> > Signed-off-by: Ilpo J=E4rvinen <ilpo.jarvinen@linux.intel.com>
> > ---
> >   tools/testing/selftests/resctrl/mba_test.c | 1 -
> >   1 file changed, 1 deletion(-)
> >=20
> > 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
> >   =09struct resctrl_val_param param =3D {
> >   =09=09.resctrl_val=09=3D MBA_STR,
> >   =09=09.ctrlgrp=09=3D "c1",
> > -=09=09.mongrp=09=09=3D "m1",
> >   =09=09.filename=09=3D RESULT_FILE_NAME,
> >   =09=09.init=09=09=3D mba_init,
> >   =09=09.setup=09=09=3D mba_setup,
>=20
> 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=20
been put on generality until now. Because of that, the solution had always=
=20
been adding new ifs, no matter the place, instead of thinking how to=20
parametrize things properly instead. It has lead to fully overlapping=20
checks, dead code, and incomplete error handling which is hopefully now=20
slowly getting less and less.

> Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>

--=20
 i.
--8323328-1244305515-1717062694=:1113--