Received-SPF: pass (google.com: domain of linux-kernel+bounces-101575-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>
Date: Wed, 13 Mar 2024 15:59:15 +0200 (EET)
To: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
cc: linux-kselftest@vger.kernel.org, 
    Reinette Chatre <reinette.chatre@intel.com>, Shuah Khan <shuah@kernel.org>, 
    Babu Moger <babu.moger@amd.com>, Fenghua Yu <fenghua.yu@intel.com>, 
    LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 08/13] selftests/resctrl: Add ->init() callback into
 resctrl_val_param
In-Reply-To: <7zhdrvbb2w2ytyhbr2pl22exkzktqhv4ykb5pht6wsefg3yfyz@72xhgz3uyawo>
Message-ID: <5105ab0f-a90e-1770-1a67-db48f74b427f@linux.intel.com>
References: <20240311135230.7007-1-ilpo.jarvinen@linux.intel.com> <20240311135230.7007-9-ilpo.jarvinen@linux.intel.com> <zxrd5yzgtbet3d42c77ifgu5t2guyhwbz76zhk7zhrp6hnamrb@pl72dxisxax6> <vzluzxmks6bqfszwoib72ufbanxucxk5xzilavp3wrwoh7fqxh@rugv6wcptofu>
 <511bb602-e29d-0a2c-3076-81fdd5a5955f@linux.intel.com> <7zhdrvbb2w2ytyhbr2pl22exkzktqhv4ykb5pht6wsefg3yfyz@72xhgz3uyawo>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/mixed; BOUNDARY="8323328-518787328-1710338233=:1167"
Content-ID: <cdf611ef-34ee-201b-da68-72df17a42dbd@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-518787328-1710338233=:1167
Content-Type: text/plain; CHARSET=ISO-8859-15
Content-Transfer-Encoding: QUOTED-PRINTABLE
Content-ID: <22bde3bc-d109-63f6-bed5-ff50d2bc2f9f@linux.intel.com>

On Wed, 13 Mar 2024, Maciej Wieczor-Retman wrote:

> On 2024-03-13 at 13:37:51 +0200, Ilpo J=E4rvinen wrote:
> >On Wed, 13 Mar 2024, Maciej Wieczor-Retman wrote:
> >> On 2024-03-13 at 11:15:30 +0100, Maciej Wieczor-Retman wrote:
> >> >On 2024-03-11 at 15:52:25 +0200, Ilpo J=E4rvinen wrote:
> >> >>diff --git a/tools/testing/selftests/resctrl/cmt_test.c b/tools/test=
ing/selftests/resctrl/cmt_test.c
> >> >>index 241c0b129b58..e79eca9346f3 100644
> >> >>--- a/tools/testing/selftests/resctrl/cmt_test.c
> >> >>+++ b/tools/testing/selftests/resctrl/cmt_test.c
> >> >>@@ -16,6 +16,17 @@
> >> >> #define MAX_DIFF=09=092000000
> >> >> #define MAX_DIFF_PERCENT=0915
> >> >>=20
> >> >>+#define CON_MON_LCC_OCCUP_PATH=09=09\
> >> >>+=09"%s/%s/mon_groups/%s/mon_data/mon_L3_%02d/llc_occupancy"
> >> >>+
> >> >>+static int set_cmt_path(const struct resctrl_val_param *param, int =
domain_id)
> >> >>+{
> >> >>+=09sprintf(llc_occup_path,=09CON_MON_LCC_OCCUP_PATH,=09RESCTRL_PATH=
,
> >> >>+=09=09param->ctrlgrp, param->mongrp, domain_id);
> >> >>+
> >> >>+=09return 0;
> >> >>+}
> >> >>+
> >> >
> >> >Is there an option to make this function (and the set_mbm_path()) glo=
bal through
> >> >the resctrl.h?
> >> >
> >> >I'd like to use it in my SNC series [1] for looping over different no=
des and
> >> >that requires changing the paths during the measure phase of the test=
s and that
> >> >part is currently in cache.c:measure_llc_resctrl().
> >> >
> >> >Or would you suggest some other way of changing these paths in cache?
> >> >
> >>=20
> >> +forgot to add the link :b
> >>=20
> >> [1] https://lore.kernel.org/all/cover.1709721159.git.maciej.wieczor-re=
tman@intel.com/
> >
> >Perhaps ->init() should just prepare an array of filenames to read from=
=20
> >to support SNC. That would keep the filename preparations out of the=20
> >measurement period.
> >
> >It feels slightly hacky to have an array of files but I cannot think of=
=20
> >anything else that would be cleaner and would not require creating the=
=20
> >filenames during the actual test.
>=20
> So the array of names would be a part of "struct resctrl_val_param"?

This series has in independent of resctrl_val_param because resctrl_val()=
=20
itself doesn't care after the separation of generic and per test logic.=20

But that meant making it globals into the per test files which I didn't=20
like that much either. So perhaps having it in resctrl_val_param is=20
still better.

--=20
 i.

> It would have to be dynamically allocated too before running the tests.
>=20
> I kinda agree, creating filenames during measurements messes the whole
> separation idea between setup and measuring. And as you said there are li=
ke 4
> nodes max so not much memory would be wasted there.
>=20
> I can experiment with it and try to add it in my series - since it's much=
 more
> tied to SNC. Unless you see it'd better fit here then that's fine with me=
 too.

--8323328-518787328-1710338233=:1167--