Received-SPF: pass (google.com: domain of linux-kernel+bounces-52699-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: Mon, 5 Feb 2024 15:16:27 +0200 (EET)
To: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
cc: fenghua.yu@intel.com, reinette.chatre@intel.com, shuah@kernel.org, 
    linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org, 
    ilpo.jarvinen@linux.intel.com
Subject: Re: [PATCH v4 2/5] selftests/resctrl: Add helpers for the non-contiguous
 test
In-Reply-To: <89b8965d563e4e61b95b20be55c26475d830b245.1707130307.git.maciej.wieczor-retman@intel.com>
Message-ID: <7cfff3da-b476-4f24-aa16-428c79bd81e2@linux.intel.com>
References: <cover.1707130307.git.maciej.wieczor-retman@intel.com> <89b8965d563e4e61b95b20be55c26475d830b245.1707130307.git.maciej.wieczor-retman@intel.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="8323328-1807511629-1707138987=:1027"

  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-1807511629-1707138987=:1027
Content-Type: text/plain; charset=ISO-8859-15
Content-Transfer-Encoding: QUOTED-PRINTABLE

On Mon, 5 Feb 2024, Maciej Wieczor-Retman wrote:

> The CAT non-contiguous selftests have to read the file responsible for
> reporting support of non-contiguous CBMs in kernel (resctrl). Then the
> test compares if that information matches what is reported by CPUID
> output.
>=20
> Add a generic helper function to read an unsigned number from a file in
> /sys/fs/resctrl/info/<RESOURCE>/<FILE>.
>=20
> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
> ---
> Changelog v4:
> - Rewrite function comment.
> - Redo ksft_perror() as ksft_print_msg(). (Reinette)
>=20
> Changelog v3:
> - Rewrite patch message.
> - Add documentation and rewrote the function. (Reinette)
>=20
> Changelog v2:
> - Add this patch.
>=20
>  tools/testing/selftests/resctrl/resctrl.h   |  1 +
>  tools/testing/selftests/resctrl/resctrlfs.c | 36 +++++++++++++++++++++
>  2 files changed, 37 insertions(+)
>=20
> diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/se=
lftests/resctrl/resctrl.h
> index a1462029998e..5116ea082d03 100644
> --- a/tools/testing/selftests/resctrl/resctrl.h
> +++ b/tools/testing/selftests/resctrl/resctrl.h
> @@ -162,6 +162,7 @@ unsigned int count_contiguous_bits(unsigned long val,=
 unsigned int *start);
>  int get_full_cbm(const char *cache_type, unsigned long *mask);
>  int get_mask_no_shareable(const char *cache_type, unsigned long *mask);
>  int get_cache_size(int cpu_no, const char *cache_type, unsigned long *ca=
che_size);
> +int resource_info_unsigned_get(const char *resource, const char *filenam=
e, unsigned int *val);
>  void ctrlc_handler(int signum, siginfo_t *info, void *ptr);
>  int signal_handler_register(void);
>  void signal_handler_unregister(void);
> diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/=
selftests/resctrl/resctrlfs.c
> index 5750662cce57..e0fbc46a917a 100644
> --- a/tools/testing/selftests/resctrl/resctrlfs.c
> +++ b/tools/testing/selftests/resctrl/resctrlfs.c
> @@ -249,6 +249,42 @@ static int get_bit_mask(const char *filename, unsign=
ed long *mask)
>  =09return 0;
>  }
> =20
> +/**
> + * resource_info_unsigned_get - Read an unsigned value from
> + * /sys/fs/resctrl/info/RESOURCE/FILENAME
> + * @resource:=09Resource name that matches directory name in
> + *=09=09/sys/fs/resctrl/info
> + * @filename:=09File in /sys/fs/resctrl/info/@resource
> + * @val:=09Contains read value on success.
> + *
> + * Return: =3D 0 on success, < 0 on failure. On success the read
> + * value is saved into the @val.
> + */
> +int resource_info_unsigned_get(const char *resource, const char *filenam=
e,
> +=09=09=09       unsigned int *val)
> +{
> +=09char file_path[PATH_MAX];
> +=09FILE *fp;
> +
> +=09snprintf(file_path, sizeof(file_path), "%s/%s/%s", INFO_PATH, resourc=
e,
> +=09=09 filename);
> +
> +=09fp =3D fopen(file_path, "r");
> +=09if (!fp) {
> +=09=09ksft_print_msg("Error in opening %s\n: %m\n", file_path);

Adding the extra \n in between there will likely mess up the test output=20
formatting (the expected prefixes will not appear). Therefore, manually=20
adding newlines should be avoided.

> +=09=09return -1;
> +=09}
> +
> +=09if (fscanf(fp, "%u", val) <=3D 0) {
> +=09=09ksft_print_msg("Could not get contents of %s\n: %m\n", file_path);

Ditto.

With those two fixed,

Reviewed-by: Ilpo J=E4rvinen <ilpo.jarvinen@linux.intel.com>


--=20
 i.

--8323328-1807511629-1707138987=:1027--