Received-SPF: pass (google.com: domain of linux-kernel+bounces-188430-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>
Date: Fri, 24 May 2024 10:57: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 02/16] selftests/resctrl: Calculate resctrl FS derived
 mem bw over sleep(1) only
In-Reply-To: <04d0a5d6-82fa-4cc7-bd80-ee5cbd35f0c3@intel.com>
Message-ID: <ea0c86b9-ae77-c2d9-b52b-239ae42603e8@linux.intel.com>
References: <20240520123020.18938-1-ilpo.jarvinen@linux.intel.com> <20240520123020.18938-3-ilpo.jarvinen@linux.intel.com> <04d0a5d6-82fa-4cc7-bd80-ee5cbd35f0c3@intel.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="8323328-355923539-1716537436=:1394"

  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-355923539-1716537436=:1394
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: QUOTED-PRINTABLE

On Thu, 23 May 2024, Reinette Chatre wrote:
> On 5/20/24 5:30 AM, Ilpo J=C3=A4rvinen wrote:
> > For MBM/MBA tests, measure_vals() calls get_mem_bw_imc() that performs
> > the measurement over a duration of sleep(1) call. The memory bandwidth
> > numbers from IMC are derived over this duration. The resctrl FS derived
> > memory bandwidth, however, is calculated inside measure_vals() and only
> > takes delta between the previous value and the current one which
> > besides the actual test, also samples inter-test noise.
> >=20
> > Rework the logic in measure_vals() and get_mem_bw_imc() such that the
> > resctrl FS memory bandwidth section covers much shorter duration
> > closely matching that of the IMC perf counters to improve measurement
> > accuracy. Open two the resctrl mem bw files twice to avoid opening
> > after the test during measurement period (reading the same file twice
> > returns the same value so two files are needed).
>=20
> I think this is only because of how the current reading is done, resctrl
> surely supports keeping a file open and reading from it multiple times.
>=20
> There seems to be two things that prevent current code from doing this
> correctly:
> (a) the fscanf() code does not take into account that resctrl also
>     prints a "\n" ... (this seems to be the part that may cause the same
>     value to be returned).
>     So:
> =09if (fscanf(fp, "%lu", mbm_total) <=3D 0) {
>     should be:
> =09if (fscanf(fp, "%lu\n", mbm_total) <=3D 0) {
> (b) the current reading does not reset the file position so a second
>     read will attempt to read past the beginning. A "rewind(fp)"
>     should help here.

(b) cannot be the cause for returning the same value again. It would=20
not be able to reread the number at all if file position is not moved.

I certainly tried with fseek() and it is when I got same value on the=20
second read which is when I just went to two files solution.

> A small program like below worked for me by showing different values
> on every read:
>=20
> #include <stdio.h>
> #include <stdlib.h>
> #include <unistd.h>
>=20
> const char *mbm_total_path =3D
> "/sys/fs/resctrl/mon_data/mon_L3_00/mbm_total_bytes";
>=20
> int main(void)
> {
> =09unsigned long mbm_total;
> =09FILE *fp;
> =09int count;
>=20
> =09fp =3D fopen(mbm_total_path, "r");
> =09if (!fp) {
> =09=09perror("Opening data file\n");
> =09=09exit(1);
> =09}
> =09for (count =3D 0; count < 100; count++) {
> =09=09if (fscanf(fp, "%lu\n", &mbm_total) <=3D 0) {
> =09=09=09perror("Unable to read from data file\n");
> =09=09=09exit(1);
> =09=09}
> =09=09printf("Read %d: %lu\n",count ,mbm_total );
> =09=09sleep(1);
> =09=09rewind(fp);
> =09}
> =09fclose(fp);
> =09return 0;
> }

Okay, so perhaps it's your explanation (a) but can libc be trusted to not=
=20
do buffering/caching for FILE *? So to be on the safe side, it would=20
need to use syscalls directly to guarantee it's read the file twice.

If I convert it into fds, fscanf() cannot be used which would complicate=20
the string processing by adding extra steps.

--=20
 i.

--8323328-355923539-1716537436=:1394--